netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] axienet clock additions
@ 2021-03-11 20:11 Robert Hancock
  2021-03-11 20:11 ` [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks Robert Hancock
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Robert Hancock @ 2021-03-11 20:11 UTC (permalink / raw)
  To: radhey.shyam.pandey, davem, kuba; +Cc: netdev, devicetree, Robert Hancock

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
  2021-03-11 20:11 [PATCH net-next 0/2] axienet clock additions Robert Hancock
@ 2021-03-11 20:11 ` Robert Hancock
  2021-03-12  7:57   ` Radhey Shyam Pandey
  2021-03-11 20:11 ` [PATCH net-next 2/2] net: axienet: Enable more clocks Robert Hancock
  2021-03-14 21:27 ` [PATCH net-next 0/2] axienet clock additions David Miller
  2 siblings, 1 reply; 8+ messages in thread
From: Robert Hancock @ 2021-03-11 20:11 UTC (permalink / raw)
  To: radhey.shyam.pandey, davem, kuba; +Cc: netdev, devicetree, Robert Hancock

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 <robert.hancock@calian.com>
---
 .../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 = <&microblaze_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 = <0x40c00000 0x40000 0x50c00000 0x40000>;
 		xlnx,rxcsum = <0x2>;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/2] net: axienet: Enable more clocks
  2021-03-11 20:11 [PATCH net-next 0/2] axienet clock additions Robert Hancock
  2021-03-11 20:11 ` [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks Robert Hancock
@ 2021-03-11 20:11 ` Robert Hancock
  2021-03-11 23:50   ` Robert Hancock
  2021-03-14 21:27 ` [PATCH net-next 0/2] axienet clock additions David Miller
  2 siblings, 1 reply; 8+ messages in thread
From: Robert Hancock @ 2021-03-11 20:11 UTC (permalink / raw)
  To: radhey.shyam.pandey, davem, kuba; +Cc: netdev, devicetree, Robert Hancock

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(-)

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;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/2] net: axienet: Enable more clocks
  2021-03-11 20:11 ` [PATCH net-next 2/2] net: axienet: Enable more clocks Robert Hancock
@ 2021-03-11 23:50   ` Robert Hancock
  2021-03-12  8:53     ` Radhey Shyam Pandey
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Hancock @ 2021-03-11 23:50 UTC (permalink / raw)
  To: davem, kuba, radhey.shyam.pandey; +Cc: netdev, devicetree

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;
>  
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks
  2021-03-11 20:11 ` [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks Robert Hancock
@ 2021-03-12  7:57   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 8+ messages in thread
From: Radhey Shyam Pandey @ 2021-03-12  7:57 UTC (permalink / raw)
  To: Robert Hancock, davem, kuba; +Cc: netdev, devicetree, Anirudha Sarangi

Thanks for the patch.

> -----Original Message-----
> From: Robert Hancock <robert.hancock@calian.com>
> Sent: Friday, March 12, 2021 1:41 AM
> To: Radhey Shyam Pandey <radheys@xilinx.com>; davem@davemloft.net;
> kuba@kernel.org
> Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; Robert Hancock
> <robert.hancock@calian.com>
> Subject: [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 <robert.hancock@calian.com>
> ---
>  .../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

Based on PG138 , axis_clk is AXI4-Stream clock for TXD RXD TXC and RXS interfaces.
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 it in separate
patch. Please refer to xilinx tree.

> +		  ref_clk: Ethernet reference clock

Better to add some more description -  used by signal delay primitives and transceivers.
> +		  mgt_clk: MGT reference clock (used by internal PCS/PMA
> PHY)
(used by optional internal PCS/PMA)
> +
> +		  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 = <&microblaze_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 = <0x40c00000 0x40000 0x50c00000 0x40000>;
>  		xlnx,rxcsum = <0x2>;
> --
> 2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH net-next 2/2] net: axienet: Enable more clocks
  2021-03-11 23:50   ` Robert Hancock
@ 2021-03-12  8:53     ` Radhey Shyam Pandey
  0 siblings, 0 replies; 8+ messages in thread
From: Radhey Shyam Pandey @ 2021-03-12  8:53 UTC (permalink / raw)
  To: Robert Hancock, davem, kuba; +Cc: netdev, devicetree, Anirudha Sarangi

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] axienet clock additions
  2021-03-11 20:11 [PATCH net-next 0/2] axienet clock additions Robert Hancock
  2021-03-11 20:11 ` [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks Robert Hancock
  2021-03-11 20:11 ` [PATCH net-next 2/2] net: axienet: Enable more clocks Robert Hancock
@ 2021-03-14 21:27 ` David Miller
  2021-03-14 23:25   ` Robert Hancock
  2 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2021-03-14 21:27 UTC (permalink / raw)
  To: robert.hancock; +Cc: radhey.shyam.pandey, kuba, netdev, devicetree

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/2] axienet clock additions
  2021-03-14 21:27 ` [PATCH net-next 0/2] axienet clock additions David Miller
@ 2021-03-14 23:25   ` Robert Hancock
  0 siblings, 0 replies; 8+ messages in thread
From: Robert Hancock @ 2021-03-14 23:25 UTC (permalink / raw)
  To: davem; +Cc: kuba, netdev, radhey.shyam.pandey, devicetree

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?

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-14 23:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 20:11 [PATCH net-next 0/2] axienet clock additions Robert Hancock
2021-03-11 20:11 ` [PATCH net-next 1/2] dt-bindings: net: xilinx_axienet: Document additional clocks Robert Hancock
2021-03-12  7:57   ` Radhey Shyam Pandey
2021-03-11 20:11 ` [PATCH net-next 2/2] net: axienet: Enable more clocks Robert Hancock
2021-03-11 23:50   ` Robert Hancock
2021-03-12  8:53     ` Radhey Shyam Pandey
2021-03-14 21:27 ` [PATCH net-next 0/2] axienet clock additions David Miller
2021-03-14 23:25   ` Robert Hancock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).