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

[AngeloGioacchino Del Regno]

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.


> 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