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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |