[PATCH 0/3] mt8188: Fix missing reset and clock-names DT properties

[PATCH 0/3] mt8188: Fix missing reset and clock-names DT properties

[Julien Massot]

Hi all,

This patch series addresses some issues found in the MediaTek MT8188 device tree
and its corresponding bindings:

The #reset-cells property was missing in the MT8188 clock controller binding and
device tree nodes. This causes DT validation errors.

The clock-names property was missing from the Global Command Engine (GCE) mailbox node,
which also triggers a dtb-check failure.

Patch 1 updates the binding to declare #reset-cells as a valid property.
Patch 2 adds #reset-cells to all affected nodes in mt8188.dtsi.
Patch 3 fixes the missing clock-names property in the GCE node.

To: kernel@collabora.com
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: Garmin Chang <garmin.chang@mediatek.com>
To: Friday Yang <friday.yang@mediatek.com>
Cc: Conor Dooley <conor.dooley@microchip.com>
Cc: linux-clk@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org

Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Julien Massot (3):
      dt-bindings: clock: mediatek: Add #reset-cells property for MT8188
      arm64: dts: mediatek: mt8188: Add missing #reset-cells property
      arm64: dts: mediatek: mt8188: gce: add missing 'clock-names'

 .../devicetree/bindings/clock/mediatek,mt8188-clock.yaml     |  3 +++
 arch/arm64/boot/dts/mediatek/mt8188.dtsi                     | 12 ++++++++++++
 2 files changed, 15 insertions(+)
---
base-commit: bdd609656ff5573db9ba1d26496a528bdd297cf2
change-id: 20250515-dtb-check-mt8188-5af50d5d5c93

Best regards,
-- 
Julien Massot <julien.massot@collabora.com>

[PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[Julien Massot]

The '#reset-cells' property is required for some of the MT8188
clock controllers, but not listed as a valid property.

Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
index 2985c8c717d72888dd49f1f6249a9e2594d8a38d..5403242545ab12a7736ed4fbac26008aa955c724 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
@@ -52,6 +52,9 @@ properties:
   '#clock-cells':
     const: 1
 
+  '#reset-cells':
+    const: 1
+
 required:
   - compatible
   - reg

-- 
2.49.0

Re: [PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[AngeloGioacchino Del Regno]

Il 15/05/25 15:31, Julien Massot ha scritto:
> The '#reset-cells' property is required for some of the MT8188
> clock controllers, but not listed as a valid property.
> 
> Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
> Signed-off-by: Julien Massot <julien.massot@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On Thu, May 15, 2025 at 03:31:43PM +0200, Julien Massot wrote:
> The '#reset-cells' property is required for some of the MT8188
> clock controllers, but not listed as a valid property.

"required for some" but not marked required on those platforms.
Why not?

> 
> Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>  Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> index 2985c8c717d72888dd49f1f6249a9e2594d8a38d..5403242545ab12a7736ed4fbac26008aa955c724 100644
> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> @@ -52,6 +52,9 @@ properties:
>    '#clock-cells':
>      const: 1
>  
> +  '#reset-cells':
> +    const: 1
> +
>  required:
>    - compatible
>    - reg
> 
> -- 
> 2.49.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[AngeloGioacchino Del Regno]

Il 15/05/25 17:03, Conor Dooley ha scritto:
> On Thu, May 15, 2025 at 03:31:43PM +0200, Julien Massot wrote:
>> The '#reset-cells' property is required for some of the MT8188
>> clock controllers, but not listed as a valid property.
> 
> "required for some" but not marked required on those platforms.
> Why not?
> 

Yeah now that I read that for the third time, the wording is a bit incorrect.

It's not "required", some clock controllers do have reset controllers, but it
is facultative to actually use the latter.

I'm not sure if the ones that do have reset controllers inside should have the
#reset-cells property as required...

Conor, what do you think?

Cheers,
Angelo

>>
>> Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
>> Signed-off-by: Julien Massot <julien.massot@collabora.com>
>> ---
>>   Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
>> index 2985c8c717d72888dd49f1f6249a9e2594d8a38d..5403242545ab12a7736ed4fbac26008aa955c724 100644
>> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
>> @@ -52,6 +52,9 @@ properties:
>>     '#clock-cells':
>>       const: 1
>>   
>> +  '#reset-cells':
>> +    const: 1
>> +
>>   required:
>>     - compatible
>>     - reg
>>
>> -- 
>> 2.49.0
>>

Re: [PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

On Thu, May 15, 2025 at 05:11:13PM +0200, AngeloGioacchino Del Regno wrote:
> Il 15/05/25 17:03, Conor Dooley ha scritto:
> > On Thu, May 15, 2025 at 03:31:43PM +0200, Julien Massot wrote:
> > > The '#reset-cells' property is required for some of the MT8188
> > > clock controllers, but not listed as a valid property.
> > 
> > "required for some" but not marked required on those platforms.
> > Why not?
> > 
> 
> Yeah now that I read that for the third time, the wording is a bit incorrect.
> 
> It's not "required", some clock controllers do have reset controllers, but it
> is facultative to actually use the latter.

I don't think I've ever seen this word before.

> I'm not sure if the ones that do have reset controllers inside should have the
> #reset-cells property as required...
> 
> Conor, what do you think?

If "required for some" in the OP was meant as "permitted for some",
change the wording to that and it is fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: clock: mediatek: Add #reset-cells property for MT8188

[Julien Massot]

Hi Conor, Angelo,
On Thu, 2025-05-15 at 17:17 +0100, Conor Dooley wrote:
> On Thu, May 15, 2025 at 05:11:13PM +0200, AngeloGioacchino Del Regno wrote:
> > Il 15/05/25 17:03, Conor Dooley ha scritto:
> > > On Thu, May 15, 2025 at 03:31:43PM +0200, Julien Massot wrote:
> > > > The '#reset-cells' property is required for some of the MT8188
> > > > clock controllers, but not listed as a valid property.
> > > 
> > > "required for some" but not marked required on those platforms.
> > > Why not?
> > > 
> > 
> > Yeah now that I read that for the third time, the wording is a bit incorrect.
> > 
> > It's not "required", some clock controllers do have reset controllers, but it
> > is facultative to actually use the latter.
> 
> I don't think I've ever seen this word before.
> 
> > I'm not sure if the ones that do have reset controllers inside should have the
> > #reset-cells property as required...
> > 
> > Conor, what do you think?
> 
> If "required for some" in the OP was meant as "permitted for some",
> change the wording to that and it is fine.
Changed 'required' by 'permitted' in v2.

Thanks,
Julien

[PATCH 2/3] arm64: dts: mediatek: mt8188: Add missing #reset-cells property

[Julien Massot]

The binding now require the '#reset-cells' property but the
devicetree has not been updated which trigger dtb-check errors.

Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8188.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
index 296090fbaf4953db8075f72073509b731dc41e51..dec6ce3e94e92c8e1e2c3680cb3584394d9058bd 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -2647,36 +2647,42 @@ imgsys1_dip_top: clock-controller@15110000 {
 			compatible = "mediatek,mt8188-imgsys1-dip-top";
 			reg = <0 0x15110000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		imgsys1_dip_nr: clock-controller@15130000 {
 			compatible = "mediatek,mt8188-imgsys1-dip-nr";
 			reg = <0 0x15130000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		imgsys_wpe1: clock-controller@15220000 {
 			compatible = "mediatek,mt8188-imgsys-wpe1";
 			reg = <0 0x15220000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		ipesys: clock-controller@15330000 {
 			compatible = "mediatek,mt8188-ipesys";
 			reg = <0 0x15330000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		imgsys_wpe2: clock-controller@15520000 {
 			compatible = "mediatek,mt8188-imgsys-wpe2";
 			reg = <0 0x15520000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		imgsys_wpe3: clock-controller@15620000 {
 			compatible = "mediatek,mt8188-imgsys-wpe3";
 			reg = <0 0x15620000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		camsys: clock-controller@16000000 {
@@ -2689,24 +2695,28 @@ camsys_rawa: clock-controller@1604f000 {
 			compatible = "mediatek,mt8188-camsys-rawa";
 			reg = <0 0x1604f000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		camsys_yuva: clock-controller@1606f000 {
 			compatible = "mediatek,mt8188-camsys-yuva";
 			reg = <0 0x1606f000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		camsys_rawb: clock-controller@1608f000 {
 			compatible = "mediatek,mt8188-camsys-rawb";
 			reg = <0 0x1608f000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		camsys_yuvb: clock-controller@160af000 {
 			compatible = "mediatek,mt8188-camsys-yuvb";
 			reg = <0 0x160af000 0 0x1000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		ccusys: clock-controller@17200000 {

-- 
2.49.0

Re: [PATCH 2/3] arm64: dts: mediatek: mt8188: Add missing #reset-cells property

[AngeloGioacchino Del Regno]

Il 15/05/25 15:31, Julien Massot ha scritto:
> The binding now require the '#reset-cells' property but the
> devicetree has not been updated which trigger dtb-check errors.
> 
> Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")

That's not really a fix though, so after you drop the Fixes tag.....

> Signed-off-by: Julien Massot <julien.massot@collabora.com>

....you can get my:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/3] arm64: dts: mediatek: mt8188: Add missing #reset-cells property

[Julien Massot]

Hi Angelo,


On Thu, 2025-05-15 at 16:53 +0200, AngeloGioacchino Del Regno wrote:
> Il 15/05/25 15:31, Julien Massot ha scritto:
> > The binding now require the '#reset-cells' property but the
> > devicetree has not been updated which trigger dtb-check errors.
> > 
> > Fixes: 9a5cd59640ac ("dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188")
> 
> That's not really a fix though, so after you drop the Fixes tag.....
> 
> > Signed-off-by: Julien Massot <julien.massot@collabora.com>
> 
> ....you can get my:
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Thanks dropped the fix tag in the v2

Regards,
Julien

[PATCH 3/3] arm64: dts: mediatek: mt8188: gce: add missing 'clock-names'

[Julien Massot]

The 'clock-names' property is required by the dt binding.

Fix the following dtb-check error:
arch/arm64/boot/dts/mediatek/mt8390-genio-700-evk.dtb: mailbox@10320000: 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#

Fixes: 45682a4fffdd ("arm64: dts: mediatek: mt8188: Add Global Command Engine mailboxes")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 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 dec6ce3e94e92c8e1e2c3680cb3584394d9058bd..d5892ca35bc29f830d7894ba592aca3f58dc3bdc 100644
--- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
@@ -1372,6 +1372,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 {
@@ -1380,6 +1381,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_cluster: scp@10720000 {

-- 
2.49.0

Re: [PATCH 3/3] arm64: dts: mediatek: mt8188: gce: add missing 'clock-names'

[AngeloGioacchino Del Regno]

Il 15/05/25 15:31, Julien Massot ha scritto:
> The 'clock-names' property is required by the dt binding.
> 

I just remembered something....


...care to respin this patch instead?
https://lore.kernel.org/all/20240911104327.123602-1-angelogioacchino.delregno@collabora.com/

Cheers,
Angelo

Re: [PATCH 3/3] arm64: dts: mediatek: mt8188: gce: add missing 'clock-names'

[Julien Massot]

On Thu, 2025-05-15 at 16:52 +0200, AngeloGioacchino Del Regno wrote:
> Il 15/05/25 15:31, Julien Massot ha scritto:
> > The 'clock-names' property is required by the dt binding.
> > 
> 
> I just remembered something....
> 
> 
> ...care to respin this patch instead?
> https://lore.kernel.org/all/20240911104327.123602-1-angelogioacchino.delregno@collabora.com/
> 

Ok, we will resend this patch instead, I dropped the patch 3/3 in the v2.

Regards,
Julien