Re: [PATCHv2 net-next 1/2] net: dsa: mv88e6xxx: Add helper for accessing port registers

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:



> @@ -585,19 +601,19 @@ static void mv88e6xxx_adjust_link(struct dsa_switch 
> *ds, int port,
> struct phy_device *phydev)
>  {
>   struct mv88e6xxx_chip *chip = ds->priv;
> - u32 reg;
> - int ret;
> + u16 reg;
> + int err;
>  
>   if (!phy_is_pseudo_fixed_link(phydev))
>   return;
>  
>   mutex_lock(&chip->reg_lock);
>  
> - ret = _mv88e6xxx_reg_read(chip, REG_PORT(port), PORT_PCS_CTRL);
> - if (ret < 0)
> + err = mv88e6xxx_port_read(chip, port, PORT_PCS_CTRL, ®);
> + if (err)
>   goto out;
>  
> - reg = ret & ~(PORT_PCS_CTRL_LINK_UP |
> + reg = reg & ~(PORT_PCS_CTRL_LINK_UP |

reg &= ~(PORT_PCS_CTRL_LINK_UP |

> PORT_PCS_CTRL_FORCE_LINK |
> PORT_PCS_CTRL_DUPLEX_FULL |
> PORT_PCS_CTRL_FORCE_DUPLEX |



>   /* Wait up to one second for reset to complete. */
>   timeout = jiffies + 1 * HZ;
>   while (time_before(jiffies, timeout)) {
> - ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, 0x00);
> - if (ret < 0)
> - return ret;
> + err = _mv88e6xxx_reg_read(chip, REG_GLOBAL, 0x00);
> + if (err)
> + return err;

Ouch! Wrong s/ret/err/ here.

>  
> - if ((ret & is_reset) == is_reset)
> + if ((err & is_reset) == is_reset)

Here as well. Keep ret or use mv88e6xxx_read().

>   break;
>   usleep_range(1000, 2000);
>   }
>   if (time_after(jiffies, timeout))
> - ret = -ETIMEDOUT;
> + err = -ETIMEDOUT;
>   else
> - ret = 0;
> + err = 0;
>  
> - return ret;
> + return err;
>  }
>  
>  static int mv88e6xxx_serdes_power_on(struct mv88e6xxx_chip *chip)

Thanks,

Vivien

[PATCHv2 net-next 1/2] net: dsa: mv88e6xxx: Add helper for accessing port registers

[Andrew Lunn]

There is a device coming soon which places its port registers
somewhere different to all other Marvell switches supported so far.
Add helper functions for reading/writing port registers, making it
easier to handle this new device.

Signed-off-by: Andrew Lunn 
---
v2:
 Call mv88e6xxx_{read|write} in wrappers
 Change some (err < 0) to plain (err)
---
 drivers/net/dsa/mv88e6xxx/chip.c  | 369 +-
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |   1 -
 2 files changed, 181 insertions(+), 189 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 70a812d159c9..6f2a145082e5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -216,6 +216,22 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, 
int reg, u16 val)
return 0;
 }
 
+int mv88e6xxx_port_read(struct mv88e6xxx_chip *chip, int port, int reg,
+   u16 *val)
+{
+   int addr = chip->info->port_base_addr + port;
+
+   return mv88e6xxx_read(chip, addr, reg, val);
+}
+
+int mv88e6xxx_port_write(struct mv88e6xxx_chip *chip, int port, int reg,
+u16 val)
+{
+   int addr = chip->info->port_base_addr + port;
+
+   return mv88e6xxx_write(chip, addr, reg, val);
+}
+
 static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
  int reg, u16 *val)
 {
@@ -585,19 +601,19 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, 
int port,
  struct phy_device *phydev)
 {
struct mv88e6xxx_chip *chip = ds->priv;
-   u32 reg;
-   int ret;
+   u16 reg;
+   int err;
 
if (!phy_is_pseudo_fixed_link(phydev))
return;
 
mutex_lock(&chip->reg_lock);
 
-   ret = _mv88e6xxx_reg_read(chip, REG_PORT(port), PORT_PCS_CTRL);
-   if (ret < 0)
+   err = mv88e6xxx_port_read(chip, port, PORT_PCS_CTRL, ®);
+   if (err)
goto out;
 
-   reg = ret & ~(PORT_PCS_CTRL_LINK_UP |
+   reg = reg & ~(PORT_PCS_CTRL_LINK_UP |
  PORT_PCS_CTRL_FORCE_LINK |
  PORT_PCS_CTRL_DUPLEX_FULL |
  PORT_PCS_CTRL_FORCE_DUPLEX |
@@ -639,7 +655,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, 
int port,
reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK |
PORT_PCS_CTRL_RGMII_DELAY_TXCLK);
}
-   _mv88e6xxx_reg_write(chip, REG_PORT(port), PORT_PCS_CTRL, reg);
+   mv88e6xxx_port_write(chip, port, PORT_PCS_CTRL, reg);
 
 out:
mutex_unlock(&chip->reg_lock);
@@ -799,22 +815,22 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct 
mv88e6xxx_chip *chip,
 {
u32 low;
u32 high = 0;
-   int ret;
+   int err;
+   u16 reg;
u64 value;
 
switch (s->type) {
case PORT:
-   ret = _mv88e6xxx_reg_read(chip, REG_PORT(port), s->reg);
-   if (ret < 0)
+   err = mv88e6xxx_port_read(chip, port, s->reg, ®);
+   if (err)
return UINT64_MAX;
 
-   low = ret;
+   low = reg;
if (s->sizeof_stat == 4) {
-   ret = _mv88e6xxx_reg_read(chip, REG_PORT(port),
- s->reg + 1);
-   if (ret < 0)
+   err = mv88e6xxx_port_read(chip, port, s->reg + 1, ®);
+   if (err)
return UINT64_MAX;
-   high = ret;
+   high = reg;
}
break;
case BANK0:
@@ -893,6 +909,8 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int 
port,
   struct ethtool_regs *regs, void *_p)
 {
struct mv88e6xxx_chip *chip = ds->priv;
+   int err;
+   u16 reg;
u16 *p = _p;
int i;
 
@@ -903,11 +921,10 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int 
port,
mutex_lock(&chip->reg_lock);
 
for (i = 0; i < 32; i++) {
-   int ret;
 
-   ret = _mv88e6xxx_reg_read(chip, REG_PORT(port), i);
-   if (ret >= 0)
-   p[i] = ret;
+   err = mv88e6xxx_port_read(chip, port, i, ®);
+   if (!err)
+   p[i] = reg;
}
 
mutex_unlock(&chip->reg_lock);
@@ -938,7 +955,7 @@ static int mv88e6xxx_get_eee(struct dsa_switch *ds, int 
port,
e->eee_enabled = !!(reg & 0x0200);
e->tx_lpi_enabled = !!(reg & 0x0100);
 
-   err = mv88e6xxx_read(chip, REG_PORT(port), PORT_STATUS, ®);
+   err = mv88e6xxx_port_read(chip, port, PORT_STATUS, ®);
if (err)
goto out;
 
@@ -1106,12 +1123,13 @@ static int _mv88e6xxx_port_state(struct mv88e6xxx_chip 
*chip, int port,
 u8 state)
 {
str