[PATCH 0/2] mips: realtek: Add reboot support

[PATCH 0/2] mips: realtek: Add reboot support

[Chris Packham]

The system reboot on the cameo-rtl9302c (and presumably many other boards based
on the realtek reference design) is connected via the switch reset register
(RST_GLB_CTRL_0).

Because the switch register block encompasses a number of functions that would
normally be separate perhipherals I've represented it as a syscon node. Right
now the only peripheral I've added is the reset (using syscon-reboot). The
binding and syscon node will be expanded in the future to add some additional
functions (e.g. I2C, GPIO, MDIO).

Chris Packham (2):
  dt-bindings: mfd: Add Realtek switch
  mips: dts: realtek: Add syscon-reboot node

 .../bindings/mfd/realtek,rtl9302c-switch.yaml | 50 +++++++++++++++++++
 arch/mips/boot/dts/realtek/rtl930x.dtsi       | 11 ++++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,rtl9302c-switch.yaml

-- 
2.46.1

[PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Chris Packham]

Add device tree schema for the Realtek switch. Currently the only
supported feature is the syscon-reboot which is needed to be able to
reboot the board.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - fix schema id to match filename
    Changes in v2:
    - use filename that matches the compatible
    - put maintainers after title
    - remove unnecessary label in example
    - Rework description to focus on what is implemented rather than what
      may be implemented in the future.

 .../bindings/mfd/realtek,rtl9302c-switch.yaml | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,rtl9302c-switch.yaml

diff --git a/Documentation/devicetree/bindings/mfd/realtek,rtl9302c-switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,rtl9302c-switch.yaml
new file mode 100644
index 000000000000..2d20dd07a7e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/realtek,rtl9302c-switch.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/realtek,rtl9302c-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek Switch with Internal CPU
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+description:
+  The RTL9302 is an Ethernet switch with an integrated CPU. A number of
+  different peripherals are accessed through a common register block,
+  represented here as a syscon node.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtl9302c-switch
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  reboot:
+    $ref: /schemas/power/reset/syscon-reboot.yaml#
+
+required:
+  - compatible
+  - reg
+  - reboot
+
+additionalProperties: false
+
+examples:
+  - |
+    ethernet-switch@1b000000 {
+      compatible = "realtek,rtl9302c-switch", "syscon", "simple-mfd";
+      reg = <0x1b000000 0x10000>;
+
+      reboot {
+        compatible = "syscon-reboot";
+        offset = <0x0c>;
+        value = <0x01>;
+      };
+    };
+
-- 
2.46.1

[PATCH 2/2] mips: dts: realtek: Add syscon-reboot node

[Chris Packham]

The board level reset on systems using the RTL9302 can be driven via the
switch. Use a syscon-reboot node to represent this.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - None
    Changes in v2:
    - drop redundant status = "okay"

 arch/mips/boot/dts/realtek/rtl930x.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/mips/boot/dts/realtek/rtl930x.dtsi b/arch/mips/boot/dts/realtek/rtl930x.dtsi
index f271940f82be..cf1b38b6c353 100644
--- a/arch/mips/boot/dts/realtek/rtl930x.dtsi
+++ b/arch/mips/boot/dts/realtek/rtl930x.dtsi
@@ -29,6 +29,17 @@ lx_clk: clock-175mhz {
 		#clock-cells = <0>;
 		clock-frequency  = <175000000>;
 	};
+
+	switch0: switch@1b000000 {
+		compatible = "realtek,rtl9302c-switch", "syscon", "simple-mfd";
+		reg = <0x1b000000 0x10000>;
+
+		reboot {
+			compatible = "syscon-reboot";
+			offset = <0x0c>;
+			value = <0x01>;
+		};
+	};
 };
 
 &soc {
-- 
2.46.1

[PATCH 0/2] mips: realtek: Add reboot support

[Chris Packham]

The system reboot on the cameo-rtl9302c (and presumably many other boards based
on the realtek reference design) is connected via the switch reset register
(RST_GLB_CTRL_0). This adds a basic ethernet-switch node with a reboot node so
we can make uses of the syscon-reboot register.

Chris Packham (2):
  dt-bindings: mfd: Add Realtek switch
  mips: dts: realtek: Add syscon-reboot node

 .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
 .../cameo-rtl9302c-2x-rtl8224-2xge.dts        |  3 ++
 arch/mips/boot/dts/realtek/rtl930x.dtsi       | 11 ++++
 3 files changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml

-- 
2.46.0

[PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Chris Packham]

Add device tree schema for the Realtek switch. Currently the only
supported feature is the syscon-reboot which is needed to be able to
reboot the board.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml

diff --git a/Documentation/devicetree/bindings/mfd/realtek,switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
new file mode 100644
index 000000000000..84b57f87bd3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/realtek,switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek Switch with Internal CPU
+
+description:
+  The RTL9302 ethernet switch has an internal CPU. The switch is a multi-port
+  networking switch that supports many interfaces. Additionally, the device can
+  support MDIO, SPI and I2C busses.
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtl9302c-switch
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  reboot:
+    $ref: /schemas/power/reset/syscon-reboot.yaml#
+
+required:
+  - compatible
+  - reg
+  - reboot
+
+additionalProperties: false
+
+examples:
+  - |
+    switch0: ethernet-switch@1b000000 {
+      compatible = "realtek,rtl9302c-switch", "syscon", "simple-mfd";
+      reg = <0x1b000000 0x10000>;
+
+      reboot {
+        compatible = "syscon-reboot";
+        offset = <0x0c>;
+        value = <0x01>;
+      };
+    };
+
-- 
2.46.0

Re: [PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Krzysztof Kozlowski]

On Mon, Sep 09, 2024 at 01:47:06PM +1200, Chris Packham wrote:
> Add device tree schema for the Realtek switch. Currently the only
> supported feature is the syscon-reboot which is needed to be able to
> reboot the board.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml

Use compatible as filename.

> 
> diff --git a/Documentation/devicetree/bindings/mfd/realtek,switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
> new file mode 100644
> index 000000000000..84b57f87bd3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/realtek,switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek Switch with Internal CPU

What sort of Switch? Like network switch? Then this should be placed in
respective net (or deeper, e.g. net/dsa/) directory.

Maintainers go here. See example-schema.

> +
> +description:
> +  The RTL9302 ethernet switch has an internal CPU. The switch is a multi-port
> +  networking switch that supports many interfaces. Additionally, the device can
> +  support MDIO, SPI and I2C busses.

I don't get why syscon node is called switch. This looks incomplete or
you used description from some other device.

But if this is DSA, then you miss dsa ref and dsa-related properties.

> +
> +maintainers:
> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl9302c-switch
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  reboot:
> +    $ref: /schemas/power/reset/syscon-reboot.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - reboot
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    switch0: ethernet-switch@1b000000 {

Drop unused label.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Chris Packham]

Hi Krzysztof,

On 9/09/24 18:38, Krzysztof Kozlowski wrote:
> On Mon, Sep 09, 2024 at 01:47:06PM +1200, Chris Packham wrote:
>> Add device tree schema for the Realtek switch. Currently the only
>> supported feature is the syscon-reboot which is needed to be able to
>> reboot the board.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
>>   1 file changed, 50 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml
> Use compatible as filename.

My hope was eventually that this would support multiple Realtek 
switches. But sure for now at least I can name it after the one in front 
of me.

>
>> diff --git a/Documentation/devicetree/bindings/mfd/realtek,switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>> new file mode 100644
>> index 000000000000..84b57f87bd3a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=55fe5gyquxahZ_dJqiHMxmkDG8M1MWjoNtZN70yrng&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmfd%2frealtek%2cswitch%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=55fe5gyquxahZ_dJqiHMxmkDG8M1MWjoNoNFvkz8nA&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Realtek Switch with Internal CPU
> What sort of Switch? Like network switch? Then this should be placed in
> respective net (or deeper, e.g. net/dsa/) directory.
Yes network switch. But this is one of those all in one chips that has a 
CPU, network switch and various peripherals. MFD seemed appropriate.
>
> Maintainers go here. See example-schema.

Ack.

>> +
>> +description:
>> +  The RTL9302 ethernet switch has an internal CPU. The switch is a multi-port
>> +  networking switch that supports many interfaces. Additionally, the device can
>> +  support MDIO, SPI and I2C busses.
> I don't get why syscon node is called switch. This looks incomplete or
> you used description from some other device.

Yes I did take a lot of inspiration from the mscc,ocelot. I am working 
on more support for the switch and some of the other peripherals so I 
figured I'd word it towards that end goal. If you prefer I could word 
this more towards the one function (reboot) that is supported right now.

> But if this is DSA, then you miss dsa ref and dsa-related properties.

So far I'm resisting DSA. The usage of the RTL9300 as a SoC+Switch 
doesn't really lend itself to the DSA architecture (there is a external 
CPU mode that would). I think eventually we'd end up with something like 
the mscc,oscelot where both switchdev and DSA usage is supported. There 
would be some properties (e.g. port/phy arrangement) that apply to both 
uses.

I have got a (kind of) working proof of concept switchdev driver which 
has some of the support you've mentioned. It's not really ready so I 
didn't include the dt-binding for that stuff in this patch.

>> +
>> +maintainers:
>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - realtek,rtl9302c-switch
>> +      - const: syscon
>> +      - const: simple-mfd
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  reboot:
>> +    $ref: /schemas/power/reset/syscon-reboot.yaml#
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reboot
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    switch0: ethernet-switch@1b000000 {
> Drop unused label.
Ack.
>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Krzysztof Kozlowski]

On 09/09/2024 22:36, Chris Packham wrote:
> Hi Krzysztof,
> 
> On 9/09/24 18:38, Krzysztof Kozlowski wrote:
>> On Mon, Sep 09, 2024 at 01:47:06PM +1200, Chris Packham wrote:
>>> Add device tree schema for the Realtek switch. Currently the only
>>> supported feature is the syscon-reboot which is needed to be able to
>>> reboot the board.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
>>>   1 file changed, 50 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>> Use compatible as filename.
> 
> My hope was eventually that this would support multiple Realtek 
> switches. But sure for now at least I can name it after the one in front 
> of me.

This might never happen, so unless you document more models now, I
strongly suggest using compatible as filename.

> 
>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/realtek,switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>>> new file mode 100644
>>> index 000000000000..84b57f87bd3a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>>> @@ -0,0 +1,50 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://scanmail.trustwave.com/?c=20988&d=55fe5gyquxahZ_dJqiHMxmkDG8M1MWjoNtZN70yrng&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmfd%2frealtek%2cswitch%2eyaml%23
>>> +$schema: http://scanmail.trustwave.com/?c=20988&d=55fe5gyquxahZ_dJqiHMxmkDG8M1MWjoNoNFvkz8nA&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>> +
>>> +title: Realtek Switch with Internal CPU
>> What sort of Switch? Like network switch? Then this should be placed in
>> respective net (or deeper, e.g. net/dsa/) directory.
> Yes network switch. But this is one of those all in one chips that has a 
> CPU, network switch and various peripherals. MFD seemed appropriate.

There is no such device as MFD. MFD is a Linux framework.

>>
>> Maintainers go here. See example-schema.
> 
> Ack.
> 
>>> +
>>> +description:
>>> +  The RTL9302 ethernet switch has an internal CPU. The switch is a multi-port
>>> +  networking switch that supports many interfaces. Additionally, the device can
>>> +  support MDIO, SPI and I2C busses.
>> I don't get why syscon node is called switch. This looks incomplete or
>> you used description from some other device.
> 
> Yes I did take a lot of inspiration from the mscc,ocelot. I am working 
> on more support for the switch and some of the other peripherals so I 
> figured I'd word it towards that end goal. If you prefer I could word 
> this more towards the one function (reboot) that is supported right now.

Your commit msg is not explaining here much. And "Currently the only
supported" feels like a driver description. We expect bindings to be
complete. It's fine to bring partial description of hardware, but this
should be explained in the commit msg and entire binding should be still
created to accommodate that full description.

However such complex devices like Ocelot should be described fully so we
can easily see how you organize entire binding.

> 
>> But if this is DSA, then you miss dsa ref and dsa-related properties.
> 
> So far I'm resisting DSA. The usage of the RTL9300 as a SoC+Switch 
> doesn't really lend itself to the DSA architecture (there is a external 
> CPU mode that would). I think eventually we'd end up with something like 
> the mscc,oscelot where both switchdev and DSA usage is supported. There 
> would be some properties (e.g. port/phy arrangement) that apply to both 
> uses.

This feels ok, although you really should create complete binding here.

> 
> I have got a (kind of) working proof of concept switchdev driver which 
> has some of the support you've mentioned. It's not really ready so I 
> didn't include the dt-binding for that stuff in this patch.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mfd: Add Realtek switch

[Chris Packham]

Hi Krzysztof,

(sorry, re-sending as plain text)

On 10/09/24 19:26, Krzysztof Kozlowski wrote:
> On 09/09/2024 22:36, Chris Packham wrote:
>> Hi Krzysztof,
>>
>> On 9/09/24 18:38, Krzysztof Kozlowski wrote:
>>> On Mon, Sep 09, 2024 at 01:47:06PM +1200, Chris Packham wrote:
>>>> Add device tree schema for the Realtek switch. Currently the only
>>>> supported feature is the syscon-reboot which is needed to be able to
>>>> reboot the board.
>>>>
>>>> Signed-off-by: Chris Packham<chris.packham@alliedtelesis.co.nz>
>>>> ---
>>>>    .../bindings/mfd/realtek,switch.yaml          | 50 +++++++++++++++++++
>>>>    1 file changed, 50 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>>> Use compatible as filename.
>> My hope was eventually that this would support multiple Realtek
>> switches. But sure for now at least I can name it after the one in front
>> of me.
> This might never happen, so unless you document more models now, I
> strongly suggest using compatible as filename.
Agreed.
>>>> diff --git a/Documentation/devicetree/bindings/mfd/realtek,switch.yaml b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>>>> new file mode 100644
>>>> index 000000000000..84b57f87bd3a
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/realtek,switch.yaml
>>>> @@ -0,0 +1,50 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id:http://scanmail.trustwave.com/?c=20988&d=s_Tf5n4B2HBkm1hFlKUTA3UKwK2Jg2EPHeQMRCQHXQ&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmfd%2frealtek%2cswitch%2eyaml%23
>>>> +$schema:http://scanmail.trustwave.com/?c=20988&d=s_Tf5n4B2HBkm1hFlKUTA3UKwK2Jg2EPHbEEFSRQXw&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>>> +
>>>> +title: Realtek Switch with Internal CPU
>>> What sort of Switch? Like network switch? Then this should be placed in
>>> respective net (or deeper, e.g. net/dsa/) directory.
>> Yes network switch. But this is one of those all in one chips that has a
>> CPU, network switch and various peripherals. MFD seemed appropriate.
> There is no such device as MFD. MFD is a Linux framework.

What I meant was the RTL9302 is a similar class of device to the 
mscc,ocelot which has a binding in 
Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml. If it's a case 
of that being historical baggage then 
Documentation/devicetree/bindings/mips/ might be appropriate for the SoC 
type properties and Documentation/devicetree/bindings/net/ for the 
switch portion.

>>> Maintainers go here. See example-schema.
>> Ack.
>>
>>>> +
>>>> +description:
>>>> +  The RTL9302 ethernet switch has an internal CPU. The switch is a multi-port
>>>> +  networking switch that supports many interfaces. Additionally, the device can
>>>> +  support MDIO, SPI and I2C busses.
>>> I don't get why syscon node is called switch. This looks incomplete or
>>> you used description from some other device.
>> Yes I did take a lot of inspiration from the mscc,ocelot. I am working
>> on more support for the switch and some of the other peripherals so I
>> figured I'd word it towards that end goal. If you prefer I could word
>> this more towards the one function (reboot) that is supported right now.
> Your commit msg is not explaining here much. And "Currently the only
> supported" feels like a driver description. We expect bindings to be
> complete. It's fine to bring partial description of hardware, but this
> should be explained in the commit msg and entire binding should be still
> created to accommodate that full description.
>
> However such complex devices like Ocelot should be described fully so we
> can easily see how you organize entire binding.

I can definitely do a better job of explaining myself in the commit 
message. Right now I just want to have a working reboot.

I'm not really using the "realtek,rtl9302c-switch" compatible for 
anything but I gather using a "syscon" node without a more specific 
compatible is frowned upon (please tell me if I'm wrong). In terms of 
the binding should I just make the description something terse like "The 
RTL9302 ethernet switch has an internal CPU. Some peripherals are 
accessed via a common register block".

>>> But if this is DSA, then you miss dsa ref and dsa-related properties.
>> So far I'm resisting DSA. The usage of the RTL9300 as a SoC+Switch
>> doesn't really lend itself to the DSA architecture (there is a external
>> CPU mode that would). I think eventually we'd end up with something like
>> the mscc,oscelot where both switchdev and DSA usage is supported. There
>> would be some properties (e.g. port/phy arrangement) that apply to both
>> uses.
> This feels ok, although you really should create complete binding here.

This is a bit of a chicken and egg thing. I don't want to commit to a 
binding until I have more code to back it up, but of course I don't want 
to spend a bunch of time writing code for a binding that then needs to 
change when that binding is reviewed.

>> I have got a (kind of) working proof of concept switchdev driver which
>> has some of the support you've mentioned. It's not really ready so I
>> didn't include the dt-binding for that stuff in this patch.
> Best regards,
> Krzysztof
>