[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