[U-Boot] [PATCH v3 01/10] dm: i2c: Add a uclass for I2C

[Simon Glass]

Hi Masahiro,

On 4 December 2014 at 00:27, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Wed, 3 Dec 2014 19:36:15 -0700
> Simon Glass <sjg at chromium.org> wrote:
>
>> Hi Masahiro,
>>
>> On 3 December 2014 at 19:01, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
>> > Hi Simon,
>> >
>> >
>> > More comments again.
>> >
>> >
>> > On Mon, 24 Nov 2014 11:57:15 -0700
>> >> +
>> >> +static int i2c_probe_chip(struct udevice *bus, uint chip_addr)
>> >> +{
>> >> +     struct dm_i2c_ops *ops = i2c_get_ops(bus);
>> >> +     struct i2c_msg msg[1];
>> >> +     uint8_t ch = 0;
>> >> +
>> >> +     if (!ops->xfer)
>> >> +             return -ENOSYS;
>> >> +
>> >> +     msg->addr = chip_addr;
>> >> +     msg->flags = 0;
>> >> +     msg->len = 1;
>> >> +     msg->buf = &ch;
>> >> +
>> >> +     return ops->xfer(bus, msg, 1);
>> >> +}
>> >
>> > i2c_probe_chip() issues a write transaction with one length,
>> > but a read transaction should be used.
>> >
>> > For most of chips, the first write data is the first byte of
>> > the offset address, so no real data will be written into the chip.
>> >
>> > But it is possible to have offset_address_length == 0.
>> >
>> > The read transaction is always safer than the write transaction.
>> >
>>
>> I originally had a probe method to allow the driver to decide what to
>> do. Many drivers can in fact just write the address and don't need a
>> data / offset byte. But not all. With a read byte the tegra driver
>> takes ages to respond and the probe takes a lot longer than it should.
>
> I can't believe this.
> Reading one byte should be done pretty fast.
> If not, the driver implemetation has crewed up.
>
>
>> Yes I agree that read is safer, but even safer is nothing. Perhaps I
>> should go back to having a probe_chip() method? Or at least it could
>> be optional.
>
> Safety takes precedence over efficiency.
>
> I don't think we should go back to a probe_chip().
>
> Probing happens only one time before the use of I2C.
> Even if it takes longer, no problem.

This would still be considered a regression. Yes I suspect that the
Tegra driver has a problem, but I'm not sure. I don't want to be in
the business of looking for bugs in all the drivers just so they can
convert to driver model without problems.

IMO the right way to probe is to write the address with no data at
all. I was hoping that there was a standard probe method which would
work with all drivers but I think that might have been a mistake. I'm
going to reinstate the probe() method and make it optional. That way
drivers with particular needs can still use it, but driver model will
have a default probe implementation. That implementation will just
send a zero-length message.

That should resolve your concern and mine.

Regards,
Simon