| Message ID | 20240909111535.528624-7-fshao@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Several fixes and supports for MediaTek MT8188 SoC | expand |
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
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 >
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
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
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
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
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>; };
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(-)