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

[Masahiro YAMADA]

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.



> - 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 think this needs a little more discussion before we decide what to do.

Agreed.
We do not rush to make a decision.



-- 
Best Regards
Masahiro Yamada