[PATCH v2] drm/bridge: cdns-mhdp8546: Add suspend resume support to the bridge driver

[PATCH v2] drm/bridge: cdns-mhdp8546: Add suspend resume support to the bridge driver

[Abhash Kumar Jha]

Add system suspend and resume hooks to the cdns-mhdp8546 bridge driver.

While resuming we either load the firmware or activate it. Firmware
is loaded only when resuming from a successful suspend-resume cycle.

If resuming due to an aborted suspend, loading the firmware is not
possible because the uCPU's IMEM is only accessible after a reset and the
bridge has not gone through a reset in this case. Hence, Activate the
firmware that is already loaded.

Use GENPD_NOTIFY_OFF genpd_notifier to get the power domain status of
the bridge and accordingly load the firmware.

Additionally, introduce phy_power_off/on to control the power to the phy.

Signed-off-by: Abhash Kumar Jha <a-kumar2@ti.com>
---
Changes in v2:
- Fixed defined but not used [-Wunused-function] warning for suspend and resume calls.
- Link to v1: https://lore.kernel.org/all/20260129112016.2448037-1-a-kumar2@ti.com/

 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 136 +++++++++++++++++-
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   4 +
 2 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 38726ae1bf150..818eef1a57ffb 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -32,6 +32,7 @@
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-dp.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/wait.h>
 
@@ -2383,6 +2384,120 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
 	}
 }
 
+static int __maybe_unused cdns_mhdp_resume(struct device *dev)
+{
+	struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
+	unsigned long rate;
+	int ret;
+
+	ret = clk_prepare_enable(mhdp->clk);
+	if (ret)
+		return ret;
+
+	rate = clk_get_rate(mhdp->clk);
+	writel(rate % 1000000, mhdp->regs + CDNS_SW_CLK_L);
+	writel(rate / 1000000, mhdp->regs + CDNS_SW_CLK_H);
+	writel(~0, mhdp->regs + CDNS_APB_INT_MASK);
+
+	ret = phy_init(mhdp->phy);
+	if (ret) {
+		dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
+		goto disable_clk;
+	}
+	ret = phy_power_on(mhdp->phy);
+	if (ret < 0) {
+		dev_err(mhdp->dev, "Failed to power on PHY: %d\n", ret);
+		goto error;
+	}
+
+	if (mhdp->powered_off) {
+		ret = cdns_mhdp_load_firmware(mhdp);
+		if (ret)
+			goto phy_off;
+
+		ret = wait_event_timeout(mhdp->fw_load_wq,
+					mhdp->hw_state == MHDP_HW_READY,
+					msecs_to_jiffies(1000));
+		if (ret == 0) {
+			dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
+				__func__);
+			ret = -ETIMEDOUT;
+			goto phy_off;
+		}
+	} else {
+		ret = cdns_mhdp_set_firmware_active(mhdp, true);
+		if (ret) {
+			dev_err(mhdp->dev, "Failed to activate firmware (%pe)\n", ERR_PTR(ret));
+			goto phy_off;
+		}
+	}
+
+	return 0;
+
+phy_off:
+	phy_power_off(mhdp->phy);
+error:
+	phy_exit(mhdp->phy);
+disable_clk:
+	clk_disable_unprepare(mhdp->clk);
+
+	return ret;
+}
+
+static int __maybe_unused cdns_mhdp_suspend(struct device *dev)
+{
+	struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
+	unsigned long timeout = msecs_to_jiffies(100);
+	int ret = 0;
+
+	cancel_work_sync(&mhdp->hpd_work);
+	ret = wait_event_timeout(mhdp->fw_load_wq,
+				 mhdp->hw_state == MHDP_HW_READY,
+				 timeout);
+
+	spin_lock(&mhdp->start_lock);
+	if (mhdp->hw_state != MHDP_HW_READY) {
+		spin_unlock(&mhdp->start_lock);
+		return -EINVAL;
+	}
+	mhdp->hw_state = MHDP_HW_STOPPED;
+	spin_unlock(&mhdp->start_lock);
+
+	if (ret == 0) {
+		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n", __func__);
+		ret = -ETIMEDOUT;
+		goto error;
+	} else {
+		ret = cdns_mhdp_set_firmware_active(mhdp, false);
+		if (ret) {
+			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n", ERR_PTR(ret));
+			goto error;
+		}
+	}
+
+	phy_power_off(mhdp->phy);
+	phy_exit(mhdp->phy);
+	clk_disable_unprepare(mhdp->clk);
+
+error:
+	return ret;
+}
+
+static int mhdp_pd_notifier_cb(struct notifier_block *nb,
+			unsigned long action, void *data)
+{
+	struct cdns_mhdp_device *mhdp = container_of(nb, struct cdns_mhdp_device, pd_nb);
+
+	if (action == GENPD_NOTIFY_OFF)
+		mhdp->powered_off = true;
+
+	return 0;
+}
+
+static const struct dev_pm_ops cdns_mhdp_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(cdns_mhdp_suspend, cdns_mhdp_resume)
+};
+
 static int cdns_mhdp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -2494,6 +2609,11 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 		dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
 		goto plat_fini;
 	}
+	ret = phy_power_on(mhdp->phy);
+	if (ret < 0) {
+		dev_err(mhdp->dev, "Failed to power on PHY: %d\n", ret);
+		goto phy_exit;
+	}
 
 	/* Initialize the work for modeset in case of link train failure */
 	INIT_WORK(&mhdp->modeset_retry_work, cdns_mhdp_modeset_retry_fn);
@@ -2504,21 +2624,33 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 
 	ret = cdns_mhdp_load_firmware(mhdp);
 	if (ret)
-		goto phy_exit;
+		goto power_off;
 
 	if (mhdp->hdcp_supported)
 		cdns_mhdp_hdcp_init(mhdp);
 
 	drm_bridge_add(&mhdp->bridge);
 
+	mhdp->powered_off = false;
+	mhdp->pd_nb.notifier_call = mhdp_pd_notifier_cb;
+	ret = dev_pm_genpd_add_notifier(mhdp->dev, &mhdp->pd_nb);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to add power domain notifier\n");
+		dev_pm_genpd_remove_notifier(mhdp->dev);
+		goto power_off;
+	}
+
 	return 0;
 
+power_off:
+	phy_power_off(mhdp->phy);
 phy_exit:
 	phy_exit(mhdp->phy);
 plat_fini:
 	if (mhdp->info && mhdp->info->ops && mhdp->info->ops->exit)
 		mhdp->info->ops->exit(mhdp);
 runtime_put:
+	mhdp->powered_off = true;
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
@@ -2550,6 +2682,7 @@ static void cdns_mhdp_remove(struct platform_device *pdev)
 				ERR_PTR(ret));
 	}
 
+	phy_power_off(mhdp->phy);
 	phy_exit(mhdp->phy);
 
 	if (mhdp->info && mhdp->info->ops && mhdp->info->ops->exit)
@@ -2581,6 +2714,7 @@ static struct platform_driver mhdp_driver = {
 	.driver	= {
 		.name		= "cdns-mhdp8546",
 		.of_match_table	= mhdp_ids,
+		.pm = &cdns_mhdp_pm_ops,
 	},
 	.probe	= cdns_mhdp_probe,
 	.remove = cdns_mhdp_remove,
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index bad2fc0c73066..b06dd5e44aafd 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -412,6 +412,10 @@ struct cdns_mhdp_device {
 
 	struct cdns_mhdp_hdcp hdcp;
 	bool hdcp_supported;
+
+	/* Power domain status notifier */
+	struct notifier_block pd_nb;
+	bool powered_off;
 };
 
 #define connector_to_mhdp(x) container_of(x, struct cdns_mhdp_device, connector)
-- 
2.34.1

Re: [PATCH v2] drm/bridge: cdns-mhdp8546: Add suspend resume support to the bridge driver

[Devarsh Thakkar]

Hi Abhash,

Thanks for the patch.

On 05/02/26 14:22, Abhash Kumar Jha wrote:
> Add system suspend and resume hooks to the cdns-mhdp8546 bridge driver.
> 
> While resuming we either load the firmware or activate it. Firmware
> is loaded only when resuming from a successful suspend-resume cycle.
> 
> If resuming due to an aborted suspend, loading the firmware is not
> possible because the uCPU's IMEM is only accessible after a reset and the
> bridge has not gone through a reset in this case. Hence, Activate the
> firmware that is already loaded.
> 
> Use GENPD_NOTIFY_OFF genpd_notifier to get the power domain status of
> the bridge and accordingly load the firmware.
> 
> Additionally, introduce phy_power_off/on to control the power to the phy.
> 
> Signed-off-by: Abhash Kumar Jha <a-kumar2@ti.com>
> ---
> Changes in v2:
> - Fixed defined but not used [-Wunused-function] warning for suspend and resume calls.
> - Link to v1: https://lore.kernel.org/all/20260129112016.2448037-1-a-kumar2@ti.com/
> 
>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 136 +++++++++++++++++-
>   .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   4 +
>   2 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 38726ae1bf150..818eef1a57ffb 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -32,6 +32,7 @@
>   #include <linux/phy/phy.h>
>   #include <linux/phy/phy-dp.h>
>   #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>   #include <linux/slab.h>
>   #include <linux/wait.h>
>   
> @@ -2383,6 +2384,120 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
>   	}
>   }
>   
> +static int __maybe_unused cdns_mhdp_resume(struct device *dev)
> +{
> +	struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> +	unsigned long rate;
> +	int ret;
> +
> +	ret = clk_prepare_enable(mhdp->clk);
> +	if (ret)
> +		return ret;
> +
> +	rate = clk_get_rate(mhdp->clk);
> +	writel(rate % 1000000, mhdp->regs + CDNS_SW_CLK_L);
> +	writel(rate / 1000000, mhdp->regs + CDNS_SW_CLK_H);
> +	writel(~0, mhdp->regs + CDNS_APB_INT_MASK);
> +
> +	ret = phy_init(mhdp->phy);
> +	if (ret) {
> +		dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
> +		goto disable_clk;
> +	}
> +	ret = phy_power_on(mhdp->phy);
> +	if (ret < 0) {
> +		dev_err(mhdp->dev, "Failed to power on PHY: %d\n", ret);
> +		goto error;
> +	}
> +
> +	if (mhdp->powered_off) {
> +		ret = cdns_mhdp_load_firmware(mhdp);
> +		if (ret)
> +			goto phy_off;
> +
> +		ret = wait_event_timeout(mhdp->fw_load_wq,
> +					mhdp->hw_state == MHDP_HW_READY,
> +					msecs_to_jiffies(1000));
> +		if (ret == 0) {
> +			dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> +				__func__);
> +			ret = -ETIMEDOUT;
> +			goto phy_off;
> +		}
> +	} else {
> +		ret = cdns_mhdp_set_firmware_active(mhdp, true);
> +		if (ret) {
> +			dev_err(mhdp->dev, "Failed to activate firmware (%pe)\n", ERR_PTR(ret));
> +			goto phy_off;
> +		}
> +	}
> +

mhdp->powered_off = false ??

> +	return 0;
> +
> +phy_off:
> +	phy_power_off(mhdp->phy);
> +error:
> +	phy_exit(mhdp->phy);
> +disable_clk:
> +	clk_disable_unprepare(mhdp->clk);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused cdns_mhdp_suspend(struct device *dev)
> +{
> +	struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> +	unsigned long timeout = msecs_to_jiffies(100);
> +	int ret = 0;
> +
> +	cancel_work_sync(&mhdp->hpd_work);
> +	ret = wait_event_timeout(mhdp->fw_load_wq,
> +				 mhdp->hw_state == MHDP_HW_READY,
> +				 timeout);
> +
> +	spin_lock(&mhdp->start_lock);
> +	if (mhdp->hw_state != MHDP_HW_READY) {
> +		spin_unlock(&mhdp->start_lock);
> +		return -EINVAL;

return -ETIMEDOUT?

> +	}
> +	mhdp->hw_state = MHDP_HW_STOPPED;

Above should be inside if block i..e if hw state was not ready.

> +	spin_unlock(&mhdp->start_lock);
> +
> +	if (ret == 0) {
> +		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n", __func__);
> +		ret = -ETIMEDOUT;
> +		goto error;
> +	} else {
> +		ret = cdns_mhdp_set_firmware_active(mhdp, false);
> +		if (ret) {
> +			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n", ERR_PTR(ret));

At this point don;t you need to
phy_power_off/phy_exit/clk_disable_unprepare, and restore hw_state to 
MHDP_HW_READY ?
> +			goto error;
> +		}
> +	}
> +
> +	phy_power_off(mhdp->phy);
> +	phy_exit(mhdp->phy);
> +	clk_disable_unprepare(mhdp->clk);
> +
> +error:
> +	return ret;
> +}
> +
> +static int mhdp_pd_notifier_cb(struct notifier_block *nb,
> +			unsigned long action, void *data)
> +{
> +	struct cdns_mhdp_device *mhdp = container_of(nb, struct cdns_mhdp_device, pd_nb);
> +
> +	if (action == GENPD_NOTIFY_OFF)
> +		mhdp->powered_off = true;

Should we reset powered_off flag for GENPD_NOTIFY_ON?
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops cdns_mhdp_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(cdns_mhdp_suspend, cdns_mhdp_resume)
> +};

Have you considered using below ?
   static DEFINE_SIMPLE_DEV_PM_OPS(cdns_mhdp_pm_ops, 
  

                                    cdns_mhdp_suspend, cdns_mhdp_resume);
> +
>   static int cdns_mhdp_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -2494,6 +2609,11 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
>   		dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
>   		goto plat_fini;
>   	}
> +	ret = phy_power_on(mhdp->phy);
> +	if (ret < 0) {
> +		dev_err(mhdp->dev, "Failed to power on PHY: %d\n", ret);
> +		goto phy_exit;
> +	}
>   
>   	/* Initialize the work for modeset in case of link train failure */
>   	INIT_WORK(&mhdp->modeset_retry_work, cdns_mhdp_modeset_retry_fn);
> @@ -2504,21 +2624,33 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
>   
>   	ret = cdns_mhdp_load_firmware(mhdp);
>   	if (ret)
> -		goto phy_exit;
> +		goto power_off;
>   
>   	if (mhdp->hdcp_supported)
>   		cdns_mhdp_hdcp_init(mhdp);
>   
>   	drm_bridge_add(&mhdp->bridge);

I think good to add bridge only after pm notifier is successfully added, 
otherwise you have to remove the bridge before adding.
>   
> +	mhdp->powered_off = false;
> +	mhdp->pd_nb.notifier_call = mhdp_pd_notifier_cb;
> +	ret = dev_pm_genpd_add_notifier(mhdp->dev, &mhdp->pd_nb);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to add power domain notifier\n");
> +		dev_pm_genpd_remove_notifier(mhdp->dev);

If add_notifier failed already why need to do above remove ?

> +		goto power_off;
> +	}

Also for non-gen-pd platforms this would pretty much restrict usage of 
this driver, is that expected/desired behaviour ?
> +
>   	return 0;
>   
> +power_off:
> +	phy_power_off(mhdp->phy);
>   phy_exit:
>   	phy_exit(mhdp->phy);
>   plat_fini:
>   	if (mhdp->info && mhdp->info->ops && mhdp->info->ops->exit)
>   		mhdp->info->ops->exit(mhdp);
>   runtime_put:
> +	mhdp->powered_off = true;

probe is anyway failing so why need to set above flag given it would be 
reset on driver probe again ?

Regards
Devarsh

>   	pm_runtime_put_sync(dev);
>   	pm_runtime_disable(dev);
>   
> @@ -2550,6 +2682,7 @@ static void cdns_mhdp_remove(struct platform_device *pdev)
>   				ERR_PTR(ret));
>   	}
>   
> +	phy_power_off(mhdp->phy);
>   	phy_exit(mhdp->phy);
>   
>   	if (mhdp->info && mhdp->info->ops && mhdp->info->ops->exit)
> @@ -2581,6 +2714,7 @@ static struct platform_driver mhdp_driver = {
>   	.driver	= {
>   		.name		= "cdns-mhdp8546",
>   		.of_match_table	= mhdp_ids,
> +		.pm = &cdns_mhdp_pm_ops,
>   	},
>   	.probe	= cdns_mhdp_probe,
>   	.remove = cdns_mhdp_remove,
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> index bad2fc0c73066..b06dd5e44aafd 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> @@ -412,6 +412,10 @@ struct cdns_mhdp_device {
>   
>   	struct cdns_mhdp_hdcp hdcp;
>   	bool hdcp_supported;
> +
> +	/* Power domain status notifier */
> +	struct notifier_block pd_nb;
> +	bool powered_off;
>   };
>   
>   #define connector_to_mhdp(x) container_of(x, struct cdns_mhdp_device, connector)

Re: [PATCH v2] drm/bridge: cdns-mhdp8546: Add suspend resume support to the bridge driver

[Kumar, Abhash]

Hi Devarsh,

Thanks for the review

Posted a v3 here: [PATCH v3] drm/bridge: cdns-mhdp8546: Add suspend 
resume support to the bridge driver - Abhash Kumar Jha 
<https://lore.kernel.org/all/20260601095041.3042950-1-a-kumar2@ti.com/>

>> +    } else {
>> +        ret = cdns_mhdp_set_firmware_active(mhdp, true);
>> +        if (ret) {
>> +            dev_err(mhdp->dev, "Failed to activate firmware 
>> (%pe)\n", ERR_PTR(ret));
>> +            goto phy_off;
>> +        }
>> +    }
>> +
>
> mhdp->powered_off = false ??

Good catch, updated in v3.

>
>> +    }
>> +    mhdp->hw_state = MHDP_HW_STOPPED;
>
> Above should be inside if block i..e if hw state was not ready.
Yes, updated this in v3
>
>> + spin_unlock(&mhdp->start_lock);
>> +
>> +    if (ret == 0) {
>> +        dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n", 
>> __func__);
>> +        ret = -ETIMEDOUT;
>> +        goto error;
>> +    } else {
>> +        ret = cdns_mhdp_set_firmware_active(mhdp, false);
>> +        if (ret) {
>> +            dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n", 
>> ERR_PTR(ret));
>
> At this point don;t you need to
> phy_power_off/phy_exit/clk_disable_unprepare, and restore hw_state to 
> MHDP_HW_READY ?

I think we should only restore the hw_state to MHDP_HW_READY.

We should not do phy_power_off/phy_exit/clk_disable_unprepare in case of 
a failure in suspend

because the device is still active. So the driver can be used and a 
suspend can be triggered later.


>> +            goto error;
>> +        }
>> +    }
>> +
>> +    phy_power_off(mhdp->phy);
>> +    phy_exit(mhdp->phy);
>> +    clk_disable_unprepare(mhdp->clk);
>> +
>> +error:
>> +    return ret;
>> +}
>> +
>> +static int mhdp_pd_notifier_cb(struct notifier_block *nb,
>> +            unsigned long action, void *data)
>> +{
>> +    struct cdns_mhdp_device *mhdp = container_of(nb, struct 
>> cdns_mhdp_device, pd_nb);
>> +
>> +    if (action == GENPD_NOTIFY_OFF)
>> +        mhdp->powered_off = true;
>
> Should we reset powered_off flag for GENPD_NOTIFY_ON?

I have reset the mhdp->powered_off to false in the resume in the v3.

>> +
>> +    return 0;
>> +}
>> +
>> +static const struct dev_pm_ops cdns_mhdp_pm_ops = {
>> +    SET_SYSTEM_SLEEP_PM_OPS(cdns_mhdp_suspend, cdns_mhdp_resume)
>> +};
>
> Have you considered using below ?
>   static DEFINE_SIMPLE_DEV_PM_OPS(cdns_mhdp_pm_ops,
>
>                                    cdns_mhdp_suspend, cdns_mhdp_resume);
Thanks for the tip, updated it in v3.
>> +
>>   static int cdns_mhdp_probe(struct platform_device *pdev)
>>   {
>>       struct device *dev = &pdev->dev;
>> @@ -2494,6 +2609,11 @@ static int cdns_mhdp_probe(struct 
>> platform_device *pdev)
>>           dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
>>           goto plat_fini;
>>       }
>>   +    mhdp->powered_off = false;
>> +    mhdp->pd_nb.notifier_call = mhdp_pd_notifier_cb;
>> +    ret = dev_pm_genpd_add_notifier(mhdp->dev, &mhdp->pd_nb);
>> +    if (ret) {
>> +        dev_err_probe(dev, ret, "failed to add power domain 
>> notifier\n");
>> +        dev_pm_genpd_remove_notifier(mhdp->dev);
>
> If add_notifier failed already why need to do above remove ?
Correct, we do not need this.
>
>> +        goto power_off;
>> +    }
>
> Also for non-gen-pd platforms this would pretty much restrict usage of 
> this driver, is that expected/desired behaviour ?

For non-gen-pd platforms, we mark the mhdp->powered_off = true at the 
end of suspend.

So if no gen-pd is available, assume that the platform has taken care of 
powering off the PD at the end of suspend

and hence always reload firmware on resume.

Additionally we only add this notifier for gen-pd platforms.


Thanks and Regards,

Abhash