Re: [RESEND v17 3/7] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0

[CK Hu]

Hi, Angelo:

On Fri, 2022-04-08 at 10:49 +0200, AngeloGioacchino Del Regno wrote:
> Il 08/04/22 03:28, CK Hu ha scritto:
> > Hi, Jason:
> > 
> > On Thu, 2022-04-07 at 14:27 +0800, Jason-JH Lin wrote:
> > > Hi CK,
> > > 
> > > Thanks for the reviews.
> > > 
> > > On Thu, 2022-04-07 at 13:45 +0800, CK Hu wrote:
> > > > Hi, Jason:
> > > > 
> > > > On Thu, 2022-04-07 at 11:04 +0800, jason-jh.lin wrote:
> > > > > 1. Add mt8195 mmsys compatible for vdosys0.
> > > > > 2. Add mt8195 routing table settings and fix build fail.
> > > > > 3. Add clock name, clock driver name and routing table into
> > > > > the
> > > > > driver data
> > > > >     of mt8195 vdosys0.
> > > > > 4. Add get match data by clock name function and clock
> > > > > platform
> > > > > labels
> > > > >     to identify which mmsys node is corresponding to vdosys0.
> > > > > 
> > > > > Signed-off-by: jason-jh.lin <jason-jh.lin@xxxxxxxxxxxx>
> > > > > ---
> > > > >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   2 +-
> > > > >   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   6 +-
> > > > >   drivers/soc/mediatek/mt8167-mmsys.h         |   2 +-
> > > > >   drivers/soc/mediatek/mt8183-mmsys.h         |   2 +-
> > > > >   drivers/soc/mediatek/mt8186-mmsys.h         |   4 +-
> > > > >   drivers/soc/mediatek/mt8192-mmsys.h         |   4 +-
> > > > >   drivers/soc/mediatek/mt8195-mmsys.h         | 370
> > > > > ++++++++++++++++++++
> > > > >   drivers/soc/mediatek/mt8365-mmsys.h         |   4 +-
> > > > >   drivers/soc/mediatek/mtk-mmsys.c            |  62 ++++
> > > > >   drivers/soc/mediatek/mtk-mmsys.h            |   1 +
> > > > >   drivers/soc/mediatek/mtk-mutex.c            |   8 +-
> > > > >   include/linux/soc/mediatek/mtk-mmsys.h      |  13 +-
> > > > >   12 files changed, 461 insertions(+), 17 deletions(-)
> > > > >   create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h
> > > > > 
> > > > 
> > > > [snip]
> > > > 
> > > > > diff --git a/drivers/soc/mediatek/mtk-mmsys.c
> > > > > b/drivers/soc/mediatek/mtk-mmsys.c
> > > > > index 4fc4c2c9ea20..b2fa239c5f5f 100644
> > > > > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > > > > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > > > > @@ -4,6 +4,8 @@
> > > > >    * Author: James Liao <jamesjj.liao@xxxxxxxxxxxx>
> > > > >    */
> > > > >   
> 
> ..snip..
> 
> > > 
> > > I think there might be another chip that needs to get driver data
> > > by
> > > clk_name .
> > > So I use "clk-mt8195" in clk_driver to identify the corresponding
> > > platform whose clk_name of mmsys is also "cfg_vod0".
> > 
> > We usually don't care the future because the future may not happen.
> > If
> 
> Hello CK,
> 
> I'm sorry, but I really have to disagree here.
> Sure, the future may not happen, but from what I can see, MediaTek's
> commitment
> on upstreaming their SoCs is continuative and they care about the
> future.
> 
> Let's also not forget that these drivers are not on a downstream
> tree, where
> you don't care about the past or the future, but on upstream, where
> you:
> - Definitely care about the past
> - Should care about the future, if you want to avoid commit noise and
>    making big changes to your drivers everytime, which would slow
> down
>    your upstreaming due to reviewers having to put 3x efforts on each
>    iteration.
> 
> And let's also not forget that this being upstream means that these
> drivers
> may (or may not) be extended even by passionate community developers,
> for
> which, having such mechanisms there for other SoCs that MediaTek
> didn't try
> to upstream yet can only be good - and when these are engineered with
> a
> certain flexibility, while keeping the codebase solid, that can only
> be good.
> 
> Besides, if I've misunderstood your "don't care the future"
> statement,
> pretend that I've never replied.

OK, let's break this patch into two patches. The first is to support
mt8195 only with clock name identification. The second patch is to
identify SoC. In this series, we just need the first patch, so move the
second patch to the series of another SoC with multiple mmsys device.
Maybe another SoC with multiple mmsys device has new property which
could be used to identify SoC, so we have no information about what is
the better implementation of second patch. I do really care the future,
but I have no information about the future. Please public any hidden
information so we could have better decision.

Regards,
CK

> 
> 
> > it's sure that would happen, I think clk_driver is not a good
> > choice.
> > For now, the clk_driver name is different for each SoC, but it
> > could be
> > the same for each SoC because only one clock driver would be
> > compiled.
> > I think "compatible" would be different for each SoC.
> > 
> 
> ...but I agree on that one (and I gave my own review and suggestions
> on
> how to improve that situation).
> 
> Regards,
> Angelo