| Message ID | 20240703145718.19951-1-ceggers@arri.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0005b2dc43f96b93fc5b0850d7ca3f7aeac9129c |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] dsa: lan9303: Fix mapping between DSA port number and PHY address | expand |
On Wed, Jul 03, 2024 at 04:57:17PM +0200, Christian Eggers wrote: > The 'phy' parameter supplied to lan9303_phy_read/_write was sometimes a > DSA port number and sometimes a PHY address. This isn't a problem as > long as they are equal. But if the external phy_addr_sel_strap pin is > wired to 'high', the PHY addresses change from 0-1-2 to 1-2-3 (CPU, > slave0, slave1). In this case, lan9303_phy_read/_write must translate > between DSA port numbers and the corresponding PHY address. > > Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the SMSC-LAN9303") > Signed-off-by: Christian Eggers <ceggers@arri.de> > --- > drivers/net/dsa/lan9303-core.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > index 02f07b870f10..268949939636 100644 > --- a/drivers/net/dsa/lan9303-core.c > +++ b/drivers/net/dsa/lan9303-core.c > @@ -1047,31 +1047,31 @@ static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset) > return ARRAY_SIZE(lan9303_mib); > } > > -static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum) > +static int lan9303_phy_read(struct dsa_switch *ds, int port, int regnum) > { > struct lan9303 *chip = ds->priv; > int phy_base = chip->phy_addr_base; > > - if (phy == phy_base) > + if (port == 0) > return lan9303_virt_phy_reg_read(chip, regnum); > - if (phy > phy_base + 2) > + if (port > 2) > return -ENODEV; > > - return chip->ops->phy_read(chip, phy, regnum); > + return chip->ops->phy_read(chip, phy_base + port, regnum); > } > > -static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum, > +static int lan9303_phy_write(struct dsa_switch *ds, int port, int regnum, > u16 val) > { > struct lan9303 *chip = ds->priv; > int phy_base = chip->phy_addr_base; > > - if (phy == phy_base) > + if (port == 0) > return lan9303_virt_phy_reg_write(chip, regnum, val); > - if (phy > phy_base + 2) > + if (port > 2) > return -ENODEV; > > - return chip->ops->phy_write(chip, phy, regnum, val); > + return chip->ops->phy_write(chip, phy_base + port, regnum, val); > } > > static int lan9303_port_enable(struct dsa_switch *ds, int port, > @@ -1099,7 +1099,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port) > vlan_vid_del(dsa_port_to_conduit(dp), htons(ETH_P_8021Q), port); > > lan9303_disable_processing_port(chip, port); > - lan9303_phy_write(ds, chip->phy_addr_base + port, MII_BMCR, BMCR_PDOWN); > + lan9303_phy_write(ds, port, MII_BMCR, BMCR_PDOWN); > } > > static int lan9303_port_bridge_join(struct dsa_switch *ds, int port, > @@ -1374,8 +1374,6 @@ static const struct dsa_switch_ops lan9303_switch_ops = { > > static int lan9303_register_switch(struct lan9303 *chip) > { > - int base; > - > chip->ds = devm_kzalloc(chip->dev, sizeof(*chip->ds), GFP_KERNEL); > if (!chip->ds) > return -ENOMEM; > @@ -1385,8 +1383,7 @@ static int lan9303_register_switch(struct lan9303 *chip) > chip->ds->priv = chip; > chip->ds->ops = &lan9303_switch_ops; > chip->ds->phylink_mac_ops = &lan9303_phylink_mac_ops; > - base = chip->phy_addr_base; > - chip->ds->phys_mii_mask = GENMASK(LAN9303_NUM_PORTS - 1 + base, base); > + chip->ds->phys_mii_mask = GENMASK(LAN9303_NUM_PORTS - 1, 0); > > return dsa_register_switch(chip->ds); > } > -- > 2.43.0 > > The patch looks OK. Thanks, Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
On 7/3/2024 3:57 PM, Christian Eggers wrote: > The 'phy' parameter supplied to lan9303_phy_read/_write was sometimes a > DSA port number and sometimes a PHY address. This isn't a problem as > long as they are equal. But if the external phy_addr_sel_strap pin is > wired to 'high', the PHY addresses change from 0-1-2 to 1-2-3 (CPU, > slave0, slave1). In this case, lan9303_phy_read/_write must translate > between DSA port numbers and the corresponding PHY address. > > Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the SMSC-LAN9303") > Signed-off-by: Christian Eggers <ceggers@arri.de> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
On Wed, Jul 03, 2024 at 04:57:17PM +0200, Christian Eggers wrote: > The 'phy' parameter supplied to lan9303_phy_read/_write was sometimes a > DSA port number and sometimes a PHY address. This isn't a problem as > long as they are equal. But if the external phy_addr_sel_strap pin is > wired to 'high', the PHY addresses change from 0-1-2 to 1-2-3 (CPU, > slave0, slave1). In this case, lan9303_phy_read/_write must translate > between DSA port numbers and the corresponding PHY address. > > Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the SMSC-LAN9303") > Signed-off-by: Christian Eggers <ceggers@arri.de> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 3 Jul 2024 16:57:17 +0200 you wrote: > The 'phy' parameter supplied to lan9303_phy_read/_write was sometimes a > DSA port number and sometimes a PHY address. This isn't a problem as > long as they are equal. But if the external phy_addr_sel_strap pin is > wired to 'high', the PHY addresses change from 0-1-2 to 1-2-3 (CPU, > slave0, slave1). In this case, lan9303_phy_read/_write must translate > between DSA port numbers and the corresponding PHY address. > > [...] Here is the summary with links: - [net,1/2] dsa: lan9303: Fix mapping between DSA port number and PHY address https://git.kernel.org/netdev/net/c/0005b2dc43f9 - [net,2/2] dsa: lan9303: consistent naming for PHY address parameter (no matching commit) You are awesome, thank you!
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index 02f07b870f10..268949939636 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -1047,31 +1047,31 @@ static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset) return ARRAY_SIZE(lan9303_mib); } -static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum) +static int lan9303_phy_read(struct dsa_switch *ds, int port, int regnum) { struct lan9303 *chip = ds->priv; int phy_base = chip->phy_addr_base; - if (phy == phy_base) + if (port == 0) return lan9303_virt_phy_reg_read(chip, regnum); - if (phy > phy_base + 2) + if (port > 2) return -ENODEV; - return chip->ops->phy_read(chip, phy, regnum); + return chip->ops->phy_read(chip, phy_base + port, regnum); } -static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum, +static int lan9303_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val) { struct lan9303 *chip = ds->priv; int phy_base = chip->phy_addr_base; - if (phy == phy_base) + if (port == 0) return lan9303_virt_phy_reg_write(chip, regnum, val); - if (phy > phy_base + 2) + if (port > 2) return -ENODEV; - return chip->ops->phy_write(chip, phy, regnum, val); + return chip->ops->phy_write(chip, phy_base + port, regnum, val); } static int lan9303_port_enable(struct dsa_switch *ds, int port, @@ -1099,7 +1099,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port) vlan_vid_del(dsa_port_to_conduit(dp), htons(ETH_P_8021Q), port); lan9303_disable_processing_port(chip, port); - lan9303_phy_write(ds, chip->phy_addr_base + port, MII_BMCR, BMCR_PDOWN); + lan9303_phy_write(ds, port, MII_BMCR, BMCR_PDOWN); } static int lan9303_port_bridge_join(struct dsa_switch *ds, int port, @@ -1374,8 +1374,6 @@ static const struct dsa_switch_ops lan9303_switch_ops = { static int lan9303_register_switch(struct lan9303 *chip) { - int base; - chip->ds = devm_kzalloc(chip->dev, sizeof(*chip->ds), GFP_KERNEL); if (!chip->ds) return -ENOMEM; @@ -1385,8 +1383,7 @@ static int lan9303_register_switch(struct lan9303 *chip) chip->ds->priv = chip; chip->ds->ops = &lan9303_switch_ops; chip->ds->phylink_mac_ops = &lan9303_phylink_mac_ops; - base = chip->phy_addr_base; - chip->ds->phys_mii_mask = GENMASK(LAN9303_NUM_PORTS - 1 + base, base); + chip->ds->phys_mii_mask = GENMASK(LAN9303_NUM_PORTS - 1, 0); return dsa_register_switch(chip->ds); }
The 'phy' parameter supplied to lan9303_phy_read/_write was sometimes a DSA port number and sometimes a PHY address. This isn't a problem as long as they are equal. But if the external phy_addr_sel_strap pin is wired to 'high', the PHY addresses change from 0-1-2 to 1-2-3 (CPU, slave0, slave1). In this case, lan9303_phy_read/_write must translate between DSA port numbers and the corresponding PHY address. Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the SMSC-LAN9303") Signed-off-by: Christian Eggers <ceggers@arri.de> --- drivers/net/dsa/lan9303-core.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)