[PATCH v5 25/34] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP

AngeloGioacchino Del Regno posted 34 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v5 25/34] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP
Posted by AngeloGioacchino Del Regno 11 months, 1 week ago
Since the SIMPLE_DEV_PM_OPS macro and the pm pointer are anyway
defined when CONFIG_PM_SLEEP is not set, remove the ifdef for it
and indicate that the mtk_hdmi_{remove,suspend} functions may be
unused (as they are, in case PM support is not built-in).

While at it, to improve readability, also compress the
SIMPLE_DEV_PM_OPS declaration as it even fits in less
than 80 columns.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 0ec3bfe528f8..f6b463aaba47 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1692,8 +1692,7 @@ static void mtk_hdmi_remove(struct platform_device *pdev)
 	mtk_hdmi_clk_disable_audio(hdmi);
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int mtk_hdmi_suspend(struct device *dev)
+static __maybe_unused int mtk_hdmi_suspend(struct device *dev)
 {
 	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
 
@@ -1702,7 +1701,7 @@ static int mtk_hdmi_suspend(struct device *dev)
 	return 0;
 }
 
-static int mtk_hdmi_resume(struct device *dev)
+static __maybe_unused int mtk_hdmi_resume(struct device *dev)
 {
 	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
 	int ret = 0;
@@ -1715,9 +1714,8 @@ static int mtk_hdmi_resume(struct device *dev)
 
 	return 0;
 }
-#endif
-static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
-			 mtk_hdmi_suspend, mtk_hdmi_resume);
+
+static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops, mtk_hdmi_suspend, mtk_hdmi_resume);
 
 static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701 = {
 	.tz_disabled = true,
-- 
2.47.0
Re: [PATCH v5 25/34] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP
Posted by CK Hu (胡俊光) 10 months, 2 weeks ago
Hi, Angelo:

On Mon, 2025-01-13 at 15:52 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> Since the SIMPLE_DEV_PM_OPS macro and the pm pointer are anyway
> defined when CONFIG_PM_SLEEP is not set, remove the ifdef for it
> and indicate that the mtk_hdmi_{remove,suspend} functions may be
> unused (as they are, in case PM support is not built-in).
> 
> While at it, to improve readability, also compress the
> SIMPLE_DEV_PM_OPS declaration as it even fits in less
> than 80 columns.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 0ec3bfe528f8..f6b463aaba47 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1692,8 +1692,7 @@ static void mtk_hdmi_remove(struct platform_device *pdev)
>         mtk_hdmi_clk_disable_audio(hdmi);
>  }
> 
> -#ifdef CONFIG_PM_SLEEP
> -static int mtk_hdmi_suspend(struct device *dev)
> +static __maybe_unused int mtk_hdmi_suspend(struct device *dev)

I see many driver use CONFIG_PM_SLEEP,
and SIMPLE_DEV_PM_OPS() is defined to support suspend/resume function is not exist when CONFIG_PM_SLEEP is not defined,
so there is no strong purpose to apply this patch.

Regards,
CK

>  {
>         struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> 
> @@ -1702,7 +1701,7 @@ static int mtk_hdmi_suspend(struct device *dev)
>         return 0;
>  }
> 
> -static int mtk_hdmi_resume(struct device *dev)
> +static __maybe_unused int mtk_hdmi_resume(struct device *dev)
>  {
>         struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
>         int ret = 0;
> @@ -1715,9 +1714,8 @@ static int mtk_hdmi_resume(struct device *dev)
> 
>         return 0;
>  }
> -#endif
> -static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
> -                        mtk_hdmi_suspend, mtk_hdmi_resume);
> +
> +static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops, mtk_hdmi_suspend, mtk_hdmi_resume);
> 
>  static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701 = {
>         .tz_disabled = true,
> --
> 2.47.0
> 

Re: [PATCH v5 25/34] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP
Posted by AngeloGioacchino Del Regno 10 months, 1 week ago
Il 07/02/25 07:10, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
> 
> On Mon, 2025-01-13 at 15:52 +0100, AngeloGioacchino Del Regno wrote:
>> External email : Please do not click links or open attachments until you have verified the sender or the content.
>>
>>
>> Since the SIMPLE_DEV_PM_OPS macro and the pm pointer are anyway
>> defined when CONFIG_PM_SLEEP is not set, remove the ifdef for it
>> and indicate that the mtk_hdmi_{remove,suspend} functions may be
>> unused (as they are, in case PM support is not built-in).
>>
>> While at it, to improve readability, also compress the
>> SIMPLE_DEV_PM_OPS declaration as it even fits in less
>> than 80 columns.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_hdmi.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> index 0ec3bfe528f8..f6b463aaba47 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -1692,8 +1692,7 @@ static void mtk_hdmi_remove(struct platform_device *pdev)
>>          mtk_hdmi_clk_disable_audio(hdmi);
>>   }
>>
>> -#ifdef CONFIG_PM_SLEEP
>> -static int mtk_hdmi_suspend(struct device *dev)
>> +static __maybe_unused int mtk_hdmi_suspend(struct device *dev)
> 
> I see many driver use CONFIG_PM_SLEEP,
> and SIMPLE_DEV_PM_OPS() is defined to support suspend/resume function is not exist when CONFIG_PM_SLEEP is not defined,
> so there is no strong purpose to apply this patch.
> 

This allows to improve the compile time coverage and at the same, this will not
increase the binary size of this driver, because the compiler (or, all of the
linkers associated to the compilers that can be used to build the kernel, anyway!)
will drop the two functions when CONFIG_PM_SLEEP is *not* defined.

Also:
  * This ifdef is redundant, as it's already handled by SIMPLE_DEV_PM_OPS
  * Removing ugly ifdeffery increases human readability while also slightly
    reducing the number of lines

Regards,
Angelo

> Regards,
> CK
> 
>>   {
>>          struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
>>
>> @@ -1702,7 +1701,7 @@ static int mtk_hdmi_suspend(struct device *dev)
>>          return 0;
>>   }
>>
>> -static int mtk_hdmi_resume(struct device *dev)
>> +static __maybe_unused int mtk_hdmi_resume(struct device *dev)
>>   {
>>          struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
>>          int ret = 0;
>> @@ -1715,9 +1714,8 @@ static int mtk_hdmi_resume(struct device *dev)
>>
>>          return 0;
>>   }
>> -#endif
>> -static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
>> -                        mtk_hdmi_suspend, mtk_hdmi_resume);
>> +
>> +static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops, mtk_hdmi_suspend, mtk_hdmi_resume);
>>
>>   static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701 = {
>>          .tz_disabled = true,
>> --
>> 2.47.0
>>
> 

Re: [PATCH v5 25/34] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP
Posted by CK Hu (胡俊光) 10 months, 1 week ago
On Mon, 2025-02-10 at 12:43 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> Il 07/02/25 07:10, CK Hu (胡俊光) ha scritto:
> > Hi, Angelo:
> > 
> > On Mon, 2025-01-13 at 15:52 +0100, AngeloGioacchino Del Regno wrote:
> > > External email : Please do not click links or open attachments until you have verified the sender or the content.
> > > 
> > > 
> > > Since the SIMPLE_DEV_PM_OPS macro and the pm pointer are anyway
> > > defined when CONFIG_PM_SLEEP is not set, remove the ifdef for it
> > > and indicate that the mtk_hdmi_{remove,suspend} functions may be
> > > unused (as they are, in case PM support is not built-in).
> > > 
> > > While at it, to improve readability, also compress the
> > > SIMPLE_DEV_PM_OPS declaration as it even fits in less
> > > than 80 columns.
> > > 
> > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > ---
> > >   drivers/gpu/drm/mediatek/mtk_hdmi.c | 10 ++++------
> > >   1 file changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > index 0ec3bfe528f8..f6b463aaba47 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > > @@ -1692,8 +1692,7 @@ static void mtk_hdmi_remove(struct platform_device *pdev)
> > >          mtk_hdmi_clk_disable_audio(hdmi);
> > >   }
> > > 
> > > -#ifdef CONFIG_PM_SLEEP
> > > -static int mtk_hdmi_suspend(struct device *dev)
> > > +static __maybe_unused int mtk_hdmi_suspend(struct device *dev)
> > 
> > I see many driver use CONFIG_PM_SLEEP,
> > and SIMPLE_DEV_PM_OPS() is defined to support suspend/resume function is not exist when CONFIG_PM_SLEEP is not defined,
> > so there is no strong purpose to apply this patch.
> > 
> 
> This allows to improve the compile time coverage and at the same, this will not
> increase the binary size of this driver, because the compiler (or, all of the
> linkers associated to the compilers that can be used to build the kernel, anyway!)
> will drop the two functions when CONFIG_PM_SLEEP is *not* defined.
> 
> Also:
>   * This ifdef is redundant, as it's already handled by SIMPLE_DEV_PM_OPS
>   * Removing ugly ifdeffery increases human readability while also slightly
>     reducing the number of lines

Add these information in commit message, then

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Regards,
> Angelo
> 
> > Regards,
> > CK
> > 
> > >   {
> > >          struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> > > 
> > > @@ -1702,7 +1701,7 @@ static int mtk_hdmi_suspend(struct device *dev)
> > >          return 0;
> > >   }
> > > 
> > > -static int mtk_hdmi_resume(struct device *dev)
> > > +static __maybe_unused int mtk_hdmi_resume(struct device *dev)
> > >   {
> > >          struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> > >          int ret = 0;
> > > @@ -1715,9 +1714,8 @@ static int mtk_hdmi_resume(struct device *dev)
> > > 
> > >          return 0;
> > >   }
> > > -#endif
> > > -static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
> > > -                        mtk_hdmi_suspend, mtk_hdmi_resume);
> > > +
> > > +static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops, mtk_hdmi_suspend, mtk_hdmi_resume);
> > > 
> > >   static const struct mtk_hdmi_conf mtk_hdmi_conf_mt2701 = {
> > >          .tz_disabled = true,
> > > --
> > > 2.47.0
> > > 
> > 
>