linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v1 1/5] dt-bindings: arm: mediatek: mmsys: add mt8195 SoC binding
       [not found] ` <20210722092624.14401-2-jason-jh.lin@mediatek.com>
@ 2021-07-23 11:13   ` Enric Balletbo Serra
       [not found]     ` <4c0fe16988c559a5a4b1ce714eeaa31f4628f68f.camel@mediatek.com>
  0 siblings, 1 reply; 2+ messages in thread
From: Enric Balletbo Serra @ 2021-07-23 11:13 UTC (permalink / raw)
  To: jason-jh.lin
  Cc: Rob Herring, Chun-Kuang Hu, Philipp Zabel, devicetree, Jitao shi,
	fshao, David Airlie, singo.chang, linux-kernel, dri-devel,
	Fabien Parent, Nancy.Lin,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Linux ARM

Hi Jason,

Thank you for your patch.

Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj., 22
de jul. 2021 a les 11:26:
>
> There are 2 display hardware path in mt8195, namely vdosys0 and
> vdosys1, so add their definition in mtk-mmsys documentation.
>

Just having 2 display hardware paths is not a reason to have two
compatibles, isn't the IP block the same? Why do you need to introduce
the two compatibles?

Thanks,
  Enric

> Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> ---
> this patch is base on [1][2]
>
> [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> - https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/
> [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> - https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/
> ---
>  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> index 2d4ff0ce387b..0789a9614f12 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> @@ -30,6 +30,8 @@ properties:
>                - mediatek,mt8173-mmsys
>                - mediatek,mt8183-mmsys
>                - mediatek,mt8365-mmsys
> +              - mediatek,mt8195-vdosys0
> +              - mediatek,mt8195-vdosys1
>            - const: syscon
>        - items:
>            - const: mediatek,mt7623-mmsys
> --
> 2.18.0
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v1 1/5] dt-bindings: arm: mediatek: mmsys: add mt8195 SoC binding
       [not found]     ` <4c0fe16988c559a5a4b1ce714eeaa31f4628f68f.camel@mediatek.com>
@ 2021-07-26 10:08       ` Enric Balletbo Serra
  0 siblings, 0 replies; 2+ messages in thread
From: Enric Balletbo Serra @ 2021-07-26 10:08 UTC (permalink / raw)
  To: Jason-JH Lin
  Cc: Rob Herring, Chun-Kuang Hu, Philipp Zabel, devicetree, Jitao shi,
	fshao, David Airlie, singo.chang, linux-kernel, dri-devel,
	Fabien Parent, Nancy.Lin,
	moderated list:ARM/Mediatek SoC support, Matthias Brugger,
	Linux ARM

Hi Jason,

Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dl., 26
de jul. 2021 a les 9:02:
>
> On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> > Hi Jason,
> >
> > Thank you for your patch.
> >
> > Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj., 22
> > de jul. 2021 a les 11:26:
> > >
> > > There are 2 display hardware path in mt8195, namely vdosys0 and
> > > vdosys1, so add their definition in mtk-mmsys documentation.
> > >
> >
> > Just having 2 display hardware paths is not a reason to have two
> > compatibles, isn't the IP block the same? Why do you need to
> > introduce
> > the two compatibles?
> >
> > Thanks,
> >   Enric
> >
>
> Hi Enric,
>
> Thanks for reviewing my patch.
>
> The reason for using two compatibles is that vdosys0 and vdosys1 are
> different IP blocks.
>

With that there are different IP blocks, what do you mean? Do you mean
that there are two completely different blocks with completely
different functionalities?

Or that there is the same IP block twice? I mean, of course, the
registers are different but has exactly the same functionality.

> Because mmsys provides clock control, other display function blocks may
> use them as clock provider.
>
> E.g.
> 1. mmsys with compatible="mediatek,mt8195-vdosys0"
> [v4,1/6] arm64: dts: mt8195: add display node for vdosys0
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/
>
> ovl0: disp_ovl@1c000000 {
>         ...
>         clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
>         ...
> };
>
> 2. mmsys with compatible="mediatek,mt8195-vdosys1"
> [v2,06/14] arm64: dts: mt8195: add display node for vdosys1
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/
>
> vdo1_rdma0: vdo1_rdma@1c104000 {
>         ...
>         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
>         ...
> };
>

Note that I am talking without knowing the hardware in detail, but I
am wondering why I can't have something like this, where every mmsys
is a clock and reset controller provider.

vdosys0: syscon@14000000 {
          compatible = "mediatek,mt8195-mmsys", "syscon";
          reg = <0 0x14000000 0 0x1000>;
          #clock-cells = <1>;
          #reset-cells = <1>;
};

vdosys1: syscon@15000000 {
          compatible = "mediatek,mt8195-mmsys", "syscon";
          reg = <0 0x15000000 0 0x1000>;
          #clock-cells = <1>;
          #reset-cells = <1>;
};

ovl0: disp_ovl@1c000000 {
        ...
       clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
        ...
};

vdo1_rdma0: vdo1_rdma@1c104000 {
        ...
        clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
        ...
};

What are the differences between vdosys0 and vdosys1 from a hardware
point of view?

Cheers,
  Enric

> Regards,
> Jason-JH.Lin
>
> > > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > > ---
> > > this patch is base on [1][2]
> > >
> > > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> > > -
> > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmknH8f2P$
> > >
> > > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> > > -
> > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmju2GBrD$
> > >
> > > ---
> > >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        |
> > > 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > index 2d4ff0ce387b..0789a9614f12 100644
> > > ---
> > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > +++
> > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > @@ -30,6 +30,8 @@ properties:
> > >                - mediatek,mt8173-mmsys
> > >                - mediatek,mt8183-mmsys
> > >                - mediatek,mt8365-mmsys
> > > +              - mediatek,mt8195-vdosys0
> > > +              - mediatek,mt8195-vdosys1
> > >            - const: syscon
> > >        - items:
> > >            - const: mediatek,mt7623-mmsys
> > > --
> > > 2.18.0
> > >
> --

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-26 10:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210722092624.14401-1-jason-jh.lin@mediatek.com>
     [not found] ` <20210722092624.14401-2-jason-jh.lin@mediatek.com>
2021-07-23 11:13   ` [PATCH v1 1/5] dt-bindings: arm: mediatek: mmsys: add mt8195 SoC binding Enric Balletbo Serra
     [not found]     ` <4c0fe16988c559a5a4b1ce714eeaa31f4628f68f.camel@mediatek.com>
2021-07-26 10:08       ` Enric Balletbo Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).