* [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD
2022-08-26 19:13 [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Lucas Stach
@ 2022-08-26 19:13 ` Lucas Stach
2022-08-29 1:56 ` Peng Fan
2022-12-31 5:41 ` Shawn Guo
2022-08-26 19:13 ` [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes Lucas Stach
` (3 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Lucas Stach @ 2022-08-26 19:13 UTC (permalink / raw)
To: Shawn Guo
Cc: Fabio Estevam, NXP Linux Team, Peng Fan, Kieran Bingham,
linux-arm-kernel, patchwork-lst, kernel
NXP internal information shows that the PHY refclk is gated by the
GLOBAL_TX_PIX_CLK_EN bit, so to allow the PHY PLL to lock without the
LCDIF being already active, tie this bit to the HDMI_TX_PHY power
domain.
Fixes: e3442022f543 ("soc: imx: add i.MX8MP HDMI blk-ctrl")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/soc/imx/imx8mp-blk-ctrl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index 9401df25c85a..73f672104aaa 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -192,7 +192,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc,
break;
case IMX8MP_HDMIBLK_PD_LCDIF:
regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
- BIT(7) | BIT(16) | BIT(17) | BIT(18) |
+ BIT(16) | BIT(17) | BIT(18) |
BIT(19) | BIT(20));
regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(11));
regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0,
@@ -221,6 +221,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct imx8mp_blk_ctrl *bc,
regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(1));
break;
case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
+ regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0, BIT(7));
regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(22) | BIT(24));
regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0, BIT(12));
regmap_clear_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(3));
@@ -243,7 +244,7 @@ static void imx8mp_hdmi_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
BIT(4) | BIT(5) | BIT(6));
regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(11));
regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
- BIT(7) | BIT(16) | BIT(17) | BIT(18) |
+ BIT(16) | BIT(17) | BIT(18) |
BIT(19) | BIT(20));
break;
case IMX8MP_HDMIBLK_PD_PAI:
@@ -271,6 +272,7 @@ static void imx8mp_hdmi_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(3));
regmap_clear_bits(bc->regmap, HDMI_RTX_RESET_CTL0, BIT(12));
+ regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0, BIT(7));
regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(22) | BIT(24));
break;
default:
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD
2022-08-26 19:13 ` [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD Lucas Stach
@ 2022-08-29 1:56 ` Peng Fan
2022-12-15 15:09 ` Lucas Stach
2022-12-31 5:41 ` Shawn Guo
1 sibling, 1 reply; 11+ messages in thread
From: Peng Fan @ 2022-08-29 1:56 UTC (permalink / raw)
To: Lucas Stach, Shawn Guo, Sandor Yu
Cc: Fabio Estevam, dl-linux-imx, Kieran Bingham,
linux-arm-kernel@lists.infradead.org,
patchwork-lst@pengutronix.de, kernel@pengutronix.de
+ Sandor
> Subject: [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with
> HDMI_TX_PHY PD
>
> NXP internal information shows that the PHY refclk is gated by the
> GLOBAL_TX_PIX_CLK_EN bit, so to allow the PHY PLL to lock without the
> LCDIF being already active, tie this bit to the HDMI_TX_PHY power domain.
>
> Fixes: e3442022f543 ("soc: imx: add i.MX8MP HDMI blk-ctrl")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/soc/imx/imx8mp-blk-ctrl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-
> blk-ctrl.c
> index 9401df25c85a..73f672104aaa 100644
> --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -192,7 +192,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct
> imx8mp_blk_ctrl *bc,
> break;
> case IMX8MP_HDMIBLK_PD_LCDIF:
> regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> - BIT(7) | BIT(16) | BIT(17) | BIT(18) |
> + BIT(16) | BIT(17) | BIT(18) |
> BIT(19) | BIT(20));
> regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> BIT(11));
> regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0, @@
> -221,6 +221,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct
> imx8mp_blk_ctrl *bc,
> regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(1));
> break;
> case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
> + regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0, BIT(7));
> regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(22)
> | BIT(24));
> regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0,
> BIT(12));
> regmap_clear_bits(bc->regmap, HDMI_TX_CONTROL0,
> BIT(3)); @@ -243,7 +244,7 @@ static void
> imx8mp_hdmi_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
> BIT(4) | BIT(5) | BIT(6));
> regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> BIT(11));
> regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> - BIT(7) | BIT(16) | BIT(17) | BIT(18) |
> + BIT(16) | BIT(17) | BIT(18) |
> BIT(19) | BIT(20));
> break;
> case IMX8MP_HDMIBLK_PD_PAI:
> @@ -271,6 +272,7 @@ static void imx8mp_hdmi_blk_ctrl_power_off(struct
> imx8mp_blk_ctrl *bc,
> case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
> regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(3));
> regmap_clear_bits(bc->regmap, HDMI_RTX_RESET_CTL0,
> BIT(12));
> + regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> BIT(7));
> regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> BIT(22) | BIT(24));
> break;
> default:
> --
> 2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD
2022-08-29 1:56 ` Peng Fan
@ 2022-12-15 15:09 ` Lucas Stach
0 siblings, 0 replies; 11+ messages in thread
From: Lucas Stach @ 2022-12-15 15:09 UTC (permalink / raw)
To: Peng Fan, Shawn Guo, Sandor Yu
Cc: Fabio Estevam, dl-linux-imx, Kieran Bingham,
linux-arm-kernel@lists.infradead.org,
patchwork-lst@pengutronix.de, kernel@pengutronix.de
Hi Shawn, Peng,
Am Montag, dem 29.08.2022 um 01:56 +0000 schrieb Peng Fan:
> + Sandor
>
Could you please pull this patch in? It's based on information I got
from Sandor.
Regards,
Lucas
> > Subject: [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with
> > HDMI_TX_PHY PD
> >
> > NXP internal information shows that the PHY refclk is gated by the
> > GLOBAL_TX_PIX_CLK_EN bit, so to allow the PHY PLL to lock without the
> > LCDIF being already active, tie this bit to the HDMI_TX_PHY power domain.
> >
> > Fixes: e3442022f543 ("soc: imx: add i.MX8MP HDMI blk-ctrl")
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/soc/imx/imx8mp-blk-ctrl.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-
> > blk-ctrl.c
> > index 9401df25c85a..73f672104aaa 100644
> > --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > @@ -192,7 +192,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct
> > imx8mp_blk_ctrl *bc,
> > break;
> > case IMX8MP_HDMIBLK_PD_LCDIF:
> > regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> > - BIT(7) | BIT(16) | BIT(17) | BIT(18) |
> > + BIT(16) | BIT(17) | BIT(18) |
> > BIT(19) | BIT(20));
> > regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> > BIT(11));
> > regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0, @@
> > -221,6 +221,7 @@ static void imx8mp_hdmi_blk_ctrl_power_on(struct
> > imx8mp_blk_ctrl *bc,
> > regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(1));
> > break;
> > case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
> > + regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL0, BIT(7));
> > regmap_set_bits(bc->regmap, HDMI_RTX_CLK_CTL1, BIT(22)
> > > BIT(24));
> > regmap_set_bits(bc->regmap, HDMI_RTX_RESET_CTL0,
> > BIT(12));
> > regmap_clear_bits(bc->regmap, HDMI_TX_CONTROL0,
> > BIT(3)); @@ -243,7 +244,7 @@ static void
> > imx8mp_hdmi_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
> > BIT(4) | BIT(5) | BIT(6));
> > regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> > BIT(11));
> > regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> > - BIT(7) | BIT(16) | BIT(17) | BIT(18) |
> > + BIT(16) | BIT(17) | BIT(18) |
> > BIT(19) | BIT(20));
> > break;
> > case IMX8MP_HDMIBLK_PD_PAI:
> > @@ -271,6 +272,7 @@ static void imx8mp_hdmi_blk_ctrl_power_off(struct
> > imx8mp_blk_ctrl *bc,
> > case IMX8MP_HDMIBLK_PD_HDMI_TX_PHY:
> > regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0, BIT(3));
> > regmap_clear_bits(bc->regmap, HDMI_RTX_RESET_CTL0,
> > BIT(12));
> > + regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL0,
> > BIT(7));
> > regmap_clear_bits(bc->regmap, HDMI_RTX_CLK_CTL1,
> > BIT(22) | BIT(24));
> > break;
> > default:
> > --
> > 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD
2022-08-26 19:13 ` [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD Lucas Stach
2022-08-29 1:56 ` Peng Fan
@ 2022-12-31 5:41 ` Shawn Guo
1 sibling, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2022-12-31 5:41 UTC (permalink / raw)
To: Lucas Stach
Cc: Fabio Estevam, NXP Linux Team, Peng Fan, Kieran Bingham,
linux-arm-kernel, patchwork-lst, kernel
On Fri, Aug 26, 2022 at 09:13:04PM +0200, Lucas Stach wrote:
> NXP internal information shows that the PHY refclk is gated by the
> GLOBAL_TX_PIX_CLK_EN bit, so to allow the PHY PLL to lock without the
> LCDIF being already active, tie this bit to the HDMI_TX_PHY power
> domain.
>
> Fixes: e3442022f543 ("soc: imx: add i.MX8MP HDMI blk-ctrl")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Applied, thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
2022-08-26 19:13 [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Lucas Stach
2022-08-26 19:13 ` [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD Lucas Stach
@ 2022-08-26 19:13 ` Lucas Stach
2022-08-29 1:55 ` Peng Fan
2022-08-29 1:51 ` [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Peng Fan
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2022-08-26 19:13 UTC (permalink / raw)
To: Shawn Guo
Cc: Fabio Estevam, NXP Linux Team, Peng Fan, Kieran Bingham,
linux-arm-kernel, patchwork-lst, kernel
Since 5a46079a9645 ("PM: domains: Delete usage of driver_deferred_probe_check_state()")
power domain consumers attached by the driver core do not support probe
deferral anymore, as it is assumed that they are only probed after the
provider is present, as a devlink should have been established between
the two.
With the GPCv2 and its slightly unusual mix between platform devices and DT
description for the PGC domains, devlink fails to add the neccessary probe
dependency. Now that probe deferral is not an option anymore, the domain
drivers for nested GPC domains simply fail to probe, leaving parts of the
SoC unusable. Rather than trying to teach devlink about our one-off usage
of DT and platform devices, just split the registration of the nested
power domains into a second pass, so that we never need any dependency
handling.
Fixes: 5a46079a9645 ("PM: domains: Delete usage of driver_deferred_probe_check_state()")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/soc/imx/gpcv2.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 6383a4edc360..d1bbadbcb034 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -1446,7 +1446,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
struct device_node *pgc_np, *np;
struct regmap *regmap;
void __iomem *base;
- int ret;
+ int ret, pass = 0;
pgc_np = of_get_child_by_name(dev->of_node, "pgc");
if (!pgc_np) {
@@ -1465,7 +1465,16 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
return ret;
}
+ /*
+ * Run two passes for the registration of the PGC domain platform
+ * devices: first all devices that are not part of a power-domain
+ * themselves, then all the others. This avoids -EPROBE_DEFER being
+ * returned for nested domains, that need their parent PGC domains
+ * to be present on probe.
+ */
+again:
for_each_child_of_node(pgc_np, np) {
+ bool child_domain = of_property_read_bool(np, "power-domains");
struct platform_device *pd_pdev;
struct imx_pgc_domain *domain;
u32 domain_index;
@@ -1473,6 +1482,9 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
if (!of_device_is_available(np))
continue;
+ if ((pass == 0 && child_domain) || (pass == 1 && !child_domain))
+ continue;
+
ret = of_property_read_u32(np, "reg", &domain_index);
if (ret) {
dev_err(dev, "Failed to read 'reg' property\n");
@@ -1522,6 +1534,11 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
}
}
+ if (pass == 0) {
+ pass++;
+ goto again;
+ }
+
return 0;
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
2022-08-26 19:13 ` [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes Lucas Stach
@ 2022-08-29 1:55 ` Peng Fan
2022-08-29 7:15 ` Lucas Stach
0 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2022-08-29 1:55 UTC (permalink / raw)
To: Lucas Stach, Shawn Guo
Cc: Fabio Estevam, dl-linux-imx, Kieran Bingham,
linux-arm-kernel@lists.infradead.org,
patchwork-lst@pengutronix.de, kernel@pengutronix.de
> Subject: [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
>
> Since 5a46079a9645 ("PM: domains: Delete usage of
> driver_deferred_probe_check_state()")
> power domain consumers attached by the driver core do not support probe
> deferral anymore, as it is assumed that they are only probed after the
> provider is present, as a devlink should have been established between the
> two.
>
> With the GPCv2 and its slightly unusual mix between platform devices and
> DT description for the PGC domains, devlink fails to add the neccessary
> probe dependency. Now that probe deferral is not an option anymore, the
> domain drivers for nested GPC domains simply fail to probe, leaving parts of
> the SoC unusable. Rather than trying to teach devlink about our one-off
> usage of DT and platform devices, just split the registration of the nested
> power domains into a second pass, so that we never need any dependency
> handling.
>
> Fixes: 5a46079a9645 ("PM: domains: Delete usage of
> driver_deferred_probe_check_state()")
This regression has been fixed, the patch author reverted that patch.
Do we still need this fix?
Thanks,
Peng.
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/soc/imx/gpcv2.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index
> 6383a4edc360..d1bbadbcb034 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -1446,7 +1446,7 @@ static int imx_gpcv2_probe(struct
> platform_device *pdev)
> struct device_node *pgc_np, *np;
> struct regmap *regmap;
> void __iomem *base;
> - int ret;
> + int ret, pass = 0;
>
> pgc_np = of_get_child_by_name(dev->of_node, "pgc");
> if (!pgc_np) {
> @@ -1465,7 +1465,16 @@ static int imx_gpcv2_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> + /*
> + * Run two passes for the registration of the PGC domain platform
> + * devices: first all devices that are not part of a power-domain
> + * themselves, then all the others. This avoids -EPROBE_DEFER being
> + * returned for nested domains, that need their parent PGC domains
> + * to be present on probe.
> + */
> +again:
> for_each_child_of_node(pgc_np, np) {
> + bool child_domain = of_property_read_bool(np, "power-
> domains");
> struct platform_device *pd_pdev;
> struct imx_pgc_domain *domain;
> u32 domain_index;
> @@ -1473,6 +1482,9 @@ static int imx_gpcv2_probe(struct
> platform_device *pdev)
> if (!of_device_is_available(np))
> continue;
>
> + if ((pass == 0 && child_domain) || (pass == 1
> && !child_domain))
> + continue;
> +
> ret = of_property_read_u32(np, "reg", &domain_index);
> if (ret) {
> dev_err(dev, "Failed to read 'reg' property\n"); @@
> -1522,6 +1534,11 @@ static int imx_gpcv2_probe(struct platform_device
> *pdev)
> }
> }
>
> + if (pass == 0) {
> + pass++;
> + goto again;
> + }
> +
> return 0;
> }
>
> --
> 2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
2022-08-29 1:55 ` Peng Fan
@ 2022-08-29 7:15 ` Lucas Stach
0 siblings, 0 replies; 11+ messages in thread
From: Lucas Stach @ 2022-08-29 7:15 UTC (permalink / raw)
To: Peng Fan, Shawn Guo
Cc: Fabio Estevam, dl-linux-imx, Kieran Bingham,
linux-arm-kernel@lists.infradead.org,
patchwork-lst@pengutronix.de, kernel@pengutronix.de
Am Montag, dem 29.08.2022 um 01:55 +0000 schrieb Peng Fan:
> > Subject: [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
> >
> > Since 5a46079a9645 ("PM: domains: Delete usage of
> > driver_deferred_probe_check_state()")
> > power domain consumers attached by the driver core do not support probe
> > deferral anymore, as it is assumed that they are only probed after the
> > provider is present, as a devlink should have been established between the
> > two.
> >
> > With the GPCv2 and its slightly unusual mix between platform devices and
> > DT description for the PGC domains, devlink fails to add the neccessary
> > probe dependency. Now that probe deferral is not an option anymore, the
> > domain drivers for nested GPC domains simply fail to probe, leaving parts of
> > the SoC unusable. Rather than trying to teach devlink about our one-off
> > usage of DT and platform devices, just split the registration of the nested
> > power domains into a second pass, so that we never need any dependency
> > handling.
> >
> > Fixes: 5a46079a9645 ("PM: domains: Delete usage of
> > driver_deferred_probe_check_state()")
>
> This regression has been fixed, the patch author reverted that patch.
>
Thanks, didn't know this.
> Do we still need this fix?
>
We certainly do not need it as a fix then. Depending on how people feel
about the code change, I would still like to land it, as we can avoid a
probe deferral cycle for some GPC power domains this way and thus avoid
unnecessary work during kernel boot.
Regards,
Lucas
> Thanks,
> Peng.
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/soc/imx/gpcv2.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index
> > 6383a4edc360..d1bbadbcb034 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c
> > @@ -1446,7 +1446,7 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> > struct device_node *pgc_np, *np;
> > struct regmap *regmap;
> > void __iomem *base;
> > - int ret;
> > + int ret, pass = 0;
> >
> > pgc_np = of_get_child_by_name(dev->of_node, "pgc");
> > if (!pgc_np) {
> > @@ -1465,7 +1465,16 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> > return ret;
> > }
> >
> > + /*
> > + * Run two passes for the registration of the PGC domain platform
> > + * devices: first all devices that are not part of a power-domain
> > + * themselves, then all the others. This avoids -EPROBE_DEFER being
> > + * returned for nested domains, that need their parent PGC domains
> > + * to be present on probe.
> > + */
> > +again:
> > for_each_child_of_node(pgc_np, np) {
> > + bool child_domain = of_property_read_bool(np, "power-
> > domains");
> > struct platform_device *pd_pdev;
> > struct imx_pgc_domain *domain;
> > u32 domain_index;
> > @@ -1473,6 +1482,9 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> > if (!of_device_is_available(np))
> > continue;
> >
> > + if ((pass == 0 && child_domain) || (pass == 1
> > && !child_domain))
> > + continue;
> > +
> > ret = of_property_read_u32(np, "reg", &domain_index);
> > if (ret) {
> > dev_err(dev, "Failed to read 'reg' property\n"); @@
> > -1522,6 +1534,11 @@ static int imx_gpcv2_probe(struct platform_device
> > *pdev)
> > }
> > }
> >
> > + if (pass == 0) {
> > + pass++;
> > + goto again;
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in
2022-08-26 19:13 [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Lucas Stach
2022-08-26 19:13 ` [PATCH 2/3] soc: imx: imx8mp-blk-ctrl: enable global pixclk with HDMI_TX_PHY PD Lucas Stach
2022-08-26 19:13 ` [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes Lucas Stach
@ 2022-08-29 1:51 ` Peng Fan
2022-12-31 5:31 ` Shawn Guo
2023-01-01 3:15 ` Shawn Guo
4 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2022-08-29 1:51 UTC (permalink / raw)
To: Lucas Stach, Shawn Guo
Cc: Fabio Estevam, dl-linux-imx, Kieran Bingham,
linux-arm-kernel@lists.infradead.org,
patchwork-lst@pengutronix.de, kernel@pengutronix.de
> Subject: [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device
> name in
>
> Setting the device name after it has been registered confuses the sysfs
> cleanup paths. This has already been fixed for the imx8m-blk-ctrl driver in
> b64b46fbaa1d ("Revert "soc: imx: imx8m-blk-ctrl: set power device name""),
> but the same problem exists in imx8mp-blk-ctrl.
>
> Fixes: 556f5cf9568a ("soc: imx: add i.MX8MP HSIO blk-ctrl")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/soc/imx/imx8mp-blk-ctrl.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-
> blk-ctrl.c
> index 4ca2ede6871b..9401df25c85a 100644
> --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -529,7 +529,6 @@ static int imx8mp_blk_ctrl_probe(struct
> platform_device *pdev)
> ret = PTR_ERR(domain->power_dev);
> goto cleanup_pds;
> }
> - dev_set_name(domain->power_dev, "%s", data->name);
>
> domain->genpd.name = data->name;
> domain->genpd.power_on = imx8mp_blk_ctrl_power_on;
> --
> 2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in
2022-08-26 19:13 [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Lucas Stach
` (2 preceding siblings ...)
2022-08-29 1:51 ` [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Peng Fan
@ 2022-12-31 5:31 ` Shawn Guo
2023-01-01 3:15 ` Shawn Guo
4 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2022-12-31 5:31 UTC (permalink / raw)
To: Lucas Stach
Cc: Fabio Estevam, NXP Linux Team, Peng Fan, Kieran Bingham,
linux-arm-kernel, patchwork-lst, kernel
On Fri, Aug 26, 2022 at 09:13:03PM +0200, Lucas Stach wrote:
> Setting the device name after it has been registered confuses the sysfs
> cleanup paths. This has already been fixed for the imx8m-blk-ctrl driver in
> b64b46fbaa1d ("Revert "soc: imx: imx8m-blk-ctrl: set power device name""),
> but the same problem exists in imx8mp-blk-ctrl.
>
> Fixes: 556f5cf9568a ("soc: imx: add i.MX8MP HSIO blk-ctrl")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
For patch subject, what does "in" mean there? Should we just drop it or
add something after it?
Shawn
> ---
> drivers/soc/imx/imx8mp-blk-ctrl.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> index 4ca2ede6871b..9401df25c85a 100644
> --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -529,7 +529,6 @@ static int imx8mp_blk_ctrl_probe(struct platform_device *pdev)
> ret = PTR_ERR(domain->power_dev);
> goto cleanup_pds;
> }
> - dev_set_name(domain->power_dev, "%s", data->name);
>
> domain->genpd.name = data->name;
> domain->genpd.power_on = imx8mp_blk_ctrl_power_on;
> --
> 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in
2022-08-26 19:13 [PATCH 1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in Lucas Stach
` (3 preceding siblings ...)
2022-12-31 5:31 ` Shawn Guo
@ 2023-01-01 3:15 ` Shawn Guo
4 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2023-01-01 3:15 UTC (permalink / raw)
To: Lucas Stach
Cc: Fabio Estevam, NXP Linux Team, Peng Fan, Kieran Bingham,
linux-arm-kernel, patchwork-lst, kernel
On Fri, Aug 26, 2022 at 09:13:03PM +0200, Lucas Stach wrote:
> Setting the device name after it has been registered confuses the sysfs
> cleanup paths. This has already been fixed for the imx8m-blk-ctrl driver in
> b64b46fbaa1d ("Revert "soc: imx: imx8m-blk-ctrl: set power device name""),
> but the same problem exists in imx8mp-blk-ctrl.
>
> Fixes: 556f5cf9568a ("soc: imx: add i.MX8MP HSIO blk-ctrl")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Dropped "in" from subject and applied, thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread