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

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@xxxxxxxxxxxx>
> Sent: Tuesday, October 8, 2024 9:12 AM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> Simek, Michal <michal.simek@xxxxxxx>; Katakam, Harini
> <harini.katakam@xxxxxxx>; netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; git (AMD-Xilinx)
> <git@xxxxxxx>; Joseph, Abin <Abin.Joseph@xxxxxxx>
> 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@xxxxxxx> wrote:
> >
> > From: Abin Joseph <abin.joseph@xxxxxxx>
> >
> > 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@xxxxxxx>
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
> > ---
> > 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