Hi Shawn,

Am Montag, dem 06.12.2021 um 08:45 +0800 schrieb Shawn Guo:

Hi Lucas,

What's your take on this one?

Thanks for asking. This patch is incorrect. The handshake will only be
completed once all the clocks for the ADB are enabled, which is done in
the blk-ctrl driver. The blk-ctrl driver however can only do so _after_
the power-up sequence of the GPC domain is completed, so waiting for
the handshake in the middle of the power-up sequence is not going to
work for most of the GPC domains. Please drop this patch.

Regards,
Lucas

Shawn

On Sat, Nov 20, 2021 at 01:49:00PM -0600, Adam Ford wrote:

quoted

There is a comment in the code that states the driver needs to
wait for the handshake, but it's only available when the bus
has been enabled from the blk-ctrl.  Since both the
vpumix and dispmix are called from the blk-ctl, it seems
reasonable to assume the bus is enabled. Add a bool to determine
which power-domains are able to properly wait for this
handshake and set the corresping boolean for the two domains
activated by the blk-ctrl.

Signed-off-by: Adam Ford <redacted>

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 7b6dfa33dcb9..a957f7fff968 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c

@@ -204,6 +204,7 @@ struct imx_pgc_domain {
 	const int voltage;
 	const bool keep_clocks;
 	struct device *dev;
+	bool blkctrl_bus_enabled;
 };
 
 struct imx_pgc_domain_data {

@@ -282,17 +283,14 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				   domain->bits.hskreq, domain->bits.hskreq);
 
 		/*
-		 * ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK, reg_val,
-		 *				  (reg_val & domain->bits.hskack), 0,
-		 *				  USEC_PER_MSEC);
-		 * Technically we need the commented code to wait handshake. But that needs
-		 * the BLK-CTL module BUS clk-en bit being set.
-		 *
-		 * There is a separate BLK-CTL module and we will have such a driver for it,
-		 * that driver will set the BUS clk-en bit and handshake will be triggered
-		 * automatically there. Just add a delay and suppose the handshake finish
-		 * after that.
+		 * blkctrl_bus_enabled implies that the GPC is being invoked from a blk-ctrl
+		 * and not from a peripheral or other GPC power domain.  The blk-ctrl is required
+		 * to support the handshake.
 		 */
+		if (domain->blkctrl_bus_enabled)
+			ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK, reg_val,
+							(reg_val & domain->bits.hskack), 0,
+							USEC_PER_MSEC);
 	}
 
 	/* Disable reset clocks for all devices in the domain */

@@ -701,6 +699,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 		},
 		.pgc   = BIT(IMX8MM_PGC_VPUMIX),
 		.keep_clocks = true,
+		.blkctrl_bus_enabled = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {

@@ -749,6 +748,7 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 		},
 		.pgc   = BIT(IMX8MM_PGC_DISPMIX),
 		.keep_clocks = true,
+		.blkctrl_bus_enabled = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_MIPI] = {

-- 
2.32.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel