Re: Discuss Mediatek Power Domain Driver Support Mulitle Smi Clamp Protection

["Yu-chang Lee (李禹璋)" <Yu-chang.Lee@mediatek.com>]

On Wed, 2023-12-20 at 14:11 +0100, AngeloGioacchino Del Regno wrote:
> Il 20/12/23 10:36, Yu-chang Lee (李禹璋) ha scritto:
> > Dear Markus, Matthias, Angelo:
> > 
> > I am writing to seek your expertise and advice regarding the
> > revision
> > of the Mediatek power domain driver for the MT8188 platform.
> > Specifically, the display power domain requires multiple SMI clamp
> > protections (refer to [1] in mt8188.dts "MT8188_POWER_DOMAIN_DIP"
> > "mediatek,smi"). However, the current version on the main branch,
> > which
> > has already merged SMI and infra bus protection (mainly [2]),
> > complicates the mapping of individual SMIs to their respective bus
> > protections.
> > 
> > While not a complete solution, I believe the patch in [3] may
> > better
> > illustrate my intentions.
> > 
> > Markus has provided some thoughts and recommendations that I want
> > to
> > bring forward, hoping to gain further direction based on your
> > expertise.
> > 
> > 1.My initial thought is to revert the changes made in [2], which
> > separated the SMI infra bus protection operations. This approach
> > would
> > simplify the mapping of multiple SMIs to bus protections by their
> > order
> > in the DTS. However, reverting to an older version may not be
> > desired.
> > 
> 
> Relying on the order of bus protections in DTS can be flaky and prone
> to generate
> future mistakes; remember that whatever we send upstream is something
> that not
> only has to be working, clean, solid and reliable, but we should also
> take care
> of avoiding future mistakes that could happen when somebody from the
> community, or
> other MediaTek engineers, implement new platforms.

> Reverting commits is not prohibited as long as we're not breaking
> support for any
> platform (so, as long as we're not removing features), but still, I
> don't think
> that this is the best solution, honestly.
> 
> Besides, I don't get why the order of SMI phandles is important, can
> you please
> explain why?

I apologize for the confusion, The bp_cfg order in mt8188-pm-domain.h
is important because the order of bus protection is depned on it.
However smi clamp protection should be done before infra one. So rely
on this is also a little shaky I think. Also it will take some overhead
to loop through bp_cfg, mixed with infra and smi, to find the correct
name smi to do clamp protection.

That why the first approach seems straight forward to me because it
separate smi and infra. And we can do different protection with them.

> 
> > 2.The second approach, suggested by Markus, is to introduce
> > "mediatek,smi-names" and convert "mediatek,smi" to an array of
> > phandles. We could add a new field to the struct
> > scpsys_bus_prot_data,
> > such as const char *smi_name, which could be NULL (for device trees
> > with only a single property). This way, we can use the smi-names to
> > specify the desired SMI. While this method retains the current code
> > structure, the implementation may not be as straightforward.
> > 
> 
> To be completely honest, neither of the two approaches are state of
> the art,
> but the mediatek,smi-names one is better than the first, granted that
> we
> really need to guarantee that the SMIs are ordered.
> 
> But again, my question is: why should those be ordered?
> Can't we clamp IMG1 before clamping IMG0?
> 
We can clamp IMG1 before clamping IMG0, I am more concerned with the
order between infra and smi rather than the order between smi. And I
think current structure mixed smi and infra may also introduce
misunderstanding in the future. Maybe separate the bp_cfg and match the
smi name with smi's bus protection step could be a direction to fix the
problem?

Thanks for your timely feedback!

Best Regards,
yu-chang.lee


> Cheers,
> Angelo
> 
> > Attempting to resolve this issue, I find the first option easier
> > and
> > more direct. However, I would greatly appreciate hearing the
> > thoughts
> > of experts on this matter before proceeding with upstreaming.
> > 
> > I look forward to your valued response.
> > 
> > Cheers!
> > Best Regards,
> > Yu-chang.lee
> > 
> > [1]:
> > 
https://urldefense.com/v3/__https://chromium.googlesource.com/chromiumos/third_party/kernel/*/87222665c8f974e4ff9dca408cedf6b540a9e770__;Kw!!CTRNKA9wMg0ARbw!kDNdDHejJnDRc6HgYp1-LOvU7Ij4m23UJizlQOGgaJET_rH1jNegg7tYmhOf8XkHVZF_dqG94-kHyhzX6nZE_6aGNBv2CSyx6g$
> >  
> > [2]:
> > 
https://lore.kernel.org/all/20230918093751.1188668-6-msp@baylibre.com/
> > [3]:
> > 
https://urldefense.com/v3/__https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/*/5098295__;Kw!!CTRNKA9wMg0ARbw!kDNdDHejJnDRc6HgYp1-LOvU7Ij4m23UJizlQOGgaJET_rH1jNegg7tYmhOf8XkHVZF_dqG94-kHyhzX6nZE_6aGNBvmL2Hiaw$
> >  
> 
> 
>