[PATCH] pmdomain: imx8mp-blk-ctrl: fix adb handshake handling

[PATCH] pmdomain: imx8mp-blk-ctrl: fix adb handshake handling

[Marco Felsch]

Fix powering the DWC3 USB subsystem which is part of the HSIO BLKCTRL.
Currently we enable the USB clocks and the USB module clock within the
HSIO BLKCTRL. The later get stuck during the first call of
imx8mp_blk_ctrl_power_on() since the parent GPCv2 device is not powered
yet.

Fix this by porting the Linux imx8mp_blk_ctrl::power_nb notifier_block
logic. The Linux driver enable/disable the USB module to propagate the
ADB handshake instead of powering it permanently. The logic is executed
after the parent GPCv2 power domain is powered so we can access the HSIO
BLKCTRL registers.

Fixes: 5cb41f0b62dc ("pmdomain: imx: add i.MX8MP HSIO blk-ctrl driver")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 28 ++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
index 38de9d88a139..b71a4a793761 100644
--- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
@@ -192,20 +192,34 @@ static void imx8mp_hsio_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
 	}
 }
 
-static int imx8mp_hsio_enable_usb_clk(struct imx8mp_blk_ctrl *bc)
+static int imx8mp_hsio_propagate_adb_handshake(struct imx8mp_blk_ctrl *bc)
 {
 	int ret;
 	struct clk_bulk_data *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clks;
 	int num_clks = bc->domains[IMX8MP_HSIOBLK_PD_USB].data->num_clks;
+	static int once;
 
+	if (once)
+		return 0;
+
+	/*
+	 * enable USB clock for a moment for the power-on ADB handshake
+	 * to proceed
+	 */
 	ret = clk_bulk_prepare_enable(num_clks, usb_clk);
 	if (ret)
 		return ret;
 
 	regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
 
-	return 0;
+	udelay(5);
+
+	regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+	clk_bulk_disable_unprepare(num_clks, usb_clk);
 
+	once++;
+
+	return 0;
 }
 
 static const struct imx8mp_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
@@ -251,10 +265,6 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 	struct imx8mp_blk_ctrl *bc = domain->bc;
 	int ret;
 
-	ret = imx8mp_hsio_enable_usb_clk(bc);
-	if (ret)
-		return ret;
-
 	/* make sure bus domain is awake */
 	ret = pm_runtime_resume_and_get_genpd(bc->bus_power_dev);
 	if (ret < 0) {
@@ -262,6 +272,12 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 		return ret;
 	}
 
+	ret = imx8mp_hsio_propagate_adb_handshake(bc);
+	if (ret) {
+		dev_err(bc->dev, "failed to propagate adb handshake\n");
+		goto bus_put;
+	}
+
 	/* enable upstream clocks */
 	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
 	if (ret) {
-- 
2.39.2

Re: [PATCH] pmdomain: imx8mp-blk-ctrl: fix adb handshake handling

[Ahmad Fatoum]

Hello Marco,

On 03.04.24 17:31, Marco Felsch wrote:
> Fix powering the DWC3 USB subsystem which is part of the HSIO BLKCTRL.
> Currently we enable the USB clocks and the USB module clock within the
> HSIO BLKCTRL. The later get stuck during the first call of
> imx8mp_blk_ctrl_power_on() since the parent GPCv2 device is not powered
> yet.
> 
> Fix this by porting the Linux imx8mp_blk_ctrl::power_nb notifier_block
> logic. The Linux driver enable/disable the USB module to propagate the
> ADB handshake instead of powering it permanently. The logic is executed
> after the parent GPCv2 power domain is powered so we can access the HSIO
> BLKCTRL registers.
> 
> Fixes: 5cb41f0b62dc ("pmdomain: imx: add i.MX8MP HSIO blk-ctrl driver")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Given that we currently never disable power domains, this looks ok:

Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thanks for the fix. This should go into master.

Cheers,
Ahmad

> ---
>  drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 28 ++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index 38de9d88a139..b71a4a793761 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -192,20 +192,34 @@ static void imx8mp_hsio_blk_ctrl_power_off(struct imx8mp_blk_ctrl *bc,
>  	}
>  }
>  
> -static int imx8mp_hsio_enable_usb_clk(struct imx8mp_blk_ctrl *bc)
> +static int imx8mp_hsio_propagate_adb_handshake(struct imx8mp_blk_ctrl *bc)
>  {
>  	int ret;
>  	struct clk_bulk_data *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clks;
>  	int num_clks = bc->domains[IMX8MP_HSIOBLK_PD_USB].data->num_clks;
> +	static int once;
>  
> +	if (once)
> +		return 0;
> +
> +	/*
> +	 * enable USB clock for a moment for the power-on ADB handshake
> +	 * to proceed
> +	 */
>  	ret = clk_bulk_prepare_enable(num_clks, usb_clk);
>  	if (ret)
>  		return ret;
>  
>  	regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
>  
> -	return 0;
> +	udelay(5);
> +
> +	regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> +	clk_bulk_disable_unprepare(num_clks, usb_clk);
>  
> +	once++;
> +
> +	return 0;
>  }
>  
>  static const struct imx8mp_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> @@ -251,10 +265,6 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>  	struct imx8mp_blk_ctrl *bc = domain->bc;
>  	int ret;
>  
> -	ret = imx8mp_hsio_enable_usb_clk(bc);
> -	if (ret)
> -		return ret;
> -
>  	/* make sure bus domain is awake */
>  	ret = pm_runtime_resume_and_get_genpd(bc->bus_power_dev);
>  	if (ret < 0) {
> @@ -262,6 +272,12 @@ static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>  		return ret;
>  	}
>  
> +	ret = imx8mp_hsio_propagate_adb_handshake(bc);
> +	if (ret) {
> +		dev_err(bc->dev, "failed to propagate adb handshake\n");
> +		goto bus_put;
> +	}
> +
>  	/* enable upstream clocks */
>  	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
>  	if (ret) {

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] pmdomain: imx8mp-blk-ctrl: fix adb handshake handling

[Sascha Hauer]

On Wed, 03 Apr 2024 17:31:51 +0200, Marco Felsch wrote:
> Fix powering the DWC3 USB subsystem which is part of the HSIO BLKCTRL.
> Currently we enable the USB clocks and the USB module clock within the
> HSIO BLKCTRL. The later get stuck during the first call of
> imx8mp_blk_ctrl_power_on() since the parent GPCv2 device is not powered
> yet.
> 
> Fix this by porting the Linux imx8mp_blk_ctrl::power_nb notifier_block
> logic. The Linux driver enable/disable the USB module to propagate the
> ADB handshake instead of powering it permanently. The logic is executed
> after the parent GPCv2 power domain is powered so we can access the HSIO
> BLKCTRL registers.
> 
> [...]

Applied, thanks!

[1/1] pmdomain: imx8mp-blk-ctrl: fix adb handshake handling
      https://git.pengutronix.de/cgit/barebox/commit/?id=aa03dc194997 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>