[PATCH net-next v4 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
Update DT bindings to describe all of the clocks that the axienet driver will now be able to make use of. Acked-by: Rob Herring Signed-off-by: Robert Hancock --- .../bindings/net/xilinx_axienet.txt | 25 ++- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 2cd452419ed0..b8e4894bc634 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -42,11 +42,23 @@ Optional properties: support both 1000BaseX and SGMII modes. If set, the phy-mode should be set to match the mode selected on core reset (i.e. by the basex_or_sgmii core input line). -- clocks : AXI bus clock for the device. Refer to common clock bindings. - Used to calculate MDIO clock divisor. If not specified, it is - auto-detected from the CPU clock (but only on platforms where - this is possible). New device trees should specify this - the - auto detection is only for backward compatibility. +- clock-names: Tuple listing input clock names. Possible clocks: + s_axi_lite_clk: Clock for AXI register slave interface + axis_clk: AXI4-Stream clock for TXD RXD TXC and RXS interfaces + ref_clk: Ethernet reference clock, used by signal delay + primitives and transceivers + mgt_clk: MGT reference clock (used by optional internal + PCS/PMA PHY) + + Note that if s_axi_lite_clk is not specified by name, the + first clock of any name is used for this. If that is also not + specified, the clock rate is auto-detected from the CPU clock + (but only on platforms where this is possible). New device + trees should specify all applicable clocks by name - the + fallbacks to an unnamed clock or to CPU clock are only for + backward compatibility. +- clocks:Phandles to input clocks matching clock-names. Refer to common + clock bindings. - axistream-connected: Reference to another node which contains the resources for the AXI DMA controller used by this device. If this is specified, the DMA-related resources from that @@ -62,7 +74,8 @@ Example: device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; interrupts = <2 0 1>; - clocks = <&axi_clk>; + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk"; + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, <&mgt_clk>; phy-mode = "mii"; reg = <0x40c0 0x4 0x50c0 0x4>; xlnx,rxcsum = <0x2>; -- 2.27.0
[PATCH net-next v4 2/2] net: axienet: Enable more clocks
This driver was only enabling the first clock on the device, regardless of its name. However, this controller logic can have multiple clocks which should all be enabled. Add support for enabling additional clocks. The clock names used are matching those used in the Xilinx version of this driver as well as the Xilinx device tree generator, except for mgt_clk which is not present there. For backward compatibility, if no named clocks are present, the first clock present is used for determining the MDIO bus clock divider. Reviewed-by: Radhey Shyam Pandey Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 1e966a39967e..708769349f76 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -376,6 +376,8 @@ struct axidma_bd { struct sk_buff *skb; } __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); +#define XAE_NUM_MISC_CLOCKS 3 + /** * struct axienet_local - axienet private per device data * @ndev: Pointer for net_device to which it will be attached. @@ -385,7 +387,8 @@ struct axidma_bd { * @phylink_config: phylink configuration settings * @pcs_phy: Reference to PCS/PMA PHY if used * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core - * @clk: Clock for AXI bus + * @axi_clk: AXI4-Lite bus clock + * @misc_clks: Misc ethernet clocks (AXI4-Stream, Ref, MGT clocks) * @mii_bus: Pointer to MII bus structure * @mii_clk_div: MII bus clock divider value * @regs_start: Resource start for axienet device addresses @@ -434,7 +437,8 @@ struct axienet_local { bool switch_x_sgmii; - struct clk *clk; + struct clk *axi_clk; + struct clk_bulk_data misc_clks[XAE_NUM_MISC_CLOCKS]; struct mii_bus *mii_bus; u8 mii_clk_div; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 5d677db0aee5..9635101fbb88 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1863,17 +1863,35 @@ static int axienet_probe(struct platform_device *pdev) lp->rx_bd_num = RX_BD_NUM_DEFAULT; lp->tx_bd_num = TX_BD_NUM_DEFAULT; - lp->clk = devm_clk_get_optional(&pdev->dev, NULL); - if (IS_ERR(lp->clk)) { - ret = PTR_ERR(lp->clk); + lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk"); + if (!lp->axi_clk) { + /* For backward compatibility, if named AXI clock is not present, +* treat the first clock specified as the AXI clock. +*/ + lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL); + } + if (IS_ERR(lp->axi_clk)) { + ret = PTR_ERR(lp->axi_clk); goto free_netdev; } - ret = clk_prepare_enable(lp->clk); + ret = clk_prepare_enable(lp->axi_clk); if (ret) { - dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); + dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n", ret); goto free_netdev; } + lp->misc_clks[0].id = "axis_clk"; + lp->misc_clks[1].id = "ref_clk"; + lp->misc_clks[2].id = "mgt_clk"; + + ret = devm_clk_bulk_get_optional(&pdev->dev, XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + + ret = clk_bulk_prepare_enable(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); lp->regs = devm_ioremap_resource(&pdev->dev, ethres); @@ -2109,7 +2127,8 @@ static int axienet_probe(struct platform_device *pdev) of_node_put(lp->phy_node); cleanup_clk: - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); free_netdev: free_netdev(ndev); @@ -2132,7 +2151,8 @@ static int axienet_remove(struct platform_device *pdev) axienet_mdio_teardown(lp); - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); of_node_put(lp->phy_node); lp->phy_node = NULL; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_
[PATCH net-next v4 0/2] axienet clock additions
Add support to the axienet driver for controlling all of the clocks that the logic core may utilize. Changed since v3: -Added Acked-by to patch 1 -Now applies to net-next tree after earlier patches merged in - code unchanged from v3 Changed since v2: -Additional clock description clarification Changed since v1: -Clarified clock usages in documentation and code comments Robert Hancock (2): dt-bindings: net: xilinx_axienet: Document additional clocks net: axienet: Enable more clocks .../bindings/net/xilinx_axienet.txt | 25 ++ drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 4 files changed, 54 insertions(+), 17 deletions(-) -- 2.27.0
Re: [PATCH net-next v3 v3 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
On Wed, 2021-03-24 at 11:08 -0600, Rob Herring wrote: > On Fri, Mar 12, 2021 at 01:52:13PM -0600, Robert Hancock wrote: > > Update DT bindings to describe all of the clocks that the axienet > > driver will now be able to make use of. > > > > Signed-off-by: Robert Hancock > > --- > > .../bindings/net/xilinx_axienet.txt | 25 ++- > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > index 2cd452419ed0..b8e4894bc634 100644 > > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > @@ -42,11 +42,23 @@ Optional properties: > > support both 1000BaseX and SGMII modes. If set, the phy-mode > > should be set to match the mode selected on core reset (i.e. > > by the basex_or_sgmii core input line). > > -- clocks : AXI bus clock for the device. Refer to common clock bindings. > > - Used to calculate MDIO clock divisor. If not specified, it is > > - auto-detected from the CPU clock (but only on platforms where > > - this is possible). New device trees should specify this - the > > - auto detection is only for backward compatibility. > > +- clock-names: Tuple listing input clock names. Possible clocks: > > + s_axi_lite_clk: Clock for AXI register slave interface > > + axis_clk: AXI4-Stream clock for TXD RXD TXC and RXS > > interfaces > > + ref_clk: Ethernet reference clock, used by signal delay > > + primitives and transceivers > > + mgt_clk: MGT reference clock (used by optional internal > > + PCS/PMA PHY) > > '_clk' is redundant. True, but there are existing device trees which already referenced these names because those are what was used by the Xilinx version of this driver and hence the Xilinx device tree generation software. So for compatibility I think we are kind of stuck with those names.. > > > + > > + Note that if s_axi_lite_clk is not specified by name, the > > + first clock of any name is used for this. If that is also not > > + specified, the clock rate is auto-detected from the CPU clock > > + (but only on platforms where this is possible). New device > > + trees should specify all applicable clocks by name - the > > + fallbacks to an unnamed clock or to CPU clock are only for > > + backward compatibility. > > +- clocks:Phandles to input clocks matching clock-names. Refer to > > common > > + clock bindings. > > - axistream-connected: Reference to another node which contains the > > resources > >for the AXI DMA controller used by this device. > >If this is specified, the DMA-related resources from > > that > > @@ -62,7 +74,8 @@ Example: > > device_type = "network"; > > interrupt-parent = <µblaze_0_axi_intc>; > > interrupts = <2 0 1>; > > - clocks = <&axi_clk>; > > + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", > > "mgt_clk"; > > + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, > > <&mgt_clk>; > > phy-mode = "mii"; > > reg = <0x40c0 0x4 0x50c0 0x4>; > > xlnx,rxcsum = <0x2>; > > -- > > 2.27.0 > > -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
Re: [PATCH net-next 0/2] axienet clock additions
On Sun, 2021-03-14 at 14:27 -0700, David Miller wrote: > From: Robert Hancock > Date: Thu, 11 Mar 2021 14:11:15 -0600 > > > Add support to the axienet driver for controlling all of the clocks that > > the logic core may utilize. > > This series does not apply to net-next, please respin. > > Thanks. It's dependent on this patch which is now in the net tree: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=59cd4f19267a0aab87a8c07e4426eb7187ee548d I could respin a version without that dependency to apply to net-next now, but that will cause some conflicts later when net is merged into net-next.. not sure how this should be handled?
Re: [PATCH net-next 2/2] net: macb: Disable PCS auto-negotiation for SGMII fixed-link mode
On Sat, 2021-03-13 at 02:45 +0100, Andrew Lunn wrote: > On Thu, Mar 11, 2021 at 02:18:13PM -0600, Robert Hancock wrote: > > When using a fixed-link configuration in SGMII mode, it's not really > > sensible to have auto-negotiation enabled since the link settings are > > fixed by definition. In other configurations, such as an SGMII > > connection to a PHY, it should generally be enabled. > > So how do you tell the PCS it should be doing 10Mbps over the SGMII > link? I'm assuming it is the PCS which does the bit replication, not > the MAC? I'm not sure if this is the same for all devices using this Cadence IP, but the register documentation I have for the Xilinx UltraScale+ MPSoC we are using indicates this PCS is only capable of 1000 Mbps speeds: https://www.xilinx.com/html_docs/registers/ug1087/gem___pcs_control.html So it doesn't actually seem applicable in this case. > > I'm surprised you are even using SGMII with a fixed link. 1000BaseX is > the norm, and then you don't need to worry about the speed. > That would be a bit simpler, yes - but it seems like this hardware is set up more for SGMII mode - it's not entirely clear to me that 1000BaseX is supported in the hardware, and it's not currently supported in the driver that I can see. -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
[PATCH net-next v3 v3 2/2] net: axienet: Enable more clocks
This driver was only enabling the first clock on the device, regardless of its name. However, this controller logic can have multiple clocks which should all be enabled. Add support for enabling additional clocks. The clock names used are matching those used in the Xilinx version of this driver as well as the Xilinx device tree generator, except for mgt_clk which is not present there. For backward compatibility, if no named clocks are present, the first clock present is used for determining the MDIO bus clock divider. Reviewed-by: Radhey Shyam Pandey Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 1e966a39967e..708769349f76 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -376,6 +376,8 @@ struct axidma_bd { struct sk_buff *skb; } __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); +#define XAE_NUM_MISC_CLOCKS 3 + /** * struct axienet_local - axienet private per device data * @ndev: Pointer for net_device to which it will be attached. @@ -385,7 +387,8 @@ struct axidma_bd { * @phylink_config: phylink configuration settings * @pcs_phy: Reference to PCS/PMA PHY if used * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core - * @clk: Clock for AXI bus + * @axi_clk: AXI4-Lite bus clock + * @misc_clks: Misc ethernet clocks (AXI4-Stream, Ref, MGT clocks) * @mii_bus: Pointer to MII bus structure * @mii_clk_div: MII bus clock divider value * @regs_start: Resource start for axienet device addresses @@ -434,7 +437,8 @@ struct axienet_local { bool switch_x_sgmii; - struct clk *clk; + struct clk *axi_clk; + struct clk_bulk_data misc_clks[XAE_NUM_MISC_CLOCKS]; struct mii_bus *mii_bus; u8 mii_clk_div; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 5d677db0aee5..9635101fbb88 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1863,17 +1863,35 @@ static int axienet_probe(struct platform_device *pdev) lp->rx_bd_num = RX_BD_NUM_DEFAULT; lp->tx_bd_num = TX_BD_NUM_DEFAULT; - lp->clk = devm_clk_get_optional(&pdev->dev, NULL); - if (IS_ERR(lp->clk)) { - ret = PTR_ERR(lp->clk); + lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk"); + if (!lp->axi_clk) { + /* For backward compatibility, if named AXI clock is not present, +* treat the first clock specified as the AXI clock. +*/ + lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL); + } + if (IS_ERR(lp->axi_clk)) { + ret = PTR_ERR(lp->axi_clk); goto free_netdev; } - ret = clk_prepare_enable(lp->clk); + ret = clk_prepare_enable(lp->axi_clk); if (ret) { - dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); + dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n", ret); goto free_netdev; } + lp->misc_clks[0].id = "axis_clk"; + lp->misc_clks[1].id = "ref_clk"; + lp->misc_clks[2].id = "mgt_clk"; + + ret = devm_clk_bulk_get_optional(&pdev->dev, XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + + ret = clk_bulk_prepare_enable(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); lp->regs = devm_ioremap_resource(&pdev->dev, ethres); @@ -2109,7 +2127,8 @@ static int axienet_probe(struct platform_device *pdev) of_node_put(lp->phy_node); cleanup_clk: - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); free_netdev: free_netdev(ndev); @@ -2132,7 +2151,8 @@ static int axienet_remove(struct platform_device *pdev) axienet_mdio_teardown(lp); - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); of_node_put(lp->phy_node); lp->phy_node = NULL; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_
[PATCH net-next v3 v3 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
Update DT bindings to describe all of the clocks that the axienet driver will now be able to make use of. Signed-off-by: Robert Hancock --- .../bindings/net/xilinx_axienet.txt | 25 ++- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 2cd452419ed0..b8e4894bc634 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -42,11 +42,23 @@ Optional properties: support both 1000BaseX and SGMII modes. If set, the phy-mode should be set to match the mode selected on core reset (i.e. by the basex_or_sgmii core input line). -- clocks : AXI bus clock for the device. Refer to common clock bindings. - Used to calculate MDIO clock divisor. If not specified, it is - auto-detected from the CPU clock (but only on platforms where - this is possible). New device trees should specify this - the - auto detection is only for backward compatibility. +- clock-names: Tuple listing input clock names. Possible clocks: + s_axi_lite_clk: Clock for AXI register slave interface + axis_clk: AXI4-Stream clock for TXD RXD TXC and RXS interfaces + ref_clk: Ethernet reference clock, used by signal delay + primitives and transceivers + mgt_clk: MGT reference clock (used by optional internal + PCS/PMA PHY) + + Note that if s_axi_lite_clk is not specified by name, the + first clock of any name is used for this. If that is also not + specified, the clock rate is auto-detected from the CPU clock + (but only on platforms where this is possible). New device + trees should specify all applicable clocks by name - the + fallbacks to an unnamed clock or to CPU clock are only for + backward compatibility. +- clocks:Phandles to input clocks matching clock-names. Refer to common + clock bindings. - axistream-connected: Reference to another node which contains the resources for the AXI DMA controller used by this device. If this is specified, the DMA-related resources from that @@ -62,7 +74,8 @@ Example: device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; interrupts = <2 0 1>; - clocks = <&axi_clk>; + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk"; + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, <&mgt_clk>; phy-mode = "mii"; reg = <0x40c0 0x4 0x50c0 0x4>; xlnx,rxcsum = <0x2>; -- 2.27.0
[PATCH net-next v3 v3 0/2] axienet clock additions
Add support to the axienet driver for controlling all of the clocks that the logic core may utilize. Note this patch set is dependent on "net: axienet: Fix probe error cleanup" which has been submitted for the net tree. Changed since v2: -Additional clock description clarification Changed since v1: -Clarified clock usages in documentation and code comments Robert Hancock (2): dt-bindings: net: xilinx_axienet: Document additional clocks net: axienet: Enable more clocks .../bindings/net/xilinx_axienet.txt | 25 ++ drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 4 files changed, 54 insertions(+), 17 deletions(-) -- 2.27.0
Re: [PATCH net-next v2 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
On Fri, 2021-03-12 at 18:39 +, Radhey Shyam Pandey wrote: > > -Original Message- > > From: Robert Hancock > > Sent: Friday, March 12, 2021 11:13 PM > > To: Radhey Shyam Pandey ; da...@davemloft.net; > > k...@kernel.org > > Cc: netdev@vger.kernel.org; devicet...@vger.kernel.org; Robert Hancock > > > > Subject: [PATCH net-next v2 1/2] dt-bindings: net: xilinx_axienet: Document > > additional clocks > > > > Update DT bindings to describe all of the clocks that the axienet driver > > will > > now be able to make use of. > > > > Signed-off-by: Robert Hancock > > --- > > .../bindings/net/xilinx_axienet.txt | 25 ++- > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > index 2cd452419ed0..5df5ba449b8f 100644 > > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > @@ -42,11 +42,23 @@ Optional properties: > > support both 1000BaseX and SGMII modes. If set, the phy- > > mode > > should be set to match the mode selected on core reset > > (i.e. > > by the basex_or_sgmii core input line). > > -- clocks : AXI bus clock for the device. Refer to common clock > > bindings. > > - Used to calculate MDIO clock divisor. If not specified, it is > > - auto-detected from the CPU clock (but only on platforms > > where > > - this is possible). New device trees should specify this - the > > - auto detection is only for backward compatibility. > > +- clock-names: Tuple listing input clock names. Possible clocks: > > + s_axi_lite_clk: Clock for AXI register slave interface > > + axis_clk: AXI stream clock for DMA block > > Description for axis_clk should be- > axis_clk: AXI4-Stream clock for TXD RXD TXC and RXS interfaces. Missed this, will add in. > In this patch I assume we are only adding additional clocks for > 1G ethernet subsystem. For dma clocking support we need to add > more clocks and better to add them in a separate patch. Please refer to > xilinx tree. Correct, here I am just focusing on clocks which are connected to the Ethernet core itself. It seems the DMA core clocks would be more difficult since those clocks are attached to the DMA node in the device tree (based on the Vitis- generated PL device tree content) and not to the node belonging to the Ethernet platform device. Looking at the Xilinx version of this driver, it seems to implement retrieving these clocks, but it is using devm_clk_get on the same platform device used for the Ethernet core, so I am confused how that is working (or maybe it isn't?) The DMA core clocks are less of a concern for us, since (at least in our design) they are driven by the same clock source as one of the Ethernet core clocks, so they should always be enabled when the Ethernet core clocks are enabled. > > + ref_clk: Ethernet reference clock, used by signal delay > > + primitives and transceivers > > + mgt_clk: MGT reference clock (used by optional internal > > + PCS/PMA PHY) > > + > > + Note that if s_axi_lite_clk is not specified by name, the > > + first clock of any name is used for this. If that is also not > > + specified, the clock rate is auto-detected from the CPU > > clock > > + (but only on platforms where this is possible). New device > > + trees should specify all applicable clocks by name - the > > + fallbacks to an unnamed clock or to CPU clock are only for > > + backward compatibility. > > +- clocks:Phandles to input clocks matching clock-names. Refer to > > common > > + clock bindings. > > - axistream-connected: Reference to another node which contains the > > resources > >for the AXI DMA controller used by this device. > >If this is specified, the DMA-related resources from > > that > > @@ -62,7 +74,8 @@ Example: > > device_type = "network"; > > interrupt-parent = <µblaze_0_axi_intc>; > > interrupts = <2 0 1>; > > - clocks = <&axi_clk>; > > + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", > > "mgt_clk"; > > + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, > > <&mgt_clk>; > > phy-mode = "mii"; > > reg = <0x40c0 0x4 0x50c0 0x4>; > > xlnx,rxcsum = <0x2>; > > -- > > 2.27.0
[PATCH net-next v2 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
Update DT bindings to describe all of the clocks that the axienet driver will now be able to make use of. Signed-off-by: Robert Hancock --- .../bindings/net/xilinx_axienet.txt | 25 ++- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 2cd452419ed0..5df5ba449b8f 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -42,11 +42,23 @@ Optional properties: support both 1000BaseX and SGMII modes. If set, the phy-mode should be set to match the mode selected on core reset (i.e. by the basex_or_sgmii core input line). -- clocks : AXI bus clock for the device. Refer to common clock bindings. - Used to calculate MDIO clock divisor. If not specified, it is - auto-detected from the CPU clock (but only on platforms where - this is possible). New device trees should specify this - the - auto detection is only for backward compatibility. +- clock-names: Tuple listing input clock names. Possible clocks: + s_axi_lite_clk: Clock for AXI register slave interface + axis_clk: AXI stream clock for DMA block + ref_clk: Ethernet reference clock, used by signal delay + primitives and transceivers + mgt_clk: MGT reference clock (used by optional internal + PCS/PMA PHY) + + Note that if s_axi_lite_clk is not specified by name, the + first clock of any name is used for this. If that is also not + specified, the clock rate is auto-detected from the CPU clock + (but only on platforms where this is possible). New device + trees should specify all applicable clocks by name - the + fallbacks to an unnamed clock or to CPU clock are only for + backward compatibility. +- clocks:Phandles to input clocks matching clock-names. Refer to common + clock bindings. - axistream-connected: Reference to another node which contains the resources for the AXI DMA controller used by this device. If this is specified, the DMA-related resources from that @@ -62,7 +74,8 @@ Example: device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; interrupts = <2 0 1>; - clocks = <&axi_clk>; + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk"; + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, <&mgt_clk>; phy-mode = "mii"; reg = <0x40c0 0x4 0x50c0 0x4>; xlnx,rxcsum = <0x2>; -- 2.27.0
[PATCH net-next v2 0/2] axienet clock additions
Add support to the axienet driver for controlling all of the clocks that the logic core may utilize. Note this patch set is dependent on "net: axienet: Fix probe error cleanup" which has been submitted for the net tree. Changed since v1: -Clarified clock usages in documentation and code comments Robert Hancock (2): dt-bindings: net: xilinx_axienet: Document additional clocks net: axienet: Enable more clocks .../bindings/net/xilinx_axienet.txt | 25 ++ drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 4 files changed, 54 insertions(+), 17 deletions(-) -- 2.27.0
[PATCH net-next v2 2/2] net: axienet: Enable more clocks
This driver was only enabling the first clock on the device, regardless of its name. However, this controller logic can have multiple clocks which should all be enabled. Add support for enabling additional clocks. The clock names used are matching those used in the Xilinx version of this driver as well as the Xilinx device tree generator, except for mgt_clk which is not present there. For backward compatibility, if no named clocks are present, the first clock present is used for determining the MDIO bus clock divider. Reviewed-by: Radhey Shyam Pandey Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 1e966a39967e..708769349f76 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -376,6 +376,8 @@ struct axidma_bd { struct sk_buff *skb; } __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); +#define XAE_NUM_MISC_CLOCKS 3 + /** * struct axienet_local - axienet private per device data * @ndev: Pointer for net_device to which it will be attached. @@ -385,7 +387,8 @@ struct axidma_bd { * @phylink_config: phylink configuration settings * @pcs_phy: Reference to PCS/PMA PHY if used * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core - * @clk: Clock for AXI bus + * @axi_clk: AXI4-Lite bus clock + * @misc_clks: Misc ethernet clocks (AXI4-Stream, Ref, MGT clocks) * @mii_bus: Pointer to MII bus structure * @mii_clk_div: MII bus clock divider value * @regs_start: Resource start for axienet device addresses @@ -434,7 +437,8 @@ struct axienet_local { bool switch_x_sgmii; - struct clk *clk; + struct clk *axi_clk; + struct clk_bulk_data misc_clks[XAE_NUM_MISC_CLOCKS]; struct mii_bus *mii_bus; u8 mii_clk_div; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 5d677db0aee5..9635101fbb88 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1863,17 +1863,35 @@ static int axienet_probe(struct platform_device *pdev) lp->rx_bd_num = RX_BD_NUM_DEFAULT; lp->tx_bd_num = TX_BD_NUM_DEFAULT; - lp->clk = devm_clk_get_optional(&pdev->dev, NULL); - if (IS_ERR(lp->clk)) { - ret = PTR_ERR(lp->clk); + lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk"); + if (!lp->axi_clk) { + /* For backward compatibility, if named AXI clock is not present, +* treat the first clock specified as the AXI clock. +*/ + lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL); + } + if (IS_ERR(lp->axi_clk)) { + ret = PTR_ERR(lp->axi_clk); goto free_netdev; } - ret = clk_prepare_enable(lp->clk); + ret = clk_prepare_enable(lp->axi_clk); if (ret) { - dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); + dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n", ret); goto free_netdev; } + lp->misc_clks[0].id = "axis_clk"; + lp->misc_clks[1].id = "ref_clk"; + lp->misc_clks[2].id = "mgt_clk"; + + ret = devm_clk_bulk_get_optional(&pdev->dev, XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + + ret = clk_bulk_prepare_enable(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); lp->regs = devm_ioremap_resource(&pdev->dev, ethres); @@ -2109,7 +2127,8 @@ static int axienet_probe(struct platform_device *pdev) of_node_put(lp->phy_node); cleanup_clk: - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); free_netdev: free_netdev(ndev); @@ -2132,7 +2151,8 @@ static int axienet_remove(struct platform_device *pdev) axienet_mdio_teardown(lp); - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); of_node_put(lp->phy_node); lp->phy_node = NULL; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_
Re: [PATCH net-next 2/2] net: axienet: Enable more clocks
On Thu, 2021-03-11 at 14:11 -0600, Robert Hancock wrote: > This driver was only enabling the first clock on the device, regardless > of its name. However, this controller logic can have multiple clocks > which should all be enabled. Add support for enabling additional clocks. > The clock names used are matching those used in the Xilinx version of this > driver as well as the Xilinx device tree generator, except for mgt_clk > which is not present there. > > For backward compatibility, if no named clocks are present, the first > clock present is used for determining the MDIO bus clock divider. > > Signed-off-by: Robert Hancock > Note that this patch is dependent on "net: axienet: Fix probe error cleanup" ( https://patchwork.kernel.org/project/netdevbpf/patch/20210311200518.3801916-1-robert.hanc...@calian.com/ ) which is tagged for the net tree. > --- > drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- > .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ > .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- > 3 files changed, 35 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > index 1e966a39967e..92f7cefb345e 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > @@ -376,6 +376,8 @@ struct axidma_bd { > struct sk_buff *skb; > } __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); > > +#define XAE_NUM_MISC_CLOCKS 3 > + > /** > * struct axienet_local - axienet private per device data > * @ndev:Pointer for net_device to which it will be attached. > @@ -385,7 +387,8 @@ struct axidma_bd { > * @phylink_config: phylink configuration settings > * @pcs_phy: Reference to PCS/PMA PHY if used > * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in > the core > - * @clk: Clock for AXI bus > + * @axi_clk: AXI bus clock > + * @misc_clks: Other device clocks > * @mii_bus: Pointer to MII bus structure > * @mii_clk_div: MII bus clock divider value > * @regs_start: Resource start for axienet device addresses > @@ -434,7 +437,8 @@ struct axienet_local { > > bool switch_x_sgmii; > > - struct clk *clk; > + struct clk *axi_clk; > + struct clk_bulk_data misc_clks[XAE_NUM_MISC_CLOCKS]; > > struct mii_bus *mii_bus; > u8 mii_clk_div; > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index 5d677db0aee5..9635101fbb88 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -1863,17 +1863,35 @@ static int axienet_probe(struct platform_device > *pdev) > lp->rx_bd_num = RX_BD_NUM_DEFAULT; > lp->tx_bd_num = TX_BD_NUM_DEFAULT; > > - lp->clk = devm_clk_get_optional(&pdev->dev, NULL); > - if (IS_ERR(lp->clk)) { > - ret = PTR_ERR(lp->clk); > + lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk"); > + if (!lp->axi_clk) { > + /* For backward compatibility, if named AXI clock is not > present, > + * treat the first clock specified as the AXI clock. > + */ > + lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL); > + } > + if (IS_ERR(lp->axi_clk)) { > + ret = PTR_ERR(lp->axi_clk); > goto free_netdev; > } > - ret = clk_prepare_enable(lp->clk); > + ret = clk_prepare_enable(lp->axi_clk); > if (ret) { > - dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); > + dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n", ret); > goto free_netdev; > } > > + lp->misc_clks[0].id = "axis_clk"; > + lp->misc_clks[1].id = "ref_clk"; > + lp->misc_clks[2].id = "mgt_clk"; > + > + ret = devm_clk_bulk_get_optional(&pdev->dev, XAE_NUM_MISC_CLOCKS, lp- > >misc_clks); > + if (ret) > + goto cleanup_clk; > + > + ret = clk_bulk_prepare_enable(XAE_NUM_MISC_CLOCKS, lp->misc_clks); > + if (ret) > + goto cleanup_clk; > + > /* Map device registers */ > ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); > lp->regs = devm_ioremap_resource(&pdev->dev, ethres); > @@ -2109,7 +2127,8 @@ static int axienet_probe(struct platform_device *pdev) > of_node_put(lp->phy_node); > > cleanup_clk: > -
[PATCH net-next 0/2] macb SGMII fixed-link fixes
Some fixes to the macb driver for use in SGMII mode with a fixed-link (such as for chip-to-chip connectivity). Robert Hancock (2): net: macb: poll for fixed link state in SGMII mode net: macb: Disable PCS auto-negotiation for SGMII fixed-link mode drivers/net/ethernet/cadence/macb.h | 14 +++ drivers/net/ethernet/cadence/macb_main.c | 30 2 files changed, 44 insertions(+) -- 2.27.0
[PATCH net-next 2/2] net: macb: Disable PCS auto-negotiation for SGMII fixed-link mode
When using a fixed-link configuration in SGMII mode, it's not really sensible to have auto-negotiation enabled since the link settings are fixed by definition. In other configurations, such as an SGMII connection to a PHY, it should generally be enabled. Signed-off-by: Robert Hancock --- drivers/net/ethernet/cadence/macb.h | 14 ++ drivers/net/ethernet/cadence/macb_main.c | 16 2 files changed, 30 insertions(+) diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index d8c68906525a..d8d87213697c 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -159,6 +159,16 @@ #define GEM_PEFTN 0x01f4 /* PTP Peer Event Frame Tx Ns */ #define GEM_PEFRSL 0x01f8 /* PTP Peer Event Frame Rx Sec Low */ #define GEM_PEFRN 0x01fc /* PTP Peer Event Frame Rx Ns */ +#define GEM_PCSCNTRL 0x0200 /* PCS Control */ +#define GEM_PCSSTS 0x0204 /* PCS Status */ +#define GEM_PCSPHYTOPID0x0208 /* PCS PHY Top ID */ +#define GEM_PCSPHYBOTID0x020c /* PCS PHY Bottom ID */ +#define GEM_PCSANADV 0x0210 /* PCS AN Advertisement */ +#define GEM_PCSANLPBASE0x0214 /* PCS AN Link Partner Base */ +#define GEM_PCSANEXP 0x0218 /* PCS AN Expansion */ +#define GEM_PCSANNPTX 0x021c /* PCS AN Next Page TX */ +#define GEM_PCSANNPLP 0x0220 /* PCS AN Next Page LP */ +#define GEM_PCSANEXTSTS0x023c /* PCS AN Extended Status */ #define GEM_DCFG1 0x0280 /* Design Config 1 */ #define GEM_DCFG2 0x0284 /* Design Config 2 */ #define GEM_DCFG3 0x0288 /* Design Config 3 */ @@ -478,6 +488,10 @@ #define GEM_HS_MAC_SPEED_OFFSET0 #define GEM_HS_MAC_SPEED_SIZE 3 +/* Bitfields in PCSCNTRL */ +#define GEM_PCSAUTONEG_OFFSET 12 +#define GEM_PCSAUTONEG_SIZE1 + /* Bitfields in DCFG1. */ #define GEM_IRQCOR_OFFSET 23 #define GEM_IRQCOR_SIZE1 diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index ca72a16c8da3..e7c123aadf56 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -694,6 +694,22 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode, if (old_ncr ^ ncr) macb_or_gem_writel(bp, NCR, ncr); + /* Disable AN for SGMII fixed link configuration, enable otherwise. +* Must be written after PCSSEL is set in NCFGR, +* otherwise writes will not take effect. +*/ + if (macb_is_gem(bp) && state->interface == PHY_INTERFACE_MODE_SGMII) { + u32 pcsctrl, old_pcsctrl; + + old_pcsctrl = gem_readl(bp, PCSCNTRL); + if (mode == MLO_AN_FIXED) + pcsctrl = old_pcsctrl & ~GEM_BIT(PCSAUTONEG); + else + pcsctrl = old_pcsctrl | GEM_BIT(PCSAUTONEG); + if (old_pcsctrl != pcsctrl) + gem_writel(bp, PCSCNTRL, pcsctrl); + } + spin_unlock_irqrestore(&bp->lock, flags); } -- 2.27.0
[PATCH net-next 1/2] net: macb: poll for fixed link state in SGMII mode
When using a fixed-link configuration with GEM in SGMII mode, such as for a chip-to-chip interconnect, the link state was always showing as established regardless of the actual connectivity state. We can monitor the pcs_link_state bit in the Network Status register to determine whether the PCS link state is actually up. Signed-off-by: Robert Hancock --- drivers/net/ethernet/cadence/macb_main.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 15362d016a87..ca72a16c8da3 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -847,6 +847,15 @@ static int macb_phylink_connect(struct macb *bp) return 0; } +static void macb_get_pcs_fixed_state(struct phylink_config *config, +struct phylink_link_state *state) +{ + struct net_device *ndev = to_net_dev(config->dev); + struct macb *bp = netdev_priv(ndev); + + state->link = (macb_readl(bp, NSR) & MACB_BIT(NSR_LINK)) != 0; +} + /* based on au1000_eth. c*/ static int macb_mii_probe(struct net_device *dev) { @@ -855,6 +864,11 @@ static int macb_mii_probe(struct net_device *dev) bp->phylink_config.dev = &dev->dev; bp->phylink_config.type = PHYLINK_NETDEV; + if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) { + bp->phylink_config.poll_fixed_state = true; + bp->phylink_config.get_fixed_state = macb_get_pcs_fixed_state; + } + bp->phylink = phylink_create(&bp->phylink_config, bp->pdev->dev.fwnode, bp->phy_interface, &macb_phylink_ops); if (IS_ERR(bp->phylink)) { -- 2.27.0
[PATCH net-next 2/2] net: axienet: Enable more clocks
This driver was only enabling the first clock on the device, regardless of its name. However, this controller logic can have multiple clocks which should all be enabled. Add support for enabling additional clocks. The clock names used are matching those used in the Xilinx version of this driver as well as the Xilinx device tree generator, except for mgt_clk which is not present there. For backward compatibility, if no named clocks are present, the first clock present is used for determining the MDIO bus clock divider. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 1e966a39967e..92f7cefb345e 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -376,6 +376,8 @@ struct axidma_bd { struct sk_buff *skb; } __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); +#define XAE_NUM_MISC_CLOCKS 3 + /** * struct axienet_local - axienet private per device data * @ndev: Pointer for net_device to which it will be attached. @@ -385,7 +387,8 @@ struct axidma_bd { * @phylink_config: phylink configuration settings * @pcs_phy: Reference to PCS/PMA PHY if used * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core - * @clk: Clock for AXI bus + * @axi_clk: AXI bus clock + * @misc_clks: Other device clocks * @mii_bus: Pointer to MII bus structure * @mii_clk_div: MII bus clock divider value * @regs_start: Resource start for axienet device addresses @@ -434,7 +437,8 @@ struct axienet_local { bool switch_x_sgmii; - struct clk *clk; + struct clk *axi_clk; + struct clk_bulk_data misc_clks[XAE_NUM_MISC_CLOCKS]; struct mii_bus *mii_bus; u8 mii_clk_div; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 5d677db0aee5..9635101fbb88 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1863,17 +1863,35 @@ static int axienet_probe(struct platform_device *pdev) lp->rx_bd_num = RX_BD_NUM_DEFAULT; lp->tx_bd_num = TX_BD_NUM_DEFAULT; - lp->clk = devm_clk_get_optional(&pdev->dev, NULL); - if (IS_ERR(lp->clk)) { - ret = PTR_ERR(lp->clk); + lp->axi_clk = devm_clk_get_optional(&pdev->dev, "s_axi_lite_clk"); + if (!lp->axi_clk) { + /* For backward compatibility, if named AXI clock is not present, +* treat the first clock specified as the AXI clock. +*/ + lp->axi_clk = devm_clk_get_optional(&pdev->dev, NULL); + } + if (IS_ERR(lp->axi_clk)) { + ret = PTR_ERR(lp->axi_clk); goto free_netdev; } - ret = clk_prepare_enable(lp->clk); + ret = clk_prepare_enable(lp->axi_clk); if (ret) { - dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); + dev_err(&pdev->dev, "Unable to enable AXI clock: %d\n", ret); goto free_netdev; } + lp->misc_clks[0].id = "axis_clk"; + lp->misc_clks[1].id = "ref_clk"; + lp->misc_clks[2].id = "mgt_clk"; + + ret = devm_clk_bulk_get_optional(&pdev->dev, XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + + ret = clk_bulk_prepare_enable(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + if (ret) + goto cleanup_clk; + /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); lp->regs = devm_ioremap_resource(&pdev->dev, ethres); @@ -2109,7 +2127,8 @@ static int axienet_probe(struct platform_device *pdev) of_node_put(lp->phy_node); cleanup_clk: - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); free_netdev: free_netdev(ndev); @@ -2132,7 +2151,8 @@ static int axienet_remove(struct platform_device *pdev) axienet_mdio_teardown(lp); - clk_disable_unprepare(lp->clk); + clk_bulk_disable_unprepare(XAE_NUM_MISC_CLOCKS, lp->misc_clks); + clk_disable_unprepare(lp->axi_clk); of_node_put(lp->phy_node); lp->phy_node = NULL; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c index 9c014cee34b2..48f544f6c999 100644 --- a/drivers/net
[PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
Update DT bindings to describe all of the clocks that the axienet driver will now be able to make use of. Signed-off-by: Robert Hancock --- .../bindings/net/xilinx_axienet.txt | 23 ++- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 2cd452419ed0..1ee1c6c5fc66 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -42,11 +42,21 @@ Optional properties: support both 1000BaseX and SGMII modes. If set, the phy-mode should be set to match the mode selected on core reset (i.e. by the basex_or_sgmii core input line). -- clocks : AXI bus clock for the device. Refer to common clock bindings. - Used to calculate MDIO clock divisor. If not specified, it is - auto-detected from the CPU clock (but only on platforms where - this is possible). New device trees should specify this - the - auto detection is only for backward compatibility. +- clock-names: Tuple listing input clock names. Possible clocks: + s_axi_lite_clk: Clock for AXI register slave interface + axis_clk: AXI stream clock for DMA block + ref_clk: Ethernet reference clock + mgt_clk: MGT reference clock (used by internal PCS/PMA PHY) + + Note that if s_axi_lite_clk is not specified by name, the + first clock of any name is used for this. If that is also not + specified, the clock rate is auto-detected from the CPU clock + (but only on platforms where this is possible). New device + trees should specify all applicable clocks by name - the + fallbacks to an unnamed clock or to CPU clock are only for + backward compatibility. +- clocks:Phandles to input clocks matching clock-names. Refer to common + clock bindings. - axistream-connected: Reference to another node which contains the resources for the AXI DMA controller used by this device. If this is specified, the DMA-related resources from that @@ -62,7 +72,8 @@ Example: device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; interrupts = <2 0 1>; - clocks = <&axi_clk>; + clock-names = "s_axi_lite_clk", "axis_clk", "ref_clk", "mgt_clk"; + clocks = <&axi_clk>, <&axi_clk>, <&pl_enet_ref_clk>, <&mgt_clk>; phy-mode = "mii"; reg = <0x40c0 0x4 0x50c0 0x4>; xlnx,rxcsum = <0x2>; -- 2.27.0
[PATCH net-next 0/2] axienet clock additions
Add support to the axienet driver for controlling all of the clocks that the logic core may utilize. Robert Hancock (2): dt-bindings: net: xilinx_axienet: Document additional clocks net: axienet: Enable more clocks .../bindings/net/xilinx_axienet.txt | 23 + drivers/net/ethernet/xilinx/xilinx_axienet.h | 8 +++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++ .../net/ethernet/xilinx/xilinx_axienet_mdio.c | 4 +-- 4 files changed, 52 insertions(+), 17 deletions(-) -- 2.27.0
[PATCH net] net: axienet: Fix probe error cleanup
The driver did not always clean up all allocated resources when probe failed. Fix the probe cleanup path to clean up everything that was allocated. Fixes: 57baf8cc70 ("net: axienet: Handle deferred probe on clock properly") Signed-off-by: Robert Hancock --- .../net/ethernet/xilinx/xilinx_axienet_main.c | 37 +-- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 3a8775e0ca55..5d677db0aee5 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1880,7 +1880,7 @@ static int axienet_probe(struct platform_device *pdev) if (IS_ERR(lp->regs)) { dev_err(&pdev->dev, "could not map Axi Ethernet regs.\n"); ret = PTR_ERR(lp->regs); - goto free_netdev; + goto cleanup_clk; } lp->regs_start = ethres->start; @@ -1958,18 +1958,18 @@ static int axienet_probe(struct platform_device *pdev) break; default: ret = -EINVAL; - goto free_netdev; + goto cleanup_clk; } } else { ret = of_get_phy_mode(pdev->dev.of_node, &lp->phy_mode); if (ret) - goto free_netdev; + goto cleanup_clk; } if (lp->switch_x_sgmii && lp->phy_mode != PHY_INTERFACE_MODE_SGMII && lp->phy_mode != PHY_INTERFACE_MODE_1000BASEX) { dev_err(&pdev->dev, "xlnx,switch-x-sgmii only supported with SGMII or 1000BaseX\n"); ret = -EINVAL; - goto free_netdev; + goto cleanup_clk; } /* Find the DMA node, map the DMA registers, and decode the DMA IRQs */ @@ -1982,7 +1982,7 @@ static int axienet_probe(struct platform_device *pdev) dev_err(&pdev->dev, "unable to get DMA resource\n"); of_node_put(np); - goto free_netdev; + goto cleanup_clk; } lp->dma_regs = devm_ioremap_resource(&pdev->dev, &dmares); @@ -2002,12 +2002,12 @@ static int axienet_probe(struct platform_device *pdev) if (IS_ERR(lp->dma_regs)) { dev_err(&pdev->dev, "could not map DMA regs\n"); ret = PTR_ERR(lp->dma_regs); - goto free_netdev; + goto cleanup_clk; } if ((lp->rx_irq <= 0) || (lp->tx_irq <= 0)) { dev_err(&pdev->dev, "could not determine irqs\n"); ret = -ENOMEM; - goto free_netdev; + goto cleanup_clk; } /* Autodetect the need for 64-bit DMA pointers. @@ -2037,7 +2037,7 @@ static int axienet_probe(struct platform_device *pdev) ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(addr_width)); if (ret) { dev_err(&pdev->dev, "No suitable DMA available\n"); - goto free_netdev; + goto cleanup_clk; } /* Check for Ethernet core IRQ (optional) */ @@ -2068,12 +2068,12 @@ static int axienet_probe(struct platform_device *pdev) if (!lp->phy_node) { dev_err(&pdev->dev, "phy-handle required for 1000BaseX/SGMII\n"); ret = -EINVAL; - goto free_netdev; + goto cleanup_mdio; } lp->pcs_phy = of_mdio_find_device(lp->phy_node); if (!lp->pcs_phy) { ret = -EPROBE_DEFER; - goto free_netdev; + goto cleanup_mdio; } lp->phylink_config.pcs_poll = true; } @@ -2087,17 +2087,30 @@ static int axienet_probe(struct platform_device *pdev) if (IS_ERR(lp->phylink)) { ret = PTR_ERR(lp->phylink); dev_err(&pdev->dev, "phylink_create error (%i)\n", ret); - goto free_netdev; + goto cleanup_mdio; } ret = register_netdev(lp->ndev); if (ret) { dev_err(lp->dev, "register_netdev() error (%i)\n", ret); - goto free_netdev; + goto cleanup_phylink; } return 0; +cleanup_phylink: + phylink_destroy(lp->phylink); + +cleanup_mdio: + if (lp->pcs_phy) + put_device(&lp->pcs_phy->dev); + if (lp->mii_bus) + axienet_mdio_teardown(lp); + of_node_put(lp->phy_node); + +cleanup_clk: + clk_disable_unprepare(lp->clk); + free_netdev: free_netdev(ndev); -- 2.27.0
[PATCH net-next v4 2/3] net: phy: Add is_on_sfp_module flag and phy_on_sfp helper
Add a flag and helper function to indicate that a PHY device is part of an SFP module, which is set on attach. This can be used by PHY drivers to handle SFP-specific quirks or behavior. Signed-off-by: Robert Hancock --- drivers/net/phy/phy_device.c | 2 ++ include/linux/phy.h | 11 +++ 2 files changed, 13 insertions(+) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 05261698bf74..d6ac3ed38197 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1377,6 +1377,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, if (phydev->sfp_bus_attached) dev->sfp_bus = phydev->sfp_bus; + else if (dev->sfp_bus) + phydev->is_on_sfp_module = true; } /* Some Ethernet drivers try to connect to a PHY device before diff --git a/include/linux/phy.h b/include/linux/phy.h index 0d537f59b77f..1a12e4436b5b 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -492,6 +492,7 @@ struct macsec_ops; * @sysfs_links: Internal boolean tracking sysfs symbolic links setup/removal. * @loopback_enabled: Set true if this PHY has been loopbacked successfully. * @downshifted_rate: Set true if link speed has been downshifted. + * @is_on_sfp_module: Set true if PHY is located on an SFP module. * @state: State of the PHY for management purposes * @dev_flags: Device-specific flags used by the PHY driver. * @irq: IRQ number of the PHY's interrupt (-1 if none) @@ -565,6 +566,7 @@ struct phy_device { unsigned sysfs_links:1; unsigned loopback_enabled:1; unsigned downshifted_rate:1; + unsigned is_on_sfp_module:1; unsigned autoneg:1; /* The most recently read link state */ @@ -1296,6 +1298,15 @@ static inline bool phy_is_internal(struct phy_device *phydev) return phydev->is_internal; } +/** + * phy_on_sfp - Convenience function for testing if a PHY is on an SFP module + * @phydev: the phy_device struct + */ +static inline bool phy_on_sfp(struct phy_device *phydev) +{ + return phydev->is_on_sfp_module; +} + /** * phy_interface_mode_is_rgmii - Convenience function for testing if a * PHY interface mode is RGMII (all variants) -- 2.27.0
[PATCH net-next v4 3/3] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs
bcm54xx_config_init was modifying the PHY LED configuration to enable link and activity indications. However, some SFP modules (such as Bel-Fuse SFP-1GBT-06) have no LEDs but use the LED outputs to control the SFP LOS signal, and modifying the LED settings will cause the LOS output to malfunction. Skip this configuration for PHYs which are bound to an SFP bus. Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 8e7fc3368380..fa0be591ae79 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -366,18 +366,24 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_phydsp_config(phydev); - /* Encode link speed into LED1 and LED3 pair (green/amber). + /* For non-SFP setups, encode link speed into LED1 and LED3 pair +* (green/amber). * Also flash these two LEDs on activity. This means configuring * them for MULTICOLOR and encoding link/activity into them. +* Don't do this for devices on an SFP module, since some of these +* use the LED outputs to control the SFP LOS signal, and changing +* these settings will cause LOS to malfunction. */ - val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); - bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); - - val = BCM_LED_MULTICOLOR_IN_PHASE | - BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); - bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + if (!phy_on_sfp(phydev)) { + val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); + bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); + + val = BCM_LED_MULTICOLOR_IN_PHASE | + BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); + bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + } return 0; } -- 2.27.0
[PATCH net-next v4 1/3] net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S
The default configuration for the BCM54616S PHY may not match the desired mode when using 1000BaseX or SGMII interface modes, such as when it is on an SFP module. Add code to explicitly set the correct mode using programming sequences provided by Bel-Fuse: https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf Signed-off-by: Robert Hancock Acked-by: Florian Fainelli --- drivers/net/phy/broadcom.c | 84 -- include/linux/brcmphy.h| 4 ++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 91fbd26c809e..8e7fc3368380 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -103,6 +103,64 @@ static int bcm54612e_config_init(struct phy_device *phydev) return 0; } +static int bcm54616s_config_init(struct phy_device *phydev) +{ + int rc, val; + + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) + return 0; + + /* Ensure proper interface mode is selected. */ + /* Disable RGMII mode */ + val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC); + if (val < 0) + return val; + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; + val |= MII_BCM54XX_AUXCTL_MISC_WREN; + rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC, + val); + if (rc < 0) + return rc; + + /* Select 1000BASE-X register set (primary SerDes) */ + val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE); + if (val < 0) + return val; + val |= BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power down SerDes interface */ + rc = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select proper interface mode */ + val &= ~BCM54XX_SHD_INTF_SEL_MASK; + val |= phydev->interface == PHY_INTERFACE_MODE_SGMII ? + BCM54XX_SHD_INTF_SEL_SGMII : + BCM54XX_SHD_INTF_SEL_GBIC; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up SerDes interface */ + rc = phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select copper register set */ + val &= ~BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up copper interface */ + return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); +} + /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */ static int bcm50610_a0_workaround(struct phy_device *phydev) { @@ -283,15 +341,17 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_adjust_rxrefclk(phydev); - if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E) { + switch (BRCM_PHY_MODEL(phydev)) { + case PHY_ID_BCM54210E: err = bcm54210e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54612E) { + break; + case PHY_ID_BCM54612E: err = bcm54612e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) { + break; + case PHY_ID_BCM54616S: + err = bcm54616s_config_init(phydev); + break; + case PHY_ID_BCM54810: /* For BCM54810, we need to disable BroadR-Reach function */ val = bcm_phy_read_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL); @@ -299,9 +359,10 @@ static int bcm54xx_config_init(struct phy_device *phydev) err = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, val); - if (err < 0) - return err; + break; } + if (err) + return err; bcm54xx_phydsp_config(phydev); @@ -390,7 +451,7 @@ struct bcm54616s_phy_priv { static int bcm54616s_probe(struct phy_device *phydev) { struct bcm54616s_phy_priv *priv; - int val, intf_sel; + int val; priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -408,8 +469,7 @@ static int bcm54616s_probe(struct phy_device *phydev) * RGMII-1000Base-X is prope
[PATCH net-next v4 0/3] Broadcom PHY driver updates
Updates to the Broadcom PHY driver related to use with copper SFP modules. Changed since v3: -fixed kerneldoc error Changed since v2: -Create flag for PHY on SFP module and use that rather than accessing attached_dev directly in PHY driver Changed since v1: -Reversed conditional to reduce indentation -Added missing setting of MII_BCM54XX_AUXCTL_MISC_WREN in MII_BCM54XX_AUXCTL_SHDWSEL_MISC register Robert Hancock (3): net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S net: phy: Add is_on_sfp_module flag and phy_on_sfp helper net: phy: broadcom: Do not modify LED configuration for SFP module PHYs drivers/net/phy/broadcom.c | 108 --- drivers/net/phy/phy_device.c | 2 + include/linux/brcmphy.h | 4 ++ include/linux/phy.h | 11 4 files changed, 104 insertions(+), 21 deletions(-) -- 2.27.0
[PATCH net-next v3 3/3] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs
bcm54xx_config_init was modifying the PHY LED configuration to enable link and activity indications. However, some SFP modules (such as Bel-Fuse SFP-1GBT-06) have no LEDs but use the LED outputs to control the SFP LOS signal, and modifying the LED settings will cause the LOS output to malfunction. Skip this configuration for PHYs which are bound to an SFP bus. Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 8e7fc3368380..fa0be591ae79 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -366,18 +366,24 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_phydsp_config(phydev); - /* Encode link speed into LED1 and LED3 pair (green/amber). + /* For non-SFP setups, encode link speed into LED1 and LED3 pair +* (green/amber). * Also flash these two LEDs on activity. This means configuring * them for MULTICOLOR and encoding link/activity into them. +* Don't do this for devices on an SFP module, since some of these +* use the LED outputs to control the SFP LOS signal, and changing +* these settings will cause LOS to malfunction. */ - val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); - bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); - - val = BCM_LED_MULTICOLOR_IN_PHASE | - BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); - bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + if (!phy_on_sfp(phydev)) { + val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); + bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); + + val = BCM_LED_MULTICOLOR_IN_PHASE | + BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); + bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + } return 0; } -- 2.27.0
[PATCH net-next v3 2/3] net: phy: Add is_on_sfp_module flag and phy_on_sfp helper
Add a flag and helper function to indicate that a PHY device is part of an SFP module, which is set on attach. This can be used by PHY drivers to handle SFP-specific quirks or behavior. Signed-off-by: Robert Hancock --- drivers/net/phy/phy_device.c | 2 ++ include/linux/phy.h | 11 +++ 2 files changed, 13 insertions(+) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 05261698bf74..d6ac3ed38197 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1377,6 +1377,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, if (phydev->sfp_bus_attached) dev->sfp_bus = phydev->sfp_bus; + else if (dev->sfp_bus) + phydev->is_on_sfp_module = true; } /* Some Ethernet drivers try to connect to a PHY device before diff --git a/include/linux/phy.h b/include/linux/phy.h index 5d7c4084ade9..eb48c686c423 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -488,6 +488,7 @@ struct macsec_ops; * @sysfs_links: Internal boolean tracking sysfs symbolic links setup/removal. * @loopback_enabled: Set true if this PHY has been loopbacked successfully. * @downshifted_rate: Set true if link speed has been downshifted. + * @on_sfp_module: Set true if PHY is located on an SFP module. * @state: State of the PHY for management purposes * @dev_flags: Device-specific flags used by the PHY driver. * @irq: IRQ number of the PHY's interrupt (-1 if none) @@ -561,6 +562,7 @@ struct phy_device { unsigned sysfs_links:1; unsigned loopback_enabled:1; unsigned downshifted_rate:1; + unsigned is_on_sfp_module:1; unsigned autoneg:1; /* The most recently read link state */ @@ -1292,6 +1294,15 @@ static inline bool phy_is_internal(struct phy_device *phydev) return phydev->is_internal; } +/** + * phy_on_sfp - Convenience function for testing if a PHY is on an SFP module + * @phydev: the phy_device struct + */ +static inline bool phy_on_sfp(struct phy_device *phydev) +{ + return phydev->is_on_sfp_module; +} + /** * phy_interface_mode_is_rgmii - Convenience function for testing if a * PHY interface mode is RGMII (all variants) -- 2.27.0
[PATCH net-next v3 1/3] net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S
The default configuration for the BCM54616S PHY may not match the desired mode when using 1000BaseX or SGMII interface modes, such as when it is on an SFP module. Add code to explicitly set the correct mode using programming sequences provided by Bel-Fuse: https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf Signed-off-by: Robert Hancock Acked-by: Florian Fainelli --- drivers/net/phy/broadcom.c | 84 -- include/linux/brcmphy.h| 4 ++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 91fbd26c809e..8e7fc3368380 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -103,6 +103,64 @@ static int bcm54612e_config_init(struct phy_device *phydev) return 0; } +static int bcm54616s_config_init(struct phy_device *phydev) +{ + int rc, val; + + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) + return 0; + + /* Ensure proper interface mode is selected. */ + /* Disable RGMII mode */ + val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC); + if (val < 0) + return val; + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; + val |= MII_BCM54XX_AUXCTL_MISC_WREN; + rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC, + val); + if (rc < 0) + return rc; + + /* Select 1000BASE-X register set (primary SerDes) */ + val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE); + if (val < 0) + return val; + val |= BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power down SerDes interface */ + rc = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select proper interface mode */ + val &= ~BCM54XX_SHD_INTF_SEL_MASK; + val |= phydev->interface == PHY_INTERFACE_MODE_SGMII ? + BCM54XX_SHD_INTF_SEL_SGMII : + BCM54XX_SHD_INTF_SEL_GBIC; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up SerDes interface */ + rc = phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select copper register set */ + val &= ~BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up copper interface */ + return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); +} + /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */ static int bcm50610_a0_workaround(struct phy_device *phydev) { @@ -283,15 +341,17 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_adjust_rxrefclk(phydev); - if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E) { + switch (BRCM_PHY_MODEL(phydev)) { + case PHY_ID_BCM54210E: err = bcm54210e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54612E) { + break; + case PHY_ID_BCM54612E: err = bcm54612e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) { + break; + case PHY_ID_BCM54616S: + err = bcm54616s_config_init(phydev); + break; + case PHY_ID_BCM54810: /* For BCM54810, we need to disable BroadR-Reach function */ val = bcm_phy_read_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL); @@ -299,9 +359,10 @@ static int bcm54xx_config_init(struct phy_device *phydev) err = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, val); - if (err < 0) - return err; + break; } + if (err) + return err; bcm54xx_phydsp_config(phydev); @@ -390,7 +451,7 @@ struct bcm54616s_phy_priv { static int bcm54616s_probe(struct phy_device *phydev) { struct bcm54616s_phy_priv *priv; - int val, intf_sel; + int val; priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -408,8 +469,7 @@ static int bcm54616s_probe(struct phy_device *phydev) * RGMII-1000Base-X is prope
[PATCH net-next v3 0/3] Broadcom PHY driver updates
Updates to the Broadcom PHY driver related to use with copper SFP modules. Changed since v2: -Create flag for PHY on SFP module and use that rather than accessing attached_dev directly in PHY driver Changed since v1: -Reversed conditional to reduce indentation -Added missing setting of MII_BCM54XX_AUXCTL_MISC_WREN in MII_BCM54XX_AUXCTL_SHDWSEL_MISC register Robert Hancock (3): net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S net: phy: Add is_on_sfp_module flag and phy_on_sfp helper net: phy: broadcom: Do not modify LED configuration for SFP module PHYs drivers/net/phy/broadcom.c | 108 --- drivers/net/phy/phy_device.c | 2 + include/linux/brcmphy.h | 4 ++ include/linux/phy.h | 11 4 files changed, 104 insertions(+), 21 deletions(-) -- 2.27.0
[PATCH net-next v2] net: phy: marvell: Ensure SGMII auto-negotiation is enabled for 88E1111
When 88E is operating in SGMII mode, auto-negotiation should be enabled on the SGMII side so that the link will come up properly with PCSes which normally have auto-negotiation enabled. This is normally the case when the PHY defaults to SGMII mode at power-up, however if we switched it from some other mode like 1000Base-X, as may happen in some SFP module situations, it may not be, particularly for modules which have 1000Base-X auto-negotiation defaulting to disabled. Call genphy_check_and_restart_aneg on the fiber page to ensure that auto- negotiation is properly enabled on the SGMII interface. Signed-off-by: Robert Hancock --- Changed since v1: Fixed typo and added more explanation in commit message drivers/net/phy/marvell.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 3238d0fbf437..e26a5d663f8a 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -684,16 +684,19 @@ static int m88e_config_aneg(struct phy_device *phydev) if (err < 0) goto error; - /* Do not touch the fiber page if we're in copper->sgmii mode */ - if (phydev->interface == PHY_INTERFACE_MODE_SGMII) - return 0; - /* Then the fiber link */ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); if (err < 0) goto error; - err = marvell_config_aneg_fiber(phydev); + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + /* Do not touch the fiber advertisement if we're in copper->sgmii mode. +* Just ensure that SGMII-side autonegotiation is enabled. +* If we switched from some other mode to SGMII it may not be. +*/ + err = genphy_check_and_restart_aneg(phydev, false); + else + err = marvell_config_aneg_fiber(phydev); if (err < 0) goto error; -- 2.27.0
Re: [PATCH net-next v2 2/2] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs
On Sat, 2021-02-13 at 10:45 +, Russell King - ARM Linux admin wrote: > On Fri, Feb 12, 2021 at 08:18:40PM -0600, Robert Hancock wrote: > > + if (!phydev->sfp_bus && > > + (!phydev->attached_dev || !phydev->attached_dev->sfp_bus)) { > > First, do we want this to be repeated in every driver? > > Second, are you sure this is the correct condition to be using for > this? phydev->sfp_bus is non-NULL when _this_ PHY has a SFP bus > connected to its fibre side, it will never be set for a PHY on a > SFP. The fact that it is non-NULL or NULL shouldn't have a bearing > on whether we configure the LED register. I think you're correct, the phydev->sfp_bus portion is probably not useful and could be dropped. What we're really concerned about is whether this PHY is on an SFP module or not. I'm not sure that a module-specific quirk makes sense here since there are probably other models which have a similar design where the LED outputs from the PHY are used for other purposes, and there's really no benefit to playing with the LED outputs on SFP modules in any case, so it would be safer to skip the LED reconfiguration for anything on an SFP. So we could either have a condition for "!phydev->attached_dev || !phydev->attached_dev- >sfp_bus" here and anywhere else that needs a similar check, or we do something different, like have a specific flag to indicate that this PHY is on an SFP module? What do people think is best here? > -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
Re: [PATCH net-next 3/3] net: axienet: Support dynamic switching between 1000BaseX and SGMII
On Sat, 2021-02-13 at 17:43 +0100, Andrew Lunn wrote: > On Fri, Feb 12, 2021 at 06:23:56PM -0600, Robert Hancock wrote: > > Newer versions of the Xilinx AXI Ethernet core (specifically version 7.2 or > > later) allow the core to be configured with a PHY interface mode of "Both", > > Hi Robert > > Is it possible to read the version of the core from a register? Is it > possible to synthesizer a version 7.2 or > without this feature? I'm > just wondering if the DT property is actually needed? The core can still be synthesized with a fixed 1000Base-X or SGMII interface mode in addition to the "Both" option, and I'm not aware of a way to determine what mode has been used based on registers, so I don't think there's really another option. > > > /** > > * struct axidma_bd - Axi Dma buffer descriptor layout > > * @next: MM2S/S2MM Next Descriptor Pointer > > @@ -377,22 +381,29 @@ struct axidma_bd { > > * @ndev: Pointer for net_device to which it will be attached. > > * @dev: Pointer to device structure > > * @phy_node: Pointer to device node structure > > + * @phylink: Pointer to phylink instance > > + * @phylink_config: phylink configuration settings > > + * @pcs_phy: Reference to PCS/PMA PHY if used > > + * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in > > the core > > + * @clk: Clock for AXI bus > > * @mii_bus: Pointer to MII bus structure > > * @mii_clk_div: MII bus clock divider value > > * @regs_start: Resource start for axienet device addresses > > * @regs: Base address for the axienet_local device address space > > * @dma_regs: Base address for the axidma device address space > > - * @dma_err_tasklet: Tasklet structure to process Axi DMA errors > > + * @dma_err_task: Work structure to process Axi DMA errors > > * @tx_irq:Axidma TX IRQ number > > * @rx_irq:Axidma RX IRQ number > > + * @eth_irq: Ethernet core IRQ number > > * @phy_mode: Phy type to identify between MII/GMII/RGMII/SGMII/1000 > > Base-X > > * @options: AxiEthernet option word > > - * @last_link: Phy link state in which the PHY was negotiated earlier > > * @features: Stores the extended features supported by the axienet > > hw > > * @tx_bd_v: Virtual address of the TX buffer descriptor ring > > * @tx_bd_p: Physical address(start address) of the TX buffer descr. > > ring > > + * @tx_bd_num: Size of TX buffer descriptor ring > > * @rx_bd_v: Virtual address of the RX buffer descriptor ring > > * @rx_bd_p: Physical address(start address) of the RX buffer descr. > > ring > > + * @rx_bd_num: Size of RX buffer descriptor ring > > * @tx_bd_ci: Stores the index of the Tx buffer descriptor in the > > ring being > > * accessed currently. Used while alloc. BDs before a TX starts > > * @tx_bd_tail:Stores the index of the Tx buffer descriptor in the > > ring being > > @@ -414,23 +425,20 @@ struct axienet_local { > > struct net_device *ndev; > > struct device *dev; > > > > - /* Connection to PHY device */ > > struct device_node *phy_node; > > > > struct phylink *phylink; > > struct phylink_config phylink_config; > > > > - /* Reference to PCS/PMA PHY if used */ > > struct mdio_device *pcs_phy; > > This really should of been two patches. One moving the comments > around, and a second one adding the new fields. > > > +static int axienet_mac_prepare(struct phylink_config *config, unsigned int > > mode, > > + phy_interface_t iface) > > +{ > > + struct net_device *ndev = to_net_dev(config->dev); > > + struct axienet_local *lp = netdev_priv(ndev); > > + int ret; > > + > > + switch (iface) { > > + case PHY_INTERFACE_MODE_SGMII: > > + case PHY_INTERFACE_MODE_1000BASEX: > > + if (!lp->switch_x_sgmii) > > + return 0; > > Maybe -EOPNOTSUPP would be better? From my reading of the code it appears that this function is called on startup initially even if dynamic switching is not supported, so we would need to return 0 here for that case. The validate callback should trap cases where we attempt to switch modes and that isn't supported. > > Andrew -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
[PATCH net-next v2 2/2] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs
bcm54xx_config_init was modifying the PHY LED configuration to enable link and activity indications. However, some SFP modules (such as Bel-Fuse SFP-1GBT-06) have no LEDs but use the LED outputs to control the SFP LOS signal, and modifying the LED settings will cause the LOS output to malfunction. Skip this configuration for PHYs which are bound to an SFP bus. Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 484791ac236b..9d73bfe0a1c2 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -12,6 +12,7 @@ #include "bcm-phy-lib.h" #include +#include #include #include #include @@ -366,18 +367,25 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_phydsp_config(phydev); - /* Encode link speed into LED1 and LED3 pair (green/amber). + /* For non-SFP setups, encode link speed into LED1 and LED3 pair +* (green/amber). * Also flash these two LEDs on activity. This means configuring * them for MULTICOLOR and encoding link/activity into them. +* Don't do this for devices that may be on an SFP module, since +* some of these use the LED outputs to control the SFP LOS signal, +* and changing these settings will cause LOS to malfunction. */ - val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); - bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); - - val = BCM_LED_MULTICOLOR_IN_PHASE | - BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); - bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + if (!phydev->sfp_bus && + (!phydev->attached_dev || !phydev->attached_dev->sfp_bus)) { + val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); + bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); + + val = BCM_LED_MULTICOLOR_IN_PHASE | + BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); + bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + } return 0; } -- 2.27.0
[PATCH net-next v2 1/2] net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S
The default configuration for the BCM54616S PHY may not match the desired mode when using 1000BaseX or SGMII interface modes, such as when it is on an SFP module. Add code to explicitly set the correct mode using programming sequences provided by Bel-Fuse: https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 84 -- include/linux/brcmphy.h| 4 ++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 0472b3470c59..484791ac236b 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -64,6 +64,64 @@ static int bcm54612e_config_init(struct phy_device *phydev) return 0; } +static int bcm54616s_config_init(struct phy_device *phydev) +{ + int rc, val; + + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) + return 0; + + /* Ensure proper interface mode is selected. */ + /* Disable RGMII mode */ + val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC); + if (val < 0) + return val; + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; + val |= MII_BCM54XX_AUXCTL_MISC_WREN; + rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC, + val); + if (rc < 0) + return rc; + + /* Select 1000BASE-X register set (primary SerDes) */ + val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE); + if (val < 0) + return val; + val |= BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power down SerDes interface */ + rc = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select proper interface mode */ + val &= ~BCM54XX_SHD_INTF_SEL_MASK; + val |= phydev->interface == PHY_INTERFACE_MODE_SGMII ? + BCM54XX_SHD_INTF_SEL_SGMII : + BCM54XX_SHD_INTF_SEL_GBIC; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up SerDes interface */ + rc = phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select copper register set */ + val &= ~BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up copper interface */ + return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); +} + static int bcm54xx_config_clock_delay(struct phy_device *phydev) { int rc, val; @@ -283,15 +341,17 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_adjust_rxrefclk(phydev); - if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E) { + switch (BRCM_PHY_MODEL(phydev)) { + case PHY_ID_BCM54210E: err = bcm54210e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54612E) { + break; + case PHY_ID_BCM54612E: err = bcm54612e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) { + break; + case PHY_ID_BCM54616S: + err = bcm54616s_config_init(phydev); + break; + case PHY_ID_BCM54810: /* For BCM54810, we need to disable BroadR-Reach function */ val = bcm_phy_read_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL); @@ -299,9 +359,10 @@ static int bcm54xx_config_init(struct phy_device *phydev) err = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, val); - if (err < 0) - return err; + break; } + if (err) + return err; bcm54xx_phydsp_config(phydev); @@ -385,7 +446,7 @@ static int bcm5481_config_aneg(struct phy_device *phydev) static int bcm54616s_probe(struct phy_device *phydev) { - int val, intf_sel; + int val; val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE); if (val < 0) @@ -397,8 +458,7 @@ static int bcm54616s_probe(struct phy_device *phydev) * RGMII-1000Base-X is properly supported, but RGMII-100Base-FX * support is still missing as of now. *
[PATCH net-next v2 0/2] Broadcom PHY driver updates
Updates to the Broadcom PHY driver related to use with copper SFP modules. Changed since v1: -Reversed conditional to reduce indentation -Added missing setting of MII_BCM54XX_AUXCTL_MISC_WREN in MII_BCM54XX_AUXCTL_SHDWSEL_MISC register Robert Hancock (2): net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S net: phy: broadcom: Do not modify LED configuration for SFP module PHYs drivers/net/phy/broadcom.c | 110 ++--- include/linux/brcmphy.h| 4 ++ 2 files changed, 93 insertions(+), 21 deletions(-) -- 2.27.0
Re: [PATCH net-next 1/2] net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S
On Fri, 2021-02-12 at 17:26 -0800, Florian Fainelli wrote: > > On 2/12/2021 4:28 PM, 'Robert Hancock' via BCM-KERNEL-FEEDBACK-LIST,PDL > wrote: > > The default configuration for the BCM54616S PHY may not match the desired > > mode when using 1000BaseX or SGMII interface modes, such as when it is on > > an SFP module. Add code to explicitly set the correct mode using > > programming sequences provided by Bel-Fuse: > > > > https://urldefense.com/v3/__https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf__;!!IOGos0k!20FhZqRHEiz2-qFJ7J8XC4xX2qG-ajZ17Ma1W-VwDgwdQZeIhHEpWKlNldWW8DyFaQo$ > > > > https://urldefense.com/v3/__https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf__;!!IOGos0k!20FhZqRHEiz2-qFJ7J8XC4xX2qG-ajZ17Ma1W-VwDgwdQZeIhHEpWKlNldWW58K3fY4$ > > > > > > Signed-off-by: Robert Hancock > > --- > > drivers/net/phy/broadcom.c | 83 -- > > include/linux/brcmphy.h| 4 ++ > > 2 files changed, 75 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c > > index 0472b3470c59..78542580f2b2 100644 > > --- a/drivers/net/phy/broadcom.c > > +++ b/drivers/net/phy/broadcom.c > > @@ -64,6 +64,63 @@ static int bcm54612e_config_init(struct phy_device > > *phydev) > > return 0; > > } > > > > +static int bcm54616s_config_init(struct phy_device *phydev) > > +{ > > + int rc, val; > > + > > + if (phydev->interface == PHY_INTERFACE_MODE_SGMII || > > + phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { > > Can you reverse the condition so as to save a level of identation? Can do. > > > + /* Ensure proper interface mode is selected. */ > > + /* Disable RGMII mode */ > > + val = bcm54xx_auxctl_read(phydev, > > MII_BCM54XX_AUXCTL_SHDWSEL_MISC); > > + if (val < 0) > > + return val; > > + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; > > + rc = bcm54xx_auxctl_write(phydev, > > MII_BCM54XX_AUXCTL_SHDWSEL_MISC, > > + val); > > + if (rc < 0) > > + return rc; > > I don't think this write is making it through since you are not setting > MII_BCM54XX_AUXCTL_MISC_WREN in val, I know this is an annoying detail, > and we could probably fold that to be within bcm54xx_auxctl_write() > directly, similarly to what bcm_phy_write_shadow() does. Ah, indeed. I assume that is specific to the MII_BCM54XX_AUXCTL_SHDWSEL_MISC register? I suppose bcm54xx_auxctl_write could add that automatically for writes to that register. Not sure if that is too much magic for that function or not.. > > The reset of the sequence and changes looks fine to me. -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
Re: [PATCH net-next] net: phy: marvell: Ensure SGMII auto-negotiation is enabled for 88E1111
On Sat, 2021-02-13 at 01:09 +, Russell King - ARM Linux admin wrote: > On Fri, Feb 12, 2021 at 06:26:29PM -0600, Robert Hancock wrote: > > When 88E111 is operating in SGMII mode, auto-negotiation should be enabled > > 88E. yup.. > > > on the SGMII side so that the link will come up properly with PCSes which > > normally have auto-negotiation enabled. This is normally the case when the > > PHY defaults to SGMII mode at power-up, however if we switched it from some > > other mode like 1000BaseX, as may happen in some SFP module situations, > > it may not be. > > Do you actually have a module where this applies? > > I have modules that do come up in 1000base-X mode, but do switch to > SGMII mode with AN just fine. So I'm wondering what the difference is. I saw this with a Finisar FCLF8520P2BTL, which defaults to 1000Base-X with host-side auto-negotiation disabled. So presumably the auto-negotiation disabled state carries over when it is switched to SGMII mode. I previously wrote a patch ("net: phy: marvell: add special handling of Finisar modules with 88E") which enabled auto-negotiation for 1000Base-X mode, but we are trying to switch over to using SGMII with these now. > -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
[PATCH net-next] net: phy: marvell: Ensure SGMII auto-negotiation is enabled for 88E1111
When 88E111 is operating in SGMII mode, auto-negotiation should be enabled on the SGMII side so that the link will come up properly with PCSes which normally have auto-negotiation enabled. This is normally the case when the PHY defaults to SGMII mode at power-up, however if we switched it from some other mode like 1000BaseX, as may happen in some SFP module situations, it may not be. Call genphy_check_and_restart_aneg on the fiber page to ensure that auto- negotiation is properly enabled on the SGMII interface. Signed-off-by: Robert Hancock --- drivers/net/phy/marvell.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 3238d0fbf437..e26a5d663f8a 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -684,16 +684,19 @@ static int m88e_config_aneg(struct phy_device *phydev) if (err < 0) goto error; - /* Do not touch the fiber page if we're in copper->sgmii mode */ - if (phydev->interface == PHY_INTERFACE_MODE_SGMII) - return 0; - /* Then the fiber link */ err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); if (err < 0) goto error; - err = marvell_config_aneg_fiber(phydev); + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + /* Do not touch the fiber advertisement if we're in copper->sgmii mode. +* Just ensure that SGMII-side autonegotiation is enabled. +* If we switched from some other mode to SGMII it may not be. +*/ + err = genphy_check_and_restart_aneg(phydev, false); + else + err = marvell_config_aneg_fiber(phydev); if (err < 0) goto error; -- 2.27.0
[PATCH net-next 2/3] dt-bindings: net: xilinx_axienet: add xlnx,switch-x-sgmii attribute
Document the new xlnx,switch-x-sgmii attribute which is used to indicate that the Ethernet core supports dynamic switching between 1000BaseX and SGMII. Signed-off-by: Robert Hancock --- Documentation/devicetree/bindings/net/xilinx_axienet.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 7360617cdedb..2cd452419ed0 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -38,6 +38,10 @@ Optional properties: 1 to enable partial TX checksum offload, 2 to enable full TX checksum offload - xlnx,rxcsum : Same values as xlnx,txcsum but for RX checksum offload +- xlnx,switch-x-sgmii : Boolean to indicate the Ethernet core is configured to + support both 1000BaseX and SGMII modes. If set, the phy-mode + should be set to match the mode selected on core reset (i.e. + by the basex_or_sgmii core input line). - clocks : AXI bus clock for the device. Refer to common clock bindings. Used to calculate MDIO clock divisor. If not specified, it is auto-detected from the CPU clock (but only on platforms where -- 2.27.0
[PATCH net-next 1/3] net: axienet: hook up nway_reset ethtool operation
Hook up the nway_reset ethtool operation to the corresponding phylink function so that "ethtool -r" can be supported. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index b4a0bfce5b76..3ef31bae71fb 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1469,6 +1469,13 @@ axienet_ethtools_set_link_ksettings(struct net_device *ndev, return phylink_ethtool_ksettings_set(lp->phylink, cmd); } +static int axienet_ethtools_nway_reset(struct net_device *dev) +{ + struct axienet_local *lp = netdev_priv(dev); + + return phylink_ethtool_nway_reset(lp->phylink); +} + static const struct ethtool_ops axienet_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES, .get_drvinfo= axienet_ethtools_get_drvinfo, @@ -1483,6 +1490,7 @@ static const struct ethtool_ops axienet_ethtool_ops = { .set_coalesce = axienet_ethtools_set_coalesce, .get_link_ksettings = axienet_ethtools_get_link_ksettings, .set_link_ksettings = axienet_ethtools_set_link_ksettings, + .nway_reset = axienet_ethtools_nway_reset, }; static void axienet_validate(struct phylink_config *config, -- 2.27.0
[PATCH net-next 0/2] Broadcom PHY driver updates
Updates to the Broadcom PHY driver related to use with copper SFP modules. Robert Hancock (2): net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S net: phy: broadcom: Do not modify LED configuration for SFP module PHYs drivers/net/phy/broadcom.c | 109 ++--- include/linux/brcmphy.h| 4 ++ 2 files changed, 92 insertions(+), 21 deletions(-) -- 2.27.0
[PATCH net-next 3/3] net: axienet: Support dynamic switching between 1000BaseX and SGMII
Newer versions of the Xilinx AXI Ethernet core (specifically version 7.2 or later) allow the core to be configured with a PHY interface mode of "Both", allowing either 1000BaseX or SGMII modes to be selected at runtime. Add support for this in the driver to allow better support for applications which can use both fiber and copper SFP modules. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 29 + .../net/ethernet/xilinx/xilinx_axienet_main.c | 60 --- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index a03c3ca1b28d..1e966a39967e 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -339,6 +339,10 @@ #define DELAY_OF_ONE_MILLISEC 1000 +/* Xilinx PCS/PMA PHY register for switching 1000BaseX or SGMII */ +#define XLNX_MII_STD_SELECT_REG0x11 +#define XLNX_MII_STD_SELECT_SGMII BIT(0) + /** * struct axidma_bd - Axi Dma buffer descriptor layout * @next: MM2S/S2MM Next Descriptor Pointer @@ -377,22 +381,29 @@ struct axidma_bd { * @ndev: Pointer for net_device to which it will be attached. * @dev: Pointer to device structure * @phy_node: Pointer to device node structure + * @phylink: Pointer to phylink instance + * @phylink_config: phylink configuration settings + * @pcs_phy: Reference to PCS/PMA PHY if used + * @switch_x_sgmii: Whether switchable 1000BaseX/SGMII mode is enabled in the core + * @clk: Clock for AXI bus * @mii_bus: Pointer to MII bus structure * @mii_clk_div: MII bus clock divider value * @regs_start: Resource start for axienet device addresses * @regs: Base address for the axienet_local device address space * @dma_regs: Base address for the axidma device address space - * @dma_err_tasklet: Tasklet structure to process Axi DMA errors + * @dma_err_task: Work structure to process Axi DMA errors * @tx_irq:Axidma TX IRQ number * @rx_irq:Axidma RX IRQ number + * @eth_irq: Ethernet core IRQ number * @phy_mode: Phy type to identify between MII/GMII/RGMII/SGMII/1000 Base-X * @options: AxiEthernet option word - * @last_link: Phy link state in which the PHY was negotiated earlier * @features: Stores the extended features supported by the axienet hw * @tx_bd_v: Virtual address of the TX buffer descriptor ring * @tx_bd_p: Physical address(start address) of the TX buffer descr. ring + * @tx_bd_num: Size of TX buffer descriptor ring * @rx_bd_v: Virtual address of the RX buffer descriptor ring * @rx_bd_p: Physical address(start address) of the RX buffer descr. ring + * @rx_bd_num: Size of RX buffer descriptor ring * @tx_bd_ci: Stores the index of the Tx buffer descriptor in the ring being * accessed currently. Used while alloc. BDs before a TX starts * @tx_bd_tail:Stores the index of the Tx buffer descriptor in the ring being @@ -414,23 +425,20 @@ struct axienet_local { struct net_device *ndev; struct device *dev; - /* Connection to PHY device */ struct device_node *phy_node; struct phylink *phylink; struct phylink_config phylink_config; - /* Reference to PCS/PMA PHY if used */ struct mdio_device *pcs_phy; - /* Clock for AXI bus */ + bool switch_x_sgmii; + struct clk *clk; - /* MDIO bus data */ - struct mii_bus *mii_bus;/* MII bus reference */ - u8 mii_clk_div; /* MII bus clock divider value */ + struct mii_bus *mii_bus; + u8 mii_clk_div; - /* IO registers, dma functions and IRQs */ resource_size_t regs_start; void __iomem *regs; void __iomem *dma_regs; @@ -442,10 +450,9 @@ struct axienet_local { int eth_irq; phy_interface_t phy_mode; - u32 options;/* Current options word */ + u32 options; u32 features; - /* Buffer descriptors */ struct axidma_bd *tx_bd_v; dma_addr_t tx_bd_p; u32 tx_bd_num; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 3ef31bae71fb..3a8775e0ca55 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1502,13 +1502,22 @@ static void axienet_validate(struct phylink_config *config, __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; /* Only support the mode we are configured for */ - if (state->interface != PHY_INTERFACE_MODE_NA && - state->interface != lp->phy_mode) { - netdev_warn(ndev, "Cannot use PHY mode %s, supported: %s\n", - phy_modes(state->interface), - phy_modes(lp->phy_mo
[PATCH net-next 2/2] net: phy: broadcom: Do not modify LED configuration for SFP module PHYs
bcm54xx_config_init was modifying the PHY LED configuration to enable link and activity indications. However, some SFP modules (such as Bel-Fuse SFP-1GBT-06) have no LEDs but use the LED outputs to control the SFP LOS signal, and modifying the LED settings will cause the LOS output to malfunction. Skip this configuration for PHYs which are bound to an SFP bus. Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 78542580f2b2..81a5721f732a 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -12,6 +12,7 @@ #include "bcm-phy-lib.h" #include +#include #include #include #include @@ -365,18 +366,25 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_phydsp_config(phydev); - /* Encode link speed into LED1 and LED3 pair (green/amber). + /* For non-SFP setups, encode link speed into LED1 and LED3 pair +* (green/amber). * Also flash these two LEDs on activity. This means configuring * them for MULTICOLOR and encoding link/activity into them. +* Don't do this for devices that may be on an SFP module, since +* some of these use the LED outputs to control the SFP LOS signal, +* and changing these settings will cause LOS to malfunction. */ - val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); - bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); - - val = BCM_LED_MULTICOLOR_IN_PHASE | - BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | - BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); - bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + if (!phydev->sfp_bus && + (!phydev->attached_dev || !phydev->attached_dev->sfp_bus)) { + val = BCM5482_SHD_LEDS1_LED1(BCM_LED_SRC_MULTICOLOR1) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_SRC_MULTICOLOR1); + bcm_phy_write_shadow(phydev, BCM5482_SHD_LEDS1, val); + + val = BCM_LED_MULTICOLOR_IN_PHASE | + BCM5482_SHD_LEDS1_LED1(BCM_LED_MULTICOLOR_LINK_ACT) | + BCM5482_SHD_LEDS1_LED3(BCM_LED_MULTICOLOR_LINK_ACT); + bcm_phy_write_exp(phydev, BCM_EXP_MULTICOLOR, val); + } return 0; } -- 2.27.0
[PATCH net-next 1/2] net: phy: broadcom: Set proper 1000BaseX/SGMII interface mode for BCM54616S
The default configuration for the BCM54616S PHY may not match the desired mode when using 1000BaseX or SGMII interface modes, such as when it is on an SFP module. Add code to explicitly set the correct mode using programming sequences provided by Bel-Fuse: https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf Signed-off-by: Robert Hancock --- drivers/net/phy/broadcom.c | 83 -- include/linux/brcmphy.h| 4 ++ 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 0472b3470c59..78542580f2b2 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -64,6 +64,63 @@ static int bcm54612e_config_init(struct phy_device *phydev) return 0; } +static int bcm54616s_config_init(struct phy_device *phydev) +{ + int rc, val; + + if (phydev->interface == PHY_INTERFACE_MODE_SGMII || + phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { + /* Ensure proper interface mode is selected. */ + /* Disable RGMII mode */ + val = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC); + if (val < 0) + return val; + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; + rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC, + val); + if (rc < 0) + return rc; + + /* Select 1000BASE-X register set (primary SerDes) */ + val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE); + if (val < 0) + return val; + val |= BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power down SerDes interface */ + rc = phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select proper interface mode */ + val &= ~BCM54XX_SHD_INTF_SEL_MASK; + val |= phydev->interface == PHY_INTERFACE_MODE_SGMII ? + BCM54XX_SHD_INTF_SEL_SGMII : + BCM54XX_SHD_INTF_SEL_GBIC; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up SerDes interface */ + rc = phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); + if (rc < 0) + return rc; + + /* Select copper register set */ + val &= ~BCM54XX_SHD_MODE_1000BX; + rc = bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE, val); + if (rc < 0) + return rc; + + /* Power up copper interface */ + return phy_clear_bits(phydev, MII_BMCR, BMCR_PDOWN); + } + return 0; +} + static int bcm54xx_config_clock_delay(struct phy_device *phydev) { int rc, val; @@ -283,15 +340,17 @@ static int bcm54xx_config_init(struct phy_device *phydev) bcm54xx_adjust_rxrefclk(phydev); - if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54210E) { + switch (BRCM_PHY_MODEL(phydev)) { + case PHY_ID_BCM54210E: err = bcm54210e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54612E) { + break; + case PHY_ID_BCM54612E: err = bcm54612e_config_init(phydev); - if (err) - return err; - } else if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) { + break; + case PHY_ID_BCM54616S: + err = bcm54616s_config_init(phydev); + break; + case PHY_ID_BCM54810: /* For BCM54810, we need to disable BroadR-Reach function */ val = bcm_phy_read_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL); @@ -299,9 +358,10 @@ static int bcm54xx_config_init(struct phy_device *phydev) err = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, val); - if (err < 0) - return err; + break; } + if (err) + return err; bcm54xx_phydsp_config(phydev); @@ -385,7 +445,7 @@ static int bcm5481_config_aneg(struct phy_device *phydev) static int bcm54616s_probe(struct phy_device *phydev) { - int val, intf_sel; + int val; va
[PATCH net] net: axienet: Handle deferred probe on clock properly
This driver is set up to use a clock mapping in the device tree if it is present, but still work without one for backward compatibility. However, if getting the clock returns -EPROBE_DEFER, then we need to abort and return that error from our driver initialization so that the probe can be retried later after the clock is set up. Move clock initialization to earlier in the process so we do not waste as much effort if the clock is not yet available. Switch to use devm_clk_get_optional and abort initialization on any error reported. Also enable the clock regardless of whether the controller is using an MDIO bus, as the clock is required in any case. Fixes: 09a0354cadec267be7f ("net: axienet: Use clock framework to get device clock rate") Signed-off-by: Robert Hancock --- .../net/ethernet/xilinx/xilinx_axienet_main.c | 26 +-- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 6fea980acf64..b4a0bfce5b76 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1817,6 +1817,18 @@ static int axienet_probe(struct platform_device *pdev) lp->options = XAE_OPTION_DEFAULTS; lp->rx_bd_num = RX_BD_NUM_DEFAULT; lp->tx_bd_num = TX_BD_NUM_DEFAULT; + + lp->clk = devm_clk_get_optional(&pdev->dev, NULL); + if (IS_ERR(lp->clk)) { + ret = PTR_ERR(lp->clk); + goto free_netdev; + } + ret = clk_prepare_enable(lp->clk); + if (ret) { + dev_err(&pdev->dev, "Unable to enable clock: %d\n", ret); + goto free_netdev; + } + /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); lp->regs = devm_ioremap_resource(&pdev->dev, ethres); @@ -1992,20 +2004,6 @@ static int axienet_probe(struct platform_device *pdev) lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); if (lp->phy_node) { - lp->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(lp->clk)) { - dev_warn(&pdev->dev, "Failed to get clock: %ld\n", -PTR_ERR(lp->clk)); - lp->clk = NULL; - } else { - ret = clk_prepare_enable(lp->clk); - if (ret) { - dev_err(&pdev->dev, "Unable to enable clock: %d\n", - ret); - goto free_netdev; - } - } - ret = axienet_mdio_setup(lp); if (ret) dev_warn(&pdev->dev, -- 2.27.0
[PATCH net-next 0/3] Xilinx axienet updates
Updates to the Xilinx AXI Ethernet driver to add support for an additional ethtool operation, and to support dynamic switching between 1000BaseX and SGMII interface modes. Robert Hancock (3): net: axienet: hook up nway_reset ethtool operation dt-bindings: net: xilinx_axienet: add xlnx,switch-x-sgmii attribute net: axienet: Support dynamic switching between 1000BaseX and SGMII .../bindings/net/xilinx_axienet.txt | 4 ++ drivers/net/ethernet/xilinx/xilinx_axienet.h | 29 +--- .../net/ethernet/xilinx/xilinx_axienet_main.c | 68 +-- 3 files changed, 83 insertions(+), 18 deletions(-) -- 2.27.0
Patch for stable: iwlwifi: provide gso_type to GSO packets
Figured I would poke someone to add this patch to the stable queue - I don't see it in https://patchwork.kernel.org/bundle/netdev/stable/?state=* right now. This patch is reported to fix a severe upload speed regression in many Intel wireless adapters existing since kernel 5.9, as described in https://bugzilla.kernel.org/show_bug.cgi?id=209913 commit 81a86e1bd8e7060ebba1718b284d54f1238e9bf9 Author: Eric Dumazet Date: Mon Jan 25 07:09:49 2021 -0800 iwlwifi: provide gso_type to GSO packets net/core/tso.c got recent support for USO, and this broke iwlfifi because the driver implemented a limited form of GSO. Providing ->gso_type allows for skb_is_gso_tcp() to provide a correct result. Fixes: 3d5b459ba0e3 ("net: tso: add UDP segmentation support") Signed-off-by: Eric Dumazet Reported-by: Ben Greear Tested-by: Ben Greear Cc: Luca Coelho Cc: Johannes Berg Link: https://bugzilla.kernel.org/show_bug.cgi?id=209913 Link: https://lore.kernel.org/r/20210125150949.619309-1-eric.duma...@gmail.com Signed-off-by: Jakub Kicinski
[PATCH net-next v5] net: phy: marvell: add special handling of Finisar modules with 88E1111
The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel 88E PHY with a modified PHY ID. Add support for this ID using the 88E methods. By default these modules do not have 1000BaseX auto-negotiation enabled, which is not generally desirable with Linux networking drivers. Add handling to enable 1000BaseX auto-negotiation when these modules are used in 1000BaseX mode. Also, some special handling is required to ensure that 1000BaseT auto-negotiation is enabled properly when desired. Based on existing handling in the AMD xgbe driver and the information in the Finisar FAQ: https://www.finisar.com/sites/default/files/resources/an-2036_1000base-t_sfp_faqreve1.pdf Signed-off-by: Robert Hancock Reviewed-by: Russell King --- Changed since v4: Fixed coding style in m88e_config_init_1000basex. drivers/net/phy/marvell.c | 100 +++- include/linux/marvell_phy.h | 3 ++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 5aec673a0120..1f062e6699c8 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -80,8 +80,11 @@ #define MII_M_HWCFG_MODE_FIBER_RGMII 0x3 #define MII_M_HWCFG_MODE_SGMII_NO_CLK 0x4 #define MII_M_HWCFG_MODE_RTBI 0x7 +#define MII_M_HWCFG_MODE_COPPER_1000X_AN 0x8 #define MII_M_HWCFG_MODE_COPPER_RTBI 0x9 #define MII_M_HWCFG_MODE_COPPER_RGMII 0xb +#define MII_M_HWCFG_MODE_COPPER_1000X_NOAN 0xc +#define MII_M_HWCFG_SERIAL_AN_BYPASS BIT(12) #define MII_M_HWCFG_FIBER_COPPER_RES BIT(13) #define MII_M_HWCFG_FIBER_COPPER_AUTO BIT(15) @@ -629,6 +632,51 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev) return genphy_check_and_restart_aneg(phydev, changed); } +static int m88e_config_aneg(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + int err; + + if (extsr < 0) + return extsr; + + /* If not using SGMII or copper 1000BaseX modes, use normal process. +* Steps below are only required for these modes. +*/ + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + (extsr & MII_M_HWCFG_MODE_MASK) != + MII_M_HWCFG_MODE_COPPER_1000X_AN) + return marvell_config_aneg(phydev); + + err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + if (err < 0) + goto error; + + /* Configure the copper link first */ + err = marvell_config_aneg(phydev); + if (err < 0) + goto error; + + /* Do not touch the fiber page if we're in copper->sgmii mode */ + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + return 0; + + /* Then the fiber link */ + err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); + if (err < 0) + goto error; + + err = marvell_config_aneg_fiber(phydev); + if (err < 0) + goto error; + + return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + +error: + marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + return err; +} + static int m88e1510_config_aneg(struct phy_device *phydev) { int err; @@ -814,6 +862,28 @@ static int m88e_config_init_rtbi(struct phy_device *phydev) MII_M_HWCFG_FIBER_COPPER_AUTO); } +static int m88e_config_init_1000basex(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + int err, mode; + + if (extsr < 0) + return extsr; + + /* If using copper mode, ensure 1000BaseX auto-negotiation is enabled */ + mode = extsr & MII_M_HWCFG_MODE_MASK; + if (mode == MII_M_HWCFG_MODE_COPPER_1000X_NOAN) { + err = phy_modify(phydev, MII_M_PHY_EXT_SR, +MII_M_HWCFG_MODE_MASK | +MII_M_HWCFG_SERIAL_AN_BYPASS, +MII_M_HWCFG_MODE_COPPER_1000X_AN | +MII_M_HWCFG_SERIAL_AN_BYPASS); + if (err < 0) + return err; + } + return 0; +} + static int m88e_config_init(struct phy_device *phydev) { int err; @@ -836,6 +906,12 @@ static int m88e_config_init(struct phy_device *phydev) return err; } + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { + err = m88e_config_init_1000basex(phydev); + if (err < 0) + return err; + } + err = marvell_of_reg_init(phydev); if (err < 0) return err; @@ -2658,7 +2734,28 @@ static struct phy_driver marvell_drivers[] = { /* PHY_GBIT_FEATURES */
[PATCH net-next v3] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
Update the axienet driver to properly support the Xilinx PCS/PMA PHY component which is used for 1000BaseX and SGMII modes, including properly configuring the auto-negotiation mode of the PHY and reading the negotiated state from the PHY. Signed-off-by: Robert Hancock --- Changed since v2: Removed some duplicate code in axienet_validate using fallthrough. drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + .../net/ethernet/xilinx/xilinx_axienet_main.c | 94 ++- 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f34c7903ff52..7326ad4d5e1c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -419,6 +419,9 @@ struct axienet_local { struct phylink *phylink; struct phylink_config phylink_config; + /* Reference to PCS/PMA PHY if used */ + struct mdio_device *pcs_phy; + /* Clock for AXI bus */ struct clk *clk; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 9aafd3ecdaa4..529c167cd5a6 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1517,10 +1517,27 @@ static void axienet_validate(struct phylink_config *config, phylink_set(mask, Asym_Pause); phylink_set(mask, Pause); - phylink_set(mask, 1000baseX_Full); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 1000baseT_Full); + + switch (state->interface) { + case PHY_INTERFACE_MODE_NA: + case PHY_INTERFACE_MODE_1000BASEX: + case PHY_INTERFACE_MODE_SGMII: + 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: + phylink_set(mask, 1000baseX_Full); + phylink_set(mask, 1000baseT_Full); + if (state->interface == PHY_INTERFACE_MODE_1000BASEX) + break; + fallthrough; + case PHY_INTERFACE_MODE_MII: + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + default: + break; + } bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS); @@ -1533,38 +1550,46 @@ static void axienet_mac_pcs_get_state(struct phylink_config *config, { struct net_device *ndev = to_net_dev(config->dev); struct axienet_local *lp = netdev_priv(ndev); - u32 emmc_reg, fcc_reg; - - state->interface = lp->phy_mode; - - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); - if (emmc_reg & XAE_EMMC_LINKSPD_1000) - state->speed = SPEED_1000; - else if (emmc_reg & XAE_EMMC_LINKSPD_100) - state->speed = SPEED_100; - else - state->speed = SPEED_10; - - state->pause = 0; - fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET); - if (fcc_reg & XAE_FCC_FCTX_MASK) - state->pause |= MLO_PAUSE_TX; - if (fcc_reg & XAE_FCC_FCRX_MASK) - state->pause |= MLO_PAUSE_RX; - state->an_complete = 0; - state->duplex = 1; + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + phylink_mii_c22_pcs_get_state(lp->pcs_phy, state); + break; + default: + break; + } } static void axienet_mac_an_restart(struct phylink_config *config) { - /* Unsupported, do nothing */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + + phylink_mii_c22_pcs_an_restart(lp->pcs_phy); } static void axienet_mac_config(struct phylink_config *config, unsigned int mode, const struct phylink_link_state *state) { - /* nothing meaningful to do */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + int ret; + + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode, +state->interface, +state->advertising); + if (ret < 0) + netdev_warn(ndev, "Failed to configure PCS: %d\n", + ret); + break; + + default: + break; + } } static void axienet_mac_link_down(struct phyli
Re: [PATCH net-next v2] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
On Tue, 2020-10-27 at 14:25 +, Radhey Shyam Pandey wrote: > > -Original Message- > > From: Robert Hancock > > Sent: Tuesday, October 27, 2020 1:15 AM > > To: Radhey Shyam Pandey ; da...@davemloft.net; > > k...@kernel.org > > Cc: li...@armlinux.org.uk; Michal Simek ; > > netdev@vger.kernel.org; and...@lunn.ch > > Subject: Re: [PATCH net-next v2] net: axienet: Properly handle > > PCS/PMA PHY > > for 1000BaseX mode > > > > On Mon, 2020-10-26 at 18:57 +, Radhey Shyam Pandey wrote: > > > Thanks for the patch. > > > > > > > -Original Message- > > > > From: Robert Hancock > > > > Sent: Monday, October 26, 2020 11:26 PM > > > > To: Radhey Shyam Pandey ; > > da...@davemloft.net; > > > > k...@kernel.org > > > > Cc: Michal Simek ; li...@armlinux.org.uk; > > > > and...@lunn.ch; netdev@vger.kernel.org; Robert Hancock > > > > > > > > Subject: [PATCH net-next v2] net: axienet: Properly handle > > > > PCS/PMA > > > > PHY for > > > > 1000BaseX mode > > > > > > > > Update the axienet driver to properly support the Xilinx > > > > PCS/PMA > > > > PHY > > > > component which is used for 1000BaseX and SGMII modes, > > > > including > > > > properly configuring the auto-negotiation mode of the PHY and > > > > reading > > > > the negotiated state from the PHY. > > > > > > > > Signed-off-by: Robert Hancock > > > > --- > > > > > > > > Resubmit of v2 tagged for net-next. > > > > > > > > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + > > > > .../net/ethernet/xilinx/xilinx_axienet_main.c | 96 > > > > ++- > > > > > > > > 2 files changed, 73 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > > > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > > > index f34c7903ff52..7326ad4d5e1c 100644 > > > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > > > @@ -419,6 +419,9 @@ struct axienet_local { > > > > struct phylink *phylink; > > > > struct phylink_config phylink_config; > > > > > > > > + /* Reference to PCS/PMA PHY if used */ > > > > + struct mdio_device *pcs_phy; > > > > + > > > > /* Clock for AXI bus */ > > > > struct clk *clk; > > > > > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > > > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > > > index 9aafd3ecdaa4..f46595ef2822 100644 > > > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > > > @@ -1517,10 +1517,29 @@ static void axienet_validate(struct > > > > phylink_config *config, > > > > > > > > phylink_set(mask, Asym_Pause); > > > > phylink_set(mask, Pause); > > > > - phylink_set(mask, 1000baseX_Full); > > > > - phylink_set(mask, 10baseT_Full); > > > > - phylink_set(mask, 100baseT_Full); > > > > - phylink_set(mask, 1000baseT_Full); > > > > + > > > > + switch (state->interface) { > > > > + case PHY_INTERFACE_MODE_NA: > > > > + case PHY_INTERFACE_MODE_1000BASEX: > > > > + case PHY_INTERFACE_MODE_SGMII: > > > > + 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: > > > > + phylink_set(mask, 1000baseX_Full); > > > > + phylink_set(mask, 1000baseT_Full); > > > > + if (state->interface == > > > > PHY_INTERFACE_MODE_1000BASEX) > > > > + break; > > > > > > 100BaseT and 10BaseT can be set in PHY_INTERFACE_MODE_MII if we > > > allow fallthrough here. > > > > Not quite sure what you are saying here? > > I was saying to allow switch case fall through. Ah, I see. Yes, that would work to save a couple duplicate lines - just not sure if using the switch fall-through is preferable. Any thoughts from people on what's preferred? -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH net-next v2] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
On Mon, 2020-10-26 at 18:57 +, Radhey Shyam Pandey wrote: > Thanks for the patch. > > > -Original Message- > > From: Robert Hancock > > Sent: Monday, October 26, 2020 11:26 PM > > To: Radhey Shyam Pandey ; da...@davemloft.net; > > k...@kernel.org > > Cc: Michal Simek ; li...@armlinux.org.uk; > > and...@lunn.ch; netdev@vger.kernel.org; Robert Hancock > > > > Subject: [PATCH net-next v2] net: axienet: Properly handle PCS/PMA > > PHY for > > 1000BaseX mode > > > > Update the axienet driver to properly support the Xilinx PCS/PMA > > PHY > > component which is used for 1000BaseX and SGMII modes, including > > properly configuring the auto-negotiation mode of the PHY and > > reading > > the negotiated state from the PHY. > > > > Signed-off-by: Robert Hancock > > --- > > > > Resubmit of v2 tagged for net-next. > > > > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + > > .../net/ethernet/xilinx/xilinx_axienet_main.c | 96 ++- > > > > 2 files changed, 73 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > index f34c7903ff52..7326ad4d5e1c 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > @@ -419,6 +419,9 @@ struct axienet_local { > > struct phylink *phylink; > > struct phylink_config phylink_config; > > > > + /* Reference to PCS/PMA PHY if used */ > > + struct mdio_device *pcs_phy; > > + > > /* Clock for AXI bus */ > > struct clk *clk; > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > index 9aafd3ecdaa4..f46595ef2822 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > @@ -1517,10 +1517,29 @@ static void axienet_validate(struct > > phylink_config *config, > > > > phylink_set(mask, Asym_Pause); > > phylink_set(mask, Pause); > > - phylink_set(mask, 1000baseX_Full); > > - phylink_set(mask, 10baseT_Full); > > - phylink_set(mask, 100baseT_Full); > > - phylink_set(mask, 1000baseT_Full); > > + > > + switch (state->interface) { > > + case PHY_INTERFACE_MODE_NA: > > + case PHY_INTERFACE_MODE_1000BASEX: > > + case PHY_INTERFACE_MODE_SGMII: > > + 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: > > + phylink_set(mask, 1000baseX_Full); > > + phylink_set(mask, 1000baseT_Full); > > + if (state->interface == PHY_INTERFACE_MODE_1000BASEX) > > + break; > > 100BaseT and 10BaseT can be set in PHY_INTERFACE_MODE_MII if we > allow fallthrough here. Not quite sure what you are saying here? > > + phylink_set(mask, 100baseT_Full); > > + phylink_set(mask, 10baseT_Full); > > + break; > > + case PHY_INTERFACE_MODE_MII: > > + phylink_set(mask, 100baseT_Full); > > + phylink_set(mask, 10baseT_Full); > > + default: > > + break; > > + } > > > > bitmap_and(supported, supported, mask, > >__ETHTOOL_LINK_MODE_MASK_NBITS); > > @@ -1533,38 +1552,46 @@ static void > > axienet_mac_pcs_get_state(struct > > phylink_config *config, > > { > > struct net_device *ndev = to_net_dev(config->dev); > > struct axienet_local *lp = netdev_priv(ndev); > > - u32 emmc_reg, fcc_reg; > > - > > - state->interface = lp->phy_mode; > > > > - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); > > - if (emmc_reg & XAE_EMMC_LINKSPD_1000) > > - state->speed = SPEED_1000; > > - else if (emmc_reg & XAE_EMMC_LINKSPD_100) > > - state->speed = SPEED_100; > > - else > > - state->speed = SPEED_10; > > - > > - state->pause = 0; > > - fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET); > > - if (fcc_reg & XAE_FCC_FCTX_MASK) > > - state->pause |= MLO_PAUSE_TX; > > - if (fcc_reg & XAE_FCC_FCRX_MASK) > > - state->pause |= MLO_PAUSE_RX; > > - > > - state->an_complete = 0; > > - state->duplex = 1; > &g
[PATCH net-next] net: phylink: disable BMCR_ISOLATE in phylink_mii_c22_pcs_config
The Xilinx PCS/PMA PHY requires that BMCR_ISOLATE be disabled for proper operation in 1000BaseX mode. It should be safe to ensure this bit is disabled in phylink_mii_c22_pcs_config in all cases. Signed-off-by: Robert Hancock Reviewed-by: Russell King --- Resubmit tagged for net-next. drivers/net/phy/phylink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index fe2296fdda19..5d8c015bc9f2 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -2515,9 +2515,10 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode, changed = ret > 0; + /* Ensure ISOLATE bit is disabled */ bmcr = mode == MLO_AN_INBAND ? BMCR_ANENABLE : 0; ret = mdiobus_modify(pcs->bus, pcs->addr, MII_BMCR, -BMCR_ANENABLE, bmcr); +BMCR_ANENABLE | BMCR_ISOLATE, bmcr); if (ret < 0) return ret; -- 2.18.4
[PATCH net-next v4] net: phy: marvell: add special handling of Finisar modules with 88E1111
The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel 88E PHY with a modified PHY ID. Add support for this ID using the 88E methods. By default these modules do not have 1000BaseX auto-negotiation enabled, which is not generally desirable with Linux networking drivers. Add handling to enable 1000BaseX auto-negotiation when these modules are used in 1000BaseX mode. Also, some special handling is required to ensure that 1000BaseT auto-negotiation is enabled properly when desired. Based on existing handling in the AMD xgbe driver and the information in the Finisar FAQ: https://www.finisar.com/sites/default/files/resources/an-2036_1000base-t_sfp_faqreve1.pdf Signed-off-by: Robert Hancock Reviewed-by: Russell King --- Changed in v4: Fixed variable order in m88e_config_aneg, added Russell's Reviewed-by drivers/net/phy/marvell.c | 99 - include/linux/marvell_phy.h | 3 ++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 5aec673a0120..2ed4b873a0cc 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -80,8 +80,11 @@ #define MII_M_HWCFG_MODE_FIBER_RGMII 0x3 #define MII_M_HWCFG_MODE_SGMII_NO_CLK 0x4 #define MII_M_HWCFG_MODE_RTBI 0x7 +#define MII_M_HWCFG_MODE_COPPER_1000X_AN 0x8 #define MII_M_HWCFG_MODE_COPPER_RTBI 0x9 #define MII_M_HWCFG_MODE_COPPER_RGMII 0xb +#define MII_M_HWCFG_MODE_COPPER_1000X_NOAN 0xc +#define MII_M_HWCFG_SERIAL_AN_BYPASS BIT(12) #define MII_M_HWCFG_FIBER_COPPER_RES BIT(13) #define MII_M_HWCFG_FIBER_COPPER_AUTO BIT(15) @@ -629,6 +632,51 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev) return genphy_check_and_restart_aneg(phydev, changed); } +static int m88e_config_aneg(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + int err; + + if (extsr < 0) + return extsr; + + /* If not using SGMII or copper 1000BaseX modes, use normal process. +* Steps below are only required for these modes. +*/ + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + (extsr & MII_M_HWCFG_MODE_MASK) != + MII_M_HWCFG_MODE_COPPER_1000X_AN) + return marvell_config_aneg(phydev); + + err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + if (err < 0) + goto error; + + /* Configure the copper link first */ + err = marvell_config_aneg(phydev); + if (err < 0) + goto error; + + /* Do not touch the fiber page if we're in copper->sgmii mode */ + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + return 0; + + /* Then the fiber link */ + err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); + if (err < 0) + goto error; + + err = marvell_config_aneg_fiber(phydev); + if (err < 0) + goto error; + + return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + +error: + marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + return err; +} + static int m88e1510_config_aneg(struct phy_device *phydev) { int err; @@ -814,6 +862,27 @@ static int m88e_config_init_rtbi(struct phy_device *phydev) MII_M_HWCFG_FIBER_COPPER_AUTO); } +static int m88e_config_init_1000basex(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If using copper mode, ensure 1000BaseX auto-negotiation is enabled */ + if ((extsr & MII_M_HWCFG_MODE_MASK) == + MII_M_HWCFG_MODE_COPPER_1000X_NOAN) { + int err = phy_modify(phydev, MII_M_PHY_EXT_SR, + MII_M_HWCFG_MODE_MASK | + MII_M_HWCFG_SERIAL_AN_BYPASS, + MII_M_HWCFG_MODE_COPPER_1000X_AN | + MII_M_HWCFG_SERIAL_AN_BYPASS); + if (err < 0) + return err; + } + return 0; +} + static int m88e_config_init(struct phy_device *phydev) { int err; @@ -836,6 +905,12 @@ static int m88e_config_init(struct phy_device *phydev) return err; } + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { + err = m88e_config_init_1000basex(phydev); + if (err < 0) + return err; + } + err = marvell_of_reg_init(phydev); if (err < 0) return err; @@ -2658,7 +2733,28 @@ static struct phy_driver marvell_drivers[] = { /* PHY_GBIT_FEATURES */ .probe = marvell_probe,
[PATCH net-next v2] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
Update the axienet driver to properly support the Xilinx PCS/PMA PHY component which is used for 1000BaseX and SGMII modes, including properly configuring the auto-negotiation mode of the PHY and reading the negotiated state from the PHY. Signed-off-by: Robert Hancock --- Resubmit of v2 tagged for net-next. drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + .../net/ethernet/xilinx/xilinx_axienet_main.c | 96 ++- 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f34c7903ff52..7326ad4d5e1c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -419,6 +419,9 @@ struct axienet_local { struct phylink *phylink; struct phylink_config phylink_config; + /* Reference to PCS/PMA PHY if used */ + struct mdio_device *pcs_phy; + /* Clock for AXI bus */ struct clk *clk; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 9aafd3ecdaa4..f46595ef2822 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1517,10 +1517,29 @@ static void axienet_validate(struct phylink_config *config, phylink_set(mask, Asym_Pause); phylink_set(mask, Pause); - phylink_set(mask, 1000baseX_Full); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 1000baseT_Full); + + switch (state->interface) { + case PHY_INTERFACE_MODE_NA: + case PHY_INTERFACE_MODE_1000BASEX: + case PHY_INTERFACE_MODE_SGMII: + 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: + phylink_set(mask, 1000baseX_Full); + phylink_set(mask, 1000baseT_Full); + if (state->interface == PHY_INTERFACE_MODE_1000BASEX) + break; + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + break; + case PHY_INTERFACE_MODE_MII: + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + default: + break; + } bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS); @@ -1533,38 +1552,46 @@ static void axienet_mac_pcs_get_state(struct phylink_config *config, { struct net_device *ndev = to_net_dev(config->dev); struct axienet_local *lp = netdev_priv(ndev); - u32 emmc_reg, fcc_reg; - - state->interface = lp->phy_mode; - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); - if (emmc_reg & XAE_EMMC_LINKSPD_1000) - state->speed = SPEED_1000; - else if (emmc_reg & XAE_EMMC_LINKSPD_100) - state->speed = SPEED_100; - else - state->speed = SPEED_10; - - state->pause = 0; - fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET); - if (fcc_reg & XAE_FCC_FCTX_MASK) - state->pause |= MLO_PAUSE_TX; - if (fcc_reg & XAE_FCC_FCRX_MASK) - state->pause |= MLO_PAUSE_RX; - - state->an_complete = 0; - state->duplex = 1; + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + phylink_mii_c22_pcs_get_state(lp->pcs_phy, state); + break; + default: + break; + } } static void axienet_mac_an_restart(struct phylink_config *config) { - /* Unsupported, do nothing */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + + phylink_mii_c22_pcs_an_restart(lp->pcs_phy); } static void axienet_mac_config(struct phylink_config *config, unsigned int mode, const struct phylink_link_state *state) { - /* nothing meaningful to do */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + int ret; + + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode, +state->interface, +state->advertising); + if (ret < 0) + netdev_warn(ndev, "Failed to configure PCS: %d\n", + ret); + break; + + default: + break; + } } sta
[PATCH v3] net: phy: marvell: add special handling of Finisar modules with 88E1111
The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel 88E PHY with a modified PHY ID. Add support for this ID using the 88E methods. By default these modules do not have 1000BaseX auto-negotiation enabled, which is not generally desirable with Linux networking drivers. Add handling to enable 1000BaseX auto-negotiation when these modules are used in 1000BaseX mode. Also, some special handling is required to ensure that 1000BaseT auto-negotiation is enabled properly when desired. Based on existing handling in the AMD xgbe driver and the information in the Finisar FAQ: https://www.finisar.com/sites/default/files/resources/an-2036_1000base-t_sfp_faqreve1.pdf Signed-off-by: Robert Hancock --- Changed since v2: Renamed 1000BX -> 1000X to avoid confusion with 1000Base-BX drivers/net/phy/marvell.c | 99 - include/linux/marvell_phy.h | 3 ++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 5aec673a0120..f9cef5ef6f5e 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -80,8 +80,11 @@ #define MII_M_HWCFG_MODE_FIBER_RGMII 0x3 #define MII_M_HWCFG_MODE_SGMII_NO_CLK 0x4 #define MII_M_HWCFG_MODE_RTBI 0x7 +#define MII_M_HWCFG_MODE_COPPER_1000X_AN 0x8 #define MII_M_HWCFG_MODE_COPPER_RTBI 0x9 #define MII_M_HWCFG_MODE_COPPER_RGMII 0xb +#define MII_M_HWCFG_MODE_COPPER_1000X_NOAN 0xc +#define MII_M_HWCFG_SERIAL_AN_BYPASS BIT(12) #define MII_M_HWCFG_FIBER_COPPER_RES BIT(13) #define MII_M_HWCFG_FIBER_COPPER_AUTO BIT(15) @@ -629,6 +632,51 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev) return genphy_check_and_restart_aneg(phydev, changed); } +static int m88e_config_aneg(struct phy_device *phydev) +{ + int err; + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If not using SGMII or copper 1000BaseX modes, use normal process. +* Steps below are only required for these modes. +*/ + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + (extsr & MII_M_HWCFG_MODE_MASK) != + MII_M_HWCFG_MODE_COPPER_1000X_AN) + return marvell_config_aneg(phydev); + + err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + if (err < 0) + goto error; + + /* Configure the copper link first */ + err = marvell_config_aneg(phydev); + if (err < 0) + goto error; + + /* Do not touch the fiber page if we're in copper->sgmii mode */ + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + return 0; + + /* Then the fiber link */ + err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); + if (err < 0) + goto error; + + err = marvell_config_aneg_fiber(phydev); + if (err < 0) + goto error; + + return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + +error: + marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + return err; +} + static int m88e1510_config_aneg(struct phy_device *phydev) { int err; @@ -814,6 +862,27 @@ static int m88e_config_init_rtbi(struct phy_device *phydev) MII_M_HWCFG_FIBER_COPPER_AUTO); } +static int m88e_config_init_1000basex(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If using copper mode, ensure 1000BaseX auto-negotiation is enabled */ + if ((extsr & MII_M_HWCFG_MODE_MASK) == + MII_M_HWCFG_MODE_COPPER_1000X_NOAN) { + int err = phy_modify(phydev, MII_M_PHY_EXT_SR, + MII_M_HWCFG_MODE_MASK | + MII_M_HWCFG_SERIAL_AN_BYPASS, + MII_M_HWCFG_MODE_COPPER_1000X_AN | + MII_M_HWCFG_SERIAL_AN_BYPASS); + if (err < 0) + return err; + } + return 0; +} + static int m88e_config_init(struct phy_device *phydev) { int err; @@ -836,6 +905,12 @@ static int m88e_config_init(struct phy_device *phydev) return err; } + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { + err = m88e_config_init_1000basex(phydev); + if (err < 0) + return err; + } + err = marvell_of_reg_init(phydev); if (err < 0) return err; @@ -2658,7 +2733,28 @@ static struct phy_driver marvell_drivers[] = { /* PHY_GBIT_FEATURES */ .probe = marvell_probe, .config_init = m88e_
[PATCH] net: phylink: disable BMCR_ISOLATE in phylink_mii_c22_pcs_config
The Xilinx PCS/PMA PHY requires that BMCR_ISOLATE be disabled for proper operation in 1000BaseX mode. It should be safe to ensure this bit is disabled in phylink_mii_c22_pcs_config in all cases. Signed-off-by: Robert Hancock --- drivers/net/phy/phylink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index fe2296fdda19..5d8c015bc9f2 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -2515,9 +2515,10 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode, changed = ret > 0; + /* Ensure ISOLATE bit is disabled */ bmcr = mode == MLO_AN_INBAND ? BMCR_ANENABLE : 0; ret = mdiobus_modify(pcs->bus, pcs->addr, MII_BMCR, -BMCR_ANENABLE, bmcr); +BMCR_ANENABLE | BMCR_ISOLATE, bmcr); if (ret < 0) return ret; -- 2.18.4
[PATCH v2] net: phy: marvell: add special handling of Finisar modules with 88E1111
The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel 88E PHY with a modified PHY ID. Add support for this ID using the 88E methods. By default these modules do not have 1000BaseX auto-negotiation enabled, which is not generally desirable with Linux networking drivers. Add handling to enable 1000BaseX auto-negotiation when these modules are used in 1000BaseX mode. Also, some special handling is required to ensure that 1000BaseT auto-negotiation is enabled properly when desired. Based on existing handling in the AMD xgbe driver and the information in the Finisar FAQ: https://www.finisar.com/sites/default/files/resources/an-2036_1000base-t_sfp_faqreve1.pdf Signed-off-by: Robert Hancock --- drivers/net/phy/marvell.c | 99 - include/linux/marvell_phy.h | 3 ++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 5aec673a0120..49392d15035c 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -80,8 +80,11 @@ #define MII_M_HWCFG_MODE_FIBER_RGMII 0x3 #define MII_M_HWCFG_MODE_SGMII_NO_CLK 0x4 #define MII_M_HWCFG_MODE_RTBI 0x7 +#define MII_M_HWCFG_MODE_COPPER_1000BX_AN 0x8 #define MII_M_HWCFG_MODE_COPPER_RTBI 0x9 #define MII_M_HWCFG_MODE_COPPER_RGMII 0xb +#define MII_M_HWCFG_MODE_COPPER_1000BX_NOAN 0xc +#define MII_M_HWCFG_SERIAL_AN_BYPASS BIT(12) #define MII_M_HWCFG_FIBER_COPPER_RES BIT(13) #define MII_M_HWCFG_FIBER_COPPER_AUTO BIT(15) @@ -629,6 +632,51 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev) return genphy_check_and_restart_aneg(phydev, changed); } +static int m88e_config_aneg(struct phy_device *phydev) +{ + int err; + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If not using SGMII or copper 1000BaseX modes, use normal process. +* Steps below are only required for these modes. +*/ + if (phydev->interface != PHY_INTERFACE_MODE_SGMII && + (extsr & MII_M_HWCFG_MODE_MASK) != + MII_M_HWCFG_MODE_COPPER_1000BX_AN) + return marvell_config_aneg(phydev); + + err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + if (err < 0) + goto error; + + /* Configure the copper link first */ + err = marvell_config_aneg(phydev); + if (err < 0) + goto error; + + /* Do not touch the fiber page if we're in copper->sgmii mode */ + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + return 0; + + /* Then the fiber link */ + err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); + if (err < 0) + goto error; + + err = marvell_config_aneg_fiber(phydev); + if (err < 0) + goto error; + + return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + +error: + marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + return err; +} + static int m88e1510_config_aneg(struct phy_device *phydev) { int err; @@ -814,6 +862,27 @@ static int m88e_config_init_rtbi(struct phy_device *phydev) MII_M_HWCFG_FIBER_COPPER_AUTO); } +static int m88e_config_init_1000basex(struct phy_device *phydev) +{ + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If using copper mode, ensure 1000BaseX auto-negotiation is enabled */ + if ((extsr & MII_M_HWCFG_MODE_MASK) == + MII_M_HWCFG_MODE_COPPER_1000BX_NOAN) { + int err = phy_modify(phydev, MII_M_PHY_EXT_SR, + MII_M_HWCFG_MODE_MASK | + MII_M_HWCFG_SERIAL_AN_BYPASS, + MII_M_HWCFG_MODE_COPPER_1000BX_AN | + MII_M_HWCFG_SERIAL_AN_BYPASS); + if (err < 0) + return err; + } + return 0; +} + static int m88e_config_init(struct phy_device *phydev) { int err; @@ -836,6 +905,12 @@ static int m88e_config_init(struct phy_device *phydev) return err; } + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { + err = m88e_config_init_1000basex(phydev); + if (err < 0) + return err; + } + err = marvell_of_reg_init(phydev); if (err < 0) return err; @@ -2658,7 +2733,28 @@ static struct phy_driver marvell_drivers[] = { /* PHY_GBIT_FEATURES */ .probe = marvell_probe, .config_init = m88e_config_init, - .config_aneg = marvell_config_aneg, + .conf
[PATCH v2] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
Update the axienet driver to properly support the Xilinx PCS/PMA PHY component which is used for 1000BaseX and SGMII modes, including properly configuring the auto-negotiation mode of the PHY and reading the negotiated state from the PHY. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + .../net/ethernet/xilinx/xilinx_axienet_main.c | 96 ++- 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f34c7903ff52..7326ad4d5e1c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -419,6 +419,9 @@ struct axienet_local { struct phylink *phylink; struct phylink_config phylink_config; + /* Reference to PCS/PMA PHY if used */ + struct mdio_device *pcs_phy; + /* Clock for AXI bus */ struct clk *clk; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 9aafd3ecdaa4..f46595ef2822 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1517,10 +1517,29 @@ static void axienet_validate(struct phylink_config *config, phylink_set(mask, Asym_Pause); phylink_set(mask, Pause); - phylink_set(mask, 1000baseX_Full); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 1000baseT_Full); + + switch (state->interface) { + case PHY_INTERFACE_MODE_NA: + case PHY_INTERFACE_MODE_1000BASEX: + case PHY_INTERFACE_MODE_SGMII: + 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: + phylink_set(mask, 1000baseX_Full); + phylink_set(mask, 1000baseT_Full); + if (state->interface == PHY_INTERFACE_MODE_1000BASEX) + break; + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + break; + case PHY_INTERFACE_MODE_MII: + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + default: + break; + } bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS); @@ -1533,38 +1552,46 @@ static void axienet_mac_pcs_get_state(struct phylink_config *config, { struct net_device *ndev = to_net_dev(config->dev); struct axienet_local *lp = netdev_priv(ndev); - u32 emmc_reg, fcc_reg; - - state->interface = lp->phy_mode; - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); - if (emmc_reg & XAE_EMMC_LINKSPD_1000) - state->speed = SPEED_1000; - else if (emmc_reg & XAE_EMMC_LINKSPD_100) - state->speed = SPEED_100; - else - state->speed = SPEED_10; - - state->pause = 0; - fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET); - if (fcc_reg & XAE_FCC_FCTX_MASK) - state->pause |= MLO_PAUSE_TX; - if (fcc_reg & XAE_FCC_FCRX_MASK) - state->pause |= MLO_PAUSE_RX; - - state->an_complete = 0; - state->duplex = 1; + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + phylink_mii_c22_pcs_get_state(lp->pcs_phy, state); + break; + default: + break; + } } static void axienet_mac_an_restart(struct phylink_config *config) { - /* Unsupported, do nothing */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + + phylink_mii_c22_pcs_an_restart(lp->pcs_phy); } static void axienet_mac_config(struct phylink_config *config, unsigned int mode, const struct phylink_link_state *state) { - /* nothing meaningful to do */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + int ret; + + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode, +state->interface, +state->advertising); + if (ret < 0) + netdev_warn(ndev, "Failed to configure PCS: %d\n", + ret); + break; + + default: + break; + } } static void axienet_mac_link_down(struct phyli
Re: [PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
On Tue, 2020-10-20 at 00:12 +0200, Andrew Lunn wrote: > > I think in my case those extra modes only supported in SGMII mode, > > like > > 10 and 100Mbps modes, effectively get filtered out because the MAC > > doesn't support them in the 1000BaseX mode either. > > There are different things here. What ethtool reports, and what is > programmed into the advertise register. Clearly, you don't want it > advertising 10 and 100 modes. If somebody connects it to a 10Half > link > partner, you need auto-get to fail. You don't want auto-neg to > succeed, and then all the frames get thrown away with bad CRCs, > overruns etc. > Right, I don't know that we have direct control over what the PHY is advertising to the copper side in this case. I think it's based on its auto-magical translation of the 1000BaseX advertisements from the PCS/PMA PHY in this case, where we only ever advertise 1000 modes in 1000BaseX mode (not sure if any other speeds are even defined for those messages). Presumably the 88E is smart enough to only advertise 1000 modes on the copper side when in 1000BaseX mode. > > The auto-negotiation is a bit of a weird thing in this case, as > > there > > are two negotiations occurring, the 1000BaseX between the PCS/PMA > > PHY > > and the module PHY, and the 1000BaseT between the module PHY and > > the > > copper link partner. I believe the 88E has some smarts to delay > > the > > copper negotiation until it gets the advertisement over 1000BaseX, > > uses > > those to figure out its advertisement, and then uses the copper > > link > > partner's response to determine the 1000BaseX response. > > But as far as i know you can only report duplex and pause over > 1000BaseX, not speed, since it is always 1G. > > It would be interesting to run ethtool on the link partner to see > what > it thinks the SFP is advertising. If it just 1000BaseT/Full? Well the device is plugged into a Linux PC on the other end, but unfortunately ethtool doesn't seem to report the link partner advertisements on that adapter, only the host advertisements (Intel 82574L, e1000e). -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
On Mon, 2020-10-19 at 23:59 +0200, Andrew Lunn wrote: > > I suppose that part would be pretty harmless, as you would likely > > want > > that behavior whenever that if condition was triggered. So > > m88e_finisar_config_init could likely be merged into > > m88e_config_init. > > I think so as well. > > > Mainly what stopped me from making all of these changes generic to > > all > > 88E is that I'm a bit less confident in the different > > config_aneg > > behavior, it might be more specific to this SFP copper module > > case? > > Well, for 1000BaseX, i don't think we currently have an SFP devices > using it, since phylink does not support it. So it is a question are > there any none-SFP m88e out there you might break? Yeah, I don't really know the answer to that question as I'm not familiar with all the other setups where this part would be used. So I'm inclined to leave that part specific to this ID. -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
On Mon, 2020-10-19 at 23:45 +0200, Andrew Lunn wrote: > > I have a local patch that just falls back to trying 1000BaseX mode > > if the driver reports SGMII isn't supported and it seems like it > > might be a copper module, but that is a bit of a hack that may need > > to be handled differently. > > Do you also modify what the PHY is advertising to remove the modes > your cannot support? I think in my case those extra modes only supported in SGMII mode, like 10 and 100Mbps modes, effectively get filtered out because the MAC doesn't support them in the 1000BaseX mode either. But yes, that probably should be fixed up in the PHY capabilities in a "proper" solution. The auto-negotiation is a bit of a weird thing in this case, as there are two negotiations occurring, the 1000BaseX between the PCS/PMA PHY and the module PHY, and the 1000BaseT between the module PHY and the copper link partner. I believe the 88E has some smarts to delay the copper negotiation until it gets the advertisement over 1000BaseX, uses those to figure out its advertisement, and then uses the copper link partner's response to determine the 1000BaseX response. -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
On Mon, 2020-10-19 at 23:36 +0200, Andrew Lunn wrote: > > static void axienet_mac_config(struct phylink_config *config, > > unsigned int mode, > >const struct phylink_link_state *state) > > { > > - /* nothing meaningful to do */ > > + struct net_device *ndev = to_net_dev(config->dev); > > + struct axienet_local *lp = netdev_priv(ndev); > > + int ret; > > + > > + switch (state->interface) { > > + case PHY_INTERFACE_MODE_SGMII: > > + case PHY_INTERFACE_MODE_1000BASEX: > > + ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode, > > +state->interface, > > +state->advertising); > > + if (ret < 0) > > + netdev_warn(ndev, "Failed to configure PCS: > > %d\n", > > + ret); > > + > > + /* Ensure isolate bit is cleared */ > > + ret = mdiobus_modify(lp->pcs_phy->bus, lp->pcs_phy- > > >addr, > > +MII_BMCR, BMCR_ISOLATE, 0); > > Hi Robert > > That looks like a layering violation. Maybe move this into > phylink_mii_c22_pcs_config(), it is accessing MII_BMCR anyway. Could do - do we think there's any harm in just disabling BMCR_ISOLATE in all cases in that function? -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
On Mon, 2020-10-19 at 23:00 +0200, Andrew Lunn wrote: > > +static int m88e_finisar_config_init(struct phy_device *phydev) > > +{ > > + int err; > > + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); > > + > > + if (extsr < 0) > > + return extsr; > > + > > + /* If using 1000BaseX and 1000BaseX auto-negotiation is > > disabled, enable it */ > > + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX && > > + (extsr & MII_M_HWCFG_MODE_MASK) == > > + MII_M_HWCFG_MODE_COPPER_1000BX_NOAN) { > > + err = phy_modify(phydev, MII_M_PHY_EXT_SR, > > +MII_M_HWCFG_MODE_MASK | > > +MII_M_HWCFG_SERIAL_AN_BYPASS, > > +MII_M_HWCFG_MODE_COPPER_1000BX_AN > > | > > +MII_M_HWCFG_SERIAL_AN_BYPASS); > > + if (err < 0) > > + return err; > > + } > > + > > + return m88e_config_init(phydev); > > +} > > Hi Robert > > Is this really specific to the Finisar? It seems like any application > of the m88e in 1000BaseX would benefit from this? I suppose that part would be pretty harmless, as you would likely want that behavior whenever that if condition was triggered. So m88e_finisar_config_init could likely be merged into m88e_config_init. Mainly what stopped me from making all of these changes generic to all 88E is that I'm a bit less confident in the different config_aneg behavior, it might be more specific to this SFP copper module case? -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
Re: [PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
On Mon, 2020-10-19 at 22:08 +0100, Russell King - ARM Linux admin wrote: > On Mon, Oct 19, 2020 at 02:49:13PM -0600, Robert Hancock wrote: > > The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel > > 81E PHY > > You mean 88E here. > Whoops, will fix in an updated version. > > with a modified PHY ID, and by default does not have 1000BaseX > > auto-negotiation enabled, which is not generally desirable with > > Linux > > networking drivers. Add handling to enable 1000BaseX auto- > > negotiation. > > Also, it requires some special handling to ensure that 1000BaseT > > auto- > > negotiation is enabled properly when desired. > > > > Based on existing handling in the AMD xgbe driver and the > > information in > > the Finisar FAQ: > > https://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.finisar.com%2Fsites%2Fdefault%2Ffiles%2Fresources%2Fan-2036_1000base-t_sfp_faqreve1.pdf&data=04%7C01%7Crobert.hancock%40calian.com%7C6eda7d636fbf4a70ff2408d8747332a2%7C23b57807562f49ad92c43bb0f07a1fdf%7C0%7C0%7C637387385403382018%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=cfChA4TBw3f70alrANXPR0HHgNg3Vs%2FNeEYhzZc%2BF9A%3D&reserved=0 > > There's lots of modules that have the 88E PHY on, and we switch > it to SGMII mode if it's not already in SGMII mode if we have access > to it. Why is your setup different? This is in our setup using the Xilinx axienet driver, which is stuck with whatever interface mode the FPGA logic is set up for at synthesis time. In our case since we need to support fiber modules, that means we are stuck with 1000BaseX mode with the copper modules as well. Note that there is some more work that needs to be done for this to work completely, as phylink currently will only attempt to use SGMII with copper modules and fails out if that's not supported. I have a local patch that just falls back to trying 1000BaseX mode if the driver reports SGMII isn't supported and it seems like it might be a copper module, but that is a bit of a hack that may need to be handled differently. -- Robert Hancock Senior Hardware Designer, Advanced Technologies www.calian.com
[PATCH] net: phy: marvell: add special handling of Finisar modules with 81E1111
The Finisar FCLF8520P2BTL 1000BaseT SFP module uses a Marvel 81E PHY with a modified PHY ID, and by default does not have 1000BaseX auto-negotiation enabled, which is not generally desirable with Linux networking drivers. Add handling to enable 1000BaseX auto-negotiation. Also, it requires some special handling to ensure that 1000BaseT auto- negotiation is enabled properly when desired. Based on existing handling in the AMD xgbe driver and the information in the Finisar FAQ: https://www.finisar.com/sites/default/files/resources/an-2036_1000base-t_sfp_faqreve1.pdf Signed-off-by: Robert Hancock --- drivers/net/phy/marvell.c | 82 + include/linux/marvell_phy.h | 3 ++ 2 files changed, 85 insertions(+) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 5aec673a0120..8d85c96209ad 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -80,8 +80,11 @@ #define MII_M_HWCFG_MODE_FIBER_RGMII 0x3 #define MII_M_HWCFG_MODE_SGMII_NO_CLK 0x4 #define MII_M_HWCFG_MODE_RTBI 0x7 +#define MII_M_HWCFG_MODE_COPPER_1000BX_AN 0x8 #define MII_M_HWCFG_MODE_COPPER_RTBI 0x9 #define MII_M_HWCFG_MODE_COPPER_RGMII 0xb +#define MII_M_HWCFG_MODE_COPPER_1000BX_NOAN 0xc +#define MII_M_HWCFG_SERIAL_AN_BYPASS BIT(12) #define MII_M_HWCFG_FIBER_COPPER_RES BIT(13) #define MII_M_HWCFG_FIBER_COPPER_AUTO BIT(15) @@ -629,6 +632,39 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev) return genphy_check_and_restart_aneg(phydev, changed); } +static int m88e_finisar_config_aneg(struct phy_device *phydev) +{ + int err; + + err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + if (err < 0) + goto error; + + /* Configure the copper link first */ + err = marvell_config_aneg(phydev); + if (err < 0) + goto error; + + /* Do not touch the fiber page if we're in copper->sgmii mode */ + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) + return 0; + + /* Then the fiber link */ + err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE); + if (err < 0) + goto error; + + err = marvell_config_aneg_fiber(phydev); + if (err < 0) + goto error; + + return marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + +error: + marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE); + return err; +} + static int m88e1510_config_aneg(struct phy_device *phydev) { int err; @@ -843,6 +879,30 @@ static int m88e_config_init(struct phy_device *phydev) return genphy_soft_reset(phydev); } +static int m88e_finisar_config_init(struct phy_device *phydev) +{ + int err; + int extsr = phy_read(phydev, MII_M_PHY_EXT_SR); + + if (extsr < 0) + return extsr; + + /* If using 1000BaseX and 1000BaseX auto-negotiation is disabled, enable it */ + if (phydev->interface == PHY_INTERFACE_MODE_1000BASEX && + (extsr & MII_M_HWCFG_MODE_MASK) == + MII_M_HWCFG_MODE_COPPER_1000BX_NOAN) { + err = phy_modify(phydev, MII_M_PHY_EXT_SR, +MII_M_HWCFG_MODE_MASK | +MII_M_HWCFG_SERIAL_AN_BYPASS, +MII_M_HWCFG_MODE_COPPER_1000BX_AN | +MII_M_HWCFG_SERIAL_AN_BYPASS); + if (err < 0) + return err; + } + + return m88e_config_init(phydev); +} + static int m88e_get_downshift(struct phy_device *phydev, u8 *data) { int val, cnt, enable; @@ -2672,6 +2732,27 @@ static struct phy_driver marvell_drivers[] = { .get_tunable = m88e_get_tunable, .set_tunable = m88e_set_tunable, }, + { + .phy_id = MARVELL_PHY_ID_88E_FINISAR, + .phy_id_mask = MARVELL_PHY_ID_MASK, + .name = "Marvell 88E (Finisar)", + /* PHY_GBIT_FEATURES */ + .probe = marvell_probe, + .config_init = &m88e_finisar_config_init, + .config_aneg = &m88e_finisar_config_aneg, + .read_status = &marvell_read_status, + .ack_interrupt = &marvell_ack_interrupt, + .config_intr = &marvell_config_intr, + .resume = &genphy_resume, + .suspend = &genphy_suspend, + .read_page = marvell_read_page, + .write_page = marvell_write_page, + .get_sset_count = marvell_get_sset_count, + .get_strings = marvell_get_strings, + .get_stats = marvell_get_stats, + .get_t
[PATCH] net: axienet: Properly handle PCS/PMA PHY for 1000BaseX mode
Update the axienet driver to properly support the Xilinx PCS/PMA PHY component which is used for 1000BaseX and SGMII modes, including properly configuring the auto-negotiation mode of the PHY and reading the negotiated state from the PHY. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 + .../net/ethernet/xilinx/xilinx_axienet_main.c | 104 +- 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f34c7903ff52..7326ad4d5e1c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -419,6 +419,9 @@ struct axienet_local { struct phylink *phylink; struct phylink_config phylink_config; + /* Reference to PCS/PMA PHY if used */ + struct mdio_device *pcs_phy; + /* Clock for AXI bus */ struct clk *clk; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 9aafd3ecdaa4..895c2c7a7f03 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1517,10 +1517,29 @@ static void axienet_validate(struct phylink_config *config, phylink_set(mask, Asym_Pause); phylink_set(mask, Pause); - phylink_set(mask, 1000baseX_Full); - phylink_set(mask, 10baseT_Full); - phylink_set(mask, 100baseT_Full); - phylink_set(mask, 1000baseT_Full); + + switch (state->interface) { + case PHY_INTERFACE_MODE_NA: + case PHY_INTERFACE_MODE_1000BASEX: + case PHY_INTERFACE_MODE_SGMII: + 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: + phylink_set(mask, 1000baseX_Full); + phylink_set(mask, 1000baseT_Full); + if (state->interface == PHY_INTERFACE_MODE_1000BASEX) + break; + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + break; + case PHY_INTERFACE_MODE_MII: + phylink_set(mask, 100baseT_Full); + phylink_set(mask, 10baseT_Full); + default: + break; + } bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS); @@ -1533,38 +1552,54 @@ static void axienet_mac_pcs_get_state(struct phylink_config *config, { struct net_device *ndev = to_net_dev(config->dev); struct axienet_local *lp = netdev_priv(ndev); - u32 emmc_reg, fcc_reg; - - state->interface = lp->phy_mode; - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); - if (emmc_reg & XAE_EMMC_LINKSPD_1000) - state->speed = SPEED_1000; - else if (emmc_reg & XAE_EMMC_LINKSPD_100) - state->speed = SPEED_100; - else - state->speed = SPEED_10; - - state->pause = 0; - fcc_reg = axienet_ior(lp, XAE_FCC_OFFSET); - if (fcc_reg & XAE_FCC_FCTX_MASK) - state->pause |= MLO_PAUSE_TX; - if (fcc_reg & XAE_FCC_FCRX_MASK) - state->pause |= MLO_PAUSE_RX; - - state->an_complete = 0; - state->duplex = 1; + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + phylink_mii_c22_pcs_get_state(lp->pcs_phy, state); + break; + default: + break; + } } static void axienet_mac_an_restart(struct phylink_config *config) { - /* Unsupported, do nothing */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + + phylink_mii_c22_pcs_an_restart(lp->pcs_phy); } static void axienet_mac_config(struct phylink_config *config, unsigned int mode, const struct phylink_link_state *state) { - /* nothing meaningful to do */ + struct net_device *ndev = to_net_dev(config->dev); + struct axienet_local *lp = netdev_priv(ndev); + int ret; + + switch (state->interface) { + case PHY_INTERFACE_MODE_SGMII: + case PHY_INTERFACE_MODE_1000BASEX: + ret = phylink_mii_c22_pcs_config(lp->pcs_phy, mode, +state->interface, +state->advertising); + if (ret < 0) + netdev_warn(ndev, "Failed to configure PCS: %d\n", + ret); + + /* Ensure isolate bit is cleared */ + ret = mdiobus_modify(lp->pcs_phy->bus, lp->pcs
Re: Xilinx axienet 1000BaseX support
On 2020-04-28 5:01 p.m., Russell King - ARM Linux admin wrote: On Tue, Apr 28, 2020 at 03:59:45PM -0600, Robert Hancock wrote: On 2020-04-22 1:51 a.m., Russell King - ARM Linux admin wrote: On Tue, Apr 21, 2020 at 07:45:47PM -0600, Robert Hancock wrote: Hi Andre/Russell, Just wondering where things got to with the changes for SGMII on Xilinx axienet that you were discussing (below)? I am looking into our Xilinx setup using 1000BaseX SFP and trying to get it working "properly" with newer kernels. My understanding is that the requirements for 1000BaseX and SGMII are somewhat similar. I gathered that SGMII was working somewhat already, but that not all link modes had been tested. However, it appears 1000BaseX is not yet working in the stock kernel. The way I had this working before with a 4.19-based kernel was basically a hack to phylink to allow the Xilinx PCS/PMA PHY to be configured sufficiently as a PHY for it to work, and mostly ignored the link status of the SFP PHY itself, even though we were using in-band signalling mode with an SFP module. That was using this patch: https://patchwork.ozlabs.org/project/netdev/patch/1559330285-30246-5-git-send-email-hanc...@sedsystems.ca/ Of course, that's basically just a hack which I suspect mostly worked by luck. I see that there are some helpers that were added to phylink to allow setting PHY advertisements and reading PHY status from clause 22 PHY devices, so I'm guessing that is the way to go in this case? Something like: axienet_mac_config: if using in-band mode, use phylink_mii_c22_pcs_set_advertisement to configure the Xilinx PHY. axienet_mac_pcs_get_state: use phylink_mii_c22_pcs_get_state to get the MAC PCS state from the Xilinx PHY axienet_mac_an_restart: if using in-band mode, use phylink_mii_c22_pcs_an_restart to restart autonegotiation on Xilinx PHY To use those c22 functions, we need to find the mdio_device that's referenced by the phy-handle in the device tree - I guess we can just use some of the guts of of_phy_find_device to do that? Please see the code for DPAA2 - it's changed slightly since I sent a copy to the netdev mailing list, and it still isn't clear whether this is the final approach (DPAA2 has some fun stuff such as several different PHYs at address 0.) NXP basically didn't like the approach I had in the patches I sent to netdev, we had a call, they presented an alternative appraoch, I implemented it, then they decided my original approach was the better solution for their situation. See http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7 specifically the patches from: "dpaa2-mac: add 1000BASE-X/SGMII PCS support" through to: "net: phylink: add interface to configure clause 22 PCS PHY" You may also need some of the patches further down in the net-queue branch: "net: phylink: avoid mac_config calls" through to: "net: phylink: rejig link state tracking" I've been playing with this a bit on a 5.4 kernel with some of these patches backported. However, I'm running into something that my previous hacks for this basically dealt with as a side effect: when phylink_start is called, sfp_upstream_start gets called, an SFP module is detected, phylink_connect_phy gets called, but then it hits this condition and bails out, because we are using INBAND mode with 1000BaseX: if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED || (pl->cfg_link_an_mode == MLO_AN_INBAND && phy_interface_mode_is_8023z(interface return -EINVAL; I'm expecting SGMII mode to be used when there's an external PHY as that gives greatest flexibility (as it allows 10 and 100Mbps speeds as well.) From what I remember, these blocks support SGMII, so it should just be a matter of adding that. They do support SGMII, but unfortunately it's not a runtime configurable parameter, it's a synthesis-level parameter on the FPGA IP core so you have to pick one or the other for any given build. We want to be able to support various fiber module types as well, and my understanding is that at least some of those only do 1000BaseX, so that ends up being the standard in common that we are using. I guess I'm not sure how this is supposed to work when the PHY on the SFP module gets detected, i.e. if there's supposed to be another code path that this is supposed to go down, or this is something that just hasn't been fully implemented yet? Copper PHYs work fine - using SGMII mode everywhere so far. The problem is, if you want to use them as 1000BASE-X, you generally have to ensure that the PHY is appropriately programmed for 1000BASE-X negotiation, and the copper side advertisement only indicates 1G support. Not all copper PHYs have the PHY accessible for such programming, and in that case it becomes an exercise of "read the SFP docume
Re: Xilinx axienet 1000BaseX support
On 2020-04-22 1:51 a.m., Russell King - ARM Linux admin wrote: On Tue, Apr 21, 2020 at 07:45:47PM -0600, Robert Hancock wrote: Hi Andre/Russell, Just wondering where things got to with the changes for SGMII on Xilinx axienet that you were discussing (below)? I am looking into our Xilinx setup using 1000BaseX SFP and trying to get it working "properly" with newer kernels. My understanding is that the requirements for 1000BaseX and SGMII are somewhat similar. I gathered that SGMII was working somewhat already, but that not all link modes had been tested. However, it appears 1000BaseX is not yet working in the stock kernel. The way I had this working before with a 4.19-based kernel was basically a hack to phylink to allow the Xilinx PCS/PMA PHY to be configured sufficiently as a PHY for it to work, and mostly ignored the link status of the SFP PHY itself, even though we were using in-band signalling mode with an SFP module. That was using this patch: https://patchwork.ozlabs.org/project/netdev/patch/1559330285-30246-5-git-send-email-hanc...@sedsystems.ca/ Of course, that's basically just a hack which I suspect mostly worked by luck. I see that there are some helpers that were added to phylink to allow setting PHY advertisements and reading PHY status from clause 22 PHY devices, so I'm guessing that is the way to go in this case? Something like: axienet_mac_config: if using in-band mode, use phylink_mii_c22_pcs_set_advertisement to configure the Xilinx PHY. axienet_mac_pcs_get_state: use phylink_mii_c22_pcs_get_state to get the MAC PCS state from the Xilinx PHY axienet_mac_an_restart: if using in-band mode, use phylink_mii_c22_pcs_an_restart to restart autonegotiation on Xilinx PHY To use those c22 functions, we need to find the mdio_device that's referenced by the phy-handle in the device tree - I guess we can just use some of the guts of of_phy_find_device to do that? Please see the code for DPAA2 - it's changed slightly since I sent a copy to the netdev mailing list, and it still isn't clear whether this is the final approach (DPAA2 has some fun stuff such as several different PHYs at address 0.) NXP basically didn't like the approach I had in the patches I sent to netdev, we had a call, they presented an alternative appraoch, I implemented it, then they decided my original approach was the better solution for their situation. See http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=cex7 specifically the patches from: "dpaa2-mac: add 1000BASE-X/SGMII PCS support" through to: "net: phylink: add interface to configure clause 22 PCS PHY" You may also need some of the patches further down in the net-queue branch: "net: phylink: avoid mac_config calls" through to: "net: phylink: rejig link state tracking" I've been playing with this a bit on a 5.4 kernel with some of these patches backported. However, I'm running into something that my previous hacks for this basically dealt with as a side effect: when phylink_start is called, sfp_upstream_start gets called, an SFP module is detected, phylink_connect_phy gets called, but then it hits this condition and bails out, because we are using INBAND mode with 1000BaseX: if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED || (pl->cfg_link_an_mode == MLO_AN_INBAND && phy_interface_mode_is_8023z(interface return -EINVAL; That same code is still in the latest version in the arm-linux cex7 branch, except now in phylink_attach_phy, and from what I can see would behave similarly. I guess I'm not sure how this is supposed to work when the PHY on the SFP module gets detected, i.e. if there's supposed to be another code path that this is supposed to go down, or this is something that just hasn't been fully implemented yet? -- Robert Hancock Senior Hardware Designer SED Systems, a division of Calian Ltd. Email: hanc...@sedsystems.ca
[PATCH net-next 0/2] Microchip KSZ driver enhancements
A couple of enhancements to the Microchip KSZ switch driver: one to add PHY register settings for errata workarounds for more stable operation, and another to add a device tree option to change the output clock rate as required by some board designs. Robert Hancock (2): net: dsa: microchip: Add PHY errata workarounds net: dsa: microchip: Support optional 125MHz SYNCLKO output Documentation/devicetree/bindings/net/dsa/ksz.txt | 2 + drivers/net/dsa/microchip/ksz9477.c | 66 +++ drivers/net/dsa/microchip/ksz_common.c| 2 + drivers/net/dsa/microchip/ksz_priv.h | 2 + 4 files changed, 72 insertions(+) -- 1.8.3.1
[PATCH net-next 2/2] net: dsa: microchip: Support optional 125MHz SYNCLKO output
The KSZ9477 series chips have a SYNCLKO pin which by default outputs a 25MHz clock, but some board setups require a 125MHz clock instead. Added a microchip,synclko-125 device tree property to allow indicating a 125MHz clock output is required. Signed-off-by: Robert Hancock --- Documentation/devicetree/bindings/net/dsa/ksz.txt | 2 ++ drivers/net/dsa/microchip/ksz9477.c | 4 drivers/net/dsa/microchip/ksz_common.c| 2 ++ drivers/net/dsa/microchip/ksz_priv.h | 1 + 4 files changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt index e7db726..4ac21ce 100644 --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt @@ -16,6 +16,8 @@ Required properties: Optional properties: - reset-gpios : Should be a gpio specifier for a reset line +- microchip,synclko-125 : Set if the output SYNCLKO frequency should be set to + 125MHz instead of 25MHz. See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional required and optional properties. diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 7be6d84..508380f 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -258,6 +258,10 @@ static int ksz9477_reset_switch(struct ksz_device *dev) data16 |= (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100; ksz_write16(dev, REG_SW_MAC_CTRL_2, data16); + if (dev->synclko_125) + ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, + SW_ENABLE_REFCLKO | SW_REFCLKO_IS_125MHZ); + return 0; } diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 39dace8..40c57d8 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -460,6 +460,8 @@ int ksz_switch_register(struct ksz_device *dev, ret = of_get_phy_mode(dev->dev->of_node); if (ret >= 0) dev->interface = ret; + dev->synclko_125 = of_property_read_bool(dev->dev->of_node, + "microchip,synclko-125"); } ret = dsa_register_switch(dev->ds); diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h index 724301d..c615d2a 100644 --- a/drivers/net/dsa/microchip/ksz_priv.h +++ b/drivers/net/dsa/microchip/ksz_priv.h @@ -78,6 +78,7 @@ struct ksz_device { phy_interface_t interface; u32 regs_size; bool phy_errata_9477; + bool synclko_125; struct vlan_table *vlan_cache; -- 1.8.3.1
[PATCH net-next 1/2] net: dsa: microchip: Add PHY errata workarounds
The Silicon Errata and Data Sheet Clarification documents for the KSZ9477 series of chips describe a number of otherwise undocumented PHY register settings which are required to work around various chip errata. Apply these settings when initializing the PHY ports on these chips. Signed-off-by: Robert Hancock --- drivers/net/dsa/microchip/ksz9477.c | 62 drivers/net/dsa/microchip/ksz_priv.h | 1 + 2 files changed, 63 insertions(+) diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index c026d15..7be6d84 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -1165,6 +1165,62 @@ static phy_interface_t ksz9477_get_interface(struct ksz_device *dev, int port) return interface; } +static void ksz9477_port_mmd_write(struct ksz_device *dev, int port, + u8 dev_addr, u16 reg_addr, u16 val) +{ + ksz_pwrite16(dev, port, REG_PORT_PHY_MMD_SETUP, +MMD_SETUP(PORT_MMD_OP_INDEX, dev_addr)); + ksz_pwrite16(dev, port, REG_PORT_PHY_MMD_INDEX_DATA, reg_addr); + ksz_pwrite16(dev, port, REG_PORT_PHY_MMD_SETUP, +MMD_SETUP(PORT_MMD_OP_DATA_NO_INCR, dev_addr)); + ksz_pwrite16(dev, port, REG_PORT_PHY_MMD_INDEX_DATA, val); +} + +static void ksz9477_phy_errata_setup(struct ksz_device *dev, int port) +{ + /* Apply PHY settings to address errata listed in +* KSZ9477, KSZ9897, KSZ9896, KSZ9567, KSZ8565 +* Silicon Errata and Data Sheet Clarification documents: +* +* Register settings are needed to improve PHY receive performance +*/ + ksz9477_port_mmd_write(dev, port, 0x01, 0x6f, 0xdd0b); + ksz9477_port_mmd_write(dev, port, 0x01, 0x8f, 0x6032); + ksz9477_port_mmd_write(dev, port, 0x01, 0x9d, 0x248c); + ksz9477_port_mmd_write(dev, port, 0x01, 0x75, 0x0060); + ksz9477_port_mmd_write(dev, port, 0x01, 0xd3, 0x); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x06, 0x3008); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x08, 0x2001); + + /* Transmit waveform amplitude can be improved +* (1000BASE-T, 100BASE-TX, 10BASE-Te) +*/ + ksz9477_port_mmd_write(dev, port, 0x1c, 0x04, 0x00d0); + + /* Energy Efficient Ethernet (EEE) feature select must +* be manually disabled (except on KSZ8565 which is 100Mbit) +*/ + if (dev->features & GBIT_SUPPORT) + ksz9477_port_mmd_write(dev, port, 0x07, 0x3c, 0x); + + /* Register settings are required to meet data sheet +* supply current specifications +*/ + ksz9477_port_mmd_write(dev, port, 0x1c, 0x13, 0x6eff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x14, 0xe6ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x15, 0x6eff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x16, 0xe6ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x17, 0x00ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x18, 0x43ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x19, 0xc3ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x1a, 0x6fff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x1b, 0x07ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x1c, 0x0fff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x1d, 0xe7ff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x1e, 0xefff); + ksz9477_port_mmd_write(dev, port, 0x1c, 0x20, 0x); +} + static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port) { u8 data8; @@ -1203,6 +1259,8 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port) PORT_FORCE_TX_FLOW_CTRL | PORT_FORCE_RX_FLOW_CTRL, false); + if (dev->phy_errata_9477) + ksz9477_phy_errata_setup(dev, port); } else { /* force flow control */ ksz_port_cfg(dev, port, REG_PORT_CTRL_0, @@ -1474,6 +1532,7 @@ struct ksz_chip_data { int num_statics; int cpu_ports; int port_cnt; + bool phy_errata_9477; }; static const struct ksz_chip_data ksz9477_switch_chips[] = { @@ -1485,6 +1544,7 @@ struct ksz_chip_data { .num_statics = 16, .cpu_ports = 0x7F, /* can be configured as cpu port */ .port_cnt = 7, /* total physical port count */ + .phy_errata_9477 = true, }, { .chip_id = 0x00989700, @@ -1494,6 +1554,7 @@ struct ksz_chip_data { .num_statics = 16, .cpu_ports = 0x7F, /* can be configured as cpu port */ .port_cnt = 7, /* total physical port count */ + .phy_errata_9477 = true, }, { .chip_id = 0x00989300, @@ -1522,6 +1583,7 @@ static int ksz9477_switch_init(struct
[PATCH net] net: dsa: microchip: Don't try to read stats for unused ports
If some of the switch ports were not listed in the device tree, due to being unused, the ksz_mib_read_work function ended up accessing a NULL dp->slave pointer and causing an oops. Skip checking statistics for any unused ports. Fixes: 7c6ff470aa867f53 ("net: dsa: microchip: add MIB counter reading support") Signed-off-by: Robert Hancock --- drivers/net/dsa/microchip/ksz_common.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 39dace8..f46086f 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -83,6 +83,9 @@ static void ksz_mib_read_work(struct work_struct *work) int i; for (i = 0; i < dev->mib_port_cnt; i++) { + if (dsa_is_unused_port(dev->ds, i)) + continue; + p = &dev->ports[i]; mib = &p->mib; mutex_lock(&mib->cnt_mutex); -- 1.8.3.1
[PATCH net-next] net: phy: Add more 1000BaseX support detection
Commit "net: phy: Add detection of 1000BaseX link mode support" added support for not filtering out 1000BaseX mode from the PHY's supported modes in genphy_config_init, but we have to make a similar change in genphy_read_abilities in order to actually detect it as a supported mode in the first place. Add this in. Signed-off-by: Robert Hancock --- drivers/net/phy/phy_device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 03c885e..5387890 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1984,6 +1984,8 @@ int genphy_read_abilities(struct phy_device *phydev) phydev->supported, val & ESTATUS_1000_TFULL); linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, phydev->supported, val & ESTATUS_1000_THALF); + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, +phydev->supported, val & ESTATUS_1000_XFULL); } return 0; -- 1.8.3.1
net-next: KSZ switch driver oops in ksz_mib_read_work
We are using an embedded platform with a KSZ9897 switch. I am getting the oops below in ksz_mib_read_work when testing with net-next branch. After adding in some debug output, the problem is in this code: for (i = 0; i < dev->mib_port_cnt; i++) { p = &dev->ports[i]; mib = &p->mib; mutex_lock(&mib->cnt_mutex); /* Only read MIB counters when the port is told to do. * If not, read only dropped counters when link is not up. */ if (!p->read) { const struct dsa_port *dp = dsa_to_port(dev->ds, i); if (!netif_carrier_ok(dp->slave)) mib->cnt_ptr = dev->reg_mib_cnt; } The oops is happening on port index 3 (i.e. 4th port) which is not connected on our platform and so has no entry in the device tree. For that port, dp->slave is NULL and so netif_carrier_ok explodes. If I change the code to skip the port entirely in the loop if dp->slave is NULL it seems to fix the crash, but I'm not that familiar with this code. Can someone confirm whether that is the proper fix? [ 17.842829] Unable to handle kernel NULL pointer dereference at virtual address 002c [ 17.850983] pgd = (ptrval) [ 17.853711] [002c] *pgd= [ 17.857317] Internal error: Oops: 5 [#1] PREEMPT SMP ARM [ 17.862632] Modules linked in: [ 17.865695] CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc3 #1 [ 17.872142] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [ 17.878688] Workqueue: events ksz_mib_read_work [ 17.883227] PC is at ksz_mib_read_work+0x58/0x94 [ 17.887848] LR is at ksz_mib_read_work+0x38/0x94 [ 17.887852] pc : []lr : []psr: 60070113 [ 17.887857] sp : e8147f08 ip : e8148000 fp : e000 [ 17.887860] r10: r9 : e8aa7040 r8 : e867cc44 [ 17.887865] r7 : 0c20 r6 : e8aa7120 r5 : 0003 r4 : e867c958 [ 17.887868] r3 : r2 : r1 : 0003 r0 : e8aa7040 [ 17.887879] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 17.948224] Control: 10c5387d Table: 38d9404a DAC: 0051 [ 17.948230] Process kworker/1:1 (pid: 21, stack limit = 0x(ptrval)) [ 17.948236] Stack: (0xe8147f08 to 0xe8148000) [ 17.948245] 7f00: e8aa7120 e80a8080 eb7aef40 eb7b2000 e8aa7124 [ 17.948254] 7f20: c013865c 0008 c0b03d00 e80a8080 e80a8094 eb7aef40 0008 [ 17.958073] systemd[1]: storage.mount: Unit is bound to inactive unit dev-mmcblk1p2.device. Stopping, too. [ 17.963306] 7f40: c0b03d00 eb7aef58 eb7aef40 c01393a0 e000 c0b46b09 c084e464 [ 17.963314] 7f60: e000 e8053140 e80530c0 e8146000 e80a8080 c013935c e80a1eac [ 17.963322] 7f80: e805315c c013e78c e80530c0 c013e648 [ 17.969893] random: systemd: uninitialized urandom read (16 bytes read) [ 17.973942] 7fa0: c01010e8 [ 17.973949] 7fc0: [ 17.973958] 7fe0: 0013 [ 17.982246] random: systemd: uninitialized urandom read (16 bytes read) [ 17.990329] [] (ksz_mib_read_work) from [] (process_one_work+0x17c/0x390) [ 17.990345] [] (process_one_work) from [] (worker_thread+0x44/0x518) [ 18.009394] random: systemd: uninitialized urandom read (16 bytes read) [ 18.016344] [] (worker_thread) from [] (kthread+0x144/0x14c) [ 18.016358] [] (kthread) from [] (ret_from_fork+0x14/0x2c) [ 18.016362] Exception stack(0xe8147fb0 to 0xe8147ff8) [ 18.016369] 7fa0: [ 18.031159] 7fc0: [ 18.031166] 7fe0: 0013 [ 18.031176] Code: 1a06 e51630e0 e0833405 e5933050 (e593302c) [ 18.031279] ---[ end trace ca82392a6c2aa959 ]--- -- Robert Hancock Senior Software Developer SED Systems, a division of Calian Ltd. Email: hanc...@sedsystems.ca
[PATCH net-next] net: axienet: move use of resource after validity check
We were accessing the pointer returned from platform_get_resource before checking if it was valid, causing an oops if it was not. Move this access after the call to devm_ioremap_resource which does the validity check. Signed-off-by: Robert Hancock --- This bug was introduced in my recent axienet patch series and so is only needed on net-next. drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index da420c8..561e28a 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1679,13 +1679,13 @@ static int axienet_probe(struct platform_device *pdev) lp->tx_bd_num = TX_BD_NUM_DEFAULT; /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); - lp->regs_start = ethres->start; lp->regs = devm_ioremap_resource(&pdev->dev, ethres); if (IS_ERR(lp->regs)) { dev_err(&pdev->dev, "could not map Axi Ethernet regs.\n"); ret = PTR_ERR(lp->regs); goto free_netdev; } + lp->regs_start = ethres->start; /* Setup checksum offload, but default to off if not specified */ lp->features = 0; -- 1.8.3.1
Re: [PATCH net-next] net: phy: phylink: support using device PHY in fixed or 802.3z mode
is because the early boards I > had access to when designing this setup were direct MAC to SFP cage > setups - there was no intermediate PHY. Then came the Macchiatobin > board which does have a PHY, which brings with it additional > complexities, but various hardware problems have stood in the way of > having SFP modules in the 10G slots. > >> In our case the controller is supporting 1000BaseX only and is mainly >> intended for fiber modules. We do want to be able to get copper modules >> to work - obviously only ones that are set up for 1000BaseX mode are >> possible. > > So, what I say below applies: > >>> If the former, then I'm pretty certain you're going about it the wrong >>> way - as I've said before, there is nothing in the EEPROM that >>> indicates definitively what format the control word is (and therefore >>> whether it is SGMII or 1000base-X.) >>> >>> Some network controllers may be able to tell the difference, but that >>> is not true of all controllers. >>> >>> The only way I can see to support such modules would be to have a table >>> of quirks to set the interface mode accordingly, and not try this "lets >>> pick one, try to validate it with the network controller, otherwise try >>> the other." >>> >>> In any case, the format of the connection between the SFP module and >>> the network controller isn't one that should appear in the ethtool link >>> modes - I view what you've done there as a hack rather than proper >>> design. > > I do have the beginnings of a quirk system for the sfp-bus layer, > since I've been conversing with someone with a GPON module that > does appear to follow the SFP MSA, in particular with regard to the > time it takes the module to start responding on I2C, and in regard > of the speeds it actually supports (basically, the EEPROM is > misleading.) So, that should be useful for you as well. > > http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=phy > > is my playground of patches for SFP, which are in various stages of > maturity, some which have been posted for inclusion (and merged) > others that have been waiting some time. > -- Robert Hancock Senior Software Developer SED Systems, a division of Calian Ltd. Email: hanc...@sedsystems.ca
Re: [PATCH net-next] net: phy: phylink: add fallback from SGMII to 1000BaseX
On 2019-06-02 9:15 a.m., Russell King - ARM Linux admin wrote: > On Fri, May 31, 2019 at 06:17:51PM -0600, Robert Hancock wrote: >> Our device is mainly intended for fiber modules, which is why 1000BaseX >> is being used. The variant of fiber modules we are using (for example, >> Finisar FCLF8520P2BTL) are set up for 1000BaseX, and seem like they are >> kind of a hack to allow using copper on devices which only support >> 1000BaseX mode (in fact that particular one is extra hacky since you >> have to disable 1000BaseX autonegotiation on the host side). This patch >> is basically intended to allow that particular case to work. > > Looking at the data sheet for FCLF8520P2BTL, it explicit states: > > PRODUCT SELECTION > Part Number Link Indicator 1000BASE-X auto-negotiation > on RX_LOS Pin enabled by default > FCLF8520P2BTL Yes No > FCLF8521P2BTL No Yes > FCLF8522P2BTL Yes Yes > > The idea being, you buy the correct one according to what the host > equipment requires, rather than just picking one and hoping it works. > > The data sheet goes on to mention that the module uses a Marvell > 88e PHY, which seems to be quite common for copper SFPs from > multiple manufacturers (but not all) and is very flexible in how it > can be configured. > > If we detect a PHY on the SFP module, we check detect whether it is > an 88e PHY, and then read out its configured link type. We don't > have a way to deal with the difference between FCLF8520P2BTL and > FCLF8521P2BTL, but at least we'll be able to tell whether we should > be in 1000Base-X mode for these modules, rather than SGMII. It looks like that might provide a solution for modules using the Marvell PHY, however some of the modules we are supporting seem to use a Broadcom PHY, and I have no idea if there is any documentation for those. It would really be rather silly if there were absolutely no way to tell what mode the module wants from the EEPROM.. I don't have any copper modules set up for SGMII, but below is the ethtool -m output for two of the 1000BaseX modules I have. If you have access to an SGMII module, can you compare this to what it indicates? # ethtool -m eth1 Identifier: 0x03 (SFP) Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID) Connector : 0x00 (unknown or unspecified) Transceiver codes : 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00 0x00 Transceiver type : Ethernet: 1000BASE-T Encoding : 0x01 (8B/10B) BR, Nominal : 1200MBd Rate identifier : 0x00 (unspecified) Length (SMF,km) : 0km Length (SMF) : 0m Length (50um) : 0m Length (62.5um) : 0m Length (Copper) : 100m Length (OM3) : 0m Laser wavelength : 0nm Vendor name : FINISAR CORP. Vendor OUI: 00:90:65 Vendor PN : FCLF8520P2BTL Vendor rev: A Option values : 0x00 0x12 Option: RX_LOS implemented Option: TX_DISABLE implemented BR margin, max: 0% BR margin, min: 0% Vendor SN : PX90NHX Date code : 170303 # ethtool -m eth1 Identifier: 0x03 (SFP) Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID) Connector : 0x00 (unknown or unspecified) Transceiver codes : 0x00 0x00 0x00 0x08 0x00 0x00 0x00 0x00 0x00 Transceiver type : Ethernet: 1000BASE-T Encoding : 0x01 (8B/10B) BR, Nominal : 1300MBd Rate identifier : 0x00 (unspecified) Length (SMF,km) : 0km Length (SMF) : 0m Length (50um) : 0m Length (62.5um) : 0m Length (Copper) : 100m Length (OM3)
[PATCH net-next 1/2 v2] net: sfp: Stop SFP polling and interrupt handling during shutdown
SFP device polling can cause problems during the shutdown process if the parent devices of the network controller have been shut down already. This problem was seen on the iMX6 platform with PCIe devices, where accessing the device after the bus is shut down causes a hang. Free any acquired GPIO interrupts and stop all delayed work in the SFP driver during the shutdown process, so that we ensure that no pending operations are still occurring after the SFP shutdown completes. Signed-off-by: Robert Hancock --- Changed since v1: Free interrupts during shutdown to avoid need for shutdown state flag. drivers/net/phy/sfp.c | 31 ++- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 554acc8..01af080 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -185,6 +185,7 @@ struct sfp { int (*write)(struct sfp *, bool, u8, void *, size_t); struct gpio_desc *gpio[GPIO_MAX]; + int gpio_irq[GPIO_MAX]; bool attached; unsigned int state; @@ -1786,7 +1787,7 @@ static int sfp_probe(struct platform_device *pdev) struct i2c_adapter *i2c; struct sfp *sfp; bool poll = false; - int irq, err, i; + int err, i; sfp = sfp_alloc(&pdev->dev); if (IS_ERR(sfp)) @@ -1885,19 +1886,22 @@ static int sfp_probe(struct platform_device *pdev) if (gpio_flags[i] != GPIOD_IN || !sfp->gpio[i]) continue; - irq = gpiod_to_irq(sfp->gpio[i]); - if (!irq) { + sfp->gpio_irq[i] = gpiod_to_irq(sfp->gpio[i]); + if (!sfp->gpio_irq[i]) { poll = true; continue; } - err = devm_request_threaded_irq(sfp->dev, irq, NULL, sfp_irq, + err = devm_request_threaded_irq(sfp->dev, sfp->gpio_irq[i], + NULL, sfp_irq, IRQF_ONESHOT | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, dev_name(sfp->dev), sfp); - if (err) + if (err) { + sfp->gpio_irq[i] = 0; poll = true; + } } if (poll) @@ -1928,9 +1932,26 @@ static int sfp_remove(struct platform_device *pdev) return 0; } +static void sfp_shutdown(struct platform_device *pdev) +{ + struct sfp *sfp = platform_get_drvdata(pdev); + int i; + + for (i = 0; i < GPIO_MAX; i++) { + if (!sfp->gpio_irq[i]) + continue; + + devm_free_irq(sfp->dev, sfp->gpio_irq[i], sfp); + } + + cancel_delayed_work_sync(&sfp->poll); + cancel_delayed_work_sync(&sfp->timeout); +} + static struct platform_driver sfp_driver = { .probe = sfp_probe, .remove = sfp_remove, + .shutdown = sfp_shutdown, .driver = { .name = "sfp", .of_match_table = sfp_of_match, -- 1.8.3.1
[PATCH net-next 2/2] net: sfp: add mutex to prevent concurrent state checks
sfp_check_state can potentially be called by both a threaded IRQ handler and delayed work. If it is concurrently called, it could result in incorrect state management. Add a st_mutex to protect the state - this lock gets taken outside of code that checks and handle state changes, and the existing sm_mutex nests inside of it. Suggested-by: Russell King Signed-off-by: Robert Hancock --- drivers/net/phy/sfp.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 01af080..edd2de5 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -188,10 +188,11 @@ struct sfp { int gpio_irq[GPIO_MAX]; bool attached; + struct mutex st_mutex; /* Protects state */ unsigned int state; struct delayed_work poll; struct delayed_work timeout; - struct mutex sm_mutex; + struct mutex sm_mutex; /* Protects state machine */ unsigned char sm_mod_state; unsigned char sm_dev_state; unsigned short sm_state; @@ -1705,6 +1706,7 @@ static void sfp_check_state(struct sfp *sfp) { unsigned int state, i, changed; + mutex_lock(&sfp->st_mutex); state = sfp_get_state(sfp); changed = state ^ sfp->state; changed &= SFP_F_PRESENT | SFP_F_LOS | SFP_F_TX_FAULT; @@ -1730,6 +1732,7 @@ static void sfp_check_state(struct sfp *sfp) sfp_sm_event(sfp, state & SFP_F_LOS ? SFP_E_LOS_HIGH : SFP_E_LOS_LOW); rtnl_unlock(); + mutex_unlock(&sfp->st_mutex); } static irqreturn_t sfp_irq(int irq, void *data) @@ -1760,6 +1763,7 @@ static struct sfp *sfp_alloc(struct device *dev) sfp->dev = dev; mutex_init(&sfp->sm_mutex); + mutex_init(&sfp->st_mutex); INIT_DELAYED_WORK(&sfp->poll, sfp_poll); INIT_DELAYED_WORK(&sfp->timeout, sfp_timeout); -- 1.8.3.1
[PATCH net-next 0/2] SFP polling fixes
This has an updated version of an earlier patch to ensure that SFP operations are stopped during shutdown, and another patch suggested by Russell King to address a potential concurrency issue with SFP state checks. Robert Hancock (2): net: sfp: Stop SFP polling and interrupt handling during shutdown net: sfp: add mutex to prevent concurrent state checks drivers/net/phy/sfp.c | 37 +++-- 1 file changed, 31 insertions(+), 6 deletions(-) -- 1.8.3.1
Re: [PATCH net-next] net: sfp: Stop SFP polling and interrupt handling during shutdown
On 2019-06-07 4:42 a.m., Russell King - ARM Linux admin wrote: > On Thu, Jun 06, 2019 at 02:57:22PM -0600, Robert Hancock wrote: >> The idea there was to deal with the case where GPIO interrupts were >> previously raised before shutdown and not yet handled by the threaded >> interrupt handler by the time shutdown is called. After shutdown on the >> SFP completes, the bus the GPIO stuff is on could potentially be shut >> down at any moment, so we really don't want to be digging into the GPIO >> states after that. Locking the mutex there ensures that we don't read a >> stale value for the shutdown flag in the interrupt handler, since AFAIK >> there's no other synchronization around that value. > > There are two cases: > > 1) The interrupt is raised just as sfp_shutdown() is called but before >the mutex is taken. We will get the full processing in this case. > > 2) The interrupt is raised during the mutex-locked bit of sfp_shutdown() >or after the mutex in sfp_shutdown() is released. We will get the >abbreviated processing. > > This means that the mutex doesn't provide any protection against full > interrupt processing if it occurs just prior to or during the initial > execution of sfp_shutdown(). > > All that we need to ensure is that the state of sfp->shutdown is > visible by the time sfp_shutdown() returns, and that none of the > interrupt and worker functions are executing. We have the worker > functions covered by the synchronous cancelling of them, but not the > interrupts, and as Florian points out, it's probably better to disable > the interrupts, and again, that can be done synchronously to ensure > that the handlers are not running. > > If the workers and interrupt handlers are synchronously disabled, we > can be sure by the end of sfp_shutdown() that none of those paths are > executing, so the next time something attempts to trigger them, they > will see sfp->shutdown is set. > > I'm not convinced that we even need sfp->shutdown if we have cancelled > the workers and disabled the interrupts. I think you're right that if we just free the interrupts we avoid the need for that flag, as that should ensure that the interrupt handlers are no longer running or pending. Will resubmit with that approach and also add a patch to add another mutex to the state checking as you suggested. -- Robert Hancock Senior Software Developer SED Systems, a division of Calian Ltd. Email: hanc...@sedsystems.ca
[PATCH net-next v5 18/20] net: axienet: document axistream-connected attribute
The axienet driver requires the use of an axistream-connected attribute, but this isn't documented in the devicetree bindings. Document how this attribute is supposed to be used, including the upcoming change to make the usage of this attribute optional. Signed-off-by: Robert Hancock --- .../devicetree/bindings/net/xilinx_axienet.txt| 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index a8be67b..7360617 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -17,9 +17,15 @@ For more details about mdio please refer phy.txt file in the same directory. Required properties: - compatible : Must be one of "xlnx,axi-ethernet-1.00.a", "xlnx,axi-ethernet-1.01.a", "xlnx,axi-ethernet-2.01.a" -- reg : Address and length of the IO space. +- reg : Address and length of the IO space, as well as the address + and length of the AXI DMA controller IO space, unless + axistream-connected is specified, in which case the reg + attribute of the node referenced by it is used. - interrupts : Should be a list of 2 or 3 interrupts: TX DMA, RX DMA, - and optionally Ethernet core. + and optionally Ethernet core. If axistream-connected is + specified, the TX/RX DMA interrupts should be on that node + instead, and only the Ethernet core interrupt is optionally + specified here. - phy-handle : Should point to the external phy device. See ethernet.txt file in the same directory. - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the hardware @@ -37,6 +43,11 @@ Optional properties: auto-detected from the CPU clock (but only on platforms where this is possible). New device trees should specify this - the auto detection is only for backward compatibility. +- axistream-connected: Reference to another node which contains the resources + for the AXI DMA controller used by this device. + If this is specified, the DMA-related resources from that + device (DMA registers and DMA TX/RX interrupts) rather + than this one will be used. - mdio: Child node for MDIO bus. Must be defined if PHY access is required through the core's MDIO interface (i.e. always, unless the PHY is accessed through a different bus). @@ -46,10 +57,10 @@ Example: compatible = "xlnx,axi-ethernet-1.00.a"; device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; - interrupts = <2 0>; + interrupts = <2 0 1>; clocks = <&axi_clk>; phy-mode = "mii"; - reg = <0x40c0 0x4>; + reg = <0x40c0 0x4 0x50c0 0x4>; xlnx,rxcsum = <0x2>; xlnx,rxmem = <0x800>; xlnx,txcsum = <0x2>; -- 1.8.3.1
[PATCH net-next v5 14/20] net: axienet: Make missing MAC address non-fatal
Failing initialization on a missing MAC address property is excessive. We can just fall back to using a random MAC instead, which at least leaves the interface in a functioning state. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 834fafd..6e75c43 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1733,8 +1733,9 @@ static int axienet_probe(struct platform_device *pdev) /* Retrieve the MAC address */ mac_addr = of_get_mac_address(pdev->dev.of_node); if (IS_ERR(mac_addr)) { - dev_err(&pdev->dev, "could not find MAC address\n"); - goto free_netdev; + dev_warn(&pdev->dev, "could not find MAC address property: %ld\n", +PTR_ERR(mac_addr)); + mac_addr = NULL; } axienet_set_mac_address(ndev, mac_addr); -- 1.8.3.1
[PATCH net-next v5 15/20] net: axienet: stop interface during shutdown
On some platforms, such as iMX6 with PCIe devices, crashes or hangs can occur if the axienet device continues to perform DMA transfers after parent devices/busses have been shut down. Shut down the axienet interface during its shutdown callback in order to avoid this. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 6e75c43..d138db8 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1797,9 +1797,23 @@ static int axienet_remove(struct platform_device *pdev) return 0; } +static void axienet_shutdown(struct platform_device *pdev) +{ + struct net_device *ndev = platform_get_drvdata(pdev); + + rtnl_lock(); + netif_device_detach(ndev); + + if (netif_running(ndev)) + dev_close(ndev); + + rtnl_unlock(); +} + static struct platform_driver axienet_driver = { .probe = axienet_probe, .remove = axienet_remove, + .shutdown = axienet_shutdown, .driver = { .name = "xilinx_axienet", .of_match_table = axienet_of_match, -- 1.8.3.1
[PATCH net-next v5 09/20] net: axienet: Make RX/TX ring sizes configurable
Add support for setting the RX and TX ring sizes for this driver using ethtool. Also increase the default RX ring size as the previous default was far too low for good performance in some configurations. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 2 + drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 90 --- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 1ffb113..6b6d28f 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -444,8 +444,10 @@ struct axienet_local { /* Buffer descriptors */ struct axidma_bd *tx_bd_v; dma_addr_t tx_bd_p; + u32 tx_bd_num; struct axidma_bd *rx_bd_v; dma_addr_t rx_bd_p; + u32 rx_bd_num; u32 tx_bd_ci; u32 tx_bd_tail; u32 rx_bd_ci; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index bdc6e80..2c2d626 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -39,9 +39,11 @@ #include "xilinx_axienet.h" -/* Descriptors defines for Tx and Rx DMA - 2^n for the best performance */ -#define TX_BD_NUM 64 -#define RX_BD_NUM 128 +/* Descriptors defines for Tx and Rx DMA */ +#define TX_BD_NUM_DEFAULT 64 +#define RX_BD_NUM_DEFAULT 1024 +#define TX_BD_NUM_MAX 4096 +#define RX_BD_NUM_MAX 4096 /* Must be shorter than length of ethtool_drvinfo.driver field to fit */ #define DRIVER_NAME"xaxienet" @@ -157,7 +159,7 @@ static void axienet_dma_bd_release(struct net_device *ndev) int i; struct axienet_local *lp = netdev_priv(ndev); - for (i = 0; i < RX_BD_NUM; i++) { + for (i = 0; i < lp->rx_bd_num; i++) { dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys, lp->max_frm_size, DMA_FROM_DEVICE); dev_kfree_skb(lp->rx_bd_v[i].skb); @@ -165,13 +167,13 @@ static void axienet_dma_bd_release(struct net_device *ndev) if (lp->rx_bd_v) { dma_free_coherent(ndev->dev.parent, - sizeof(*lp->rx_bd_v) * RX_BD_NUM, + sizeof(*lp->rx_bd_v) * lp->rx_bd_num, lp->rx_bd_v, lp->rx_bd_p); } if (lp->tx_bd_v) { dma_free_coherent(ndev->dev.parent, - sizeof(*lp->tx_bd_v) * TX_BD_NUM, + sizeof(*lp->tx_bd_v) * lp->tx_bd_num, lp->tx_bd_v, lp->tx_bd_p); } @@ -201,27 +203,27 @@ static int axienet_dma_bd_init(struct net_device *ndev) /* Allocate the Tx and Rx buffer descriptors. */ lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent, -sizeof(*lp->tx_bd_v) * TX_BD_NUM, +sizeof(*lp->tx_bd_v) * lp->tx_bd_num, &lp->tx_bd_p, GFP_KERNEL); if (!lp->tx_bd_v) goto out; lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent, -sizeof(*lp->rx_bd_v) * RX_BD_NUM, +sizeof(*lp->rx_bd_v) * lp->rx_bd_num, &lp->rx_bd_p, GFP_KERNEL); if (!lp->rx_bd_v) goto out; - for (i = 0; i < TX_BD_NUM; i++) { + for (i = 0; i < lp->tx_bd_num; i++) { lp->tx_bd_v[i].next = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * - ((i + 1) % TX_BD_NUM); + ((i + 1) % lp->tx_bd_num); } - for (i = 0; i < RX_BD_NUM; i++) { + for (i = 0; i < lp->rx_bd_num; i++) { lp->rx_bd_v[i].next = lp->rx_bd_p + sizeof(*lp->rx_bd_v) * - ((i + 1) % RX_BD_NUM); + ((i + 1) % lp->rx_bd_num); skb = netdev_alloc_skb_ip_align(ndev, lp->max_frm_size); if (!skb) @@ -269,7 +271,7 @@ static int axienet_dma_bd_init(struct net_device *ndev) axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, cr | XAXIDMA_CR_RUNSTOP_MASK); axienet_dma_out32(lp, XAXIDMA_RX_TDESC_OFFSET, lp->rx_bd_p + - (sizeof(*lp-&
[PATCH net-next v5 05/20] net: axienet: Use clock framework to get device clock rate
This driver was previously always calculating the MDIO clock divisor (from AXI bus clock to MDIO bus clock) based on the CPU clock frequency, assuming that it is the same as the AXI bus frequency, but that simplistic method only works on the MicroBlaze platform. Add support for specifying the clock used for the device in the device tree using the clock framework. If the clock is specified then it will be used when calculating the clock divisor. The previous CPU clock detection method is left for backward compatibility if no clock is specified. Signed-off-by: Robert Hancock --- .../devicetree/bindings/net/xilinx_axienet.txt | 6 +++ drivers/net/ethernet/xilinx/xilinx_axienet.h | 5 +- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 23 +- drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 53 -- 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 38f9ec0..2dea903 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -31,6 +31,11 @@ Optional properties: 1 to enable partial TX checksum offload, 2 to enable full TX checksum offload - xlnx,rxcsum : Same values as xlnx,txcsum but for RX checksum offload +- clocks : AXI bus clock for the device. Refer to common clock bindings. + Used to calculate MDIO clock divisor. If not specified, it is + auto-detected from the CPU clock (but only on platforms where + this is possible). New device trees should specify this - the + auto detection is only for backward compatibility. Example: axi_ethernet_eth: ethernet@40c0 { @@ -38,6 +43,7 @@ Example: device_type = "network"; interrupt-parent = <µblaze_0_axi_intc>; interrupts = <2 0>; + clocks = <&axi_clk>; phy-mode = "mii"; reg = <0x40c0 0x4>; xlnx,rxcsum = <0x2>; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f9078bd..f240ff1 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -418,6 +418,9 @@ struct axienet_local { /* Connection to PHY device */ struct device_node *phy_node; + /* Clock for AXI bus */ + struct clk *clk; + /* MDIO bus data */ struct mii_bus *mii_bus;/* MII bus reference */ @@ -502,7 +505,7 @@ static inline void axienet_iow(struct axienet_local *lp, off_t offset, } /* Function prototypes visible in xilinx_axienet_mdio.c for other files */ -int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np); +int axienet_mdio_setup(struct axienet_local *lp); int axienet_mdio_wait_until_ready(struct axienet_local *lp); void axienet_mdio_teardown(struct axienet_local *lp); diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index ffbd4d7..42b343c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -21,6 +21,7 @@ * - Add support for extended VLAN support. */ +#include #include #include #include @@ -1611,9 +1612,24 @@ static int axienet_probe(struct platform_device *pdev) lp->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); if (lp->phy_node) { - ret = axienet_mdio_setup(lp, pdev->dev.of_node); + lp->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(lp->clk)) { + dev_warn(&pdev->dev, "Failed to get clock: %ld\n", +PTR_ERR(lp->clk)); + lp->clk = NULL; + } else { + ret = clk_prepare_enable(lp->clk); + if (ret) { + dev_err(&pdev->dev, "Unable to enable clock: %d\n", + ret); + goto free_netdev; + } + } + + ret = axienet_mdio_setup(lp); if (ret) - dev_warn(&pdev->dev, "error registering MDIO bus\n"); + dev_warn(&pdev->dev, +"error registering MDIO bus: %d\n", ret); } ret = register_netdev(lp->ndev); @@ -1638,6 +1654,9 @@ static int axienet_remove(struct platform_device *pdev) axienet_mdio_teardown(lp); unregister_netdev(ndev); +
[PATCH net-next v5 10/20] net: axienet: Add DMA registers to ethtool register dump
These registers are important for troubleshooting the state of the DMA cores. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 2c2d626..4df424c 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -50,7 +50,7 @@ #define DRIVER_DESCRIPTION "Xilinx Axi Ethernet driver" #define DRIVER_VERSION "1.00a" -#define AXIENET_REGS_N 32 +#define AXIENET_REGS_N 40 /* Match table for of_platform binding */ static const struct of_device_id axienet_of_match[] = { @@ -1179,6 +1179,14 @@ static void axienet_ethtools_get_regs(struct net_device *ndev, data[29] = axienet_ior(lp, XAE_FMI_OFFSET); data[30] = axienet_ior(lp, XAE_AF0_OFFSET); data[31] = axienet_ior(lp, XAE_AF1_OFFSET); + data[32] = axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET); + data[33] = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET); + data[34] = axienet_dma_in32(lp, XAXIDMA_TX_CDESC_OFFSET); + data[35] = axienet_dma_in32(lp, XAXIDMA_TX_TDESC_OFFSET); + data[36] = axienet_dma_in32(lp, XAXIDMA_RX_CR_OFFSET); + data[37] = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET); + data[38] = axienet_dma_in32(lp, XAXIDMA_RX_CDESC_OFFSET); + data[39] = axienet_dma_in32(lp, XAXIDMA_RX_TDESC_OFFSET); } static void axienet_ethtools_get_ringparam(struct net_device *ndev, -- 1.8.3.1
[PATCH net-next v5 03/20] net: axienet: fix MDIO bus naming
The MDIO bus for this driver was being named using the result of of_address_to_resource on a node which may not have any resource on it, but the return value of that call was not checked so it was using some random value in the bus name. Change to name the MDIO bus based on the resource start of the actual Ethernet register block. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 2 ++ drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 + drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 11 +-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index d82e3b6..f9078bd 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -380,6 +380,7 @@ struct axidma_bd { * @dev: Pointer to device structure * @phy_node: Pointer to device node structure * @mii_bus: Pointer to MII bus structure + * @regs_start: Resource start for axienet device addresses * @regs: Base address for the axienet_local device address space * @dma_regs: Base address for the axidma device address space * @dma_err_tasklet: Tasklet structure to process Axi DMA errors @@ -421,6 +422,7 @@ struct axienet_local { struct mii_bus *mii_bus;/* MII bus reference */ /* IO registers, dma functions and IRQs */ + resource_size_t regs_start; void __iomem *regs; void __iomem *dma_regs; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 55beca1..ffbd4d7 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1480,6 +1480,7 @@ static int axienet_probe(struct platform_device *pdev) lp->options = XAE_OPTION_DEFAULTS; /* Map device registers */ ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0); + lp->regs_start = ethres->start; lp->regs = devm_ioremap_resource(&pdev->dev, ethres); if (IS_ERR(lp->regs)) { dev_err(&pdev->dev, "could not map Axi Ethernet regs.\n"); diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c index 704babd..665ae1d 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c @@ -127,7 +127,7 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np) int ret; u32 clk_div, host_clock; struct mii_bus *bus; - struct resource res; + struct device_node *mdio_node; struct device_node *np1; /* clk_div can be calculated by deriving it from the equation: @@ -199,10 +199,9 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np) if (!bus) return -ENOMEM; - np1 = of_get_parent(lp->phy_node); - of_address_to_resource(np1, 0, &res); - snprintf(bus->id, MII_BUS_ID_SIZE, "%.8llx", -(unsigned long long) res.start); + mdio_node = of_get_parent(lp->phy_node); + snprintf(bus->id, MII_BUS_ID_SIZE, "axienet-%.8llx", +(unsigned long long)lp->regs_start); bus->priv = lp; bus->name = "Xilinx Axi Ethernet MDIO"; @@ -211,7 +210,7 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np) bus->parent = lp->dev; lp->mii_bus = bus; - ret = of_mdiobus_register(bus, np1); + ret = of_mdiobus_register(bus, mdio_node); if (ret) { mdiobus_free(bus); lp->mii_bus = NULL; -- 1.8.3.1
[PATCH net-next v5 16/20] net: axienet: document device tree mdio child node
The mdio child node for the MDIO bus is generally required when using this driver but was not documented other than being shown in the example. Document it as an optional (but usually required) parameter. Signed-off-by: Robert Hancock --- Documentation/devicetree/bindings/net/xilinx_axienet.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 0be335c..a8be67b 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -37,6 +37,9 @@ Optional properties: auto-detected from the CPU clock (but only on platforms where this is possible). New device trees should specify this - the auto detection is only for backward compatibility. + - mdio: Child node for MDIO bus. Must be defined if PHY access is + required through the core's MDIO interface (i.e. always, + unless the PHY is accessed through a different bus). Example: axi_ethernet_eth: ethernet@40c0 { -- 1.8.3.1
[PATCH net-next v5 20/20] net: axienet: convert to phylink API
Convert this driver to use the phylink API rather than the legacy PHY API. This allows for better support for SFP modules connected using a 1000BaseX or SGMII interface. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/Kconfig | 2 +- drivers/net/ethernet/xilinx/xilinx_axienet.h | 5 +- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 286 ++ 3 files changed, 192 insertions(+), 101 deletions(-) diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig index 5f50764..8d994ce 100644 --- a/drivers/net/ethernet/xilinx/Kconfig +++ b/drivers/net/ethernet/xilinx/Kconfig @@ -27,7 +27,7 @@ config XILINX_EMACLITE config XILINX_AXI_EMAC tristate "Xilinx 10/100/1000 AXI Ethernet support" depends on MICROBLAZE || X86 || ARM || COMPILE_TEST - select PHYLIB + select PHYLINK ---help--- This driver supports the 10/100/1000 Ethernet from Xilinx for the AXI bus interface used in Xilinx Virtex FPGAs. diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 8e605a8..2dacfc8 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -13,6 +13,7 @@ #include #include #include +#include /* Packet size info */ #define XAE_HDR_SIZE 14 /* Size of Ethernet header */ @@ -420,6 +421,9 @@ struct axienet_local { /* Connection to PHY device */ struct device_node *phy_node; + struct phylink *phylink; + struct phylink_config phylink_config; + /* Clock for AXI bus */ struct clk *clk; @@ -439,7 +443,6 @@ struct axienet_local { phy_interface_t phy_mode; u32 options;/* Current options word */ - u32 last_link; u32 features; /* Buffer descriptors */ diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 898eabf..da420c8 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -7,6 +7,7 @@ * Copyright (c) 2008-2009 Secret Lab Technologies Ltd. * Copyright (c) 2010 - 2011 Michal Simek * Copyright (c) 2010 - 2011 PetaLogix + * Copyright (c) 2019 SED Systems, a division of Calian Ltd. * Copyright (c) 2010 - 2012 Xilinx, Inc. All rights reserved. * * This is a driver for the Xilinx Axi Ethernet which is used in the Virtex6 @@ -520,63 +521,6 @@ static void axienet_device_reset(struct net_device *ndev) } /** - * axienet_adjust_link - Adjust the PHY link speed/duplex. - * @ndev: Pointer to the net_device structure - * - * This function is called to change the speed and duplex setting after - * auto negotiation is done by the PHY. This is the function that gets - * registered with the PHY interface through the "of_phy_connect" call. - */ -static void axienet_adjust_link(struct net_device *ndev) -{ - u32 emmc_reg; - u32 link_state; - u32 setspeed = 1; - struct axienet_local *lp = netdev_priv(ndev); - struct phy_device *phy = ndev->phydev; - - link_state = phy->speed | (phy->duplex << 1) | phy->link; - if (lp->last_link != link_state) { - if ((phy->speed == SPEED_10) || (phy->speed == SPEED_100)) { - if (lp->phy_mode == PHY_INTERFACE_MODE_1000BASEX) - setspeed = 0; - } else { - if ((phy->speed == SPEED_1000) && - (lp->phy_mode == PHY_INTERFACE_MODE_MII)) - setspeed = 0; - } - - if (setspeed == 1) { - emmc_reg = axienet_ior(lp, XAE_EMMC_OFFSET); - emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK; - - switch (phy->speed) { - case SPEED_1000: - emmc_reg |= XAE_EMMC_LINKSPD_1000; - break; - case SPEED_100: - emmc_reg |= XAE_EMMC_LINKSPD_100; - break; - case SPEED_10: - emmc_reg |= XAE_EMMC_LINKSPD_10; - break; - default: - dev_err(&ndev->dev, "Speed other than 10, 100 " - "or 1Gbps is not supported\n"); - break; - } - - axienet_iow(lp, XAE_EMMC_OFFSET, emmc_reg); - lp->last_link = link_state; - phy_print_status(phy); - } else { -
[PATCH net-next v5 07/20] net: axienet: Re-initialize MDIO registers properly after reset
The MDIO clock divisor register setting was only applied on the initial startup when the driver was loaded. However, this setting is cleared when the device is reset, such as would occur when the interface was taken down and brought up again, and so the MDIO bus would be non-functional afterwards. Split up the MDIO bus setup and enable into separate functions and re-enable the bus after a device reset, to ensure that the MDIO registers are set properly. This also allows us to remove direct access to MDIO registers in xilinx_axienet_main.c and centralize them all in xilinx_axienet_mdio.c. Also, lock the MDIO bus lock around the device reset process, to avoid MDIO accesses from occurring while the MDIO is disabled during the reset. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 38 +++- drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 54 +-- 3 files changed, 57 insertions(+), 38 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index f240ff1..4a135ed 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -505,8 +505,9 @@ static inline void axienet_iow(struct axienet_local *lp, off_t offset, } /* Function prototypes visible in xilinx_axienet_mdio.c for other files */ +int axienet_mdio_enable(struct axienet_local *lp); +void axienet_mdio_disable(struct axienet_local *lp); int axienet_mdio_setup(struct axienet_local *lp); -int axienet_mdio_wait_until_ready(struct axienet_local *lp); void axienet_mdio_teardown(struct axienet_local *lp); #endif /* XILINX_AXI_ENET_H */ diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 5cb39de..e735ca7 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -914,27 +914,23 @@ static irqreturn_t axienet_rx_irq(int irq, void *_ndev) */ static int axienet_open(struct net_device *ndev) { - int ret, mdio_mcreg; + int ret; struct axienet_local *lp = netdev_priv(ndev); struct phy_device *phydev = NULL; dev_dbg(&ndev->dev, "axienet_open()\n"); - mdio_mcreg = axienet_ior(lp, XAE_MDIO_MC_OFFSET); - ret = axienet_mdio_wait_until_ready(lp); - if (ret < 0) - return ret; /* Disable the MDIO interface till Axi Ethernet Reset is completed. * When we do an Axi Ethernet reset, it resets the complete core -* including the MDIO. If MDIO is not disabled when the reset -* process is started, MDIO will be broken afterwards. +* including the MDIO. MDIO must be disabled before resetting +* and re-enabled afterwards. +* Hold MDIO bus lock to avoid MDIO accesses during the reset. */ - axienet_iow(lp, XAE_MDIO_MC_OFFSET, - (mdio_mcreg & (~XAE_MDIO_MC_MDIOEN_MASK))); + mutex_lock(&lp->mii_bus->mdio_lock); + axienet_mdio_disable(lp); axienet_device_reset(ndev); - /* Enable the MDIO */ - axienet_iow(lp, XAE_MDIO_MC_OFFSET, mdio_mcreg); - ret = axienet_mdio_wait_until_ready(lp); + ret = axienet_mdio_enable(lp); + mutex_unlock(&lp->mii_bus->mdio_lock); if (ret < 0) return ret; @@ -1316,28 +1312,24 @@ static void axienet_dma_err_handler(unsigned long data) { u32 axienet_status; u32 cr, i; - int mdio_mcreg; struct axienet_local *lp = (struct axienet_local *) data; struct net_device *ndev = lp->ndev; struct axidma_bd *cur_p; axienet_setoptions(ndev, lp->options & ~(XAE_OPTION_TXEN | XAE_OPTION_RXEN)); - mdio_mcreg = axienet_ior(lp, XAE_MDIO_MC_OFFSET); - axienet_mdio_wait_until_ready(lp); /* Disable the MDIO interface till Axi Ethernet Reset is completed. * When we do an Axi Ethernet reset, it resets the complete core -* including the MDIO. So if MDIO is not disabled when the reset -* process is started, MDIO will be broken afterwards. +* including the MDIO. MDIO must be disabled before resetting +* and re-enabled afterwards. +* Hold MDIO bus lock to avoid MDIO accesses during the reset. */ - axienet_iow(lp, XAE_MDIO_MC_OFFSET, (mdio_mcreg & - ~XAE_MDIO_MC_MDIOEN_MASK)); - + mutex_lock(&lp->mii_bus->mdio_lock); + axienet_mdio_disable(lp); __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET); __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET); - - axienet_iow(lp, XAE_MDIO_MC_OFFSET, mdio_mcreg); - axienet_mdio_wait_until_ready(lp); + axienet_mdio_enable(lp); + mutex_
[PATCH net-next v5 12/20] net: axienet: Add optional support for Ethernet core interrupt
Previously this driver only handled interrupts from the DMA RX and TX blocks, not from the Ethernet core itself. Add optional support for the Ethernet core interrupt, which is used to detect rx_missed and framing errors signalled by the hardware. In order to use this interrupt, a third interrupt needs to be specified in the device tree. Signed-off-by: Robert Hancock --- .../devicetree/bindings/net/xilinx_axienet.txt | 3 +- drivers/net/ethernet/xilinx/xilinx_axienet.h | 1 + drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 49 ++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index 2dea903..0be335c 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -18,7 +18,8 @@ Required properties: - compatible : Must be one of "xlnx,axi-ethernet-1.00.a", "xlnx,axi-ethernet-1.01.a", "xlnx,axi-ethernet-2.01.a" - reg : Address and length of the IO space. -- interrupts : Should be a list of two interrupt, TX and RX. +- interrupts : Should be a list of 2 or 3 interrupts: TX DMA, RX DMA, + and optionally Ethernet core. - phy-handle : Should point to the external phy device. See ethernet.txt file in the same directory. - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the hardware diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 6b6d28f..8e605a8 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -435,6 +435,7 @@ struct axienet_local { int tx_irq; int rx_irq; + int eth_irq; phy_interface_t phy_mode; u32 options;/* Current options word */ diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index f733a7a..aa51a6e 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -502,6 +502,8 @@ static void axienet_device_reset(struct net_device *ndev) axienet_status = axienet_ior(lp, XAE_IP_OFFSET); if (axienet_status & XAE_INT_RXRJECT_MASK) axienet_iow(lp, XAE_IS_OFFSET, XAE_INT_RXRJECT_MASK); + axienet_iow(lp, XAE_IE_OFFSET, lp->eth_irq > 0 ? + XAE_INT_RECV_ERROR_MASK : 0); axienet_iow(lp, XAE_FCC_OFFSET, XAE_FCC_FCRX_MASK); @@ -902,6 +904,35 @@ static irqreturn_t axienet_rx_irq(int irq, void *_ndev) return IRQ_HANDLED; } +/** + * axienet_eth_irq - Ethernet core Isr. + * @irq: irq number + * @_ndev: net_device pointer + * + * Return: IRQ_HANDLED if device generated a core interrupt, IRQ_NONE otherwise. + * + * Handle miscellaneous conditions indicated by Ethernet core IRQ. + */ +static irqreturn_t axienet_eth_irq(int irq, void *_ndev) +{ + struct net_device *ndev = _ndev; + struct axienet_local *lp = netdev_priv(ndev); + unsigned int pending; + + pending = axienet_ior(lp, XAE_IP_OFFSET); + if (!pending) + return IRQ_NONE; + + if (pending & XAE_INT_RXFIFOOVR_MASK) + ndev->stats.rx_missed_errors++; + + if (pending & XAE_INT_RXRJECT_MASK) + ndev->stats.rx_frame_errors++; + + axienet_iow(lp, XAE_IS_OFFSET, pending); + return IRQ_HANDLED; +} + static void axienet_dma_err_handler(unsigned long data); /** @@ -962,9 +993,18 @@ static int axienet_open(struct net_device *ndev) ndev->name, ndev); if (ret) goto err_rx_irq; + /* Enable interrupts for Axi Ethernet core (if defined) */ + if (lp->eth_irq > 0) { + ret = request_irq(lp->eth_irq, axienet_eth_irq, IRQF_SHARED, + ndev->name, ndev); + if (ret) + goto err_eth_irq; + } return 0; +err_eth_irq: + free_irq(lp->rx_irq, ndev); err_rx_irq: free_irq(lp->tx_irq, ndev); err_tx_irq: @@ -1028,6 +1068,8 @@ static int axienet_stop(struct net_device *ndev) tasklet_kill(&lp->dma_err_tasklet); + if (lp->eth_irq > 0) + free_irq(lp->eth_irq, ndev); free_irq(lp->tx_irq, ndev); free_irq(lp->rx_irq, ndev); @@ -1491,6 +1533,8 @@ static void axienet_dma_err_handler(unsigned long data) axienet_status = axienet_ior(lp, XAE_IP_OFFSET); if (axienet_status & XAE_INT_RXRJECT_MASK) axienet_iow(lp, XAE_IS_OFFSET, XAE_INT_RXRJECT_MASK); + axienet_iow(lp, XAE_IE_OFFSET, lp->eth_irq > 0 ? + XAE_INT_RECV_ERROR_MA
[PATCH net-next v5 17/20] net: axienet: Fix MDIO bus parent node detection
This driver was previously using the parent node of the specified PHY node as the device node to register the MDIO bus on. Andrew Lunn pointed out this is wrong as the PHY node is potentially not even underneath the MDIO bus for the current device instance. Find the MDIO node explicitly by looking it up by name under the controller's device node instead. This could potentially break existing device trees if they don't use "mdio" as the name for the MDIO bus, but I did not find any with various searches and Xilinx's examples all use mdio as the name so it seems like this should be relatively safe. Signed-off-by: Robert Hancock Reviewed-by: Andrew Lunn --- drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c index 7106810..435ed30 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c @@ -228,7 +228,6 @@ int axienet_mdio_setup(struct axienet_local *lp) if (!bus) return -ENOMEM; - mdio_node = of_get_parent(lp->phy_node); snprintf(bus->id, MII_BUS_ID_SIZE, "axienet-%.8llx", (unsigned long long)lp->regs_start); @@ -239,7 +238,9 @@ int axienet_mdio_setup(struct axienet_local *lp) bus->parent = lp->dev; lp->mii_bus = bus; + mdio_node = of_get_child_by_name(lp->dev->of_node, "mdio"); ret = of_mdiobus_register(bus, mdio_node); + of_node_put(mdio_node); if (ret) { mdiobus_free(bus); lp->mii_bus = NULL; -- 1.8.3.1
[PATCH net-next v5 04/20] net: axienet: add X86 and ARM as supported platforms
This driver should now build on (at least) X86 and ARM platforms, so add them as supported platforms for the driver in Kconfig. Signed-off-by: Robert Hancock Reviewed-by: Andrew Lunn --- drivers/net/ethernet/xilinx/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig index af96e05..5f50764 100644 --- a/drivers/net/ethernet/xilinx/Kconfig +++ b/drivers/net/ethernet/xilinx/Kconfig @@ -6,7 +6,7 @@ config NET_VENDOR_XILINX bool "Xilinx devices" default y - depends on PPC || PPC32 || MICROBLAZE || ARCH_ZYNQ || MIPS || X86 || COMPILE_TEST + depends on PPC || PPC32 || MICROBLAZE || ARCH_ZYNQ || MIPS || X86 || ARM || COMPILE_TEST ---help--- If you have a network (Ethernet) card belonging to this class, say Y. @@ -26,7 +26,7 @@ config XILINX_EMACLITE config XILINX_AXI_EMAC tristate "Xilinx 10/100/1000 AXI Ethernet support" - depends on MICROBLAZE + depends on MICROBLAZE || X86 || ARM || COMPILE_TEST select PHYLIB ---help--- This driver supports the 10/100/1000 Ethernet from Xilinx for the -- 1.8.3.1
[PATCH net-next v5 11/20] net: axienet: Support shared interrupts
Specify IRQF_SHARED to support shared interrupts. If the interrupt handler is called and the device is not indicating an interrupt, just return IRQ_NONE rather than spewing error messages. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 4df424c..f733a7a 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -809,7 +809,7 @@ static void axienet_recv(struct net_device *ndev) * @irq: irq number * @_ndev: net_device pointer * - * Return: IRQ_HANDLED for all cases. + * Return: IRQ_HANDLED if device generated a TX interrupt, IRQ_NONE otherwise. * * This is the Axi DMA Tx done Isr. It invokes "axienet_start_xmit_done" * to complete the BD processing. @@ -828,7 +828,7 @@ static irqreturn_t axienet_tx_irq(int irq, void *_ndev) goto out; } if (!(status & XAXIDMA_IRQ_ALL_MASK)) - dev_err(&ndev->dev, "No interrupts asserted in Tx path\n"); + return IRQ_NONE; if (status & XAXIDMA_IRQ_ERROR_MASK) { dev_err(&ndev->dev, "DMA Tx error 0x%x\n", status); dev_err(&ndev->dev, "Current BD is at: 0x%x\n", @@ -858,7 +858,7 @@ static irqreturn_t axienet_tx_irq(int irq, void *_ndev) * @irq: irq number * @_ndev: net_device pointer * - * Return: IRQ_HANDLED for all cases. + * Return: IRQ_HANDLED if device generated a RX interrupt, IRQ_NONE otherwise. * * This is the Axi DMA Rx Isr. It invokes "axienet_recv" to complete the BD * processing. @@ -877,7 +877,7 @@ static irqreturn_t axienet_rx_irq(int irq, void *_ndev) goto out; } if (!(status & XAXIDMA_IRQ_ALL_MASK)) - dev_err(&ndev->dev, "No interrupts asserted in Rx path\n"); + return IRQ_NONE; if (status & XAXIDMA_IRQ_ERROR_MASK) { dev_err(&ndev->dev, "DMA Rx error 0x%x\n", status); dev_err(&ndev->dev, "Current BD is at: 0x%x\n", @@ -953,11 +953,13 @@ static int axienet_open(struct net_device *ndev) (unsigned long) lp); /* Enable interrupts for Axi DMA Tx */ - ret = request_irq(lp->tx_irq, axienet_tx_irq, 0, ndev->name, ndev); + ret = request_irq(lp->tx_irq, axienet_tx_irq, IRQF_SHARED, + ndev->name, ndev); if (ret) goto err_tx_irq; /* Enable interrupts for Axi DMA Rx */ - ret = request_irq(lp->rx_irq, axienet_rx_irq, 0, ndev->name, ndev); + ret = request_irq(lp->rx_irq, axienet_rx_irq, IRQF_SHARED, + ndev->name, ndev); if (ret) goto err_rx_irq; -- 1.8.3.1
[PATCH net-next v5 01/20] net: axienet: Fix casting of pointers to u32
This driver was casting skb pointers to u32 and storing them as such in the DMA buffer descriptor, which is obviously broken on 64-bit. The area of the buffer descriptor being used is not accessed by the hardware and has sufficient room for a 32 or 64-bit pointer, so just store the skb pointer as such. Signed-off-by: Robert Hancock Reviewed-by: Andrew Lunn --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 11 +++--- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 26 --- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 011adae..e09dc14 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -356,9 +356,6 @@ * @app2: MM2S/S2MM User Application Field 2. * @app3: MM2S/S2MM User Application Field 3. * @app4: MM2S/S2MM User Application Field 4. - * @sw_id_offset: MM2S/S2MM Sw ID - * @reserved5:Reserved and not used - * @reserved6:Reserved and not used */ struct axidma_bd { u32 next; /* Physical address of next buffer descriptor */ @@ -373,11 +370,9 @@ struct axidma_bd { u32 app1; /* TX start << 16 | insert */ u32 app2; /* TX csum seed */ u32 app3; - u32 app4; - u32 sw_id_offset; - u32 reserved5; - u32 reserved6; -}; + u32 app4; /* Last field used by HW */ + struct sk_buff *skb; +} __aligned(XAXIDMA_BD_MINIMUM_ALIGNMENT); /** * struct axienet_local - axienet private per device data diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 831967f..1bace60 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -159,8 +159,7 @@ static void axienet_dma_bd_release(struct net_device *ndev) for (i = 0; i < RX_BD_NUM; i++) { dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys, lp->max_frm_size, DMA_FROM_DEVICE); - dev_kfree_skb((struct sk_buff *) - (lp->rx_bd_v[i].sw_id_offset)); + dev_kfree_skb(lp->rx_bd_v[i].skb); } if (lp->rx_bd_v) { @@ -227,7 +226,7 @@ static int axienet_dma_bd_init(struct net_device *ndev) if (!skb) goto out; - lp->rx_bd_v[i].sw_id_offset = (u32) skb; + lp->rx_bd_v[i].skb = skb; lp->rx_bd_v[i].phys = dma_map_single(ndev->dev.parent, skb->data, lp->max_frm_size, @@ -595,14 +594,15 @@ static void axienet_start_xmit_done(struct net_device *ndev) dma_unmap_single(ndev->dev.parent, cur_p->phys, (cur_p->cntrl & XAXIDMA_BD_CTRL_LENGTH_MASK), DMA_TO_DEVICE); - if (cur_p->app4) - dev_consume_skb_irq((struct sk_buff *)cur_p->app4); + if (cur_p->skb) + dev_consume_skb_irq(cur_p->skb); /*cur_p->phys = 0;*/ cur_p->app0 = 0; cur_p->app1 = 0; cur_p->app2 = 0; cur_p->app4 = 0; cur_p->status = 0; + cur_p->skb = NULL; size += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; packets++; @@ -707,7 +707,7 @@ static inline int axienet_check_tx_bd_space(struct axienet_local *lp, } cur_p->cntrl |= XAXIDMA_BD_CTRL_TXEOF_MASK; - cur_p->app4 = (unsigned long)skb; + cur_p->skb = skb; tail_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * lp->tx_bd_tail; /* Start the transfer */ @@ -742,13 +742,15 @@ static void axienet_recv(struct net_device *ndev) while ((cur_p->status & XAXIDMA_BD_STS_COMPLETE_MASK)) { tail_p = lp->rx_bd_p + sizeof(*lp->rx_bd_v) * lp->rx_bd_ci; - skb = (struct sk_buff *) (cur_p->sw_id_offset); - length = cur_p->app4 & 0x; dma_unmap_single(ndev->dev.parent, cur_p->phys, lp->max_frm_size, DMA_FROM_DEVICE); + skb = cur_p->skb; + cur_p->skb = NULL; + length = cur_p->app4 & 0x; + skb_put(skb, length); skb->protocol = eth_type_trans(skb, ndev); /*skb_checksum_none_assert(skb);*/ @@ -783,7 +785,7 @@ static void axienet_recv(struct net_device *ndev)
[PATCH net-next v5 08/20] net: axienet: Cleanup DMA device reset and halt process
The Xilinx DMA blocks each have their own reset register, but they both reset the entire DMA engine, so only one of them needs to be reset. Also, when stopping the device, we need to not just command the DMA blocks to stop, but wait for them to stop, and trigger a device reset to ensure that they are completely stopped. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 2 + drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 54 +-- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 4a135ed..1ffb113 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -83,6 +83,8 @@ #define XAXIDMA_CR_RUNSTOP_MASK0x0001 /* Start/stop DMA channel */ #define XAXIDMA_CR_RESET_MASK 0x0004 /* Reset DMA engine */ +#define XAXIDMA_SR_HALT_MASK 0x0001 /* Indicates DMA channel halted */ + #define XAXIDMA_BD_NDESC_OFFSET0x00 /* Next descriptor pointer */ #define XAXIDMA_BD_BUFA_OFFSET 0x08 /* Buffer address */ #define XAXIDMA_BD_CTRL_LEN_OFFSET 0x18 /* Control/buffer length */ diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index e735ca7..bdc6e80 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -434,17 +434,20 @@ static void axienet_setoptions(struct net_device *ndev, u32 options) lp->options |= options; } -static void __axienet_device_reset(struct axienet_local *lp, off_t offset) +static void __axienet_device_reset(struct axienet_local *lp) { u32 timeout; /* Reset Axi DMA. This would reset Axi Ethernet core as well. The reset * process of Axi DMA takes a while to complete as all pending * commands/transfers will be flushed or completed during this * reset process. +* Note that even though both TX and RX have their own reset register, +* they both reset the entire DMA core, so only one needs to be used. */ - axienet_dma_out32(lp, offset, XAXIDMA_CR_RESET_MASK); + axienet_dma_out32(lp, XAXIDMA_TX_CR_OFFSET, XAXIDMA_CR_RESET_MASK); timeout = DELAY_OF_ONE_MILLISEC; - while (axienet_dma_in32(lp, offset) & XAXIDMA_CR_RESET_MASK) { + while (axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET) & + XAXIDMA_CR_RESET_MASK) { udelay(1); if (--timeout == 0) { netdev_err(lp->ndev, "%s: DMA reset timeout!\n", @@ -470,8 +473,7 @@ static void axienet_device_reset(struct net_device *ndev) u32 axienet_status; struct axienet_local *lp = netdev_priv(ndev); - __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET); - __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET); + __axienet_device_reset(lp); lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE; lp->options |= XAE_OPTION_VLAN; @@ -981,20 +983,45 @@ static int axienet_open(struct net_device *ndev) */ static int axienet_stop(struct net_device *ndev) { - u32 cr; + u32 cr, sr; + int count; struct axienet_local *lp = netdev_priv(ndev); dev_dbg(&ndev->dev, "axienet_close()\n"); - cr = axienet_dma_in32(lp, XAXIDMA_RX_CR_OFFSET); - axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, - cr & (~XAXIDMA_CR_RUNSTOP_MASK)); - cr = axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET); - axienet_dma_out32(lp, XAXIDMA_TX_CR_OFFSET, - cr & (~XAXIDMA_CR_RUNSTOP_MASK)); axienet_setoptions(ndev, lp->options & ~(XAE_OPTION_TXEN | XAE_OPTION_RXEN)); + cr = axienet_dma_in32(lp, XAXIDMA_RX_CR_OFFSET); + cr &= ~(XAXIDMA_CR_RUNSTOP_MASK | XAXIDMA_IRQ_ALL_MASK); + axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, cr); + + cr = axienet_dma_in32(lp, XAXIDMA_TX_CR_OFFSET); + cr &= ~(XAXIDMA_CR_RUNSTOP_MASK | XAXIDMA_IRQ_ALL_MASK); + axienet_dma_out32(lp, XAXIDMA_TX_CR_OFFSET, cr); + + axienet_iow(lp, XAE_IE_OFFSET, 0); + + /* Give DMAs a chance to halt gracefully */ + sr = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET); + for (count = 0; !(sr & XAXIDMA_SR_HALT_MASK) && count < 5; ++count) { + msleep(20); + sr = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET); + } + + sr = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET); + for (count = 0; !(sr & XAXIDMA_SR_HALT_MASK) && count < 5; ++count) { + msleep(20); + sr = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET); + } + + /* Do a reset to ensure DMA is really stopped */ + mute
[PATCH net-next v5 02/20] net: axienet: Use standard IO accessors
This driver was using in_be32 and out_be32 IO accessors which do not exist on most platforms. Also, the use of big-endian accessors does not seem correct as this hardware is accessed over an AXI bus which, to the extent it has an endian-ness, is little-endian. Switch to standard ioread32/iowrite32 accessors. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 4 ++-- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index e09dc14..d82e3b6 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -476,7 +476,7 @@ struct axienet_option { */ static inline u32 axienet_ior(struct axienet_local *lp, off_t offset) { - return in_be32(lp->regs + offset); + return ioread32(lp->regs + offset); } static inline u32 axinet_ior_read_mcr(struct axienet_local *lp) @@ -496,7 +496,7 @@ static inline u32 axinet_ior_read_mcr(struct axienet_local *lp) static inline void axienet_iow(struct axienet_local *lp, off_t offset, u32 value) { - out_be32((lp->regs + offset), value); + iowrite32(value, lp->regs + offset); } /* Function prototypes visible in xilinx_axienet_mdio.c for other files */ diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 1bace60..55beca1 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -125,7 +125,7 @@ */ static inline u32 axienet_dma_in32(struct axienet_local *lp, off_t reg) { - return in_be32(lp->dma_regs + reg); + return ioread32(lp->dma_regs + reg); } /** @@ -140,7 +140,7 @@ static inline u32 axienet_dma_in32(struct axienet_local *lp, off_t reg) static inline void axienet_dma_out32(struct axienet_local *lp, off_t reg, u32 value) { - out_be32((lp->dma_regs + reg), value); + iowrite32(value, lp->dma_regs + reg); } /** -- 1.8.3.1
[PATCH net-next v5 19/20] net: axienet: make use of axistream-connected attribute optional
Currently the axienet driver requires the use of a second devicetree node, referenced by an axistream-connected attribute on the Ethernet device node, which contains the resources for the AXI DMA block used by the device. This setup is problematic for a use case we have where the Ethernet and DMA cores are behind a PCIe to AXI bridge and the memory resources for the nodes are injected into the platform devices using the multifunction device subsystem - it's not easily possible for the driver to obtain the platform-level resources from the linked device. In order to simplify that usage model, and simplify the overall use of this driver in general, allow for all of the resources to be kept on one node where the resources are retrieved using platform device APIs rather than device-tree-specific ones. The previous usage setup is still supported if the axistream-connected attribute is specified. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 43 +++ 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index d138db8..898eabf 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -1580,7 +1580,7 @@ static int axienet_probe(struct platform_device *pdev) struct axienet_local *lp; struct net_device *ndev; const void *mac_addr; - struct resource *ethres, dmares; + struct resource *ethres; u32 value; ndev = alloc_etherdev(sizeof(*lp)); @@ -1698,28 +1698,41 @@ static int axienet_probe(struct platform_device *pdev) /* Find the DMA node, map the DMA registers, and decode the DMA IRQs */ np = of_parse_phandle(pdev->dev.of_node, "axistream-connected", 0); - if (!np) { - dev_err(&pdev->dev, "could not find DMA node\n"); - ret = -ENODEV; - goto free_netdev; - } - ret = of_address_to_resource(np, 0, &dmares); - if (ret) { - dev_err(&pdev->dev, "unable to get DMA resource\n"); + if (np) { + struct resource dmares; + + ret = of_address_to_resource(np, 0, &dmares); + if (ret) { + dev_err(&pdev->dev, + "unable to get DMA resource\n"); + of_node_put(np); + goto free_netdev; + } + lp->dma_regs = devm_ioremap_resource(&pdev->dev, +&dmares); + lp->rx_irq = irq_of_parse_and_map(np, 1); + lp->tx_irq = irq_of_parse_and_map(np, 0); of_node_put(np); - goto free_netdev; + lp->eth_irq = platform_get_irq(pdev, 0); + } else { + /* Check for these resources directly on the Ethernet node. */ + struct resource *res = platform_get_resource(pdev, +IORESOURCE_MEM, 1); + if (!res) { + dev_err(&pdev->dev, "unable to get DMA memory resource\n"); + goto free_netdev; + } + lp->dma_regs = devm_ioremap_resource(&pdev->dev, res); + lp->rx_irq = platform_get_irq(pdev, 1); + lp->tx_irq = platform_get_irq(pdev, 0); + lp->eth_irq = platform_get_irq(pdev, 2); } - lp->dma_regs = devm_ioremap_resource(&pdev->dev, &dmares); if (IS_ERR(lp->dma_regs)) { dev_err(&pdev->dev, "could not map DMA regs\n"); ret = PTR_ERR(lp->dma_regs); of_node_put(np); goto free_netdev; } - lp->rx_irq = irq_of_parse_and_map(np, 1); - lp->tx_irq = irq_of_parse_and_map(np, 0); - lp->eth_irq = irq_of_parse_and_map(np, 2); - of_node_put(np); if ((lp->rx_irq <= 0) || (lp->tx_irq <= 0)) { dev_err(&pdev->dev, "could not determine irqs\n"); ret = -ENOMEM; -- 1.8.3.1
[PATCH net-next v5 13/20] net: axienet: Fix race condition causing TX hang
It is possible that the interrupt handler fires and frees up space in the TX ring in between checking for sufficient TX ring space and stopping the TX queue in axienet_start_xmit. If this happens, the queue wake from the interrupt handler will occur before the queue is stopped, causing a lost wakeup and the adapter's transmit hanging. To avoid this, after stopping the queue, check again whether there is sufficient space in the TX ring. If so, wake up the queue again. Signed-off-by: Robert Hancock --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index aa51a6e..834fafd 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -622,6 +622,10 @@ static void axienet_start_xmit_done(struct net_device *ndev) ndev->stats.tx_packets += packets; ndev->stats.tx_bytes += size; + + /* Matches barrier in axienet_start_xmit */ + smp_mb(); + netif_wake_queue(ndev); } @@ -677,9 +681,19 @@ static inline int axienet_check_tx_bd_space(struct axienet_local *lp, cur_p = &lp->tx_bd_v[lp->tx_bd_tail]; if (axienet_check_tx_bd_space(lp, num_frag)) { - if (!netif_queue_stopped(ndev)) - netif_stop_queue(ndev); - return NETDEV_TX_BUSY; + if (netif_queue_stopped(ndev)) + return NETDEV_TX_BUSY; + + netif_stop_queue(ndev); + + /* Matches barrier in axienet_start_xmit_done */ + smp_mb(); + + /* Space might have just been freed - check again */ + if (axienet_check_tx_bd_space(lp, num_frag)) + return NETDEV_TX_BUSY; + + netif_wake_queue(ndev); } if (skb->ip_summed == CHECKSUM_PARTIAL) { -- 1.8.3.1