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 <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Tested-by: Conor Dooley <[email protected]> (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);
+ 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
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 <[email protected]>
> Reviewed-by: Andrew Lunn <[email protected]>
> Tested-by: Conor Dooley <[email protected]> (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 <[email protected]>
> Reviewed-by: Andrew Lunn <[email protected]>
> Tested-by: Conor Dooley <[email protected]> (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: [email protected] <[email protected]>
> Sent: Monday, August 1, 2022 5:06 PM
> To: Pandey, Radhey Shyam <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; git (AMD-Xilinx) <[email protected]>; [email protected]
> 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 <[email protected]>
> > Reviewed-by: Andrew Lunn <[email protected]>
> > Tested-by: Conor Dooley <[email protected]> (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
> >