| Message ID | 20210311201117.3802311-3-robert.hancock@calian.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | axienet clock additions | expand |
| Context | Check | Description |
|---|---|---|
| netdev/apply | fail | Patch does not apply to net-next |
| netdev/tree_selection | success | Clearly marked for net-next |
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 <robert.hancock@calian.com> > Note that this patch is dependent on "net: axienet: Fix probe error cleanup" ( https://patchwork.kernel.org/project/netdevbpf/patch/20210311200518.3801916-1-robert.hancock@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: > - 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/ethernet/xilinx/xilinx_axienet_mdio.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c > @@ -159,8 +159,8 @@ int axienet_mdio_enable(struct axienet_local *lp) > > lp->mii_clk_div = 0; > > - if (lp->clk) { > - host_clock = clk_get_rate(lp->clk); > + if (lp->axi_clk) { > + host_clock = clk_get_rate(lp->axi_clk); > } else { > struct device_node *np1; >
Thanks for the patch. > -----Original Message----- > From: Robert Hancock <robert.hancock@calian.com> > Sent: Friday, March 12, 2021 5:21 AM > To: davem@davemloft.net; kuba@kernel.org; Radhey Shyam Pandey > <radheys@xilinx.com> > Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org > Subject: 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 <robert.hancock@calian.com> > > > > Note that this patch is dependent on "net: axienet: Fix probe error cleanup" ( > https://patchwork.kernel.org/project/netdevbpf/patch/20210311200518.38 > 01916-1-robert.hancock@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 Minor nit - Better to add " AXI4-Lite bus clock" > > + * @misc_clks: Other device clocks We can mention : Misc ethernet clocks (AXI4-Stream, Ref, MGT clocks) Apart from it - rest looks fine. Please feel free to add: Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> > > * @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/ethernet/xilinx/xilinx_axienet_mdio.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c > > @@ -159,8 +159,8 @@ int axienet_mdio_enable(struct axienet_local *lp) > > > > lp->mii_clk_div = 0; > > > > - if (lp->clk) { > > - host_clock = clk_get_rate(lp->clk); > > + if (lp->axi_clk) { > > + host_clock = clk_get_rate(lp->axi_clk); > > } else { > > struct device_node *np1; > > > -- > Robert Hancock > Senior Hardware Designer, Calian Advanced Technologies > www.calian.com
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/ethernet/xilinx/xilinx_axienet_mdio.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c @@ -159,8 +159,8 @@ int axienet_mdio_enable(struct axienet_local *lp) lp->mii_clk_div = 0; - if (lp->clk) { - host_clock = clk_get_rate(lp->clk); + if (lp->axi_clk) { + host_clock = clk_get_rate(lp->axi_clk); } else { struct device_node *np1;
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 <robert.hancock@calian.com> --- 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(-)