[PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification

Wolfram Sang posted 60 patches 2 months, 2 weeks ago
[PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Wolfram Sang 2 months, 2 weeks ago
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,
 };
 
 /* 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,
 };
 
-- 
2.43.0
Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Andi Shyti 2 months, 1 week ago
Hi Wolfram,

> 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,
>  };

OK, I managed to check and I couldn't find any definition about
the driver's state.

Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks,
Andi
Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Gregory CLEMENT 2 months ago
Hello Andi,

> Hi Wolfram,
>
>> 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,
>>  };
>
> OK, I managed to check and I couldn't find any definition about
> the driver's state.

I apologize for responding late to your previous request.

Out of curiosity, how did you manage to check it? Was it by referring to
the Marvell datasheet pointed in the Documentation, or did you use
another method?

Gregory


>
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
>
> Thanks,
> Andi
Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Andi Shyti 2 months, 1 week ago
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.

Jean and Gregory, could you please check and provide your ack
here?

Thanks,
Andi
Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Gregory CLEMENT 2 months ago
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
Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Wolfram Sang 2 months ago
> 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.

Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Wolfram Sang 2 months, 1 week ago
> > -	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.

Re: [PATCH v2 32/60] i2c: mv64xxx: reword according to newest specification
Posted by Andi Shyti 2 months, 1 week ago
> > > -	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