public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] tegra: i2c: Add function to know about current bus
Date: Thu, 08 Nov 2012 08:05:25 +0100	[thread overview]
Message-ID: <509B59B5.8090203@denx.de> (raw)
In-Reply-To: <CAPnjgZ1jWeN3trvJq1jGSiqPOxdyLta=nOyFqhNKcUOjaVtAuA@mail.gmail.com>

Hello Simon,

On 05.11.2012 21:43, Simon Glass wrote:
> Hi Heiko,
>
> On Tue, Oct 30, 2012 at 10:26 PM, Heiko Schocher<hs@denx.de>  wrote:
>> Hello Simon,
>>
>>
>> On 30.10.2012 18:28, Simon Glass wrote:
>>>
>>> Rather than using a variable in various places, add a single function,
>>> tegra_i2c_get_bus(), which returns a pointer to information about a
>>> bus.
>>>
>>> This will make it easier to move to the new i2c framework.
>>>
>>> Signed-off-by: Simon Glass<sjg@chromium.org>
>>> ---
>>>    drivers/i2c/tegra_i2c.c |   78
>>> ++++++++++++++++++++++++++++++++++++----------
>>>    1 files changed, 61 insertions(+), 17 deletions(-)
>>
>>
>> First question, did you tried the new i2c framework on some HW? Works it
>> for you?
>
> Yes, it works fine. Tried on a seaboard.

Great, thanks for testing!

>>> diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c
>>> index efc77fa..3e157f4 100644
>>> --- a/drivers/i2c/tegra_i2c.c
>>> +++ b/drivers/i2c/tegra_i2c.c
[...]
>> so the probe/read/write and set_bus_speed() functions gets now a
>> "struct i2c_adapter" pointer ... i2c driver internally, we need only
>> the "struct i2c_adapter", which the i2c core passes to the i2c driver.
>> So this function would look like now:
>>
>>> +static struct i2c_bus *tegra_i2c_get_bus(struct i2c_adapter *adap)
>
> Sounds good. Are you going to send a new patch?

Yes, want to do some more test ... and waiting for the result of the
vote, which direction we should go, see previous EMail ...

>>> +{
>>> +     struct i2c_bus *bus;
>>          ^^^^^^^^^^^^^^
>>          maybe a rename to "i2c_tegra_driver" would be good?
>>> +     bus =&i2c_controllers[adap->hwadapnr];
>>
>>> +     if (!bus->inited) {
>>> +             debug("%s: Bus %u not available\n", __func__, bus_num);
>>> +             return NULL;
>>> +     }
>>> +
>>> +     return bus;
>>> +}
>>
>> With this, we can get rid of i2c_bus_num in this driver.
>>
>> Also the probe/read/write and set_bus_speed() functions needs to
>> be adapted. I can do this for you, and add this patchset to my
>> v2 post, if it is ok for you?
>
> Of course.

Ok, do this ...

>>>    unsigned int i2c_get_bus_speed(void)
>>
>>
>> This function is not needed for the new framework!
>> [...]
>
> Yes, I thought I removed it in 2/2, perhaps not.
>
>>
>>>    int i2c_set_bus_speed(unsigned int speed)
>>>    {
>>> -       struct i2c_bus *i2c_bus;
>>> +       struct i2c_bus *bus;
>>>
>>> -       i2c_bus =&i2c_controllers[i2c_bus_num];
>>>
>>> -       i2c_bus->speed = speed;
>>> -       i2c_init_controller(i2c_bus);
>>> +       bus = tegra_i2c_get_bus(i2c_bus_num);
>>> +       if (!bus)
>>> +               return 0;
>>> +       bus->speed = speed;
>>> +       i2c_init_controller(bus);
>>>
>>>          return 0;
>>>    }
>>> @@ -426,7 +458,7 @@ void i2c_init(int speed, int slaveaddr)
>>
>> [...]
>>
>> Rest looks good to me.
>>
>> Thanks!
>
> Will await your further patch, and I am ready to test. Thanks.

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

      reply	other threads:[~2012-11-08  7:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-30 17:28 [U-Boot] [PATCH 1/2] tegra: i2c: Add function to know about current bus Simon Glass
2012-10-30 17:28 ` [U-Boot] [PATCH 2/2] WIP: tegra: i2c: Enable new CONFIG_SYS_I2C framework Simon Glass
2012-10-30 22:32   ` Stephen Warren
2012-10-31  6:00     ` Heiko Schocher
2012-10-31 15:41       ` Stephen Warren
2012-10-31 15:56         ` Simon Glass
2012-10-31 16:25           ` Stephen Warren
2012-11-01  7:42             ` Heiko Schocher
2012-11-01 17:03               ` Stephen Warren
2012-11-05 20:39                 ` Simon Glass
2012-11-08  7:02                   ` Heiko Schocher
2012-11-08 18:03                     ` Simon Glass
2012-11-08  6:47                 ` Heiko Schocher
2012-11-08 17:05                   ` Stephen Warren
2012-11-13  6:24                     ` Heiko Schocher
2012-11-13  7:48                       ` Wolfgang Denk
2012-11-13 17:30                       ` Stephen Warren
2012-10-31  5:53   ` Heiko Schocher
2012-10-31  5:26 ` [U-Boot] [PATCH 1/2] tegra: i2c: Add function to know about current bus Heiko Schocher
2012-11-05 20:43   ` Simon Glass
2012-11-08  7:05     ` Heiko Schocher [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=509B59B5.8090203@denx.de \
    --to=hs@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox