[U-Boot] [RFC PATCH 2/2] dm: i2c: support 10bit addressing in I2C uclass layer

[Simon Glass]

Hi Masahiro,

On 5 January 2015 at 07:56, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
> 2014-12-22 3:53 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> Hi Masahiro,
>>
>> On 20 December 2014 at 00:25, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
>>> Hi Simon,
>>>
>>>
>>> 2014-12-20 6:34 GMT+09:00 Simon Glass <sjg at chromium.org>:
>>>> Hi Masahiro,
>>>>
>>>> On 19 December 2014 at 11:34, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
>>>>> Master send to / receive from 10-bit addressed slave devices
>>>>> can be supported by software layer without any hardware change
>>>>> because the LSB 8bit of the slave address is treated as data part.
>>>>>
>>>>> Master Send to a 10bit-addressed slave chip is performed like this:
>>>>>
>>>>>  DIR    Format
>>>>>  M->S   11110 + address[9:8] + R/W(0)
>>>>>  M->S   address[7:0]
>>>>>  M->S   data0
>>>>>  M->S   data1
>>>>>       ...
>>>>>
>>>>> Master Receive from a 10bit-addressed slave chip is like this:
>>>>>
>>>>>  DIR    Format
>>>>>  M->S   11110 + address[9:8] + R/W(0)
>>>>>  M->S   address[7:0]
>>>>>         (Restart)
>>>>>  M->S   111110 + address[9:8] + R/W(1)
>>>>>  S->M   data0
>>>>>  S->M   data1
>>>>>       ...
>>>>>
>>>>> Signed-off-by: Masahiro Yamada <yamada.m at jp.panasonic.com>
>>>>> Cc: Heiko Schocher <hs at denx.de>
>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>> ---
>>>>>
>>>>>  drivers/i2c/i2c-uclass.c | 80 +++++++++++++++++++++++++++++++-----------------
>>>>>  include/i2c.h            |  4 +++
>>>>>  2 files changed, 56 insertions(+), 28 deletions(-)
>>>>
>>>> Seems like a good idea if we can make it work...
>>>>
>>>> But this is driver-specific. Some drivers have hardware to send the
>>>> address and it isn't part of the message. For example see the tegra
>>>> driver.
>>>>
>>>> So what you have here feels a bit like a hack to me. Can't the driver
>>>> implement it? If you are trying to avoid driver work to support 10-bit
>>>> addresses, maybe it should be an option that we can enable for each
>>>> driver, so we don't break the other drivers?
>>>
>>>
>>> I was writing two I2C drivers on DM,
>>> both of which have no dedicated hardware support for 10bit addressing.
>>>
>>> Of course, the driver could implement it, but it means
>>> I put the completely the same code in each of driver.
>>>
>>> For write transaction, for example, we create a new buffer and copy
>>> offset-address + data in Uclass layer.
>>>
>>> Do I have to create a new buffer again, in my driver,
>>> and copy  lower-slave-address + offset-address + data ?
>>
>> I see your point!
>>
>>>
>>> Perhaps, is it a good idea to have it optional?
>>>
>>> DM_I2C_FLAG_SW_TENBIT  - if set, uclass takes care of 10bit addressing
>>> by software
>>>                                                     if unset, each
>>> driver is responsible to handle I2C_M_TEN correctly
>>>
>>> altough I do think 10bit support on U-Boot is urgent necessity...
>>
>> I've thought about this quite a bit. We have only just changed the API
>> to be the same as Linux (the list of messages). It seems wrong to now
>> hack it around, such that the address field only stores the first part
>> of the address in 10-bit mode. Did we like the API or not?
>
>
> I am not sure...
> That is why I sent this series as RFC.
>
>
>> I see two options that are somewhat sane and defensible:
>
> I see another option:
> Do not support 10bit address (or leave it to each driver if necessary).
>
> Rationale:
> Until now, U-boot has not supported 10bit address and nobody has not
> complained about that.

OK, well it is certainly possible for the driver to support it, and it
isn't very hard. As you say, there demand has not been high!

>
>
>
>> - Add a helper function in the uclass that the driver can call to turn
>> the address + message into a single stream of bytes (we can avoid a
>> second malloc() by reserving space for the address bytes before the
>> message or similar similar, so long is it is clearly documented)
>> - Allow the driver to request that the message bytes should always
>> contain the address, which would remove the address-processing code
>> from the driver.
>
> How?  set a flag??

I suppose the driver could set a flag in the uclass to tell the uclass
how to behave. I'm not sure if this will make things simpler or more
complicated.

>
>> I think this needs a little more discussion before we decide what to do.
>
> Agreed.
> We do not rush to make a decision.

OK, well patch 1 looks OK anyway, so I think we should take that.

Regards,
Simon