1. Patchew
  2. linux
  3. [PATCH 00/13] Several fixes and supports for MediaTek MT8188 SoC
  4. View patch

[PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible

Fei Shao posted 13 patches 1 year, 5 months ago
[PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Fei Shao 1 year, 5 months ago 
Use and add "syscon" in VPPSYS node names and compatible to fix errors
from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index 2900d78b7ceb..14e51a11f688 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
 			#clock-cells = <1>;
 		};
 
-		vppsys0: clock-controller@14000000 {
-			compatible = "mediatek,mt8188-vppsys0";
+		vppsys0: syscon@14000000 {
+			compatible = "mediatek,mt8188-vppsys0", "syscon";
 			reg = <0 0x14000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
@@ -1817,8 +1817,8 @@ wpesys_vpp0: clock-controller@14e02000 {
 			#clock-cells = <1>;
 		};
 
-		vppsys1: clock-controller@14f00000 {
-			compatible = "mediatek,mt8188-vppsys1";
+		vppsys1: syscon@14f00000 {
+			compatible = "mediatek,mt8188-vppsys1", "syscon";
 			reg = <0 0x14f00000 0 0x1000>;
 			#clock-cells = <1>;
 		};
-- 
2.46.0.469.g59c65b2a67-goog
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Krzysztof Kozlowski 1 year, 5 months ago 
On 09/09/2024 13:14, Fei Shao wrote:
> Use and add "syscon" in VPPSYS node names and compatible to fix errors
> from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
> 
>  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> index 2900d78b7ceb..14e51a11f688 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
>  			#clock-cells = <1>;
>  		};
>  
> -		vppsys0: clock-controller@14000000 {
> -			compatible = "mediatek,mt8188-vppsys0";
> +		vppsys0: syscon@14000000 {
> +			compatible = "mediatek,mt8188-vppsys0", "syscon";

If this was working before, it looks like this is not a syscon and
bindings need to be fixed.

Best regards,
Krzysztof
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Fei Shao 1 year, 5 months ago 
On Mon, Sep 9, 2024 at 7:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 09/09/2024 13:14, Fei Shao wrote:
> > Use and add "syscon" in VPPSYS node names and compatible to fix errors
> > from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
> >
> > Signed-off-by: Fei Shao <fshao@chromium.org>
> > ---
> >
> >  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> > index 2900d78b7ceb..14e51a11f688 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> > @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
> >                       #clock-cells = <1>;
> >               };
> >
> > -             vppsys0: clock-controller@14000000 {
> > -                     compatible = "mediatek,mt8188-vppsys0";
> > +             vppsys0: syscon@14000000 {
> > +                     compatible = "mediatek,mt8188-vppsys0", "syscon";
>
> If this was working before, it looks like this is not a syscon and
> bindings need to be fixed.

I guess it's because the binding was later updated in commit
26bcd8a53098 ("dt-bindings: arm: mediatek: mmsys: Add VPPSYS
compatible for MT8188"), and the corresponding DT update was unnoticed
at the time.
If that makes sense then this should be a valid fix.

Regards,
Fei

>
> Best regards,
> Krzysztof
>
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Krzysztof Kozlowski 1 year, 5 months ago 
On 10/09/2024 07:12, Fei Shao wrote:
> On Mon, Sep 9, 2024 at 7:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 09/09/2024 13:14, Fei Shao wrote:
>>> Use and add "syscon" in VPPSYS node names and compatible to fix errors
>>> from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
>>>
>>> Signed-off-by: Fei Shao <fshao@chromium.org>
>>> ---
>>>
>>>  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>> index 2900d78b7ceb..14e51a11f688 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>> @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
>>>                       #clock-cells = <1>;
>>>               };
>>>
>>> -             vppsys0: clock-controller@14000000 {
>>> -                     compatible = "mediatek,mt8188-vppsys0";
>>> +             vppsys0: syscon@14000000 {
>>> +                     compatible = "mediatek,mt8188-vppsys0", "syscon";
>>
>> If this was working before, it looks like this is not a syscon and
>> bindings need to be fixed.
> 
> I guess it's because the binding was later updated in commit
> 26bcd8a53098 ("dt-bindings: arm: mediatek: mmsys: Add VPPSYS
> compatible for MT8188"), and the corresponding DT update was unnoticed
> at the time.
> If that makes sense then this should be a valid fix.

Not necessarily. Why not fixing bindings? Prove that bindings are
correct, not DTS, first.

Best regards,
Krzysztof
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Fei Shao 1 year, 5 months ago 
On Tue, Sep 10, 2024 at 3:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 10/09/2024 07:12, Fei Shao wrote:
> > On Mon, Sep 9, 2024 at 7:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 09/09/2024 13:14, Fei Shao wrote:
> >>> Use and add "syscon" in VPPSYS node names and compatible to fix errors
> >>> from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
> >>>
> >>> Signed-off-by: Fei Shao <fshao@chromium.org>
> >>> ---
> >>>
> >>>  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
> >>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>> index 2900d78b7ceb..14e51a11f688 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>> @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
> >>>                       #clock-cells = <1>;
> >>>               };
> >>>
> >>> -             vppsys0: clock-controller@14000000 {
> >>> -                     compatible = "mediatek,mt8188-vppsys0";
> >>> +             vppsys0: syscon@14000000 {
> >>> +                     compatible = "mediatek,mt8188-vppsys0", "syscon";
> >>
> >> If this was working before, it looks like this is not a syscon and
> >> bindings need to be fixed.
> >
> > I guess it's because the binding was later updated in commit
> > 26bcd8a53098 ("dt-bindings: arm: mediatek: mmsys: Add VPPSYS
> > compatible for MT8188"), and the corresponding DT update was unnoticed
> > at the time.
> > If that makes sense then this should be a valid fix.
>
> Not necessarily. Why not fixing bindings? Prove that bindings are
> correct, not DTS, first.

MediaTek's mmsys doesn't merely control clocks, it also provides
display pipeline routing control and other misc control registers, so
it's appropriate to categorize it as a system controller over a clock
controller.
As for vdosys and vppsys, they are likely variants or aliases of mmsys
introduced in their newer SoCs.

That description was updated in commit 1a680aa888d6 ("dt-bindings:
mediatek: Update mmsys binding to reflect it is a system controller"),
so I just assumed it's correct without thinking much...

Regards,
Fei
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Krzysztof Kozlowski 1 year, 4 months ago 
On 10/09/2024 13:06, Fei Shao wrote:
> On Tue, Sep 10, 2024 at 3:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 10/09/2024 07:12, Fei Shao wrote:
>>> On Mon, Sep 9, 2024 at 7:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 09/09/2024 13:14, Fei Shao wrote:
>>>>> Use and add "syscon" in VPPSYS node names and compatible to fix errors
>>>>> from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
>>>>>
>>>>> Signed-off-by: Fei Shao <fshao@chromium.org>
>>>>> ---
>>>>>
>>>>>  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
>>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>>>> index 2900d78b7ceb..14e51a11f688 100644
>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
>>>>> @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
>>>>>                       #clock-cells = <1>;
>>>>>               };
>>>>>
>>>>> -             vppsys0: clock-controller@14000000 {
>>>>> -                     compatible = "mediatek,mt8188-vppsys0";
>>>>> +             vppsys0: syscon@14000000 {
>>>>> +                     compatible = "mediatek,mt8188-vppsys0", "syscon";
>>>>
>>>> If this was working before, it looks like this is not a syscon and
>>>> bindings need to be fixed.
>>>
>>> I guess it's because the binding was later updated in commit
>>> 26bcd8a53098 ("dt-bindings: arm: mediatek: mmsys: Add VPPSYS
>>> compatible for MT8188"), and the corresponding DT update was unnoticed
>>> at the time.
>>> If that makes sense then this should be a valid fix.
>>
>> Not necessarily. Why not fixing bindings? Prove that bindings are
>> correct, not DTS, first.
> 
> MediaTek's mmsys doesn't merely control clocks, it also provides
> display pipeline routing control and other misc control registers, so
> it's appropriate to categorize it as a system controller over a clock
> controller.
> As for vdosys and vppsys, they are likely variants or aliases of mmsys
> introduced in their newer SoCs.

Nothing like that was in the commit msg...

> 
> That description was updated in commit 1a680aa888d6 ("dt-bindings:
> mediatek: Update mmsys binding to reflect it is a system controller"),
> so I just assumed it's correct without thinking much...


Best regards,
Krzysztof
Re: [PATCH 06/13] arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible
Posted by Fei Shao 1 year, 4 months ago 
On Mon, Sep 16, 2024 at 6:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 10/09/2024 13:06, Fei Shao wrote:
> > On Tue, Sep 10, 2024 at 3:19 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 10/09/2024 07:12, Fei Shao wrote:
> >>> On Mon, Sep 9, 2024 at 7:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>
> >>>> On 09/09/2024 13:14, Fei Shao wrote:
> >>>>> Use and add "syscon" in VPPSYS node names and compatible to fix errors
> >>>>> from `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`.
> >>>>>
> >>>>> Signed-off-by: Fei Shao <fshao@chromium.org>
> >>>>> ---
> >>>>>
> >>>>>  arch/arm64/boot/dts/mediatek/mt8188.dtsi | 8 ++++----
> >>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>>>> index 2900d78b7ceb..14e51a11f688 100644
> >>>>> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >>>>> @@ -1799,8 +1799,8 @@ mfgcfg: clock-controller@13fbf000 {
> >>>>>                       #clock-cells = <1>;
> >>>>>               };
> >>>>>
> >>>>> -             vppsys0: clock-controller@14000000 {
> >>>>> -                     compatible = "mediatek,mt8188-vppsys0";
> >>>>> +             vppsys0: syscon@14000000 {
> >>>>> +                     compatible = "mediatek,mt8188-vppsys0", "syscon";
> >>>>
> >>>> If this was working before, it looks like this is not a syscon and
> >>>> bindings need to be fixed.
> >>>
> >>> I guess it's because the binding was later updated in commit
> >>> 26bcd8a53098 ("dt-bindings: arm: mediatek: mmsys: Add VPPSYS
> >>> compatible for MT8188"), and the corresponding DT update was unnoticed
> >>> at the time.
> >>> If that makes sense then this should be a valid fix.
> >>
> >> Not necessarily. Why not fixing bindings? Prove that bindings are
> >> correct, not DTS, first.
> >
> > MediaTek's mmsys doesn't merely control clocks, it also provides
> > display pipeline routing control and other misc control registers, so
> > it's appropriate to categorize it as a system controller over a clock
> > controller.
> > As for vdosys and vppsys, they are likely variants or aliases of mmsys
> > introduced in their newer SoCs.
>
> Nothing like that was in the commit msg...

Just for a record, I've revised the commit message in the newer series:
https://lore.kernel.org/all/20240925110044.3678055-7-fshao@chromium.org/

Thanks,
Fei

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.