Replace read-modify-write code in the lan867x_config_init function to
avoid handling data type mismatch and to simplify the code.

Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
---
 drivers/net/phy/microchip_t1s.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index a42a6bb6e3bd..534d9faf8475 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -31,19 +31,19 @@
  * W   0x1F 0x0099 0x7F80 ------
  */
 
-static const int lan867x_fixup_registers[12] = {
+static const u32 lan867x_fixup_registers[12] = {
 	0x00D0, 0x00D1, 0x0084, 0x0085,
 	0x008A, 0x0087, 0x0088, 0x008B,
 	0x0080, 0x00F1, 0x0096, 0x0099,
 };
 
-static const int lan867x_fixup_values[12] = {
+static const u16 lan867x_fixup_values[12] = {
 	0x0002, 0x0000, 0x3380, 0x0006,
 	0xC000, 0x801C, 0x033F, 0x0404,
 	0x0600, 0x2400, 0x2000, 0x7F80,
 };
 
-static const int lan867x_fixup_masks[12] = {
+static const u16 lan867x_fixup_masks[12] = {
 	0x0E03, 0x0300, 0xFFC0, 0x000F,
 	0xF800, 0x801C, 0x1FFF, 0xFFFF,
 	0x0600, 0x7F00, 0x2000, 0xFFFF,
@@ -63,9 +63,7 @@ static int lan867x_config_init(struct phy_device *phydev)
 	 * used, which might then write the same value back as read + modified.
 	 */
 
-	int reg_value;
 	int err;
-	int reg;
 
 	/* Read-Modified Write Pseudocode (from AN1699)
 	 * current_val = read_register(mmd, addr) // Read current register value
@@ -74,12 +72,11 @@ static int lan867x_config_init(struct phy_device *phydev)
 	 * write_register(mmd, addr, new_val) // Write back updated register value
 	 */
 	for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) {
-		reg = lan867x_fixup_registers[i];
-		reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg);
-		reg_value &= ~lan867x_fixup_masks[i];
-		reg_value |= lan867x_fixup_values[i];
-		err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value);
-		if (err != 0)
+		err = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+				     lan867x_fixup_registers[i],
+				     lan867x_fixup_masks[i],
+				     lan867x_fixup_values[i]);
+		if (err)
 			return err;
 	}
 
-- 
2.34.1

>  
>  	/* Read-Modified Write Pseudocode (from AN1699)
>  	 * current_val = read_register(mmd, addr) // Read current register value

Hi Parthiban

Maybe extend the comment to indicate that although AN1699 says Read,
Modify, Write, the write is not required if the register already has
the required value. That is what phy_modify_mmd() actually does.

> @@ -74,12 +72,11 @@ static int lan867x_config_init(struct phy_device *phydev)
>  	 * write_register(mmd, addr, new_val) // Write back updated register value
>  	 */
>  	for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) {
> -		reg = lan867x_fixup_registers[i];
> -		reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg);
> -		reg_value &= ~lan867x_fixup_masks[i];
> -		reg_value |= lan867x_fixup_values[i];
> -		err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value);
> -		if (err != 0)
> +		err = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
> +				     lan867x_fixup_registers[i],
> +				     lan867x_fixup_masks[i],
> +				     lan867x_fixup_values[i]);
> +		if (err)
>  			return err;
>  	}


    Andrew

---
pw-bot: cr

Hi Andrew,

On 22/05/23 6:01 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>>
>>        /* Read-Modified Write Pseudocode (from AN1699)
>>         * current_val = read_register(mmd, addr) // Read current register value
> 
> Hi Parthiban
> 
> Maybe extend the comment to indicate that although AN1699 says Read,
> Modify, Write, the write is not required if the register already has
> the required value. That is what phy_modify_mmd() actually does.
Sure, I will add it in the next version.

Best Regards,
Parthiban V
> 
>> @@ -74,12 +72,11 @@ static int lan867x_config_init(struct phy_device *phydev)
>>         * write_register(mmd, addr, new_val) // Write back updated register value
>>         */
>>        for (int i = 0; i < ARRAY_SIZE(lan867x_fixup_registers); i++) {
>> -             reg = lan867x_fixup_registers[i];
>> -             reg_value = phy_read_mmd(phydev, MDIO_MMD_VEND2, reg);
>> -             reg_value &= ~lan867x_fixup_masks[i];
>> -             reg_value |= lan867x_fixup_values[i];
>> -             err = phy_write_mmd(phydev, MDIO_MMD_VEND2, reg, reg_value);
>> -             if (err != 0)
>> +             err = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
>> +                                  lan867x_fixup_registers[i],
>> +                                  lan867x_fixup_masks[i],
>> +                                  lan867x_fixup_values[i]);
>> +             if (err)
>>                        return err;
>>        }
> 
> 
>      Andrew
> 
> ---
> pw-bot: cr