| Message ID | 20240706112116.24543-33-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: reword first drivers according to newest specification | expand |
> > - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK, > > - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA, > > + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_ACK, > > + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_DATA, > > I searched online for the datasheet but couldn't find it. It > would be helpful to know if the SLAVE naming comes from the > datasheet or if it is arbitrary. I was considering datasheet names, but obviously I concluded that these are custom names.
> > > - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK, > > > - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA, > > > + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_ACK, > > > + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_DATA, > > > > I searched online for the datasheet but couldn't find it. It > > would be helpful to know if the SLAVE naming comes from the > > datasheet or if it is arbitrary. > > I was considering datasheet names, but obviously I concluded that these > are custom names. Yes, I understand, but sometimes even the action names are defined in the datasheet, e.g. as a state machine. That's why I wanted an ack from someone who has the datasheet. Or, if it's publicly available, I can check it myself, but I couldn't find it. Thanks, Andi
Hello Andi, > Hi Wolfram, > > On Sat, Jul 06, 2024 at 01:20:32PM GMT, Wolfram Sang wrote: >> Change the wording of this driver wrt. the newest I2C v7 and SMBus 3.2 >> specifications and replace "master/slave" with more appropriate terms. >> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> >> --- >> drivers/i2c/busses/i2c-mv64xxx.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c >> index dc160cbc3155..29f94efedf60 100644 >> --- a/drivers/i2c/busses/i2c-mv64xxx.c >> +++ b/drivers/i2c/busses/i2c-mv64xxx.c >> @@ -89,8 +89,8 @@ enum { >> MV64XXX_I2C_STATE_WAITING_FOR_RESTART, >> MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK, >> MV64XXX_I2C_STATE_WAITING_FOR_ADDR_2_ACK, >> - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK, >> - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA, >> + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_ACK, >> + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_DATA, > > I searched online for the datasheet but couldn't find it. It > would be helpful to know if the SLAVE naming comes from the > datasheet or if it is arbitrary. > > If it originates from the hardware specifications, I suggest > keeping the term "SLAVE." > > If anyone can share the datasheet, I would be happy to review it > myself. I think you can find the information in any Marvell datasheet that is referenced in Documentation/arch/arm/marvell.rst. > > Jean and Gregory, could you please check and provide your ack > here? I checked with the Armada 370 datasheet, and those states are not explicitly referred to as "SLAVE." See section 25 for more details. So, I think there is no problem with switching to "TARGET." Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com> Thanks, Gregory > > Thanks, > Andi
> I think you can find the information in any Marvell datasheet that is > referenced in Documentation/arch/arm/marvell.rst. Wow, this is a great collection of links. Thank you for letting us know! Very helpful.
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index dc160cbc3155..29f94efedf60 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -89,8 +89,8 @@ enum { MV64XXX_I2C_STATE_WAITING_FOR_RESTART, MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK, MV64XXX_I2C_STATE_WAITING_FOR_ADDR_2_ACK, - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK, - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA, + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_ACK, + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_DATA, }; /* Driver actions */ @@ -279,7 +279,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) } else { drv_data->action = MV64XXX_I2C_ACTION_SEND_DATA; drv_data->state = - MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK; + MV64XXX_I2C_STATE_WAITING_FOR_TARGET_ACK; drv_data->bytes_left--; } break; @@ -307,7 +307,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status) drv_data->action = MV64XXX_I2C_ACTION_RCV_DATA; drv_data->bytes_left--; } - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA; + drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_TARGET_DATA; if ((drv_data->bytes_left == 1) || drv_data->aborting) drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_ACK; @@ -797,8 +797,8 @@ static int mv64xxx_i2c_xfer_atomic(struct i2c_adapter *adap, } static const struct i2c_algorithm mv64xxx_i2c_algo = { - .master_xfer = mv64xxx_i2c_xfer, - .master_xfer_atomic = mv64xxx_i2c_xfer_atomic, + .xfer = mv64xxx_i2c_xfer, + .xfer_atomic = mv64xxx_i2c_xfer_atomic, .functionality = mv64xxx_i2c_functionality, };
Change the wording of this driver wrt. the newest I2C v7 and SMBus 3.2 specifications and replace "master/slave" with more appropriate terms. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/i2c/busses/i2c-mv64xxx.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)