[PATCH 2/3] memory: mtk-smi: Add a flag skip_rpm

[Xueqi Zhang (张雪琦)]

Hi Angelo,

Thanks for the detailed explanation. I now understand your concern 
about being future-proof and maintaining a unified pm_runtime flow.

In our current MT8196 DTS, these RTFF-managed SMI common nodes 
do not have a 'power-domains' property, as the power is handled 
automatically by the hardware. 

If we follow your suggestion to keep the pm_runtime_{en,dis}able() 
calls but set clk_required = 0, the runtime callback functions 
(smi_common_runtime_resume/suspend) will still be triggered. 

To avoid any potential conflicts with the RTFF hardware's automatic 
register restoration and to save unnecessary software overhead, 
I propose to:
1. Use the 'rtff_managed' flag as you suggested.
2. Set clk_required = 0 when 'rtff_managed' is true.
3. Add a check inside the runtime_resume/suspend callbacks: if 
   'rtff_managed' is true, return immediately without performing 
   any register backup or restoration.

This way, we keep the standard pm_runtime infrastructure as you 
requested, while ensuring the software doesn't interfere with the 
RTFF hardware's operations.

Does this sound reasonable to you?

Thanks,
Xueqi

On Mon, 2026-05-18 at 12:02 +0200, AngeloGioacchino Del Regno wrote:
> On 5/18/26 09:16, Xueqi Zhang (张雪琦) wrote:
> > Hi Angelo,
> > 
> > First of all, please accept my apologies for the delayed response.
> > I
> > have been deeply occupied with MT8196 Aluminium pKVM SMMU and SMI
> > related tasks recently.
> > 
> > Regarding your question, my previous description in the patch was
> > not
> > accurate enough and may have caused some confusion. In fact, not
> > all SMI commons have their backup/restore handled by the RTFF
> > hardware. The SMI commons are distributed across various subsystems
> > (e.g., mminfra, venc, display, cam, etc.). Currently, only the SMI
> > common under the mminfra subsystem is backed up and restored by
> > the RTFF hardware.
> > 
> > Therefore, I believe adding a specific 'skip_rpm' flag is more
> > appropriate here. If we were to differentiate this based on a new
> > MTK_SMI_GEN3 type, it would imply that all SMI common modules of
> > that generation would skip the RPM operations, which is not the
> > intended behavior.
> > 
> > To make this clearer, I plan to update the commit message in the
> > next version as follows:
> > 
> > Subject: memory: mtk-smi: Add skip_rpm flag for certain MT8196 SMI
> > commons
> 
> memory: mtk-smi: Allow no clocks for RTFF managed SMI commons
> 
> > 
> > Body:
> > On MT8196, certain SMI commons are backed up and restored by the
> > RTFF
> > hardware rather than by software.
> > 
> > For these specific SMI commons, software-controlled register backup
> > and restore in the runtime callback is no longer necessary.
> > Therefore,
> > introduce a 'skip_rpm' flag to bypass these redundant RPMoperations
> > for these SMI commons.
> > 
> > What do you think about this approach?
> > 
> 
> That would be kind-of ok, but keep in mind: pm_runtime doesn't only
> manage clocks.
> 
> I think that the best option here would be to allow having no clocks
> instead,
> and to still call pm_runtime_{en,dis}able() - as that would get a bit
> more
> future-proof, should any other (newer, older, etc) SoC need to
> declare any power
> domain but still no clocks.
> 
> So at this point, I think that just doing something like:
> 
> 	if (common->plat->has_gals) {
> 		if (common->plat->rtff_managed) <--- not "skip_rpm"
> 			clk_required = 0;
> 		else if (common->plat->type == MTK_SMI_GEN2)
> 			clk_required = MTK_SMI_COM_GALS_REQ_CLK_NR;
> 		else if (common->plat->type == MTK_SMI_GEN2_SUB_COMM)
> 			clk_required = MTK_SMI_SUB_COM_GALS_REQ_CLK_NR;
> 	}
> 
> should be sufficient (and/or check zero required clocks in
> smi_dts_clk_init).
> 
> Cheers,
> Angelo
> 
> > Thanks,
> > Xueqi
> > 
> > On Thu, 2025-03-20 at 13:11 +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 20/03/25 08:36, Xueqi Zhang ha scritto:
> > > > MT8196 SMI commons is backed up/restored by RTFF HW.
> > > > It doesn't need SW control the register backup/store
> > > > in the runtime callback.Therefore, add a flag skip_rpm
> > > > to help skip RPM operations for SMI commons.
> > > > 
> > > > Signed-off-by: Xueqi Zhang <xueqi.zhang at mediatek.com>
> > > 
> > > So the MT8196 SMI common doesn't require any clocks?
> > > 
> > > That's fine for me, but this looks bloody similar to MT6989's SMI
> > > common, which
> > > is SMI GEN3 and not GEN2....
> > > 
> > > ....so, are you sure that you need a `skip_rpm` flag and not new
> > > MTK_SMI_GEN3 and
> > > MTK_SMI_GEN3_SUB_COMM types? :-)
> > > 
> > > Regards,
> > > Angelo
> > > 
> > > > ---
> > > >    drivers/memory/mtk-smi.c | 11 ++++++++---
> > > >    1 file changed, 8 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-
> > > > smi.c
> > > > index a8f5467d6b31..b9affa3c3185 100644
> > > > --- a/drivers/memory/mtk-smi.c
> > > > +++ b/drivers/memory/mtk-smi.c
> > > > @@ -123,6 +123,7 @@ static const char * const
> > > > mtk_smi_common_clks[]
> > > > = {"apb", "smi", "gals0", "gals1
> > > >    struct mtk_smi_common_plat {
> > > >        enum mtk_smi_type       type;
> > > >        bool                    has_gals;
> > > > +     bool                    skip_rpm;
> > > >        u32                     bus_sel; /* Balance some larbs
> > > > to
> > > > enter mmu0 or mmu1 */
> > > > 
> > > >        const struct mtk_smi_reg_pair   *init;
> > > > @@ -547,6 +548,9 @@ static int mtk_smi_dts_clk_init(struct
> > > > device
> > > > *dev, struct mtk_smi *smi,
> > > >    {
> > > >        int i, ret;
> > > > 
> > > > +     if (smi->plat->skip_rpm)
> > > > +             return 0;
> > > > +
> > > >        for (i = 0; i < clk_nr_required; i++)
> > > >                smi->clks[i].id = clks[i];
> > > >        ret = devm_clk_bulk_get(dev, clk_nr_required, smi-
> > > > >clks);
> > > > @@ -783,7 +787,7 @@ static int mtk_smi_common_probe(struct
> > > > platform_device *pdev)
> > > >        common->dev = dev;
> > > >        common->plat = of_device_get_match_data(dev);
> > > > 
> > > > -     if (common->plat->has_gals) {
> > > > +     if (!common->plat->skip_rpm && common->plat->has_gals) {
> > > >                if (common->plat->type == MTK_SMI_GEN2)
> > > >                        clk_required =
> > > > MTK_SMI_COM_GALS_REQ_CLK_NR;
> > > >                else if (common->plat->type ==
> > > > MTK_SMI_GEN2_SUB_COMM)
> > > > @@ -814,13 +818,14 @@ static int mtk_smi_common_probe(struct
> > > > platform_device *pdev)
> > > >        }
> > > > 
> > > >        /* link its smi-common if this is smi-sub-common */
> > > > -     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM) {
> > > > +     if (common->plat->type == MTK_SMI_GEN2_SUB_COMM &&
> > > > !common-
> > > > > plat->skip_rpm) {
> > > > 
> > > >                ret = mtk_smi_device_link_common(dev, &common-
> > > > > smi_common_dev);
> > > > 
> > > >                if (ret < 0)
> > > >                        return ret;
> > > >        }
> > > > 
> > > > -     pm_runtime_enable(dev);
> > > > +     if (!common->plat->skip_rpm)
> > > > +             pm_runtime_enable(dev);
> > > >        platform_set_drvdata(pdev, common);
> > > >        return 0;
> > > >    }
> > > 
> > > 
> 
>