Re: [PATCH v4 1/4] i2c: imx: only poll for bus busy in multi master mode

[Lucas Stach <l.stach@pengutronix.de>]

Am Mittwoch, dem 02.10.2024 um 16:56 +0200 schrieb Stefan Eichenberger:
> On Wed, Oct 02, 2024 at 04:40:32PM +0200, Lucas Stach wrote:
> > Am Mittwoch, dem 02.10.2024 um 13:19 +0200 schrieb Stefan Eichenberger:
> > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > 
> > > According to the i.MX8M Mini reference manual chapter "16.1.4.2
> > > Generation of Start" it is only necessary to poll for bus busy and
> > > arbitration lost in multi master mode. This helps to avoid rescheduling
> > > while the i2c bus is busy and avoids SMBus devices to timeout.
> > > 
> > This is a backward incompatible change, as far as I can see. Until now
> > the driver would properly handle a multi-mastered bus, without any
> > specific configuration. Now it requires the new multi-master DT
> > property to be set, which isn't even documented in the binding to be
> > understood by this driver.
> > 
> > Are you sure that every single instance of a i.MX i2c bus is only
> > single mastered?
> > 
> > If this is a worthwhile performance improvement I guess you need to
> > flip the logic around by adding a new single-master DT property (or
> > something along those lines), which should go through proper DT binding
> > review. You can then use this property for boards/busses to opt into
> > skipping the arbitration lost check.
> 
> According to the discussion here the property documentation should not
> be added:
> https://lore.kernel.org/linux-i2c/2bbddaxyjkxfmlgmq3yqcbzo7dsb2pq5bvdatk2y4ig4iintkt@35btqkdv7sy3/
> 
Ah, I wasn't aware of that.

> However, the point regarding single-master and multi-master is correct.
> We also discussed this internally and assumed the single-master use case
> is more likely to be the default and that this patch series would fix
> issues for other devices out there.
> 
I agree that the vast majority of busses is single master only and I
can see why you did it this way.

However, I still think it's a risky change, as some boards/DTs may rely
on the fact that the driver implicitly handled multi-master until now
and we can't retroactively change DTs in the wild.

I just looked up the dtschema for i2c-controller and there is a
"single-master" property defined already. I think it would be good if
you used this instead to have boards opt in to this optimization.

Regards,
Lucas

>  However, your point is valid and if
> preferred I can change it to single-master with the next version.
> 
> Francesco Dolcini <francesco.dolcini@toradex.com> I think you once had a
> discussion regarding multi master mode for i2c on i.MX devices? Maybe
> you can remember the details?
> 
> Regards,
> Stefan