| Message ID | 20240909111535.528624-1-fshao@chromium.org |
|---|---|
| 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 09/09/2024 13:14, Fei Shao wrote: > Add performance controller node and performance-domains properties for > CPUFreq support on MT8188 SoC. > > Signed-off-by: Fei Shao <fshao@chromium.org> > --- Order of these patches is not helping. You mix new features with fixes and bindings. Please split fixes from the rest. Bindings are expected to be first in the series. 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 Mon, Sep 9, 2024 at 7:42 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 09/09/2024 13:14, Fei Shao wrote: > > Add performance controller node and performance-domains properties for > > CPUFreq support on MT8188 SoC. > > > > Signed-off-by: Fei Shao <fshao@chromium.org> > > --- > > Order of these patches is not helping. You mix new features with fixes > and bindings. > > Please split fixes from the rest. Bindings are expected to be first in > the series. Understood. I'll resend the fixes and features in separate series, and group the binding changes in the beginning. Thanks for the feedback. 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
Il 09/09/24 13:14, Fei Shao ha scritto: > Add the missing clock-names property for GCE nodes to fix errors from > `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`. I've sent a patch [1] relaxing the requirement for clock-names in the binding. There's no reason to make clock-names mandatory, as there is and there always will be one single clock for each GCE mailbox - and also the driver is not trying to get the clock by name, but rather gets the clock at index 0 anyway. Please drop this patch. Cheers, Angelo [1]: https://lore.kernel.org/all/20240911104327.123602-1-angelogioacchino.delregno@collabora.com/ > > Signed-off-by: Fei Shao <fshao@chromium.org> > --- > > arch/arm64/boot/dts/mediatek/mt8188.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi > index 445d30eee2a1..2900d78b7ceb 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi > @@ -1316,6 +1316,7 @@ gce0: mailbox@10320000 { > interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH 0>; > #mbox-cells = <2>; > clocks = <&infracfg_ao CLK_INFRA_AO_GCE>; > + clock-names = "gce"; > }; > > gce1: mailbox@10330000 { > @@ -1324,6 +1325,7 @@ gce1: mailbox@10330000 { > interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH 0>; > #mbox-cells = <2>; > clocks = <&infracfg_ao CLK_INFRA_AO_GCE2>; > + clock-names = "gce"; > }; > > scp: scp@10500000 {
On Wed, Sep 11, 2024 at 6:46 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 09/09/24 13:14, Fei Shao ha scritto: > > Add the missing clock-names property for GCE nodes to fix errors from > > `make CHECK_DTBS=y mediatek/mt8188-evb.dtb`. > > I've sent a patch [1] relaxing the requirement for clock-names in the binding. > There's no reason to make clock-names mandatory, as there is and there always > will be one single clock for each GCE mailbox - and also the driver is not > trying to get the clock by name, but rather gets the clock at index 0 anyway. > > Please drop this patch. Acknowledged. I'll drop this in the next revision. Thanks, 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
Hi all, This series contains several fixes and patches to the MediaTek MT8188 SoC. It aims to add more fundamental supports and lay the groundwork for introducing MT8188-based boards in the future: - Supports for CPUFreq, SPMI, IOMMU, PWM and audio. - Fixes for CPU big core clusters, missing dma-ranges in soc node and power-off sequence issue in vcodec pipeline. - MT8188 CHECK_DTBS fixes for dt-bindings and device tree. Note that there are more changes for MT8188 SoC on the way - PCIe, MIPI DSI panel, DRM, multimedia encoders and decoders etc. I'll keep working on organizing those changes, and I believe the feedback here will also help me improve those before posting. Thanks! Regards, Fei Fei Shao (13): arm64: dts: mediatek: mt8188: Add CPU performance controller for CPUFreq arm64: dts: mediatek: mt8188: Specify CPU big core cluster arm64: dts: mediatek: mt8188: Add missing dma-ranges to soc node arm64: dts: mediatek: mt8188: Move vdec1 power domain under vdec0 arm64: dts: mediatek: mt8188: Add missing GCE clock names arm64: dts: mediatek: mt8188: Update VPPSYS node name and compatible dt-bindings: power: mediatek: Add another nested power-domain layer arm64: dts: mediatek: mt8188: Add SMI/LARB/IOMMU support arm64: dts: mediatek: mt8188: Add PWM nodes for display backlight dt-bindings: spmi: spmi-mtk-pmif: Add compatible for MT8188 arm64: dts: mediatek: mt8188: Add SPMI support for PMIC control dt-bindings: mailbox: mtk,adsp-mbox: Add compatible for MT8188 arm64: dts: mediatek: mt8188: Add audio support .../bindings/mailbox/mtk,adsp-mbox.yaml | 12 +- .../power/mediatek,power-controller.yaml | 4 + .../bindings/spmi/mtk,spmi-mtk-pmif.yaml | 1 + arch/arm64/boot/dts/mediatek/mt8188.dtsi | 352 +++++++++++++++++- 4 files changed, 350 insertions(+), 19 deletions(-)