[PATCH 0/2] MIPS: Boston: Fix syscon devicetree binding and node

[PATCH 0/2] MIPS: Boston: Fix syscon devicetree binding and node

[Jiaxun Yang]

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Jiaxun Yang (2):
      dt-bindings: mfd: syscon: Add img,boston-platform-regs
      MIPS: dts: Boston: Add simple-mfd compatible

 Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
 arch/mips/boot/dts/img/boston.dts                 | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
---
base-commit: 6ba6c795dc73c22ce2c86006f17c4aa802db2a60
change-id: 20240513-boston-syscon-8e315e94a894

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>

[PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Jiaxun Yang]

This compatible has been used in arch/mips/boot/dts/img/boston.dts
for a while but never documented properly.

Add it to simple syscon binding.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 7ed12a938baa..034765e6a37b 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -57,6 +57,7 @@ properties:
               - hisilicon,pcie-sas-subctrl
               - hisilicon,peri-subctrl
               - hpe,gxp-sysreg
+              - img,boston-platform-regs
               - intel,lgm-syscon
               - loongson,ls1b-syscon
               - loongson,ls1c-syscon

-- 
2.34.1

[PATCH 2/2] MIPS: dts: Boston: Add simple-mfd compatible

[Jiaxun Yang]

We certainly want subnodes of system-controller node to
be populated, add simple-mfd compatible to make that happen.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/boot/dts/img/boston.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/img/boston.dts b/arch/mips/boot/dts/img/boston.dts
index 72f7605d2e31..f7a2c46f1eb8 100644
--- a/arch/mips/boot/dts/img/boston.dts
+++ b/arch/mips/boot/dts/img/boston.dts
@@ -203,7 +203,7 @@ cpc@16200000 {
 	};
 
 	plat_regs: system-controller@17ffd000 {
-		compatible = "img,boston-platform-regs", "syscon";
+		compatible = "img,boston-platform-regs", "syscon", "simple-mfd";
 		reg = <0x17ffd000 0x1000>;
 
 		clk_boston: clock {

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Krzysztof Kozlowski]

On 13/05/2024 20:58, Jiaxun Yang wrote:
> This compatible has been used in arch/mips/boot/dts/img/boston.dts
> for a while but never documented properly.
> 
> Add it to simple syscon binding.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +

It is documented in clock/img,boston-clock.txt. Please fix/convert/work
on that.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Krzysztof Kozlowski]

On 14/05/2024 11:08, Krzysztof Kozlowski wrote:
> On 13/05/2024 20:58, Jiaxun Yang wrote:
>> This compatible has been used in arch/mips/boot/dts/img/boston.dts
>> for a while but never documented properly.
>>
>> Add it to simple syscon binding.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
> 
> It is documented in clock/img,boston-clock.txt. Please fix/convert/work
> on that.

No, that's different device.

Anyway, this is wrong - does no work with your second patch. Please test
them before sending.

You need proper, dedicated schema.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Jiaxun Yang]

在2024年5月14日五月 上午10:11，Krzysztof Kozlowski写道：
> On 14/05/2024 11:08, Krzysztof Kozlowski wrote:
>> On 13/05/2024 20:58, Jiaxun Yang wrote:
>>> This compatible has been used in arch/mips/boot/dts/img/boston.dts
>>> for a while but never documented properly.
>>>
>>> Add it to simple syscon binding.
>>>
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
>> 
>> It is documented in clock/img,boston-clock.txt. Please fix/convert/work
>> on that.
>
> No, that's different device.
>
> Anyway, this is wrong - does no work with your second patch. Please test
> them before sending.

Hi Krzysztof,

Do you mind telling dumb as me how to test bindings properly?

I tried to run make check_dtbs after applying this patch and that's all
the warnings I got:
```
arch/mips/boot/dts/img/boston.dtb: Warning (interrupt_map): Failed prerequisite 'interrupt_provider'
/home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: /: 'model' is a required property
        from schema $id: http://devicetree.org/schemas/root-node.yaml#
arch/mips/boot/dts/img/boston.dtb: /: failed to match any schema with compatible: ['img,boston']
arch/mips/boot/dts/img/boston.dtb: /cpus/cpu@0: failed to match any schema with compatible: ['img,mips']
arch/mips/boot/dts/img/boston.dtb: /system-controller@17ffd000/clock: failed to match any schema with compatible: ['img,boston-clock']
/home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: uart@17ffe000: $nodename:0: 'uart@17ffe000' does not match '^serial(@.*)?$'
        from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
```

I think there is no new warning introduced by this patch.
Did I miss anything here?

Thanks
- Jiaxun
>
> You need proper, dedicated schema.
>
> Best regards,
> Krzysztof

-- 
- Jiaxun

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Krzysztof Kozlowski]

On 15/05/2024 23:44, Jiaxun Yang wrote:
> 
> 
> 在2024年5月14日五月 上午10:11，Krzysztof Kozlowski写道：
>> On 14/05/2024 11:08, Krzysztof Kozlowski wrote:
>>> On 13/05/2024 20:58, Jiaxun Yang wrote:
>>>> This compatible has been used in arch/mips/boot/dts/img/boston.dts
>>>> for a while but never documented properly.
>>>>
>>>> Add it to simple syscon binding.
>>>>
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
>>>
>>> It is documented in clock/img,boston-clock.txt. Please fix/convert/work
>>> on that.
>>
>> No, that's different device.
>>
>> Anyway, this is wrong - does no work with your second patch. Please test
>> them before sending.
> 
> Hi Krzysztof,
> 
> Do you mind telling dumb as me how to test bindings properly?
> 
> I tried to run make check_dtbs after applying this patch and that's all
> the warnings I got:
> ```
> arch/mips/boot/dts/img/boston.dtb: Warning (interrupt_map): Failed prerequisite 'interrupt_provider'
> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: /: 'model' is a required property
>         from schema $id: http://devicetree.org/schemas/root-node.yaml#
> arch/mips/boot/dts/img/boston.dtb: /: failed to match any schema with compatible: ['img,boston']
> arch/mips/boot/dts/img/boston.dtb: /cpus/cpu@0: failed to match any schema with compatible: ['img,mips']
> arch/mips/boot/dts/img/boston.dtb: /system-controller@17ffd000/clock: failed to match any schema with compatible: ['img,boston-clock']
> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: uart@17ffe000: $nodename:0: 'uart@17ffe000' does not match '^serial(@.*)?$'
>         from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
> ```
> 
> I think there is no new warning introduced by this patch.
> Did I miss anything here?

Indeed there might be no warning for this, because syscon allows a lot,
including children. You need custom binding anyway.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Jiaxun Yang]

在2024年5月17日五月 上午9:58，Krzysztof Kozlowski写道：
> On 15/05/2024 23:44, Jiaxun Yang wrote:
>> 
>> 
>> 在2024年5月14日五月 上午10:11，Krzysztof Kozlowski写道：
>>> On 14/05/2024 11:08, Krzysztof Kozlowski wrote:
>>>> On 13/05/2024 20:58, Jiaxun Yang wrote:
>>>>> This compatible has been used in arch/mips/boot/dts/img/boston.dts
>>>>> for a while but never documented properly.
>>>>>
>>>>> Add it to simple syscon binding.
>>>>>
>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
>>>>
>>>> It is documented in clock/img,boston-clock.txt. Please fix/convert/work
>>>> on that.
>>>
>>> No, that's different device.
>>>
>>> Anyway, this is wrong - does no work with your second patch. Please test
>>> them before sending.
>> 
>> Hi Krzysztof,
>> 
>> Do you mind telling dumb as me how to test bindings properly?
>> 
>> I tried to run make check_dtbs after applying this patch and that's all
>> the warnings I got:
>> ```
>> arch/mips/boot/dts/img/boston.dtb: Warning (interrupt_map): Failed prerequisite 'interrupt_provider'
>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: /: 'model' is a required property
>>         from schema $id: http://devicetree.org/schemas/root-node.yaml#
>> arch/mips/boot/dts/img/boston.dtb: /: failed to match any schema with compatible: ['img,boston']
>> arch/mips/boot/dts/img/boston.dtb: /cpus/cpu@0: failed to match any schema with compatible: ['img,mips']
>> arch/mips/boot/dts/img/boston.dtb: /system-controller@17ffd000/clock: failed to match any schema with compatible: ['img,boston-clock']
>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: uart@17ffe000: $nodename:0: 'uart@17ffe000' does not match '^serial(@.*)?$'
>>         from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>> ```
>> 
>> I think there is no new warning introduced by this patch.
>> Did I miss anything here?
>
> Indeed there might be no warning for this, because syscon allows a lot,
> including children. You need custom binding anyway.

Hi Krzysztof,

I really don't understand the reason behind having a custom binding for
such device. It even doesn't have a driver and purely rely on generic
syscon regmap to work.

I can see devices like mediatek,mt8365-syscfg being described by simple
binding have a similar nature, they all describe a register bank and with
serval sub nodes to describe devices being instanced under that register
block.

What makes img,boston-platform-regs special here?

Thanks
- Jiaxun

>
> Best regards,
> Krzysztof

-- 
- Jiaxun

Re: [PATCH 1/2] dt-bindings: mfd: syscon: Add img,boston-platform-regs

[Krzysztof Kozlowski]

On 17/05/2024 12:15, Jiaxun Yang wrote:
>>>
>>> Do you mind telling dumb as me how to test bindings properly?
>>>
>>> I tried to run make check_dtbs after applying this patch and that's all
>>> the warnings I got:
>>> ```
>>> arch/mips/boot/dts/img/boston.dtb: Warning (interrupt_map): Failed prerequisite 'interrupt_provider'
>>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: /: 'model' is a required property
>>>         from schema $id: http://devicetree.org/schemas/root-node.yaml#
>>> arch/mips/boot/dts/img/boston.dtb: /: failed to match any schema with compatible: ['img,boston']
>>> arch/mips/boot/dts/img/boston.dtb: /cpus/cpu@0: failed to match any schema with compatible: ['img,mips']
>>> arch/mips/boot/dts/img/boston.dtb: /system-controller@17ffd000/clock: failed to match any schema with compatible: ['img,boston-clock']
>>> /home/flygoat/linux-next/arch/mips/boot/dts/img/boston.dtb: uart@17ffe000: $nodename:0: 'uart@17ffe000' does not match '^serial(@.*)?$'
>>>         from schema $id: http://devicetree.org/schemas/serial/8250.yaml#
>>> ```
>>>
>>> I think there is no new warning introduced by this patch.
>>> Did I miss anything here?
>>
>> Indeed there might be no warning for this, because syscon allows a lot,
>> including children. You need custom binding anyway.
> 
> Hi Krzysztof,
> 
> I really don't understand the reason behind having a custom binding for
> such device. It even doesn't have a driver and purely rely on generic
> syscon regmap to work.
> 
> I can see devices like mediatek,mt8365-syscfg being described by simple
> binding have a similar nature, they all describe a register bank and with
> serval sub nodes to describe devices being instanced under that register
> block.
> 
> What makes img,boston-platform-regs special here?

Nothing is special here, I am in writing a patchset which will fix this
up and all will be treated similarly.

Bindings for devices should be specific, which then allows to see that
you do not have dependency of children on the parent, and your current
patch allows basically anything as a child. It allows also an
dependency, even if you did not code it.

Best regards,
Krzysztof