Re: [PATCH] eeprom: at24: Add support for large EEPROMs connected to SMBus adapters

[Wolfram Sang]

> > > At the same time multi-master access is quite rare.
> > 
> > It should still work in those rare cases. A setup of an SoC, an EC
> > (those like to be masters, as well), and an EEPROM is not very far off.

To clarify: What I meant here is, at24 should also work in those rare
cases reliably (like the described case of SoC, EC and EEPROM). Your
patch sacrificies this reliability.

> It won't work if the 24c32 is connected to a controller which only supports
> SMBus transactions. Also, there is no problem if it is connected to a
> controller supporting direct i2c accesses.

I know and agree.

> The only real solution is to not use an SMBus-only controller together
> with 24c32 or any similar chip in such situations.

Exactly.

> > Huh, using i2c_transfer to ensure 'repeated start' between messages
> > should do the trick, no? Drivers doing this via SMBus calls are broken
> > and should be fixed.
> > 
> 
> That would require all those drivers to have two separate implementations,
> or to declare that any such chips must only be connected to controllers
> supporting direct i2c accesses.

Yes, TBH I though this was the case. Which drivers did you find using
unsafe SMBus transactions?

> Both isn't really practical and defies the real world, where any kind
> of chip will be connected to SMBus-only controllers.

Is it that bad? The vast majority of controllers I come across in the
embedded world are more like I2C than SMBus controllers. They have their
quirks but most support repeated starts somehow. This is why I came up
with the quirk infrastructure BTW, to describe what the controller is
capable of and what not. So, users can check if the controller is up to
what the user needs and then they can decide what to do if they
encounter a too limited controller.

Back to topic: I2C and SMBus are multi-master busses. Drivers have to
cope with that. If we say "let's assume single master busses, because
most of them out there are exactly that", then we open a window for
very subtle and hard to find bugs for the multi-master case. And people
would get rightfully angry IMO, because they should expect specified
things to work.

However, if people are connecting I2C devices to an SMBus controller,
well, they ask for prob^H^H^H^H special cases and they need to deal with
that. And using a module parameter is not much to deal with if you ask
me. BTW another idea I got was that a user could mark an SMBus
controller as "single-master", so a driver can check for that and use
some fallback.

I am really all for the "better-safe-than-sorry" approach here. I am not
objecting if somebody wants to move out of that area. It needs some form
of "yes, I really want to do that" however.


> A worse situation would occur if the chip specifically
> (only) supports SMBus transactions and thus doesn't explicitly support
> repeated start after completion of one transaction.

You lost me here. Is "the chip" the master or the slave?

> This may or may not
> work depending on the chip or even chip revision, and it usually would
> not be specified as a valid transaction in the chip datasheet.

Can you elaborate, please?

Thanks,

   Wolfram

Attachment: signature.asc
Description: Digital signature