All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.