* [PATCH net-next 0/2] net: phy: at803x: paging support
@ 2021-02-14 1:04 Michael Walle
2021-02-14 1:04 ` [PATCH net-next 1/2] net: phy: at803x: add pages support to AR8031/33 Michael Walle
2021-02-14 1:04 ` [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done() Michael Walle
0 siblings, 2 replies; 8+ messages in thread
From: Michael Walle @ 2021-02-14 1:04 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Jakub Kicinski, Michael Walle
Add paging support to the QCA AR8031/33 PHY. This will be needed if we
add support for the .config_inband_aneg callback, see series [1]. But it
also turns out, that the driver already accessed the fiber page all along
without proper locking. Patch 2 will fix that.
[1] https://lore.kernel.org/netdev/20210212172341.3489046-1-olteanv@gmail.com/
Michael Walle (2):
net: phy: at803x: add pages support to AR8031/33
net: phy: at803x: use proper locking in at803x_aneg_done()
drivers/net/phy/at803x.c | 72 ++++++++++++++++++++++++++++------------
1 file changed, 51 insertions(+), 21 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] net: phy: at803x: add pages support to AR8031/33
2021-02-14 1:04 [PATCH net-next 0/2] net: phy: at803x: paging support Michael Walle
@ 2021-02-14 1:04 ` Michael Walle
2021-02-14 1:04 ` [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done() Michael Walle
1 sibling, 0 replies; 8+ messages in thread
From: Michael Walle @ 2021-02-14 1:04 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Jakub Kicinski, Michael Walle
The AR8031 has two register sets: Copper and Fiber. The fiber page is
used in case of 100Base-FX and 1000Base-X. But more importantly it is
also used for the SGMII link. Add support to switch between these two.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/net/phy/at803x.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index d67bddc111e3..a3aa10f14638 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -144,6 +144,9 @@
#define ATH8035_PHY_ID 0x004dd072
#define AT8030_PHY_ID_MASK 0xffffffef
+#define AT803X_FIBER_PAGE 0
+#define AT803X_COPPER_PAGE 1
+
MODULE_DESCRIPTION("Qualcomm Atheros AR803x PHY driver");
MODULE_AUTHOR("Matus Ujhelyi");
MODULE_LICENSE("GPL");
@@ -1143,6 +1146,36 @@ static int at803x_cable_test_start(struct phy_device *phydev)
return 0;
}
+static int at803x_read_page(struct phy_device *phydev)
+{
+ int val;
+
+ val = __phy_read(phydev, AT803X_REG_CHIP_CONFIG);
+ if (val < 0)
+ return val;
+
+ return (val & AT803X_BT_BX_REG_SEL) ? AT803X_COPPER_PAGE : AT803X_FIBER_PAGE;
+}
+
+static int at803x_write_page(struct phy_device *phydev, int page)
+{
+ u16 sel;
+
+ switch (page) {
+ case AT803X_FIBER_PAGE:
+ sel = 0;
+ break;
+ case AT803X_COPPER_PAGE:
+ sel = AT803X_BT_BX_REG_SEL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return __phy_modify(phydev, AT803X_REG_CHIP_CONFIG,
+ AT803X_BT_BX_REG_SEL, sel);
+}
+
static struct phy_driver at803x_driver[] = {
{
/* Qualcomm Atheros AR8035 */
@@ -1189,6 +1222,8 @@ static struct phy_driver at803x_driver[] = {
.flags = PHY_POLL_CABLE_TEST,
.probe = at803x_probe,
.remove = at803x_remove,
+ .read_page = at803x_read_page,
+ .write_page = at803x_write_page,
.config_init = at803x_config_init,
.soft_reset = genphy_soft_reset,
.set_wol = at803x_set_wol,
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 1:04 [PATCH net-next 0/2] net: phy: at803x: paging support Michael Walle
2021-02-14 1:04 ` [PATCH net-next 1/2] net: phy: at803x: add pages support to AR8031/33 Michael Walle
@ 2021-02-14 1:04 ` Michael Walle
2021-02-14 1:57 ` Vladimir Oltean
1 sibling, 1 reply; 8+ messages in thread
From: Michael Walle @ 2021-02-14 1:04 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller,
Jakub Kicinski, Michael Walle
at803x_aneg_done() checks if auto-negotiation is completed on the SGMII
side. This doesn't take the mdio bus lock and the page switching is
open-coded. Now that we have proper page support, just use
phy_read_paged(). Also use phydev->interface to check if we have an
SGMII link instead of reading the mode register and be a bit more
precise on the warning message.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/net/phy/at803x.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index a3aa10f14638..8abaea7ae6bd 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -756,32 +756,27 @@ static void at803x_link_change_notify(struct phy_device *phydev)
static int at803x_aneg_done(struct phy_device *phydev)
{
- int ccr;
-
- int aneg_done = genphy_aneg_done(phydev);
- if (aneg_done != BMSR_ANEGCOMPLETE)
- return aneg_done;
+ int ret, val;
- /*
- * in SGMII mode, if copper side autoneg is successful,
- * also check SGMII side autoneg result
- */
- ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
- if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
- return aneg_done;
- /* switch to SGMII/fiber page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
+ ret = genphy_aneg_done(phydev);
- /* check if the SGMII link is OK. */
- if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
- phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
- aneg_done = 0;
+ /* In SGMII mode, if copper side autoneg is successful, also check
+ * SGMII side autoneg result.
+ */
+ if (phydev->interface == PHY_INTERFACE_MODE_SGMII &&
+ ret == BMSR_ANEGCOMPLETE) {
+ val = phy_read_paged(phydev, AT803X_FIBER_PAGE, AT803X_PSSR);
+ if (val < 0)
+ return val;
+
+ if (!(val & AT803X_PSSR_MR_AN_COMPLETE)) {
+ phydev_warn(phydev, "SGMII autoneg isn't completed\n");
+ return 0;
+ }
}
- /* switch back to copper page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
- return aneg_done;
+ return ret;
}
static int at803x_read_status(struct phy_device *phydev)
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 1:04 ` [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done() Michael Walle
@ 2021-02-14 1:57 ` Vladimir Oltean
2021-02-14 2:18 ` Michael Walle
0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2021-02-14 1:57 UTC (permalink / raw)
To: Michael Walle
Cc: netdev, linux-kernel, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Jakub Kicinski
Hi Michael,
On Sun, Feb 14, 2021 at 02:04:05AM +0100, Michael Walle wrote:
> at803x_aneg_done() checks if auto-negotiation is completed on the SGMII
> side. This doesn't take the mdio bus lock and the page switching is
> open-coded. Now that we have proper page support, just use
> phy_read_paged(). Also use phydev->interface to check if we have an
> SGMII link instead of reading the mode register and be a bit more
> precise on the warning message.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
How did you test this patch?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 1:57 ` Vladimir Oltean
@ 2021-02-14 2:18 ` Michael Walle
2021-02-14 2:24 ` Vladimir Oltean
0 siblings, 1 reply; 8+ messages in thread
From: Michael Walle @ 2021-02-14 2:18 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, linux-kernel, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Jakub Kicinski
Am 14. Februar 2021 02:57:33 MEZ schrieb Vladimir Oltean <olteanv@gmail.com>:
>Hi Michael,
>
>On Sun, Feb 14, 2021 at 02:04:05AM +0100, Michael Walle wrote:
>> at803x_aneg_done() checks if auto-negotiation is completed on the
>SGMII
>> side. This doesn't take the mdio bus lock and the page switching is
>> open-coded. Now that we have proper page support, just use
>> phy_read_paged(). Also use phydev->interface to check if we have an
>> SGMII link instead of reading the mode register and be a bit more
>> precise on the warning message.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>
>How did you test this patch?
I'm afraid it's just compile time tested.
-michael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 2:18 ` Michael Walle
@ 2021-02-14 2:24 ` Vladimir Oltean
2021-02-14 20:48 ` Michael Walle
0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2021-02-14 2:24 UTC (permalink / raw)
To: Michael Walle
Cc: netdev, linux-kernel, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Jakub Kicinski
On Sun, Feb 14, 2021 at 03:18:49AM +0100, Michael Walle wrote:
> Am 14. Februar 2021 02:57:33 MEZ schrieb Vladimir Oltean <olteanv@gmail.com>:
> >Hi Michael,
> >
> >On Sun, Feb 14, 2021 at 02:04:05AM +0100, Michael Walle wrote:
> >> at803x_aneg_done() checks if auto-negotiation is completed on the
> >SGMII
> >> side. This doesn't take the mdio bus lock and the page switching is
> >> open-coded. Now that we have proper page support, just use
> >> phy_read_paged(). Also use phydev->interface to check if we have an
> >> SGMII link instead of reading the mode register and be a bit more
> >> precise on the warning message.
> >>
> >> Signed-off-by: Michael Walle <michael@walle.cc>
> >> ---
> >
> >How did you test this patch?
>
> I'm afraid it's just compile time tested.
I'm asking because at803x_aneg_done has been dead code for more than 2
years now. Unreachable. And while it was reachable it was buggy and an
abuse of the phylib API. So you might want to just delete this function
instead. Context:
https://lkml.org/lkml/2020/5/30/375
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 2:24 ` Vladimir Oltean
@ 2021-02-14 20:48 ` Michael Walle
2021-02-15 21:32 ` Vladimir Oltean
0 siblings, 1 reply; 8+ messages in thread
From: Michael Walle @ 2021-02-14 20:48 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, linux-kernel, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Jakub Kicinski
Am 2021-02-14 03:24, schrieb Vladimir Oltean:
> On Sun, Feb 14, 2021 at 03:18:49AM +0100, Michael Walle wrote:
>> Am 14. Februar 2021 02:57:33 MEZ schrieb Vladimir Oltean
>> <olteanv@gmail.com>:
>> >Hi Michael,
>> >
>> >On Sun, Feb 14, 2021 at 02:04:05AM +0100, Michael Walle wrote:
>> >> at803x_aneg_done() checks if auto-negotiation is completed on the
>> >SGMII
>> >> side. This doesn't take the mdio bus lock and the page switching is
>> >> open-coded. Now that we have proper page support, just use
>> >> phy_read_paged(). Also use phydev->interface to check if we have an
>> >> SGMII link instead of reading the mode register and be a bit more
>> >> precise on the warning message.
>> >>
>> >> Signed-off-by: Michael Walle <michael@walle.cc>
>> >> ---
>> >
>> >How did you test this patch?
>>
>> I'm afraid it's just compile time tested.
>
> I'm asking because at803x_aneg_done has been dead code for more than 2
> years now. Unreachable. And while it was reachable it was buggy and an
> abuse of the phylib API. So you might want to just delete this function
> instead. Context:
> https://lkml.org/lkml/2020/5/30/375
Are you sure? While it isn't called from phylib, it might be called from
some drivers directly or indirectly if they use phy_speed_down(). But
it is questionable if this is much of a use then.
That being said, if no one objects, I'd remove it, too.
-michael
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done()
2021-02-14 20:48 ` Michael Walle
@ 2021-02-15 21:32 ` Vladimir Oltean
0 siblings, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2021-02-15 21:32 UTC (permalink / raw)
To: Michael Walle
Cc: netdev, linux-kernel, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Jakub Kicinski
On Sun, Feb 14, 2021 at 09:48:53PM +0100, Michael Walle wrote:
> Am 2021-02-14 03:24, schrieb Vladimir Oltean:
> > On Sun, Feb 14, 2021 at 03:18:49AM +0100, Michael Walle wrote:
> > > Am 14. Februar 2021 02:57:33 MEZ schrieb Vladimir Oltean <olteanv@gmail.com>:
> > > >Hi Michael,
> > > >
> > > >On Sun, Feb 14, 2021 at 02:04:05AM +0100, Michael Walle wrote:
> > > >> at803x_aneg_done() checks if auto-negotiation is completed on the SGMII
> > > >> side. This doesn't take the mdio bus lock and the page switching is
> > > >> open-coded. Now that we have proper page support, just use
> > > >> phy_read_paged(). Also use phydev->interface to check if we have an
> > > >> SGMII link instead of reading the mode register and be a bit more
> > > >> precise on the warning message.
> > > >>
> > > >> Signed-off-by: Michael Walle <michael@walle.cc>
> > > >> ---
> > > >
> > > >How did you test this patch?
> > >
> > > I'm afraid it's just compile time tested.
> >
> > I'm asking because at803x_aneg_done has been dead code for more than 2
> > years now. Unreachable. And while it was reachable it was buggy and an
> > abuse of the phylib API. So you might want to just delete this function
> > instead. Context:
> > https://lkml.org/lkml/2020/5/30/375
>
> Are you sure? While it isn't called from phylib, it might be called from
> some drivers directly or indirectly if they use phy_speed_down().
> But it is questionable if this is much of a use then.
>
> That being said, if no one objects, I'd remove it, too.
It might, true, but it is certainly of no use.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-15 21:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-14 1:04 [PATCH net-next 0/2] net: phy: at803x: paging support Michael Walle
2021-02-14 1:04 ` [PATCH net-next 1/2] net: phy: at803x: add pages support to AR8031/33 Michael Walle
2021-02-14 1:04 ` [PATCH net-next 2/2] net: phy: at803x: use proper locking in at803x_aneg_done() Michael Walle
2021-02-14 1:57 ` Vladimir Oltean
2021-02-14 2:18 ` Michael Walle
2021-02-14 2:24 ` Vladimir Oltean
2021-02-14 20:48 ` Michael Walle
2021-02-15 21:32 ` Vladimir Oltean
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).