| Message ID | 20220110062117.17540-2-mohammad.athari.ismail@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: marvell: add Marvell specific PHY loopback | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 20 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 10.01.2022 07:21, Mohammad Athari Bin Ismail wrote: > Existing genphy_loopback() is not applicable for Marvell PHY. So, > adding Marvell specific PHY loopback operation by only setting(enable) or > clearing(disable) BMCR_LOOPBACK bit. > > Tested working on Marvell 88E1510. > With this change you'd basically revert the original change and loose its functionality. Did you check the Marvell datasheets? At least for few versions I found that you may have to configure bits 0..2 in MAC Specific Control Register 2 (page 2, register 21) instead of BMCR. > Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed configuration") > Cc: <stable@vger.kernel.org> # 5.15.x > Signed-off-by: Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com> > --- > drivers/net/phy/marvell.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 4fcfca4e1702..2a73a959b48b 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct phy_device *phydev, > data[i] = marvell_get_stat(phydev, i); > } > > +static int marvell_loopback(struct phy_device *phydev, bool enable) > +{ > + return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, > + enable ? BMCR_LOOPBACK : 0); > +} > + > static int marvell_vct5_wait_complete(struct phy_device *phydev) > { > int i; > @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { > .get_sset_count = marvell_get_sset_count, > .get_strings = marvell_get_strings, > .get_stats = marvell_get_stats, > - .set_loopback = genphy_loopback, > + .set_loopback = marvell_loopback, > .get_tunable = m88e1011_get_tunable, > .set_tunable = m88e1011_set_tunable, > .cable_test_start = marvell_vct7_cable_test_start,
> -----Original Message----- > From: Heiner Kallweit <hkallweit1@gmail.com> > Sent: Monday, January 10, 2022 4:34 PM > To: Ismail, Mohammad Athari <mohammad.athari.ismail@intel.com>; > Andrew Lunn <andrew@lunn.ch>; David S . Miller <davem@davemloft.net>; > Jakub Kicinski <kuba@kernel.org>; Oleksij Rempel <linux@rempel- > privat.de>; Russell King <linux@armlinux.org.uk> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > stable@vger.kernel.org > Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific PHY > loopback > > On 10.01.2022 07:21, Mohammad Athari Bin Ismail wrote: > > Existing genphy_loopback() is not applicable for Marvell PHY. So, > > adding Marvell specific PHY loopback operation by only setting(enable) > > or > > clearing(disable) BMCR_LOOPBACK bit. > > > > Tested working on Marvell 88E1510. > > > With this change you'd basically revert the original change and loose its > functionality. Did you check the Marvell datasheets? > At least for few versions I found that you may have to configure bits 0..2 in > MAC Specific Control Register 2 (page 2, register 21) instead of BMCR. May I know what datasheet version that has the bits 2:0's detail explanation? The version that I have, bits 2:0 in MAC Specific Control Register 2 shows as Reserved. The datasheet I have is "Marvell Alaska 88E1510/88E1518/88E1512/88E1514 Integrated 10/100/1000 Mbps Energy Efficient Ethernet Transceiver Rev. G December 17, 2021" Really appreciate if you could advice on PHY loopback enabling for Marvell 88E1510 because the existing genphy_loopback() function doesn't work for the PHY. Thank you. -Athari- > > > > Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed > > configuration") > > Cc: <stable@vger.kernel.org> # 5.15.x > > Signed-off-by: Mohammad Athari Bin Ismail > > <mohammad.athari.ismail@intel.com> > > --- > > drivers/net/phy/marvell.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > > index 4fcfca4e1702..2a73a959b48b 100644 > > --- a/drivers/net/phy/marvell.c > > +++ b/drivers/net/phy/marvell.c > > @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct phy_device > *phydev, > > data[i] = marvell_get_stat(phydev, i); } > > > > +static int marvell_loopback(struct phy_device *phydev, bool enable) { > > + return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, > > + enable ? BMCR_LOOPBACK : 0); > > +} > > + > > static int marvell_vct5_wait_complete(struct phy_device *phydev) { > > int i; > > @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { > > .get_sset_count = marvell_get_sset_count, > > .get_strings = marvell_get_strings, > > .get_stats = marvell_get_stats, > > - .set_loopback = genphy_loopback, > > + .set_loopback = marvell_loopback, > > .get_tunable = m88e1011_get_tunable, > > .set_tunable = m88e1011_set_tunable, > > .cable_test_start = marvell_vct7_cable_test_start,
On 10.01.2022 10:36, Ismail, Mohammad Athari wrote: > > >> -----Original Message----- >> From: Heiner Kallweit <hkallweit1@gmail.com> >> Sent: Monday, January 10, 2022 4:34 PM >> To: Ismail, Mohammad Athari <mohammad.athari.ismail@intel.com>; >> Andrew Lunn <andrew@lunn.ch>; David S . Miller <davem@davemloft.net>; >> Jakub Kicinski <kuba@kernel.org>; Oleksij Rempel <linux@rempel- >> privat.de>; Russell King <linux@armlinux.org.uk> >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; >> stable@vger.kernel.org >> Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific PHY >> loopback >> >> On 10.01.2022 07:21, Mohammad Athari Bin Ismail wrote: >>> Existing genphy_loopback() is not applicable for Marvell PHY. So, >>> adding Marvell specific PHY loopback operation by only setting(enable) >>> or >>> clearing(disable) BMCR_LOOPBACK bit. >>> >>> Tested working on Marvell 88E1510. >>> >> With this change you'd basically revert the original change and loose its >> functionality. Did you check the Marvell datasheets? >> At least for few versions I found that you may have to configure bits 0..2 in >> MAC Specific Control Register 2 (page 2, register 21) instead of BMCR. > > May I know what datasheet version that has the bits 2:0's detail explanation? The version that I have, bits 2:0 in MAC Specific Control Register 2 shows as Reserved. > The datasheet I have is "Marvell Alaska 88E1510/88E1518/88E1512/88E1514 Integrated 10/100/1000 Mbps Energy Efficient Ethernet Transceiver Rev. G December 17, 2021" > I checked the 88E6352 switch chip datasheet. The part covering the integrated PHY's lists the mentioned bits in MAC Specific Control Register 2. Table 75 in the 88E1510 datasheet says: Loopback speed is determined by Registers 21_2.6,13. So Marvell PHY's seem to use different bits (although same register) for loopback speed configuration. > Really appreciate if you could advice on PHY loopback enabling for Marvell 88E1510 because the existing genphy_loopback() function doesn't work for the PHY. > > Thank you. > > -Athari- > >> >> >>> Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed >>> configuration") >>> Cc: <stable@vger.kernel.org> # 5.15.x >>> Signed-off-by: Mohammad Athari Bin Ismail >>> <mohammad.athari.ismail@intel.com> >>> --- >>> drivers/net/phy/marvell.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c >>> index 4fcfca4e1702..2a73a959b48b 100644 >>> --- a/drivers/net/phy/marvell.c >>> +++ b/drivers/net/phy/marvell.c >>> @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct phy_device >> *phydev, >>> data[i] = marvell_get_stat(phydev, i); } >>> >>> +static int marvell_loopback(struct phy_device *phydev, bool enable) { >>> + return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, >>> + enable ? BMCR_LOOPBACK : 0); >>> +} >>> + >>> static int marvell_vct5_wait_complete(struct phy_device *phydev) { >>> int i; >>> @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { >>> .get_sset_count = marvell_get_sset_count, >>> .get_strings = marvell_get_strings, >>> .get_stats = marvell_get_stats, >>> - .set_loopback = genphy_loopback, >>> + .set_loopback = marvell_loopback, >>> .get_tunable = m88e1011_get_tunable, >>> .set_tunable = m88e1011_set_tunable, >>> .cable_test_start = marvell_vct7_cable_test_start, >
> -----Original Message----- > From: Heiner Kallweit <hkallweit1@gmail.com> > Sent: Monday, January 10, 2022 6:17 PM > To: Ismail, Mohammad Athari <mohammad.athari.ismail@intel.com>; > Andrew Lunn <andrew@lunn.ch>; David S . Miller <davem@davemloft.net>; > Jakub Kicinski <kuba@kernel.org>; Oleksij Rempel <linux@rempel- > privat.de>; Russell King <linux@armlinux.org.uk> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > stable@vger.kernel.org > Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific PHY > loopback > > On 10.01.2022 10:36, Ismail, Mohammad Athari wrote: > > > > > >> -----Original Message----- > >> From: Heiner Kallweit <hkallweit1@gmail.com> > >> Sent: Monday, January 10, 2022 4:34 PM > >> To: Ismail, Mohammad Athari <mohammad.athari.ismail@intel.com>; > >> Andrew Lunn <andrew@lunn.ch>; David S . Miller > <davem@davemloft.net>; > >> Jakub Kicinski <kuba@kernel.org>; Oleksij Rempel <linux@rempel- > >> privat.de>; Russell King <linux@armlinux.org.uk> > >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > >> stable@vger.kernel.org > >> Subject: Re: [PATCH net 1/1] net: phy: marvell: add Marvell specific > >> PHY loopback > >> > >> On 10.01.2022 07:21, Mohammad Athari Bin Ismail wrote: > >>> Existing genphy_loopback() is not applicable for Marvell PHY. So, > >>> adding Marvell specific PHY loopback operation by only > >>> setting(enable) or > >>> clearing(disable) BMCR_LOOPBACK bit. > >>> > >>> Tested working on Marvell 88E1510. > >>> > >> With this change you'd basically revert the original change and loose > >> its functionality. Did you check the Marvell datasheets? > >> At least for few versions I found that you may have to configure bits > >> 0..2 in MAC Specific Control Register 2 (page 2, register 21) instead of > BMCR. > > > > May I know what datasheet version that has the bits 2:0's detail > explanation? The version that I have, bits 2:0 in MAC Specific Control Register > 2 shows as Reserved. > > The datasheet I have is "Marvell Alaska 88E1510/88E1518/88E1512/88E1514 > Integrated 10/100/1000 Mbps Energy Efficient Ethernet Transceiver Rev. G > December 17, 2021" > > > I checked the 88E6352 switch chip datasheet. The part covering the > integrated PHY's lists the mentioned bits in MAC Specific Control Register 2. > > Table 75 in the 88E1510 datasheet says: Loopback speed is determined by > Registers 21_2.6,13. > So Marvell PHY's seem to use different bits (although same register) for > loopback speed configuration. I get your point. Registers 21_2.6,13 also need to be configured for loopback to work for all speeds. I'll send the v2 patch later. -Athari- > > > Really appreciate if you could advice on PHY loopback enabling for Marvell > 88E1510 because the existing genphy_loopback() function doesn't work for > the PHY. > > > > Thank you. > > > > -Athari- > > > >> > >> > >>> Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed > >>> configuration") > >>> Cc: <stable@vger.kernel.org> # 5.15.x > >>> Signed-off-by: Mohammad Athari Bin Ismail > >>> <mohammad.athari.ismail@intel.com> > >>> --- > >>> drivers/net/phy/marvell.c | 8 +++++++- > >>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > >>> index 4fcfca4e1702..2a73a959b48b 100644 > >>> --- a/drivers/net/phy/marvell.c > >>> +++ b/drivers/net/phy/marvell.c > >>> @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct > >>> phy_device > >> *phydev, > >>> data[i] = marvell_get_stat(phydev, i); } > >>> > >>> +static int marvell_loopback(struct phy_device *phydev, bool enable) { > >>> + return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, > >>> + enable ? BMCR_LOOPBACK : 0); > >>> +} > >>> + > >>> static int marvell_vct5_wait_complete(struct phy_device *phydev) { > >>> int i; > >>> @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { > >>> .get_sset_count = marvell_get_sset_count, > >>> .get_strings = marvell_get_strings, > >>> .get_stats = marvell_get_stats, > >>> - .set_loopback = genphy_loopback, > >>> + .set_loopback = marvell_loopback, > >>> .get_tunable = m88e1011_get_tunable, > >>> .set_tunable = m88e1011_set_tunable, > >>> .cable_test_start = marvell_vct7_cable_test_start, > >
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 4fcfca4e1702..2a73a959b48b 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1932,6 +1932,12 @@ static void marvell_get_stats(struct phy_device *phydev, data[i] = marvell_get_stat(phydev, i); } +static int marvell_loopback(struct phy_device *phydev, bool enable) +{ + return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, + enable ? BMCR_LOOPBACK : 0); +} + static int marvell_vct5_wait_complete(struct phy_device *phydev) { int i; @@ -3078,7 +3084,7 @@ static struct phy_driver marvell_drivers[] = { .get_sset_count = marvell_get_sset_count, .get_strings = marvell_get_strings, .get_stats = marvell_get_stats, - .set_loopback = genphy_loopback, + .set_loopback = marvell_loopback, .get_tunable = m88e1011_get_tunable, .set_tunable = m88e1011_set_tunable, .cable_test_start = marvell_vct7_cable_test_start,
Existing genphy_loopback() is not applicable for Marvell PHY. So, adding Marvell specific PHY loopback operation by only setting(enable) or clearing(disable) BMCR_LOOPBACK bit. Tested working on Marvell 88E1510. Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed configuration") Cc: <stable@vger.kernel.org> # 5.15.x Signed-off-by: Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com> --- drivers/net/phy/marvell.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)