* [PATCH net-next 1/7] net: freescale: ucc_geth: Drop support for the "interface" DT property
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:34 ` Andrew Lunn
2024-11-07 17:02 ` [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion Maxime Chevallier
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
In april 2007, ucc_geth was converted to phylib with :
commit 728de4c927a3 ("ucc_geth: migrate ucc_geth to phylib").
In that commit, the device-tree property "interface", that could be used to
retrieve the PHY interface mode was deprecated.
DTS files that still used that property were converted along the way, in
the following commit, also dating from april 2007 :
commit 0fd8c47cccb1 ("[POWERPC] Replace undocumented interface properties in dts files")
17 years later, there's no users of that property left and I hope it's
safe to say we can remove support from that in the ucc_geth driver,
making the probe() function a bit simpler.
Should there be any users that have a DT that was generated when 2.6.21 was
cutting-edge, print an error message with hints on how to convert the
devicetree if the 'interface' property is found.
With that property gone, we can greatly simplify the parsing of the
phy-interface-mode from the devicetree by using of_get_phy_mode(),
allowing the removal of the open-coded parsing in the driver.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 63 +++++------------------
1 file changed, 12 insertions(+), 51 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 6663c1768089..80540c817c4e 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3469,32 +3469,6 @@ static int ucc_geth_resume(struct platform_device *ofdev)
#define ucc_geth_resume NULL
#endif
-static phy_interface_t to_phy_interface(const char *phy_connection_type)
-{
- if (strcasecmp(phy_connection_type, "mii") == 0)
- return PHY_INTERFACE_MODE_MII;
- if (strcasecmp(phy_connection_type, "gmii") == 0)
- return PHY_INTERFACE_MODE_GMII;
- if (strcasecmp(phy_connection_type, "tbi") == 0)
- return PHY_INTERFACE_MODE_TBI;
- if (strcasecmp(phy_connection_type, "rmii") == 0)
- return PHY_INTERFACE_MODE_RMII;
- if (strcasecmp(phy_connection_type, "rgmii") == 0)
- return PHY_INTERFACE_MODE_RGMII;
- if (strcasecmp(phy_connection_type, "rgmii-id") == 0)
- return PHY_INTERFACE_MODE_RGMII_ID;
- if (strcasecmp(phy_connection_type, "rgmii-txid") == 0)
- return PHY_INTERFACE_MODE_RGMII_TXID;
- if (strcasecmp(phy_connection_type, "rgmii-rxid") == 0)
- return PHY_INTERFACE_MODE_RGMII_RXID;
- if (strcasecmp(phy_connection_type, "rtbi") == 0)
- return PHY_INTERFACE_MODE_RTBI;
- if (strcasecmp(phy_connection_type, "sgmii") == 0)
- return PHY_INTERFACE_MODE_SGMII;
-
- return PHY_INTERFACE_MODE_MII;
-}
-
static int ucc_geth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
@@ -3564,19 +3538,6 @@ static int ucc_geth_probe(struct platform_device* ofdev)
int err, ucc_num, max_speed = 0;
const unsigned int *prop;
phy_interface_t phy_interface;
- static const int enet_to_speed[] = {
- SPEED_10, SPEED_10, SPEED_10,
- SPEED_100, SPEED_100, SPEED_100,
- SPEED_1000, SPEED_1000, SPEED_1000, SPEED_1000,
- };
- static const phy_interface_t enet_to_phy_interface[] = {
- PHY_INTERFACE_MODE_MII, PHY_INTERFACE_MODE_RMII,
- PHY_INTERFACE_MODE_RGMII, PHY_INTERFACE_MODE_MII,
- PHY_INTERFACE_MODE_RMII, PHY_INTERFACE_MODE_RGMII,
- PHY_INTERFACE_MODE_GMII, PHY_INTERFACE_MODE_RGMII,
- PHY_INTERFACE_MODE_TBI, PHY_INTERFACE_MODE_RTBI,
- PHY_INTERFACE_MODE_SGMII,
- };
ugeth_vdbg("%s: IN", __func__);
@@ -3627,18 +3588,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
- /* get the phy interface type, or default to MII */
- prop = of_get_property(np, "phy-connection-type", NULL);
- if (!prop) {
- /* handle interface property present in old trees */
- prop = of_get_property(ug_info->phy_node, "interface", NULL);
- if (prop != NULL) {
- phy_interface = enet_to_phy_interface[*prop];
- max_speed = enet_to_speed[*prop];
- } else
- phy_interface = PHY_INTERFACE_MODE_MII;
- } else {
- phy_interface = to_phy_interface((const char *)prop);
+ prop = of_get_property(ug_info->phy_node, "interface", NULL);
+ if (prop) {
+ dev_err(&ofdev->dev,
+ "Device-tree property 'interface' is no longer supported. Please use 'phy-connection-type' instead.");
+ goto err_put_tbi;
+ }
+
+ err = of_get_phy_mode(np, &phy_interface);
+ if (err) {
+ dev_err(&ofdev->dev, "Invalid phy-connection-type");
+ goto err_put_tbi;
}
/* get speed, or derive from PHY interface */
@@ -3746,6 +3706,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
err_deregister_fixed_link:
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
+err_put_tbi:
of_node_put(ug_info->tbi_node);
of_node_put(ug_info->phy_node);
return err;
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 1/7] net: freescale: ucc_geth: Drop support for the "interface" DT property
2024-11-07 17:02 ` [PATCH net-next 1/7] net: freescale: ucc_geth: Drop support for the "interface" DT property Maxime Chevallier
@ 2024-11-07 17:34 ` Andrew Lunn
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:34 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:48PM +0100, Maxime Chevallier wrote:
> In april 2007, ucc_geth was converted to phylib with :
>
> commit 728de4c927a3 ("ucc_geth: migrate ucc_geth to phylib").
>
> In that commit, the device-tree property "interface", that could be used to
> retrieve the PHY interface mode was deprecated.
>
> DTS files that still used that property were converted along the way, in
> the following commit, also dating from april 2007 :
>
> commit 0fd8c47cccb1 ("[POWERPC] Replace undocumented interface properties in dts files")
>
> 17 years later, there's no users of that property left and I hope it's
> safe to say we can remove support from that in the ucc_geth driver,
> making the probe() function a bit simpler.
17 years seems reasonable.
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
2024-11-07 17:02 ` [PATCH net-next 1/7] net: freescale: ucc_geth: Drop support for the "interface" DT property Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:36 ` Andrew Lunn
2024-11-07 17:51 ` Russell King (Oracle)
2024-11-07 17:02 ` [PATCH net-next 3/7] net: freescale: ucc_geth: Use netdev->phydev to access the PHY Maxime Chevallier
` (4 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
Preparing the phylink conversion, split the adjust_link callbaclk, by
clearly separating the mac configuration, link_up and link_down phases.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 180 +++++++++++-----------
1 file changed, 93 insertions(+), 87 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 80540c817c4e..6286cd185a35 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1548,105 +1548,111 @@ static void ugeth_activate(struct ucc_geth_private *ugeth)
__netdev_watchdog_up(ugeth->ndev);
}
-/* Called every time the controller might need to be made
- * aware of new link state. The PHY code conveys this
- * information through variables in the ugeth structure, and this
- * function converts those variables into the appropriate
- * register values, and can bring down the device if needed.
- */
-
-static void adjust_link(struct net_device *dev)
+static void ugeth_link_up(struct ucc_geth_private *ugeth,
+ struct phy_device *phy,
+ phy_interface_t interface, int speed, int duplex)
{
- struct ucc_geth_private *ugeth = netdev_priv(dev);
- struct ucc_geth __iomem *ug_regs;
- struct ucc_fast __iomem *uf_regs;
- struct phy_device *phydev = ugeth->phydev;
+ struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
+ struct ucc_fast __iomem *uf_regs = ugeth->uccf->uf_regs;
+ u32 tempval = in_be32(&ug_regs->maccfg2);
+ u32 upsmr = in_be32(&uf_regs->upsmr);
int new_state = 0;
- ug_regs = ugeth->ug_regs;
- uf_regs = ugeth->uccf->uf_regs;
-
- if (phydev->link) {
- u32 tempval = in_be32(&ug_regs->maccfg2);
- u32 upsmr = in_be32(&uf_regs->upsmr);
- /* Now we make sure that we can be in full duplex mode.
- * If not, we operate in half-duplex mode. */
- if (phydev->duplex != ugeth->oldduplex) {
- new_state = 1;
- if (!(phydev->duplex))
- tempval &= ~(MACCFG2_FDX);
- else
- tempval |= MACCFG2_FDX;
- ugeth->oldduplex = phydev->duplex;
- }
+ /* Now we make sure that we can be in full duplex mode.
+ * If not, we operate in half-duplex mode.
+ */
+ if (duplex != ugeth->oldduplex) {
+ new_state = 1;
+ if (duplex == DUPLEX_HALF)
+ tempval &= ~(MACCFG2_FDX);
+ else
+ tempval |= MACCFG2_FDX;
+ ugeth->oldduplex = duplex;
+ }
- if (phydev->speed != ugeth->oldspeed) {
- new_state = 1;
- switch (phydev->speed) {
- case SPEED_1000:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_BYTE);
- break;
- case SPEED_100:
- case SPEED_10:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_NIBBLE);
- /* if reduced mode, re-set UPSMR.R10M */
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
- if (phydev->speed == SPEED_10)
- upsmr |= UCC_GETH_UPSMR_R10M;
- else
- upsmr &= ~UCC_GETH_UPSMR_R10M;
- }
- break;
- default:
- if (netif_msg_link(ugeth))
- pr_warn(
- "%s: Ack! Speed (%d) is not 10/100/1000!",
- dev->name, phydev->speed);
- break;
+ if (speed != ugeth->oldspeed) {
+ new_state = 1;
+ switch (speed) {
+ case SPEED_1000:
+ tempval = ((tempval &
+ ~(MACCFG2_INTERFACE_MODE_MASK)) |
+ MACCFG2_INTERFACE_MODE_BYTE);
+ break;
+ case SPEED_100:
+ case SPEED_10:
+ tempval = ((tempval &
+ ~(MACCFG2_INTERFACE_MODE_MASK)) |
+ MACCFG2_INTERFACE_MODE_NIBBLE);
+ /* if reduced mode, re-set UPSMR.R10M */
+ if (interface == PHY_INTERFACE_MODE_RMII ||
+ phy_interface_mode_is_rgmii(interface) ||
+ interface == PHY_INTERFACE_MODE_RTBI) {
+ if (speed == SPEED_10)
+ upsmr |= UCC_GETH_UPSMR_R10M;
+ else
+ upsmr &= ~UCC_GETH_UPSMR_R10M;
}
- ugeth->oldspeed = phydev->speed;
+ break;
+ default:
+ if (netif_msg_link(ugeth))
+ pr_warn("%s: Speed (%d) is not 10/100/1000!",
+ netdev_name(ugeth->ndev), speed);
+ break;
}
+ ugeth->oldspeed = speed;
+ }
- if (!ugeth->oldlink) {
- new_state = 1;
- ugeth->oldlink = 1;
- }
+ if (!ugeth->oldlink) {
+ new_state = 1;
+ ugeth->oldlink = 1;
+ }
- if (new_state) {
- /*
- * To change the MAC configuration we need to disable
- * the controller. To do so, we have to either grab
- * ugeth->lock, which is a bad idea since 'graceful
- * stop' commands might take quite a while, or we can
- * quiesce driver's activity.
- */
- ugeth_quiesce(ugeth);
- ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
+ if (new_state) {
+ /*
+ * To change the MAC configuration we need to disable
+ * the controller. To do so, we have to either grab
+ * ugeth->lock, which is a bad idea since 'graceful
+ * stop' commands might take quite a while, or we can
+ * quiesce driver's activity.
+ */
+ ugeth_quiesce(ugeth);
+ ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
- out_be32(&ug_regs->maccfg2, tempval);
- out_be32(&uf_regs->upsmr, upsmr);
+ out_be32(&ug_regs->maccfg2, tempval);
+ out_be32(&uf_regs->upsmr, upsmr);
- ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
- ugeth_activate(ugeth);
- }
- } else if (ugeth->oldlink) {
- new_state = 1;
- ugeth->oldlink = 0;
- ugeth->oldspeed = 0;
- ugeth->oldduplex = -1;
+ ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
+ ugeth_activate(ugeth);
}
- if (new_state && netif_msg_link(ugeth))
- phy_print_status(phydev);
+ if (netif_msg_link(ugeth))
+ phy_print_status(phy);
+}
+
+static void ugeth_link_down(struct ucc_geth_private *ugeth)
+{
+ ugeth->oldlink = 0;
+ ugeth->oldspeed = 0;
+ ugeth->oldduplex = -1;
+}
+
+/* Called every time the controller might need to be made
+ * aware of new link state. The PHY code conveys this
+ * information through variables in the ugeth structure, and this
+ * function converts those variables into the appropriate
+ * register values, and can bring down the device if needed.
+ */
+
+static void adjust_link(struct net_device *dev)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(dev);
+ struct phy_device *phydev = ugeth->phydev;
+
+ if (phydev->link)
+ ugeth_link_up(ugeth, phydev, phydev->interface,
+ phydev->speed, phydev->duplex);
+ else
+ ugeth_link_down(ugeth);
}
/* Initialize TBI PHY interface for communicating with the
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion
2024-11-07 17:02 ` [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion Maxime Chevallier
@ 2024-11-07 17:36 ` Andrew Lunn
2024-11-07 17:51 ` Russell King (Oracle)
1 sibling, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:36 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:49PM +0100, Maxime Chevallier wrote:
> Preparing the phylink conversion, split the adjust_link callbaclk, by
> clearly separating the mac configuration, link_up and link_down phases.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion
2024-11-07 17:02 ` [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion Maxime Chevallier
2024-11-07 17:36 ` Andrew Lunn
@ 2024-11-07 17:51 ` Russell King (Oracle)
2024-11-07 18:03 ` Maxime Chevallier
1 sibling, 1 reply; 17+ messages in thread
From: Russell King (Oracle) @ 2024-11-07 17:51 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:49PM +0100, Maxime Chevallier wrote:
> Preparing the phylink conversion, split the adjust_link callbaclk, by
> clearly separating the mac configuration, link_up and link_down phases.
I'm not entirely sure what the point of this patch is, given that in
patch 7, all this code gets deleted, or maybe moved?
If it's moved, it may be better in patch 7 to ensure that doesn't
happen, and move it in a separate patch - right now patch 7 is horrible
to review as there's no way to see what the changes are in these
link_up()/link_down() functions.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion
2024-11-07 17:51 ` Russell King (Oracle)
@ 2024-11-07 18:03 ` Maxime Chevallier
0 siblings, 0 replies; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 18:03 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
Hello Russell,
On Thu, 7 Nov 2024 17:51:05 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> On Thu, Nov 07, 2024 at 06:02:49PM +0100, Maxime Chevallier wrote:
> > Preparing the phylink conversion, split the adjust_link callbaclk, by
> > clearly separating the mac configuration, link_up and link_down phases.
>
> I'm not entirely sure what the point of this patch is, given that in
> patch 7, all this code gets deleted, or maybe moved?
>
> If it's moved, it may be better in patch 7 to ensure that doesn't
> happen, and move it in a separate patch - right now patch 7 is horrible
> to review as there's no way to see what the changes are in these
> link_up()/link_down() functions.
I agree that it's hard to review indeed... I followed the documented
approach of splitting-up the adjust_link callback then performing the
conversion, but it's true that it doesn't really make patch 7 more
readable.
I can try to move things around and make patch 7 a bit more
straightforward.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 3/7] net: freescale: ucc_geth: Use netdev->phydev to access the PHY
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
2024-11-07 17:02 ` [PATCH net-next 1/7] net: freescale: ucc_geth: Drop support for the "interface" DT property Maxime Chevallier
2024-11-07 17:02 ` [PATCH net-next 2/7] net: freescale: ucc_geth: split adjust_link for phylink conversion Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:37 ` Andrew Lunn
2024-11-07 17:02 ` [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration Maxime Chevallier
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
As this driver pre-dates phylib, it uses a private pointer to get a
reference to the attached phy_device. Drop that pointer and use the
netdev's pointer instead.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 27 ++++++++-----------
drivers/net/ethernet/freescale/ucc_geth.h | 1 -
.../net/ethernet/freescale/ucc_geth_ethtool.c | 17 ++++++------
3 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 6286cd185a35..13b8f8401c81 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1646,7 +1646,7 @@ static void ugeth_link_down(struct ucc_geth_private *ugeth)
static void adjust_link(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = dev->phydev;
if (phydev->link)
ugeth_link_up(ugeth, phydev, phydev->interface,
@@ -1727,8 +1727,6 @@ static int init_phy(struct net_device *dev)
phy_set_max_speed(phydev, priv->max_speed);
- priv->phydev = phydev;
-
return 0;
}
@@ -2001,7 +1999,7 @@ static void ucc_geth_set_multi(struct net_device *dev)
static void ucc_geth_stop(struct ucc_geth_private *ugeth)
{
struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = ugeth->ndev->phydev;
ugeth_vdbg("%s: IN", __func__);
@@ -3316,13 +3314,13 @@ static int ucc_geth_open(struct net_device *dev)
goto err;
}
- phy_start(ugeth->phydev);
+ phy_start(dev->phydev);
napi_enable(&ugeth->napi);
netdev_reset_queue(dev);
netif_start_queue(dev);
device_set_wakeup_capable(&dev->dev,
- qe_alive_during_sleep() || ugeth->phydev->irq);
+ qe_alive_during_sleep() || dev->phydev->irq);
device_set_wakeup_enable(&dev->dev, ugeth->wol_en);
return err;
@@ -3343,8 +3341,7 @@ static int ucc_geth_close(struct net_device *dev)
cancel_work_sync(&ugeth->timeout_work);
ucc_geth_stop(ugeth);
- phy_disconnect(ugeth->phydev);
- ugeth->phydev = NULL;
+ phy_disconnect(dev->phydev);
free_irq(ugeth->ug_info->uf_info.irq, ugeth->ndev);
@@ -3378,7 +3375,7 @@ static void ucc_geth_timeout_work(struct work_struct *work)
ucc_geth_stop(ugeth);
ucc_geth_init_mac(ugeth);
/* Must start PHY here */
- phy_start(ugeth->phydev);
+ phy_start(dev->phydev);
netif_tx_start_all_queues(dev);
}
@@ -3421,7 +3418,7 @@ static int ucc_geth_suspend(struct platform_device *ofdev, pm_message_t state)
setbits32(&ugeth->ug_regs->maccfg2, MACCFG2_MPE);
ucc_fast_enable(ugeth->uccf, COMM_DIR_RX_AND_TX);
} else if (!(ugeth->wol_en & WAKE_PHY)) {
- phy_stop(ugeth->phydev);
+ phy_stop(ndev->phydev);
}
return 0;
@@ -3461,8 +3458,8 @@ static int ucc_geth_resume(struct platform_device *ofdev)
ugeth->oldspeed = 0;
ugeth->oldduplex = -1;
- phy_stop(ugeth->phydev);
- phy_start(ugeth->phydev);
+ phy_stop(ndev->phydev);
+ phy_start(ndev->phydev);
napi_enable(&ugeth->napi);
netif_device_attach(ndev);
@@ -3477,15 +3474,13 @@ static int ucc_geth_resume(struct platform_device *ofdev)
static int ucc_geth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
- struct ucc_geth_private *ugeth = netdev_priv(dev);
-
if (!netif_running(dev))
return -EINVAL;
- if (!ugeth->phydev)
+ if (!dev->phydev)
return -ENODEV;
- return phy_mii_ioctl(ugeth->phydev, rq, cmd);
+ return phy_mii_ioctl(dev->phydev, rq, cmd);
}
static const struct net_device_ops ucc_geth_netdev_ops = {
diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
index 4294ed096ebb..c08a56b7c9fe 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.h
+++ b/drivers/net/ethernet/freescale/ucc_geth.h
@@ -1210,7 +1210,6 @@ struct ucc_geth_private {
u16 skb_dirtytx[NUM_TX_QUEUES];
struct ugeth_mii_info *mii_info;
- struct phy_device *phydev;
phy_interface_t phy_interface;
int max_speed;
uint32_t msg_enable;
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index 699f346faf5c..fb5254d7d1ba 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -103,8 +103,7 @@ static const char rx_fw_stat_gstrings[][ETH_GSTRING_LEN] = {
static int
uec_get_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd)
{
- struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = netdev->phydev;
if (!phydev)
return -ENODEV;
@@ -118,8 +117,7 @@ static int
uec_set_ksettings(struct net_device *netdev,
const struct ethtool_link_ksettings *cmd)
{
- struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = netdev->phydev;
if (!phydev)
return -ENODEV;
@@ -132,8 +130,10 @@ uec_get_pauseparam(struct net_device *netdev,
struct ethtool_pauseparam *pause)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
+ struct phy_device *phydev = netdev->phydev;
- pause->autoneg = ugeth->phydev->autoneg;
+ if (phydev)
+ pause->autoneg = phydev->autoneg;
if (ugeth->ug_info->receiveFlowControl)
pause->rx_pause = 1;
@@ -146,12 +146,13 @@ uec_set_pauseparam(struct net_device *netdev,
struct ethtool_pauseparam *pause)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
+ struct phy_device *phydev = netdev->phydev;
int ret = 0;
ugeth->ug_info->receiveFlowControl = pause->rx_pause;
ugeth->ug_info->transmitFlowControl = pause->tx_pause;
- if (ugeth->phydev->autoneg) {
+ if (phydev && phydev->autoneg) {
if (netif_running(netdev)) {
/* FIXME: automatically restart */
netdev_info(netdev, "Please re-open the interface\n");
@@ -343,7 +344,7 @@ uec_get_drvinfo(struct net_device *netdev,
static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = netdev->phydev;
if (phydev && phydev->irq)
wol->supported |= WAKE_PHY;
@@ -356,7 +357,7 @@ static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
static int uec_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = ugeth->phydev;
+ struct phy_device *phydev = netdev->phydev;
if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
return -EINVAL;
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 3/7] net: freescale: ucc_geth: Use netdev->phydev to access the PHY
2024-11-07 17:02 ` [PATCH net-next 3/7] net: freescale: ucc_geth: Use netdev->phydev to access the PHY Maxime Chevallier
@ 2024-11-07 17:37 ` Andrew Lunn
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:37 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:50PM +0100, Maxime Chevallier wrote:
> As this driver pre-dates phylib, it uses a private pointer to get a
> reference to the attached phy_device. Drop that pointer and use the
> netdev's pointer instead.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
` (2 preceding siblings ...)
2024-11-07 17:02 ` [PATCH net-next 3/7] net: freescale: ucc_geth: Use netdev->phydev to access the PHY Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:49 ` Andrew Lunn
2024-11-07 17:02 ` [PATCH net-next 5/7] net: freescale: ucc_geth: Simplify frame length check Maxime Chevallier
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
The get/set_wol ethtool ops rely on querying the PHY for its WoL
capabilities, checking for the presence of a PHY and a PHY interrupts
isn't enough. Address that by cleaning up the WoL configuration
sequence.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
.../net/ethernet/freescale/ucc_geth_ethtool.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index fb5254d7d1ba..2a085f8f34b2 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -346,26 +346,37 @@ static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
struct ucc_geth_private *ugeth = netdev_priv(netdev);
struct phy_device *phydev = netdev->phydev;
- if (phydev && phydev->irq)
- wol->supported |= WAKE_PHY;
+ wol->supported = 0;
+ wol->wolopts = 0;
+
+ if (phydev)
+ phy_ethtool_get_wol(phydev, wol);
+
if (qe_alive_during_sleep())
wol->supported |= WAKE_MAGIC;
- wol->wolopts = ugeth->wol_en;
+ wol->wolopts |= ugeth->wol_en;
}
static int uec_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
struct phy_device *phydev = netdev->phydev;
+ int ret = 0;
if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
return -EINVAL;
- else if (wol->wolopts & WAKE_PHY && (!phydev || !phydev->irq))
+ else if ((wol->wolopts & WAKE_PHY) && !phydev)
return -EINVAL;
else if (wol->wolopts & WAKE_MAGIC && !qe_alive_during_sleep())
return -EINVAL;
+ if (wol->wolopts & WAKE_PHY)
+ ret = phy_ethtool_set_wol(phydev, wol);
+
+ if (ret)
+ return ret;
+
ugeth->wol_en = wol->wolopts;
device_set_wakeup_enable(&netdev->dev, ugeth->wol_en);
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration
2024-11-07 17:02 ` [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration Maxime Chevallier
@ 2024-11-07 17:49 ` Andrew Lunn
2024-11-07 18:16 ` Maxime Chevallier
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:49 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:51PM +0100, Maxime Chevallier wrote:
> The get/set_wol ethtool ops rely on querying the PHY for its WoL
> capabilities, checking for the presence of a PHY and a PHY interrupts
> isn't enough. Address that by cleaning up the WoL configuration
> sequence.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> .../net/ethernet/freescale/ucc_geth_ethtool.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> index fb5254d7d1ba..2a085f8f34b2 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
> @@ -346,26 +346,37 @@ static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> struct ucc_geth_private *ugeth = netdev_priv(netdev);
> struct phy_device *phydev = netdev->phydev;
>
> - if (phydev && phydev->irq)
> - wol->supported |= WAKE_PHY;
> + wol->supported = 0;
> + wol->wolopts = 0;
> +
> + if (phydev)
> + phy_ethtool_get_wol(phydev, wol);
> +
> if (qe_alive_during_sleep())
> wol->supported |= WAKE_MAGIC;
So get WoL will return whatever methods the PHY supports, plus
WAKE_MAGIC is added because i assume the MAC can do that. So depending
on the PHY, it could be the full list:
#define WAKE_PHY (1 << 0)
#define WAKE_UCAST (1 << 1)
#define WAKE_MCAST (1 << 2)
#define WAKE_BCAST (1 << 3)
#define WAKE_ARP (1 << 4)
#define WAKE_MAGIC (1 << 5)
#define WAKE_MAGICSECURE (1 << 6) /* only meaningful if WAKE_MAGIC */
#define WAKE_FILTER (1 << 7)
>
> - wol->wolopts = ugeth->wol_en;
> + wol->wolopts |= ugeth->wol_en;
> }
>
> static int uec_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> {
> struct ucc_geth_private *ugeth = netdev_priv(netdev);
> struct phy_device *phydev = netdev->phydev;
> + int ret = 0;
>
> if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
> return -EINVAL;
> - else if (wol->wolopts & WAKE_PHY && (!phydev || !phydev->irq))
> + else if ((wol->wolopts & WAKE_PHY) && !phydev)
> return -EINVAL;
> else if (wol->wolopts & WAKE_MAGIC && !qe_alive_during_sleep())
> return -EINVAL;
>
> + if (wol->wolopts & WAKE_PHY)
> + ret = phy_ethtool_set_wol(phydev, wol);
Here, the code only calls into the PHY for WAKE_PHY, when in fact the
PHY could be handling WAKE_UCAST, WAKE_MCAST, WAKE_ARP etc.
So these conditions are wrong. It could we be that X years ago when
this code was added only WAKE_PHY and WAKE_MAGIC existed?
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration
2024-11-07 17:49 ` Andrew Lunn
@ 2024-11-07 18:16 ` Maxime Chevallier
0 siblings, 0 replies; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 18:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
Hello Andrew,
On Thu, 7 Nov 2024 18:49:00 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> > + if (phydev)
> > + phy_ethtool_get_wol(phydev, wol);
> > +
> > if (qe_alive_during_sleep())
> > wol->supported |= WAKE_MAGIC;
>
> So get WoL will return whatever methods the PHY supports, plus
> WAKE_MAGIC is added because i assume the MAC can do that. So depending
> on the PHY, it could be the full list:
> > + if (wol->wolopts & WAKE_PHY)
> > + ret = phy_ethtool_set_wol(phydev, wol);
>
> Here, the code only calls into the PHY for WAKE_PHY, when in fact the
> PHY could be handling WAKE_UCAST, WAKE_MCAST, WAKE_ARP etc.
>
> So these conditions are wrong. It could we be that X years ago when
> this code was added only WAKE_PHY and WAKE_MAGIC existed?
Ah that's right indeed. So yeah my changes aren't enough, I'll go over
that patch again.
Thanks for spotting this,
Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 5/7] net: freescale: ucc_geth: Simplify frame length check
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
` (3 preceding siblings ...)
2024-11-07 17:02 ` [PATCH net-next 4/7] net: freescale: ucc_geth: Fix WOL configuration Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:50 ` Andrew Lunn
2024-11-07 17:02 ` [PATCH net-next 6/7] net: freescale: ucc_geth: Hardcode the preamble length to 7 bytes Maxime Chevallier
2024-11-07 17:02 ` [PATCH net-next 7/7] net: freescale: ucc_geth: phylink conversion Maxime Chevallier
6 siblings, 1 reply; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
The frame length check is configured when the phy interface is setup.
However, it's configured according to an internal flag that is always
false. So, just make so that we disable the relevant bit in the MACCFG2
register upon accessing it for other MAC configuration operations.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 21 +++------------------
drivers/net/ethernet/freescale/ucc_geth.h | 1 -
2 files changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 13b8f8401c81..052f06d6f312 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1205,22 +1205,6 @@ static int init_mac_station_addr_regs(u8 address_byte_0,
return 0;
}
-static int init_check_frame_length_mode(int length_check,
- u32 __iomem *maccfg2_register)
-{
- u32 value = 0;
-
- value = in_be32(maccfg2_register);
-
- if (length_check)
- value |= MACCFG2_LC;
- else
- value &= ~MACCFG2_LC;
-
- out_be32(maccfg2_register, value);
- return 0;
-}
-
static int init_preamble_length(u8 preamble_length,
u32 __iomem *maccfg2_register)
{
@@ -1304,6 +1288,9 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
/* Set MACCFG2 */
maccfg2 = in_be32(&ug_regs->maccfg2);
+
+ /* Disable frame length check */
+ maccfg2 &= ~MACCFG2_LC;
maccfg2 &= ~MACCFG2_INTERFACE_MODE_MASK;
if ((ugeth->max_speed == SPEED_10) ||
(ugeth->max_speed == SPEED_100))
@@ -1365,8 +1352,6 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
put_device(&tbiphy->mdio.dev);
}
- init_check_frame_length_mode(ug_info->lengthCheckRx, &ug_regs->maccfg2);
-
ret_val = init_preamble_length(ug_info->prel, &ug_regs->maccfg2);
if (ret_val != 0) {
if (netif_msg_probe(ugeth))
diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
index c08a56b7c9fe..11e490398f18 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.h
+++ b/drivers/net/ethernet/freescale/ucc_geth.h
@@ -1088,7 +1088,6 @@ struct ucc_geth_info {
u8 miminumInterFrameGapEnforcement;
u8 backToBackInterFrameGap;
int ipAddressAlignment;
- int lengthCheckRx;
u32 mblinterval;
u16 nortsrbytetime;
u8 fracsiz;
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 5/7] net: freescale: ucc_geth: Simplify frame length check
2024-11-07 17:02 ` [PATCH net-next 5/7] net: freescale: ucc_geth: Simplify frame length check Maxime Chevallier
@ 2024-11-07 17:50 ` Andrew Lunn
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:50 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:52PM +0100, Maxime Chevallier wrote:
> The frame length check is configured when the phy interface is setup.
> However, it's configured according to an internal flag that is always
> false. So, just make so that we disable the relevant bit in the MACCFG2
> register upon accessing it for other MAC configuration operations.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 6/7] net: freescale: ucc_geth: Hardcode the preamble length to 7 bytes
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
` (4 preceding siblings ...)
2024-11-07 17:02 ` [PATCH net-next 5/7] net: freescale: ucc_geth: Simplify frame length check Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
2024-11-07 17:51 ` Andrew Lunn
2024-11-07 17:02 ` [PATCH net-next 7/7] net: freescale: ucc_geth: phylink conversion Maxime Chevallier
6 siblings, 1 reply; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
The preamble length can be configured in ucc_geth, however it just
ends-up always being configured to 7 bytes, as nothing ever changes the
default value of 7.
Make that value the default value when the MACCFG2 register gets
initialized, and remove the code to configure that value altogether.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 21 ---------------------
drivers/net/ethernet/freescale/ucc_geth.h | 4 ++--
2 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 052f06d6f312..0ec4ea95ad6d 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -132,7 +132,6 @@ static const struct ucc_geth_info ugeth_primary_info = {
.transmitFlowControl = 1,
.maxGroupAddrInHash = 4,
.maxIndAddrInHash = 4,
- .prel = 7,
.maxFrameLength = 1518+16, /* Add extra bytes for VLANs etc. */
.minFrameLength = 64,
.maxD1Length = 1520+16, /* Add extra bytes for VLANs etc. */
@@ -1205,18 +1204,6 @@ static int init_mac_station_addr_regs(u8 address_byte_0,
return 0;
}
-static int init_preamble_length(u8 preamble_length,
- u32 __iomem *maccfg2_register)
-{
- if ((preamble_length < 3) || (preamble_length > 7))
- return -EINVAL;
-
- clrsetbits_be32(maccfg2_register, MACCFG2_PREL_MASK,
- preamble_length << MACCFG2_PREL_SHIFT);
-
- return 0;
-}
-
static int init_rx_parameters(int reject_broadcast,
int receive_short_frames,
int promiscuous, u32 __iomem *upsmr_register)
@@ -1276,7 +1263,6 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
struct ucc_geth_info *ug_info;
struct ucc_geth __iomem *ug_regs;
struct ucc_fast __iomem *uf_regs;
- int ret_val;
u32 upsmr, maccfg2;
u16 value;
@@ -1352,13 +1338,6 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
put_device(&tbiphy->mdio.dev);
}
- ret_val = init_preamble_length(ug_info->prel, &ug_regs->maccfg2);
- if (ret_val != 0) {
- if (netif_msg_probe(ugeth))
- pr_err("Preamble length must be between 3 and 7 inclusive\n");
- return ret_val;
- }
-
return 0;
}
diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
index 11e490398f18..42fbbdf14ff2 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.h
+++ b/drivers/net/ethernet/freescale/ucc_geth.h
@@ -921,7 +921,8 @@ struct ucc_geth_hardware_statistics {
#define UCC_GETH_UPSMR_INIT UCC_GETH_UPSMR_RES1
#define UCC_GETH_MACCFG1_INIT 0
-#define UCC_GETH_MACCFG2_INIT (MACCFG2_RESERVED_1)
+#define UCC_GETH_MACCFG2_INIT (MACCFG2_RESERVED_1 | \
+ (7 << MACCFG2_PREL_SHIFT))
/* Ethernet Address Type. */
enum enet_addr_type {
@@ -1113,7 +1114,6 @@ struct ucc_geth_info {
int transmitFlowControl;
u8 maxGroupAddrInHash;
u8 maxIndAddrInHash;
- u8 prel;
u16 maxFrameLength;
u16 minFrameLength;
u16 maxD1Length;
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH net-next 6/7] net: freescale: ucc_geth: Hardcode the preamble length to 7 bytes
2024-11-07 17:02 ` [PATCH net-next 6/7] net: freescale: ucc_geth: Hardcode the preamble length to 7 bytes Maxime Chevallier
@ 2024-11-07 17:51 ` Andrew Lunn
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-11-07 17:51 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King,
Christophe Leroy, Heiner Kallweit, netdev, linux-kernel,
thomas.petazzoni, Herve Codina, Uwe Kleine-König,
linuxppc-dev
On Thu, Nov 07, 2024 at 06:02:53PM +0100, Maxime Chevallier wrote:
> The preamble length can be configured in ucc_geth, however it just
> ends-up always being configured to 7 bytes, as nothing ever changes the
> default value of 7.
>
> Make that value the default value when the MACCFG2 register gets
> initialized, and remove the code to configure that value altogether.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 7/7] net: freescale: ucc_geth: phylink conversion
2024-11-07 17:02 [PATCH net-next 0/7] net: freescale: ucc_geth: Phylink conversion Maxime Chevallier
` (5 preceding siblings ...)
2024-11-07 17:02 ` [PATCH net-next 6/7] net: freescale: ucc_geth: Hardcode the preamble length to 7 bytes Maxime Chevallier
@ 2024-11-07 17:02 ` Maxime Chevallier
6 siblings, 0 replies; 17+ messages in thread
From: Maxime Chevallier @ 2024-11-07 17:02 UTC (permalink / raw)
To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, Christophe Leroy, Heiner Kallweit
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Herve Codina, Uwe Kleine-König, linuxppc-dev
ucc_geth is quite capable in terms of supported interfaces, and even
includes an externally controlled PCS (well, TBI). Port that driver to
phylink.
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/freescale/Kconfig | 3 +-
drivers/net/ethernet/freescale/ucc_geth.c | 516 ++++++++----------
drivers/net/ethernet/freescale/ucc_geth.h | 13 +-
.../net/ethernet/freescale/ucc_geth_ethtool.c | 61 +--
4 files changed, 236 insertions(+), 357 deletions(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 75401d2a5fb4..a2d7300925a8 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -81,8 +81,7 @@ config UCC_GETH
tristate "Freescale QE Gigabit Ethernet"
depends on QUICC_ENGINE && PPC32
select FSL_PQ_MDIO
- select PHYLIB
- select FIXED_PHY
+ select PHYLINK
help
This driver supports the Gigabit Ethernet mode of the QUICC Engine,
which is available on some Freescale SOCs.
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 0ec4ea95ad6d..df74ebd8dbe8 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -26,7 +26,7 @@
#include <linux/dma-mapping.h>
#include <linux/mii.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
+#include <linux/phylink.h>
#include <linux/workqueue.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -34,6 +34,7 @@
#include <linux/of_mdio.h>
#include <linux/of_net.h>
#include <linux/platform_device.h>
+#include <linux/rtnetlink.h>
#include <linux/uaccess.h>
#include <asm/irq.h>
@@ -1258,89 +1259,6 @@ static int init_min_frame_len(u16 min_frame_length,
return 0;
}
-static int adjust_enet_interface(struct ucc_geth_private *ugeth)
-{
- struct ucc_geth_info *ug_info;
- struct ucc_geth __iomem *ug_regs;
- struct ucc_fast __iomem *uf_regs;
- u32 upsmr, maccfg2;
- u16 value;
-
- ugeth_vdbg("%s: IN", __func__);
-
- ug_info = ugeth->ug_info;
- ug_regs = ugeth->ug_regs;
- uf_regs = ugeth->uccf->uf_regs;
-
- /* Set MACCFG2 */
- maccfg2 = in_be32(&ug_regs->maccfg2);
-
- /* Disable frame length check */
- maccfg2 &= ~MACCFG2_LC;
- maccfg2 &= ~MACCFG2_INTERFACE_MODE_MASK;
- if ((ugeth->max_speed == SPEED_10) ||
- (ugeth->max_speed == SPEED_100))
- maccfg2 |= MACCFG2_INTERFACE_MODE_NIBBLE;
- else if (ugeth->max_speed == SPEED_1000)
- maccfg2 |= MACCFG2_INTERFACE_MODE_BYTE;
- maccfg2 |= ug_info->padAndCrc;
- out_be32(&ug_regs->maccfg2, maccfg2);
-
- /* Set UPSMR */
- upsmr = in_be32(&uf_regs->upsmr);
- upsmr &= ~(UCC_GETH_UPSMR_RPM | UCC_GETH_UPSMR_R10M |
- UCC_GETH_UPSMR_TBIM | UCC_GETH_UPSMR_RMM);
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
- if (ugeth->phy_interface != PHY_INTERFACE_MODE_RMII)
- upsmr |= UCC_GETH_UPSMR_RPM;
- switch (ugeth->max_speed) {
- case SPEED_10:
- upsmr |= UCC_GETH_UPSMR_R10M;
- fallthrough;
- case SPEED_100:
- if (ugeth->phy_interface != PHY_INTERFACE_MODE_RTBI)
- upsmr |= UCC_GETH_UPSMR_RMM;
- }
- }
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_TBI) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
- upsmr |= UCC_GETH_UPSMR_TBIM;
- }
- if (ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII)
- upsmr |= UCC_GETH_UPSMR_SGMM;
-
- out_be32(&uf_regs->upsmr, upsmr);
-
- /* Disable autonegotiation in tbi mode, because by default it
- comes up in autonegotiation mode. */
- /* Note that this depends on proper setting in utbipar register. */
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_TBI) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
- struct ucc_geth_info *ug_info = ugeth->ug_info;
- struct phy_device *tbiphy;
-
- if (!ug_info->tbi_node)
- pr_warn("TBI mode requires that the device tree specify a tbi-handle\n");
-
- tbiphy = of_phy_find_device(ug_info->tbi_node);
- if (!tbiphy)
- pr_warn("Could not get TBI device\n");
-
- value = phy_read(tbiphy, ENET_TBI_MII_CR);
- value &= ~0x1000; /* Turn off autonegotiation */
- phy_write(tbiphy, ENET_TBI_MII_CR, value);
-
- put_device(&tbiphy->mdio.dev);
- }
-
- return 0;
-}
-
static int ugeth_graceful_stop_tx(struct ucc_geth_private *ugeth)
{
struct ucc_fast_private *uccf;
@@ -1512,113 +1430,6 @@ static void ugeth_activate(struct ucc_geth_private *ugeth)
__netdev_watchdog_up(ugeth->ndev);
}
-static void ugeth_link_up(struct ucc_geth_private *ugeth,
- struct phy_device *phy,
- phy_interface_t interface, int speed, int duplex)
-{
- struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
- struct ucc_fast __iomem *uf_regs = ugeth->uccf->uf_regs;
- u32 tempval = in_be32(&ug_regs->maccfg2);
- u32 upsmr = in_be32(&uf_regs->upsmr);
- int new_state = 0;
-
- /* Now we make sure that we can be in full duplex mode.
- * If not, we operate in half-duplex mode.
- */
- if (duplex != ugeth->oldduplex) {
- new_state = 1;
- if (duplex == DUPLEX_HALF)
- tempval &= ~(MACCFG2_FDX);
- else
- tempval |= MACCFG2_FDX;
- ugeth->oldduplex = duplex;
- }
-
- if (speed != ugeth->oldspeed) {
- new_state = 1;
- switch (speed) {
- case SPEED_1000:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_BYTE);
- break;
- case SPEED_100:
- case SPEED_10:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_NIBBLE);
- /* if reduced mode, re-set UPSMR.R10M */
- if (interface == PHY_INTERFACE_MODE_RMII ||
- phy_interface_mode_is_rgmii(interface) ||
- interface == PHY_INTERFACE_MODE_RTBI) {
- if (speed == SPEED_10)
- upsmr |= UCC_GETH_UPSMR_R10M;
- else
- upsmr &= ~UCC_GETH_UPSMR_R10M;
- }
- break;
- default:
- if (netif_msg_link(ugeth))
- pr_warn("%s: Speed (%d) is not 10/100/1000!",
- netdev_name(ugeth->ndev), speed);
- break;
- }
- ugeth->oldspeed = speed;
- }
-
- if (!ugeth->oldlink) {
- new_state = 1;
- ugeth->oldlink = 1;
- }
-
- if (new_state) {
- /*
- * To change the MAC configuration we need to disable
- * the controller. To do so, we have to either grab
- * ugeth->lock, which is a bad idea since 'graceful
- * stop' commands might take quite a while, or we can
- * quiesce driver's activity.
- */
- ugeth_quiesce(ugeth);
- ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
-
- out_be32(&ug_regs->maccfg2, tempval);
- out_be32(&uf_regs->upsmr, upsmr);
-
- ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
- ugeth_activate(ugeth);
- }
-
- if (netif_msg_link(ugeth))
- phy_print_status(phy);
-}
-
-static void ugeth_link_down(struct ucc_geth_private *ugeth)
-{
- ugeth->oldlink = 0;
- ugeth->oldspeed = 0;
- ugeth->oldduplex = -1;
-}
-
-/* Called every time the controller might need to be made
- * aware of new link state. The PHY code conveys this
- * information through variables in the ugeth structure, and this
- * function converts those variables into the appropriate
- * register values, and can bring down the device if needed.
- */
-
-static void adjust_link(struct net_device *dev)
-{
- struct ucc_geth_private *ugeth = netdev_priv(dev);
- struct phy_device *phydev = dev->phydev;
-
- if (phydev->link)
- ugeth_link_up(ugeth, phydev, phydev->interface,
- phydev->speed, phydev->duplex);
- else
- ugeth_link_down(ugeth);
-}
-
/* Initialize TBI PHY interface for communicating with the
* SERDES lynx PHY on the chip. We communicate with this PHY
* through the MDIO bus on each controller, treating it as a
@@ -1666,32 +1477,152 @@ static void uec_configure_serdes(struct net_device *dev)
put_device(&tbiphy->mdio.dev);
}
-/* Configure the PHY for dev.
- * returns 0 if success. -1 if failure
- */
-static int init_phy(struct net_device *dev)
+static bool phy_interface_mode_is_reduced(phy_interface_t interface)
+{
+ return phy_interface_mode_is_rgmii(interface) ||
+ interface == PHY_INTERFACE_MODE_RMII ||
+ interface == PHY_INTERFACE_MODE_RTBI;
+}
+
+static void ugeth_mac_link_up(struct phylink_config *config, struct phy_device *phy,
+ unsigned int mode, phy_interface_t interface,
+ int speed, int duplex, bool tx_pause, bool rx_pause)
{
- struct ucc_geth_private *priv = netdev_priv(dev);
- struct ucc_geth_info *ug_info = priv->ug_info;
- struct phy_device *phydev;
+ struct net_device *ndev = to_net_dev(config->dev);
+ struct ucc_geth_private *ugeth = netdev_priv(ndev);
+ struct ucc_geth_info *ug_info = ugeth->ug_info;
+ struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
+ struct ucc_fast __iomem *uf_regs = ugeth->uccf->uf_regs;
+ u32 old_maccfg2, maccfg2 = in_be32(&ug_regs->maccfg2);
+ u32 old_upsmr, upsmr = in_be32(&uf_regs->upsmr);
- priv->oldlink = 0;
- priv->oldspeed = 0;
- priv->oldduplex = -1;
+ old_maccfg2 = maccfg2;
+ old_upsmr = upsmr;
- phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
- priv->phy_interface);
- if (!phydev) {
- dev_err(&dev->dev, "Could not attach to PHY\n");
- return -ENODEV;
+ /* No length check */
+ maccfg2 &= ~MACCFG2_LC;
+ maccfg2 &= ~MACCFG2_INTERFACE_MODE_MASK;
+ upsmr &= ~(UCC_GETH_UPSMR_RPM | UCC_GETH_UPSMR_R10M |
+ UCC_GETH_UPSMR_TBIM | UCC_GETH_UPSMR_RMM);
+
+ if (speed == SPEED_10 || speed == SPEED_100)
+ maccfg2 |= MACCFG2_INTERFACE_MODE_NIBBLE;
+ else if (speed == SPEED_1000)
+ maccfg2 |= MACCFG2_INTERFACE_MODE_BYTE;
+
+ maccfg2 |= ug_info->padAndCrc;
+
+ if (phy_interface_mode_is_reduced(interface)) {
+
+ if (interface != PHY_INTERFACE_MODE_RMII)
+ upsmr |= UCC_GETH_UPSMR_RPM;
+
+ switch (speed) {
+ case SPEED_10:
+ upsmr |= UCC_GETH_UPSMR_R10M;
+ fallthrough;
+ case SPEED_100:
+ if (interface != PHY_INTERFACE_MODE_RTBI)
+ upsmr |= UCC_GETH_UPSMR_RMM;
+ }
}
- if (priv->phy_interface == PHY_INTERFACE_MODE_SGMII)
- uec_configure_serdes(dev);
+ if (interface == PHY_INTERFACE_MODE_TBI ||
+ interface == PHY_INTERFACE_MODE_RTBI)
+ upsmr |= UCC_GETH_UPSMR_TBIM;
+
+ if (interface == PHY_INTERFACE_MODE_SGMII)
+ upsmr |= UCC_GETH_UPSMR_SGMM;
- phy_set_max_speed(phydev, priv->max_speed);
+ if (duplex == DUPLEX_HALF)
+ maccfg2 &= ~(MACCFG2_FDX);
+ else
+ maccfg2 |= MACCFG2_FDX;
- return 0;
+ if (maccfg2 != old_maccfg2 || upsmr != old_upsmr) {
+ /*
+ * To change the MAC configuration we need to disable
+ * the controller. To do so, we have to either grab
+ * ugeth->lock, which is a bad idea since 'graceful
+ * stop' commands might take quite a while, or we can
+ * quiesce driver's activity.
+ */
+ ugeth_quiesce(ugeth);
+ ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
+
+ out_be32(&ug_regs->maccfg2, maccfg2);
+ out_be32(&uf_regs->upsmr, upsmr);
+
+ ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
+ ugeth_activate(ugeth);
+ }
+
+ if (interface == PHY_INTERFACE_MODE_SGMII)
+ uec_configure_serdes(ndev);
+
+ if (!phylink_autoneg_inband(mode)) {
+ ug_info->aufc = 0;
+ ug_info->receiveFlowControl = rx_pause;
+ ug_info->transmitFlowControl = tx_pause;
+
+ init_flow_control_params(ug_info->aufc,
+ ug_info->receiveFlowControl,
+ ug_info->transmitFlowControl,
+ ug_info->pausePeriod,
+ ug_info->extensionField,
+ &ugeth->uccf->uf_regs->upsmr,
+ &ugeth->ug_regs->uempr,
+ &ugeth->ug_regs->maccfg1);
+ }
+
+ ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
+}
+
+static void ugeth_mac_link_down(struct phylink_config *config,
+ unsigned int mode, phy_interface_t interface)
+{
+ struct net_device *ndev = to_net_dev(config->dev);
+ struct ucc_geth_private *ugeth = netdev_priv(ndev);
+
+ ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
+}
+
+static void ugeth_mac_config(struct phylink_config *config, unsigned int mode,
+ const struct phylink_link_state *state)
+{
+ struct net_device *ndev = to_net_dev(config->dev);
+ struct ucc_geth_private *ugeth = netdev_priv(ndev);
+ struct ucc_geth_info *ug_info = ugeth->ug_info;
+ u16 value;
+
+ if (state->interface == PHY_INTERFACE_MODE_TBI ||
+ state->interface == PHY_INTERFACE_MODE_RTBI) {
+ struct phy_device *tbiphy;
+
+ if (!ug_info->tbi_node)
+ pr_warn("TBI mode requires that the device tree specify a tbi-handle\n");
+
+ tbiphy = of_phy_find_device(ug_info->tbi_node);
+ if (!tbiphy)
+ pr_warn("Could not get TBI device\n");
+
+ value = phy_read(tbiphy, ENET_TBI_MII_CR);
+ value &= ~0x1000; /* Turn off autonegotiation */
+ phy_write(tbiphy, ENET_TBI_MII_CR, value);
+
+ put_device(&tbiphy->mdio.dev);
+ }
+
+ if (phylink_autoneg_inband(mode)) {
+ ug_info->aufc = 1;
+
+ init_flow_control_params(ug_info->aufc, 1, 1,
+ ug_info->pausePeriod,
+ ug_info->extensionField,
+ &ugeth->uccf->uf_regs->upsmr,
+ &ugeth->ug_regs->uempr,
+ &ugeth->ug_regs->maccfg1);
+ }
}
static void ugeth_dump_regs(struct ucc_geth_private *ugeth)
@@ -1963,7 +1894,6 @@ static void ucc_geth_set_multi(struct net_device *dev)
static void ucc_geth_stop(struct ucc_geth_private *ugeth)
{
struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
- struct phy_device *phydev = ugeth->ndev->phydev;
ugeth_vdbg("%s: IN", __func__);
@@ -1972,7 +1902,7 @@ static void ucc_geth_stop(struct ucc_geth_private *ugeth)
* Must be done before disabling the controller
* or deadlock may happen.
*/
- phy_stop(phydev);
+ phylink_stop(ugeth->phylink);
/* Disable the controller */
ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
@@ -3214,12 +3144,6 @@ static int ucc_geth_init_mac(struct ucc_geth_private *ugeth)
goto err;
}
- err = adjust_enet_interface(ugeth);
- if (err) {
- netif_err(ugeth, ifup, dev, "Cannot configure net device, aborting\n");
- goto err;
- }
-
/* Set MACSTNADDR1, MACSTNADDR2 */
/* For more details see the hardware spec. */
init_mac_station_addr_regs(dev->dev_addr[0],
@@ -3231,12 +3155,6 @@ static int ucc_geth_init_mac(struct ucc_geth_private *ugeth)
&ugeth->ug_regs->macstnaddr1,
&ugeth->ug_regs->macstnaddr2);
- err = ugeth_enable(ugeth, COMM_DIR_RX_AND_TX);
- if (err) {
- netif_err(ugeth, ifup, dev, "Cannot enable net device, aborting\n");
- goto err;
- }
-
return 0;
err:
ucc_geth_stop(ugeth);
@@ -3259,10 +3177,10 @@ static int ucc_geth_open(struct net_device *dev)
return -EINVAL;
}
- err = init_phy(dev);
+ err = phylink_of_phy_connect(ugeth->phylink, ugeth->dev->of_node, 0);
if (err) {
- netif_err(ugeth, ifup, dev, "Cannot initialize PHY, aborting\n");
- return err;
+ dev_err(&dev->dev, "Could not attach to PHY\n");
+ return -ENODEV;
}
err = ucc_geth_init_mac(ugeth);
@@ -3278,7 +3196,7 @@ static int ucc_geth_open(struct net_device *dev)
goto err;
}
- phy_start(dev->phydev);
+ phylink_start(ugeth->phylink);
napi_enable(&ugeth->napi);
netdev_reset_queue(dev);
netif_start_queue(dev);
@@ -3305,7 +3223,7 @@ static int ucc_geth_close(struct net_device *dev)
cancel_work_sync(&ugeth->timeout_work);
ucc_geth_stop(ugeth);
- phy_disconnect(dev->phydev);
+ phylink_disconnect_phy(ugeth->phylink);
free_irq(ugeth->ug_info->uf_info.irq, ugeth->ndev);
@@ -3339,7 +3257,7 @@ static void ucc_geth_timeout_work(struct work_struct *work)
ucc_geth_stop(ugeth);
ucc_geth_init_mac(ugeth);
/* Must start PHY here */
- phy_start(dev->phydev);
+ phylink_start(ugeth->phylink);
netif_tx_start_all_queues(dev);
}
@@ -3381,10 +3299,12 @@ static int ucc_geth_suspend(struct platform_device *ofdev, pm_message_t state)
setbits32(ugeth->uccf->p_uccm, UCC_GETH_UCCE_MPD);
setbits32(&ugeth->ug_regs->maccfg2, MACCFG2_MPE);
ucc_fast_enable(ugeth->uccf, COMM_DIR_RX_AND_TX);
- } else if (!(ugeth->wol_en & WAKE_PHY)) {
- phy_stop(ndev->phydev);
}
+ rtnl_lock();
+ phylink_suspend(ugeth->phylink, ugeth->wol_en);
+ rtnl_unlock();
+
return 0;
}
@@ -3418,12 +3338,9 @@ static int ucc_geth_resume(struct platform_device *ofdev)
}
}
- ugeth->oldlink = 0;
- ugeth->oldspeed = 0;
- ugeth->oldduplex = -1;
-
- phy_stop(ndev->phydev);
- phy_start(ndev->phydev);
+ rtnl_lock();
+ phylink_resume(ugeth->phylink);
+ rtnl_unlock();
napi_enable(&ugeth->napi);
netif_device_attach(ndev);
@@ -3438,13 +3355,12 @@ static int ucc_geth_resume(struct platform_device *ofdev)
static int ucc_geth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
+ struct ucc_geth_private *ugeth = netdev_priv(dev);
+
if (!netif_running(dev))
return -EINVAL;
- if (!dev->phydev)
- return -ENODEV;
-
- return phy_mii_ioctl(dev->phydev, rq, cmd);
+ return phylink_mii_ioctl(ugeth->phylink, rq, cmd);
}
static const struct net_device_ops ucc_geth_netdev_ops = {
@@ -3452,7 +3368,6 @@ static const struct net_device_ops ucc_geth_netdev_ops = {
.ndo_stop = ucc_geth_close,
.ndo_start_xmit = ucc_geth_start_xmit,
.ndo_validate_addr = eth_validate_addr,
- .ndo_change_carrier = fixed_phy_change_carrier,
.ndo_set_mac_address = ucc_geth_set_mac_addr,
.ndo_set_rx_mode = ucc_geth_set_multi,
.ndo_tx_timeout = ucc_geth_timeout,
@@ -3492,6 +3407,12 @@ static int ucc_geth_parse_clock(struct device_node *np, const char *which,
return 0;
}
+struct phylink_mac_ops ugeth_mac_ops = {
+ .mac_link_up = ugeth_mac_link_up,
+ .mac_link_down = ugeth_mac_link_down,
+ .mac_config = ugeth_mac_config,
+};
+
static int ucc_geth_probe(struct platform_device* ofdev)
{
struct device *device = &ofdev->dev;
@@ -3499,8 +3420,10 @@ static int ucc_geth_probe(struct platform_device* ofdev)
struct net_device *dev = NULL;
struct ucc_geth_private *ugeth = NULL;
struct ucc_geth_info *ug_info;
+ struct device_node *phy_node;
+ struct phylink *phylink;
struct resource res;
- int err, ucc_num, max_speed = 0;
+ int err, ucc_num;
const unsigned int *prop;
phy_interface_t phy_interface;
@@ -3538,26 +3461,19 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info->uf_info.regs = res.start;
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
- ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
- if (!ug_info->phy_node && of_phy_is_fixed_link(np)) {
- /*
- * In the case of a fixed PHY, the DT node associated
- * to the PHY is the Ethernet MAC DT node.
- */
- err = of_phy_register_fixed_link(np);
- if (err)
- return err;
- ug_info->phy_node = of_node_get(np);
- }
-
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
- prop = of_get_property(ug_info->phy_node, "interface", NULL);
- if (prop) {
- dev_err(&ofdev->dev,
- "Device-tree property 'interface' is no longer supported. Please use 'phy-connection-type' instead.");
- goto err_put_tbi;
+ phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (phy_node) {
+ prop = of_get_property(phy_node, "interface", NULL);
+ if (prop) {
+ dev_err(&ofdev->dev,
+ "Device-tree property 'interface' is no longer supported. Please use 'phy-connection-type' instead.");
+ of_node_put(phy_node);
+ goto err_put_tbi;
+ }
+ of_node_put(phy_node);
}
err = of_get_phy_mode(np, &phy_interface);
@@ -3566,28 +3482,13 @@ static int ucc_geth_probe(struct platform_device* ofdev)
goto err_put_tbi;
}
- /* get speed, or derive from PHY interface */
- if (max_speed == 0)
- switch (phy_interface) {
- case PHY_INTERFACE_MODE_GMII:
- case PHY_INTERFACE_MODE_RGMII:
- case PHY_INTERFACE_MODE_RGMII_ID:
- case PHY_INTERFACE_MODE_RGMII_RXID:
- case PHY_INTERFACE_MODE_RGMII_TXID:
- case PHY_INTERFACE_MODE_TBI:
- case PHY_INTERFACE_MODE_RTBI:
- case PHY_INTERFACE_MODE_SGMII:
- max_speed = SPEED_1000;
- break;
- default:
- max_speed = SPEED_100;
- break;
- }
-
- if (max_speed == SPEED_1000) {
+ if (phy_interface == PHY_INTERFACE_MODE_GMII ||
+ phy_interface_mode_is_rgmii(phy_interface) ||
+ phy_interface == PHY_INTERFACE_MODE_TBI ||
+ phy_interface == PHY_INTERFACE_MODE_RTBI ||
+ phy_interface == PHY_INTERFACE_MODE_SGMII) {
unsigned int snums = qe_get_num_of_snums();
- /* configure muram FIFOs for gigabit operation */
ug_info->uf_info.urfs = UCC_GETH_URFS_GIGA_INIT;
ug_info->uf_info.urfet = UCC_GETH_URFET_GIGA_INIT;
ug_info->uf_info.urfset = UCC_GETH_URFSET_GIGA_INIT;
@@ -3616,7 +3517,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
dev = devm_alloc_etherdev(&ofdev->dev, sizeof(*ugeth));
if (!dev) {
err = -ENOMEM;
- goto err_deregister_fixed_link;
+ goto err_put_tbi;
}
ugeth = netdev_priv(dev);
@@ -3643,23 +3544,50 @@ static int ucc_geth_probe(struct platform_device* ofdev)
dev->max_mtu = 1518;
ugeth->msg_enable = netif_msg_init(debug.msg_enable, UGETH_MSG_DEFAULT);
- ugeth->phy_interface = phy_interface;
- ugeth->max_speed = max_speed;
- /* Carrier starts down, phylib will bring it up */
- netif_carrier_off(dev);
+ ugeth->phylink_config.dev = &dev->dev;
+ ugeth->phylink_config.type = PHYLINK_NETDEV;
+
+ ugeth->phylink_config.mac_capabilities =
+ MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD;
+
+ __set_bit(PHY_INTERFACE_MODE_MII,
+ ugeth->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_RMII,
+ ugeth->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_GMII,
+ ugeth->phylink_config.supported_interfaces);
+ phy_interface_set_rgmii(ugeth->phylink_config.supported_interfaces);
+
+ if (ug_info->tbi_node) {
+ __set_bit(PHY_INTERFACE_MODE_SGMII,
+ ugeth->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_TBI,
+ ugeth->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_RTBI,
+ ugeth->phylink_config.supported_interfaces);
+ }
+
+ phylink = phylink_create(&ugeth->phylink_config, dev_fwnode(&dev->dev),
+ phy_interface, &ugeth_mac_ops);
+ if (IS_ERR(phylink)) {
+ err = PTR_ERR(phylink);
+ goto err_put_tbi;
+ }
+
+ ugeth->phylink = phylink;
err = devm_register_netdev(&ofdev->dev, dev);
if (err) {
if (netif_msg_probe(ugeth))
pr_err("%s: Cannot register net device, aborting\n",
dev->name);
- goto err_deregister_fixed_link;
+ goto err_destroy_phylink;
}
err = of_get_ethdev_address(np, dev);
if (err == -EPROBE_DEFER)
- goto err_deregister_fixed_link;
+ goto err_destroy_phylink;
ugeth->ug_info = ug_info;
ugeth->dev = device;
@@ -3668,12 +3596,11 @@ static int ucc_geth_probe(struct platform_device* ofdev)
return 0;
-err_deregister_fixed_link:
- if (of_phy_is_fixed_link(np))
- of_phy_deregister_fixed_link(np);
+err_destroy_phylink:
+ phylink_destroy(phylink);
err_put_tbi:
of_node_put(ug_info->tbi_node);
- of_node_put(ug_info->phy_node);
+
return err;
}
@@ -3681,13 +3608,10 @@ static void ucc_geth_remove(struct platform_device* ofdev)
{
struct net_device *dev = platform_get_drvdata(ofdev);
struct ucc_geth_private *ugeth = netdev_priv(dev);
- struct device_node *np = ofdev->dev.of_node;
ucc_geth_memclean(ugeth);
- if (of_phy_is_fixed_link(np))
- of_phy_deregister_fixed_link(np);
+ phylink_destroy(ugeth->phylink);
of_node_put(ugeth->ug_info->tbi_node);
- of_node_put(ugeth->ug_info->phy_node);
}
static const struct of_device_id ucc_geth_match[] = {
diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
index 42fbbdf14ff2..0a3c2645e16b 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.h
+++ b/drivers/net/ethernet/freescale/ucc_geth.h
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
+#include <linux/phylink.h>
#include <linux/if_ether.h>
#include <soc/fsl/qe/immap_qe.h>
@@ -1074,6 +1075,9 @@ struct ucc_geth_tad_params {
u16 vid;
};
+struct phylink;
+struct phylink_config;
+
/* GETH protocol initialization structure */
struct ucc_geth_info {
struct ucc_fast_info uf_info;
@@ -1124,7 +1128,6 @@ struct ucc_geth_info {
u32 eventRegMask;
u16 pausePeriod;
u16 extensionField;
- struct device_node *phy_node;
struct device_node *tbi_node;
u8 weightfactor[NUM_TX_QUEUES];
u8 interruptcoalescingmaxvalue[NUM_RX_QUEUES];
@@ -1209,14 +1212,12 @@ struct ucc_geth_private {
u16 skb_dirtytx[NUM_TX_QUEUES];
struct ugeth_mii_info *mii_info;
- phy_interface_t phy_interface;
- int max_speed;
uint32_t msg_enable;
- int oldspeed;
- int oldduplex;
- int oldlink;
int wol_en;
+ struct phylink *phylink;
+ struct phylink_config phylink_config;
+
struct device_node *node;
};
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index 2a085f8f34b2..661521960cba 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -103,26 +103,18 @@ static const char rx_fw_stat_gstrings[][ETH_GSTRING_LEN] = {
static int
uec_get_ksettings(struct net_device *netdev, struct ethtool_link_ksettings *cmd)
{
- struct phy_device *phydev = netdev->phydev;
-
- if (!phydev)
- return -ENODEV;
-
- phy_ethtool_ksettings_get(phydev, cmd);
+ struct ucc_geth_private *ugeth = netdev_priv(netdev);
- return 0;
+ return phylink_ethtool_ksettings_get(ugeth->phylink, cmd);
}
static int
uec_set_ksettings(struct net_device *netdev,
const struct ethtool_link_ksettings *cmd)
{
- struct phy_device *phydev = netdev->phydev;
-
- if (!phydev)
- return -ENODEV;
+ struct ucc_geth_private *ugeth = netdev_priv(netdev);
- return phy_ethtool_ksettings_set(phydev, cmd);
+ return phylink_ethtool_ksettings_set(ugeth->phylink, cmd);
}
static void
@@ -130,15 +122,8 @@ uec_get_pauseparam(struct net_device *netdev,
struct ethtool_pauseparam *pause)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = netdev->phydev;
-
- if (phydev)
- pause->autoneg = phydev->autoneg;
- if (ugeth->ug_info->receiveFlowControl)
- pause->rx_pause = 1;
- if (ugeth->ug_info->transmitFlowControl)
- pause->tx_pause = 1;
+ return phylink_ethtool_get_pauseparam(ugeth->phylink, pause);
}
static int
@@ -146,31 +131,11 @@ uec_set_pauseparam(struct net_device *netdev,
struct ethtool_pauseparam *pause)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = netdev->phydev;
- int ret = 0;
ugeth->ug_info->receiveFlowControl = pause->rx_pause;
ugeth->ug_info->transmitFlowControl = pause->tx_pause;
- if (phydev && phydev->autoneg) {
- if (netif_running(netdev)) {
- /* FIXME: automatically restart */
- netdev_info(netdev, "Please re-open the interface\n");
- }
- } else {
- struct ucc_geth_info *ug_info = ugeth->ug_info;
-
- ret = init_flow_control_params(ug_info->aufc,
- ug_info->receiveFlowControl,
- ug_info->transmitFlowControl,
- ug_info->pausePeriod,
- ug_info->extensionField,
- &ugeth->uccf->uf_regs->upsmr,
- &ugeth->ug_regs->uempr,
- &ugeth->ug_regs->maccfg1);
- }
-
- return ret;
+ return phylink_ethtool_set_pauseparam(ugeth->phylink, pause);
}
static uint32_t
@@ -344,13 +309,8 @@ uec_get_drvinfo(struct net_device *netdev,
static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = netdev->phydev;
- wol->supported = 0;
- wol->wolopts = 0;
-
- if (phydev)
- phy_ethtool_get_wol(phydev, wol);
+ phylink_ethtool_get_wol(ugeth->phylink, wol);
if (qe_alive_during_sleep())
wol->supported |= WAKE_MAGIC;
@@ -361,19 +321,14 @@ static void uec_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
static int uec_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
{
struct ucc_geth_private *ugeth = netdev_priv(netdev);
- struct phy_device *phydev = netdev->phydev;
int ret = 0;
if (wol->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
return -EINVAL;
- else if ((wol->wolopts & WAKE_PHY) && !phydev)
- return -EINVAL;
else if (wol->wolopts & WAKE_MAGIC && !qe_alive_during_sleep())
return -EINVAL;
- if (wol->wolopts & WAKE_PHY)
- ret = phy_ethtool_set_wol(phydev, wol);
-
+ ret = phylink_ethtool_set_wol(ugeth->phylink, wol);
if (ret)
return ret;
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread