Re: [PATCH v3 24/33] drm/mediatek: mtk_hdmi: Remove ifdef for CONFIG_PM_SLEEP

[AngeloGioacchino Del Regno]

Il 23/12/24 07:35, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Tue, 2024-12-17 at 16: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.
> >
> >
> > 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).
>
> 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.
>
> > While at it, to improve readability, also compress the
> > SIMPLE_DEV_PM_OPS declaration as it even fits in less
> > than 80 columns.
>
> You have many typesetting in some patches.
> But I think typesetting is not related to the main purpose of that patch.
> I would like you to gather these typesetting modification to a readability patch.

It's a cleanup per-se, and I think that pushing a commit for one line would be
cluttering the commit history way too much... and it's the same for the other
drivers anyway.

Also, backporting (if needed) is easier when those very small changes are in
tandem with the actual bigger change.

Cheers,
Angelo

> Regards,
> CK
>
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
> >   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 5d46d486c68f..7e4536391cfb 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> > @@ -1694,8 +1694,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);
> >
> > @@ -1704,7 +1703,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;
> > @@ -1717,9 +1716,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