[PATCH net-next v2 0/3] net: xilinx: emaclite: Adopt clock support

[PATCH net-next v2 0/3] net: xilinx: emaclite: Adopt clock support

[Radhey Shyam Pandey]

This patchset adds emaclite clock support. AXI Ethernet Lite IP can also
be used on SoC platforms like Zynq UltraScale+ MPSoC which combines
powerful processing system (PS) and user-programmable logic (PL) into
the same device. On these platforms it is mandatory to explicitly enable
IP clocks for proper functionality.

Changes for v2:
- Make clocks as required property.


Abin Joseph (3):
  dt-bindings: net: emaclite: Add clock support
  net: emaclite: Replace alloc_etherdev() with devm_alloc_etherdev()
  net: emaclite: Adopt clock support

 .../bindings/net/xlnx,emaclite.yaml           |  5 +++++
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 22 ++++++++++---------
 2 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH net-next v2 1/3] dt-bindings: net: emaclite: Add clock support

[Radhey Shyam Pandey]

From: Abin Joseph <abin.joseph@amd.com>

Add s_axi_aclk AXI4 clock support. Traditionally this IP was used on
microblaze platforms which had fixed clocks enabled all the time. But
since its a PL IP, it can also be used on SoC platforms like Zynq
UltraScale+ MPSoC which combines processing system (PS) and user
programmable logic (PL) into the same device. On these platforms instead
of fixed enabled clocks it is mandatory to explicitly enable IP clocks
for proper functionality.

So make clock a required property and also define max supported clock
constraints.

Signed-off-by: Abin Joseph <abin.joseph@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
Changes for v2:
- Describe clocks as required property.
---
 Documentation/devicetree/bindings/net/xlnx,emaclite.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
index 92d8ade988f6..e16384aff557 100644
--- a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
+++ b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
@@ -29,6 +29,9 @@ properties:
   interrupts:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
   phy-handle: true
 
   local-mac-address: true
@@ -45,6 +48,7 @@ required:
   - compatible
   - reg
   - interrupts
+  - clocks
   - phy-handle
 
 additionalProperties: false
@@ -56,6 +60,7 @@ examples:
         reg = <0x40e00000 0x10000>;
         interrupt-parent = <&axi_intc_1>;
         interrupts = <1>;
+        clocks = <&dummy>;
         local-mac-address = [00 00 00 00 00 00];
         phy-handle = <&phy0>;
         xlnx,rx-ping-pong;
-- 
2.34.1

Re: [PATCH net-next v2 1/3] dt-bindings: net: emaclite: Add clock support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

On Mon, Oct 07, 2024 at 08:36:01PM +0530, Radhey Shyam Pandey wrote:
> From: Abin Joseph <abin.joseph@amd.com>
> 
> Add s_axi_aclk AXI4 clock support. Traditionally this IP was used on
> microblaze platforms which had fixed clocks enabled all the time. But
> since its a PL IP, it can also be used on SoC platforms like Zynq
> UltraScale+ MPSoC which combines processing system (PS) and user
> programmable logic (PL) into the same device. On these platforms instead
> of fixed enabled clocks it is mandatory to explicitly enable IP clocks
> for proper functionality.
> 
> So make clock a required property and also define max supported clock
> constraints.
> 
> Signed-off-by: Abin Joseph <abin.joseph@amd.com>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH net-next v2 2/3] net: emaclite: Replace alloc_etherdev() with devm_alloc_etherdev()

[Radhey Shyam Pandey]

From: Abin Joseph <abin.joseph@amd.com>

Use device managed ethernet device allocation to simplify the error
handling logic. No functional change.

Signed-off-by: Abin Joseph <abin.joseph@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
changes for v2:
- None.
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 2eb7f23538a6..418587942527 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -1097,7 +1097,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 	dev_info(dev, "Device Tree Probing\n");
 
 	/* Create an ethernet device instance */
-	ndev = alloc_etherdev(sizeof(struct net_local));
+	ndev = devm_alloc_etherdev(dev, sizeof(struct net_local));
 	if (!ndev)
 		return -ENOMEM;
 
@@ -1110,15 +1110,13 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 	/* Get IRQ for the device */
 	rc = platform_get_irq(ofdev, 0);
 	if (rc < 0)
-		goto error;
+		return rc;
 
 	ndev->irq = rc;
 
 	lp->base_addr = devm_platform_get_and_ioremap_resource(ofdev, 0, &res);
-	if (IS_ERR(lp->base_addr)) {
-		rc = PTR_ERR(lp->base_addr);
-		goto error;
-	}
+	if (IS_ERR(lp->base_addr))
+		return PTR_ERR(lp->base_addr);
 
 	ndev->mem_start = res->start;
 	ndev->mem_end = res->end;
@@ -1167,8 +1165,6 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 
 put_node:
 	of_node_put(lp->phy_node);
-error:
-	free_netdev(ndev);
 	return rc;
 }
 
@@ -1197,8 +1193,6 @@ static void xemaclite_of_remove(struct platform_device *of_dev)
 
 	of_node_put(lp->phy_node);
 	lp->phy_node = NULL;
-
-	free_netdev(ndev);
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-- 
2.34.1

[PATCH net-next v2 3/3] net: emaclite: Adopt clock support

[Radhey Shyam Pandey]

From: Abin Joseph <abin.joseph@amd.com>

Adapt to use the clock framework. Add s_axi_aclk clock from the processor
bus clock domain and make clk optional to keep DTB backward compatibility.

Signed-off-by: Abin Joseph <abin.joseph@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
changes for v2:
- None.
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 418587942527..fe901af5ddfa 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -7,6 +7,7 @@
  * Copyright (c) 2007 - 2013 Xilinx, Inc.
  */
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/uaccess.h>
@@ -1091,6 +1092,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 	struct net_device *ndev = NULL;
 	struct net_local *lp = NULL;
 	struct device *dev = &ofdev->dev;
+	struct clk *clkin;
 
 	int rc = 0;
 
@@ -1127,6 +1129,12 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 	lp->tx_ping_pong = get_bool(ofdev, "xlnx,tx-ping-pong");
 	lp->rx_ping_pong = get_bool(ofdev, "xlnx,rx-ping-pong");
 
+	clkin = devm_clk_get_optional_enabled(&ofdev->dev, NULL);
+	if (IS_ERR(clkin)) {
+		return dev_err_probe(&ofdev->dev, PTR_ERR(clkin),
+				"Failed to get and enable clock from Device Tree\n");
+	}
+
 	rc = of_get_ethdev_address(ofdev->dev.of_node, ndev);
 	if (rc) {
 		dev_warn(dev, "No MAC address found, using random\n");
-- 
2.34.1

Re: [PATCH net-next v2 3/3] net: emaclite: Adopt clock support

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On Mon, Oct 7, 2024 at 8:39 PM Radhey Shyam Pandey
<radhey.shyam.pandey@amd.com> wrote:
>
> From: Abin Joseph <abin.joseph@amd.com>
>
> Adapt to use the clock framework. Add s_axi_aclk clock from the processor
> bus clock domain and make clk optional to keep DTB backward compatibility.
>
> Signed-off-by: Abin Joseph <abin.joseph@amd.com>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> changes for v2:
> - None.
> ---
>  drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> index 418587942527..fe901af5ddfa 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> @@ -7,6 +7,7 @@
>   * Copyright (c) 2007 - 2013 Xilinx, Inc.
>   */
>
> +#include <linux/clk.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/uaccess.h>
> @@ -1091,6 +1092,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
>         struct net_device *ndev = NULL;
>         struct net_local *lp = NULL;
>         struct device *dev = &ofdev->dev;
> +       struct clk *clkin;
>
>         int rc = 0;
>
> @@ -1127,6 +1129,12 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
>         lp->tx_ping_pong = get_bool(ofdev, "xlnx,tx-ping-pong");
>         lp->rx_ping_pong = get_bool(ofdev, "xlnx,rx-ping-pong");
>
> +       clkin = devm_clk_get_optional_enabled(&ofdev->dev, NULL);
> +       if (IS_ERR(clkin)) {
> +               return dev_err_probe(&ofdev->dev, PTR_ERR(clkin),
> +                               "Failed to get and enable clock from Device Tree\n");
> +       }
[Kalesh] Braces are not needed here for a single statement block.

Also, I do not see where you use this "clkin" in this driver. I may be
missing something as I am not an expert in this area.
> +
>         rc = of_get_ethdev_address(ofdev->dev.of_node, ndev);
>         if (rc) {
>                 dev_warn(dev, "No MAC address found, using random\n");
> --
> 2.34.1
>
>


-- 
Regards,
Kalesh A P

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

RE: [PATCH net-next v2 3/3] net: emaclite: Adopt clock support

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> Sent: Tuesday, October 8, 2024 9:12 AM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> Simek, Michal <michal.simek@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>; netdev@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; git (AMD-Xilinx)
> <git@amd.com>; Joseph, Abin <Abin.Joseph@amd.com>
> Subject: Re: [PATCH net-next v2 3/3] net: emaclite: Adopt clock support
> 
> On Mon, Oct 7, 2024 at 8:39 PM Radhey Shyam Pandey
> <radhey.shyam.pandey@amd.com> wrote:
> >
> > From: Abin Joseph <abin.joseph@amd.com>
> >
> > Adapt to use the clock framework. Add s_axi_aclk clock from the processor
> > bus clock domain and make clk optional to keep DTB backward compatibility.
> >
> > Signed-off-by: Abin Joseph <abin.joseph@amd.com>
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> > ---
> > changes for v2:
> > - None.
> > ---
> >  drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > index 418587942527..fe901af5ddfa 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > @@ -7,6 +7,7 @@
> >   * Copyright (c) 2007 - 2013 Xilinx, Inc.
> >   */
> >
> > +#include <linux/clk.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/uaccess.h>
> > @@ -1091,6 +1092,7 @@ static int xemaclite_of_probe(struct platform_device
> *ofdev)
> >         struct net_device *ndev = NULL;
> >         struct net_local *lp = NULL;
> >         struct device *dev = &ofdev->dev;
> > +       struct clk *clkin;
> >
> >         int rc = 0;
> >
> > @@ -1127,6 +1129,12 @@ static int xemaclite_of_probe(struct platform_device
> *ofdev)
> >         lp->tx_ping_pong = get_bool(ofdev, "xlnx,tx-ping-pong");
> >         lp->rx_ping_pong = get_bool(ofdev, "xlnx,rx-ping-pong");
> >
> > +       clkin = devm_clk_get_optional_enabled(&ofdev->dev, NULL);
> > +       if (IS_ERR(clkin)) {
> > +               return dev_err_probe(&ofdev->dev, PTR_ERR(clkin),
> > +                               "Failed to get and enable clock from Device Tree\n");
> > +       }
> [Kalesh] Braces are not needed here for a single statement block.

Yes, braces not required. Will fix that in next version.

> 
> Also, I do not see where you use this "clkin" in this driver. I may be
> missing something as I am not an expert in this area.

devm_clk_get_optional_enabled() -> returned clk (if valid) is prepared 
and enabled. 

When the device is unbound from the bus the clock will 
automatically be disabled, unprepared and freed.

So clkin is not used later on.

> > +
> >         rc = of_get_ethdev_address(ofdev->dev.of_node, ndev);
> >         if (rc) {
> >                 dev_warn(dev, "No MAC address found, using random\n");
> > --
> > 2.34.1
> >
> >
> 
> 
> --
> Regards,
> Kalesh A P