lkml.org 
[lkml]   [2022]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 net-next 2/2] net: macb: Add zynqmp SGMII dynamic configuration support
Date
> -----Original Message-----
> From: Claudiu.Beznea@microchip.com <Claudiu.Beznea@microchip.com>
> Sent: Monday, August 1, 2022 5:06 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> michal.simek@xilinx.com; Nicolas.Ferre@microchip.com;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; gregkh@linuxfoundation.org
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>; git@xilinx.com
> Subject: Re: [PATCH v2 net-next 2/2] net: macb: Add zynqmp SGMII dynamic
> configuration support
>
> On 29.07.2022 22:35, Radhey Shyam Pandey wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> > Add support for the dynamic configuration which takes care of
> > configuring the GEM secure space configuration registers using EEMI
> > APIs. High level sequence is to:
> > - Check for the PM dynamic configuration support, if no error proceed with
> > GEM dynamic configurations(next steps) otherwise skip the dynamic
> > configuration.
> > - Configure GEM Fixed configurations.
> > - Configure GEM_CLK_CTRL (gemX_sgmii_mode).
> > - Trigger GEM reset.
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > Tested-by: Conor Dooley <conor.dooley@microchip.com> (for MPFS)
> > ---
> > Changes for v2:
> > - Add phy_exit() in error return paths.
> > ---
> > drivers/net/ethernet/cadence/macb_main.c | 25
> > +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c
> > b/drivers/net/ethernet/cadence/macb_main.c
> > index 4cd4f57ca2aa..517b40ff098b 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -38,6 +38,7 @@
> > #include <linux/pm_runtime.h>
> > #include <linux/ptp_classify.h>
> > #include <linux/reset.h>
> > +#include <linux/firmware/xlnx-zynqmp.h>
> > #include "macb.h"
> >
> > /* This structure is only used for MACB on SiFive FU540 devices */ @@
> > -4621,6 +4622,30 @@ static int init_reset_optional(struct platform_device
> *pdev)
> > "failed to init SGMII PHY\n");
> > }
> >
> > + ret = zynqmp_pm_is_function_supported(PM_IOCTL,
> IOCTL_SET_GEM_CONFIG);
> > + if (!ret) {
> > + u32 pm_info[2];
> > +
> > + ret = of_property_read_u32_array(pdev->dev.of_node, "power-
> domains",
> > + pm_info, ARRAY_SIZE(pm_info));
> > + if (ret < 0) {
> > + phy_exit(bp->sgmii_phy);
>
> Could you move this to a single exit point and jump in there with goto?
> Same for the rest of occurencies.

Ok, will make to use of common exit path and send out a new version.

>
> Also, I notice just now that phy_init() is done only if bp->phy_interface ==
> PHY_INTERFACE_MODE_SGMII, phy_exit() should be handled only if this is
> true, too.

If phy interface is not SGMII bp->sgmii_phy would be NULL and calling
phy_exit should still be fine as these phy APIs has already a NULL check.

>
> > + dev_err(&pdev->dev, "Failed to read power management
> information\n");
> > + return ret;
> > + }
> > + ret = zynqmp_pm_set_gem_config(pm_info[1],
> GEM_CONFIG_FIXED, 0);
> > + if (ret < 0) {
> > + phy_exit(bp->sgmii_phy);
> > + return ret;
> > + }
> > +
> > + ret = zynqmp_pm_set_gem_config(pm_info[1],
> GEM_CONFIG_SGMII_MODE, 1);
> > + if (ret < 0) {
> > + phy_exit(bp->sgmii_phy);
> > + return ret;
> > + }
> > + }
> > +
> > /* Fully reset controller at hardware level if mapped in device tree */
> > ret = device_reset_optional(&pdev->dev);
> > if (ret) {
> > --
> > 2.1.1
> >

\
 
 \ /
  Last update: 2022-08-01 15:03    [W:0.068 / U:0.996 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site