[PATCH v2 2/2] i2c: xiic: Support forcing single-master in DT

Laine Jaakko EXT ext-jaakko.laine at vaisala.com
Wed Aug 26 09:03:01 EDT 2020


> > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> > index 10380531d45c..5d06e6cc5d5c 100644
> > --- a/drivers/i2c/busses/i2c-xiic.c
> > +++ b/drivers/i2c/busses/i2c-xiic.c
> > @@ -58,6 +58,7 @@ enum xiic_endian {
> >   * @rx_msg: Current RX message
> >   * @rx_pos: Position within current RX message
> >   * @endianness: big/little-endian byte order
> > + * @singlemaster: Indicates bus is single master
> >   * @clk: Pointer to AXI4-lite input clock
> >   */
> >  struct xiic_i2c {
> > @@ -73,6 +74,7 @@ struct xiic_i2c {
> >  	struct i2c_msg *rx_msg;
> >  	int rx_pos;
> >  	enum xiic_endian endianness;
> > +	bool singlemaster;
>
> I would understand if this is placed above rx_msg to fill that 4bytes
> hole in the structure. But this location doesn't make any sense.

Thanks for pointing out the struct alignment issue. I agree.

> The best would be to move state to the end and add this bool behind it.
> To have nicely packed structure like this

I will move state to the end in separate commit and add single-master after that in V3
as suggested.
 
> > -	err = xiic_bus_busy(i2c);
> > -	while (err && tries--) {
> > -		msleep(1);
> > +	if (!i2c->singlemaster) {
> > +		/* for instance if previous transfer was terminated due to TX
> > +		 * error it might be that the bus is on it's way to become
> > +		 * available give it at most 3 ms to wake
> > +		 */
> >  		err = xiic_bus_busy(i2c);
> > +		while (err && tries--) {
> > +			msleep(1);
> > +			err = xiic_bus_busy(i2c);
> > +		}
> >  	}
>
> I would prefer to write this differently.
>	if (i2c->singlemaster)
>		return 0;
>
> Followed by origin code. Patch will be smaller and you don't need to add
> one more level of indentation.

That sounds better and easier than current version, thanks.
I will make the change in V3.

-Jaakko


More information about the linux-arm-kernel mailing list