| Message ID | 1659123350-10638-3-git-send-email-radhey.shyam.pandey@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | macb: add zynqmp SGMII dynamic configuration support | expand |
On 29/07/2022 20:35, Radhey Shyam Pandey wrote: > 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) Do you have cc suppression turned on or did this not get picked up b/c of the (for MPFS) you added? In the future, please CC me on later revisions if I provided a tested-by :) Thanks, Conor.
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. 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. > + 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 >
> -----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 > >
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); + 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) {