| Message ID | 20210311201117.3802311-1-robert.hancock@calian.com |
|---|---|
| Headers | show |
| Series | axienet clock additions | expand |
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
From: Robert Hancock <robert.hancock@calian.com> 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.
On Sun, 2021-03-14 at 14:27 -0700, David Miller wrote: > From: Robert Hancock <robert.hancock@calian.com> > 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?