Re: [PATCH v2 5/5] i2c: designware: Support of controller with IC_EMPTYFIFO_HOLD_MASTER disabled

["Benoît Monin" <benoit.monin@bootlin.com>]

Hi Mika,

On Monday, 3 November 2025 at 11:43:30 CET, Mika Westerberg wrote:
> On Fri, Oct 31, 2025 at 03:35:43PM +0100, Benoît Monin wrote:
[...]
> > +/*
> > + * Return true if the message needs an explicit RESTART before being sent.
> > + * Without an explicit RESTART, two consecutive messages in the same direction
> > + * will be merged into a single transfer.
> > + * The adapter always emits a RESTART when the direction changes.
> > + */
> > +static inline bool i2c_dw_msg_need_restart(struct i2c_msg msgs[], int idx)
> 
> This can take const parameters.
> 
Agreed.

> > +{
> 
> Please move the dev->flags & NO_EMPTYFIFO_HOLD_MASTER here too.
> 
Agreed.

> > +	/* No need for a RESTART on the first message */
> > +	if (idx == 0)
> > +		return false;
> 
> That's
> 
> 	if (!idx) 
> 
Ack.

> But why not pass the actual message instead of the index?
> 
Because we need the direction of the previous message and if we are checking
the first message. Passing the array of messages and the index of the
message to check give us the info we need to perform the check.

> > +
> > +	return (msgs[idx - 1].flags & I2C_M_RD) == (msgs[idx].flags & I2C_M_RD);
> 
> You don't need the outer parens.
> 
> > +}
> > +
> >  static int
> >  i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> >  {
> > @@ -918,6 +958,17 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> >  				goto done;
> >  			}
> >  
> > +			/*
> > +			 * Make sure we don't need explicit RESTART for
> > +			 * controllers that cannot emit them.
> > +			 */
> > +			if (dev->flags & NO_EMPTYFIFO_HOLD_MASTER &&
> > +			    i2c_dw_msg_need_restart(msg, cnt - 1)) {
> > +				dev_err(dev->dev, "cannot emit RESTART\n");
> > +				ret = -EINVAL;
> > +				goto done;
> > +			}
> > +
> >  			if ((msg[cnt - 1].flags & I2C_M_STOP) ||
> >  			    (msg + cnt == msgs + num))
> >  				break;
> > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> > index d7d764f7554d..4aad3dc51fbc 100644
> > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > @@ -346,6 +346,7 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
> >  
> >  static const struct of_device_id dw_i2c_of_match[] = {
> >  	{ .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
> > +	{ .compatible = "mobileye,eyeq6lplus-i2c", .data = (void *)NO_EMPTYFIFO_HOLD_MASTER },
> >  	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
> >  	{ .compatible = "snps,designware-i2c" },
> >  	{}
> > 
> 


Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com