| Message ID | 20230411023541.2372609-1-andrew@lunn.ch (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 18bb56ab4477b966e2974b96a5e3bf9ba052aac5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: mv88e6xxx: Correct cmode to PHY_INTERFACE_ | expand |
On Tue, Apr 11, 2023 at 04:35:41AM +0200, Andrew Lunn wrote: > The switch can either take the MAC or the PHY role in an MII or RMII > link. There are distinct PHY_INTERFACE_ macros for these two roles. > Correct the mapping so that the `REV` version is used for the PHY > role. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > Since this has not caused any known issues so far, i decided to not > add a Fixes: tag and submit for net. I concur. Thanks.
On Tue, Apr 11, 2023 at 04:35:41AM +0200, Andrew Lunn wrote: > The switch can either take the MAC or the PHY role in an MII or RMII > link. There are distinct PHY_INTERFACE_ macros for these two roles. > Correct the mapping so that the `REV` version is used for the PHY > role. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > Since this has not caused any known issues so far, i decided to not > add a Fixes: tag and submit for net. > > drivers/net/dsa/mv88e6xxx/chip.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 62a126402983..ffe6a88f94ce 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -611,10 +611,10 @@ static void mv88e6185_phylink_get_caps(struct mv88e6xxx_chip *chip, int port, > } > > static const u8 mv88e6xxx_phy_interface_modes[] = { > - [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_MII, > + [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_REVMII, > [MV88E6XXX_PORT_STS_CMODE_MII] = PHY_INTERFACE_MODE_MII, > [MV88E6XXX_PORT_STS_CMODE_GMII] = PHY_INTERFACE_MODE_GMII, > - [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_RMII, > + [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_REVRMII, > [MV88E6XXX_PORT_STS_CMODE_RMII] = PHY_INTERFACE_MODE_RMII, > [MV88E6XXX_PORT_STS_CMODE_100BASEX] = PHY_INTERFACE_MODE_100BASEX, > [MV88E6XXX_PORT_STS_CMODE_1000BASEX] = PHY_INTERFACE_MODE_1000BASEX, > -- > 2.40.0 >
On 4/11/2023 4:38 AM, Vladimir Oltean wrote: > On Tue, Apr 11, 2023 at 04:35:41AM +0200, Andrew Lunn wrote: >> The switch can either take the MAC or the PHY role in an MII or RMII >> link. There are distinct PHY_INTERFACE_ macros for these two roles. >> Correct the mapping so that the `REV` version is used for the PHY >> role. >> >> Signed-off-by: Andrew Lunn <andrew@lunn.ch> >> --- > > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> > >> >> Since this has not caused any known issues so far, i decided to not >> add a Fixes: tag and submit for net. >> >> drivers/net/dsa/mv88e6xxx/chip.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c >> index 62a126402983..ffe6a88f94ce 100644 >> --- a/drivers/net/dsa/mv88e6xxx/chip.c >> +++ b/drivers/net/dsa/mv88e6xxx/chip.c >> @@ -611,10 +611,10 @@ static void mv88e6185_phylink_get_caps(struct mv88e6xxx_chip *chip, int port, >> } >> >> static const u8 mv88e6xxx_phy_interface_modes[] = { >> - [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_MII, >> + [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_REVMII, Is this hunk correct? >> [MV88E6XXX_PORT_STS_CMODE_MII] = PHY_INTERFACE_MODE_MII, >> [MV88E6XXX_PORT_STS_CMODE_GMII] = PHY_INTERFACE_MODE_GMII, >> - [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_RMII, >> + [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_REVRMII, >> [MV88E6XXX_PORT_STS_CMODE_RMII] = PHY_INTERFACE_MODE_RMII, >> [MV88E6XXX_PORT_STS_CMODE_100BASEX] = PHY_INTERFACE_MODE_100BASEX, >> [MV88E6XXX_PORT_STS_CMODE_1000BASEX] = PHY_INTERFACE_MODE_1000BASEX, >> -- >> 2.40.0 >>
> On 4/11/2023 4:38 AM, Vladimir Oltean wrote: > > On Tue, Apr 11, 2023 at 04:35:41AM +0200, Andrew Lunn wrote: > > > The switch can either take the MAC or the PHY role in an MII or RMII > > > link. There are distinct PHY_INTERFACE_ macros for these two roles. > > > Correct the mapping so that the `REV` version is used for the PHY > > > role. > > > static const u8 mv88e6xxx_phy_interface_modes[] = { > > > - [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_MII, > > > + [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_REVMII, > > Is this hunk correct? Hi Florian I don't see why it is wrong, but you can be blind to bugs in your own code. What do you think is wrong? Thanks Andrew
On 4/11/2023 5:48 AM, Andrew Lunn wrote: >> On 4/11/2023 4:38 AM, Vladimir Oltean wrote: >>> On Tue, Apr 11, 2023 at 04:35:41AM +0200, Andrew Lunn wrote: >>>> The switch can either take the MAC or the PHY role in an MII or RMII >>>> link. There are distinct PHY_INTERFACE_ macros for these two roles. >>>> Correct the mapping so that the `REV` version is used for the PHY >>>> role. > >>>> static const u8 mv88e6xxx_phy_interface_modes[] = { >>>> - [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_MII, >>>> + [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_REVMII, >> >> Is this hunk correct? > > Hi Florian > > I don't see why it is wrong, but you can be blind to bugs in your own > code. What do you think is wrong? I was not thinking it was wrong, just curious about the meaning of a CMODE value suffixed with _PHY, though it seems clear(er) now that this means the port is configured as a PHY and provides PHY signals to the MAC it connects to. Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> PS: you can be blind without enough coffee as well :p
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 11 Apr 2023 04:35:41 +0200 you wrote: > The switch can either take the MAC or the PHY role in an MII or RMII > link. There are distinct PHY_INTERFACE_ macros for these two roles. > Correct the mapping so that the `REV` version is used for the PHY > role. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > [...] Here is the summary with links: - [net-next] net: dsa: mv88e6xxx: Correct cmode to PHY_INTERFACE_ https://git.kernel.org/netdev/net-next/c/18bb56ab4477 You are awesome, thank you!
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 62a126402983..ffe6a88f94ce 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -611,10 +611,10 @@ static void mv88e6185_phylink_get_caps(struct mv88e6xxx_chip *chip, int port, } static const u8 mv88e6xxx_phy_interface_modes[] = { - [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_MII, + [MV88E6XXX_PORT_STS_CMODE_MII_PHY] = PHY_INTERFACE_MODE_REVMII, [MV88E6XXX_PORT_STS_CMODE_MII] = PHY_INTERFACE_MODE_MII, [MV88E6XXX_PORT_STS_CMODE_GMII] = PHY_INTERFACE_MODE_GMII, - [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_RMII, + [MV88E6XXX_PORT_STS_CMODE_RMII_PHY] = PHY_INTERFACE_MODE_REVRMII, [MV88E6XXX_PORT_STS_CMODE_RMII] = PHY_INTERFACE_MODE_RMII, [MV88E6XXX_PORT_STS_CMODE_100BASEX] = PHY_INTERFACE_MODE_100BASEX, [MV88E6XXX_PORT_STS_CMODE_1000BASEX] = PHY_INTERFACE_MODE_1000BASEX,
The switch can either take the MAC or the PHY role in an MII or RMII link. There are distinct PHY_INTERFACE_ macros for these two roles. Correct the mapping so that the `REV` version is used for the PHY role. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- Since this has not caused any known issues so far, i decided to not add a Fixes: tag and submit for net. drivers/net/dsa/mv88e6xxx/chip.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)