From: Luiz Angelo Daros de Luca <luizluca@gmail.com> Date: 2021-12-28 07:27:09
Schema changes:
- "interrupt-controller" was not added as a required property. It might
still work polling the ports when missing
- "interrupt" property was mentioned but never used. According to its
description, it was assumed it was really "interrupt-parent"
Examples changes:
- renamed "switch_intc" to make it unique between examples
- removed "dsa-mdio" from mdio compatible property
- renamed phy@0 to ethernet-phy@0 (not tested with real HW)
phy@ requires #phy-cells
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
.../bindings/net/dsa/realtek-smi.txt | 240 --------------
.../bindings/net/dsa/realtek-smi.yaml | 310 ++++++++++++++++++
2 files changed, 310 insertions(+), 240 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml
On Tue, Dec 28, 2021 at 8:27 AM Luiz Angelo Daros de Luca
[off-list ref] wrote:
Schema changes:
- "interrupt-controller" was not added as a required property. It might
still work polling the ports when missing
- "interrupt" property was mentioned but never used. According to its
description, it was assumed it was really "interrupt-parent"
Examples changes:
- renamed "switch_intc" to make it unique between examples
- removed "dsa-mdio" from mdio compatible property
- renamed phy@0 to ethernet-phy@0 (not tested with real HW)
phy@ requires #phy-cells
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
There is some confusion with the n+m port description. Some 4+1 means
4 lan + 1 wan while in other cases it means 4 user + 1 ext port, even
in Realtek documentation. The last digit in realtek product numbers is
the port number (0 means 10) and it is the sum of user ports and
external ports. From what I investigated, the last digit numbers
normally mean:
3: 2 user + 1 ext port
4: 2 user + 2 ext port
5: 4 user + 1 ext port
6: 5 user + 1 ext port
7: 5 user + 2 ext port
0: 8 user + 2 ext port.
The description in YAML was from the TXT version but it is a good time
to improve it.
BTW, I couldn't find a datasheet for rtl8366rb. The commit message
says it is from a DIR-685 but wikidevi days that device has a
RTL8366SR, which is described as "SINGLE-CHIP 5+1-PORT 10/100/1000
MBPS SWITCH CONTROLLER WITH DUAL MAC INTERFACES".
Do you have any suggestions?
Regards,
Luiz
There is some confusion with the n+m port description. Some 4+1 means
4 lan + 1 wan while in other cases it means 4 user + 1 ext port, even
in Realtek documentation. The last digit in realtek product numbers is
the port number (0 means 10) and it is the sum of user ports and
external ports. From what I investigated, the last digit numbers
normally mean:
3: 2 user + 1 ext port
4: 2 user + 2 ext port
5: 4 user + 1 ext port
6: 5 user + 1 ext port
7: 5 user + 2 ext port
0: 8 user + 2 ext port.
The description in YAML was from the TXT version but it is a good time
to improve it.
BTW, I couldn't find a datasheet for rtl8366rb. The commit message
says it is from a DIR-685 but wikidevi days that device has a
RTL8366SR, which is described as "SINGLE-CHIP 5+1-PORT 10/100/1000
MBPS SWITCH CONTROLLER WITH DUAL MAC INTERFACES".
Do you have any suggestions?
I think Linus just meant to add spaces around the '+'.
Rob
From: Rob Herring <robh@kernel.org> Date: 2022-01-10 18:20:46
On Tue, Dec 28, 2021 at 04:26:45AM -0300, Luiz Angelo Daros de Luca wrote:
Schema changes:
- "interrupt-controller" was not added as a required property. It might
still work polling the ports when missing
- "interrupt" property was mentioned but never used. According to its
description, it was assumed it was really "interrupt-parent"
Examples changes:
- renamed "switch_intc" to make it unique between examples
- removed "dsa-mdio" from mdio compatible property
- renamed phy@0 to ethernet-phy@0 (not tested with real HW)
phy@ requires #phy-cells
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
.../bindings/net/dsa/realtek-smi.txt | 240 --------------
.../bindings/net/dsa/realtek-smi.yaml | 310 ++++++++++++++++++
2 files changed, 310 insertions(+), 240 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.txt
create mode 100644 Documentation/devicetree/bindings/net/dsa/realtek-smi.yaml
@@ -0,0 +1,310 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/net/dsa/realtek-smi.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Realtek SMI-based Switches++allOf:+-$ref:dsa.yaml#++maintainers:+-Linus Walleij <linus.walleij@linaro.org>++description:+The SMI "Simple Management Interface" is a two-wire protocol using+bit-banged GPIO that while it reuses the MDIO lines MCK and MDIO does+not use the MDIO protocol. This binding defines how to specify the+SMI-based Realtek devices. The realtek-smi driver is a platform driver+and it must be inserted inside a platform node.++properties:+compatible:+oneOf:+-enum:
Don't need oneOf when there is only 1 entry.
quoted hunk
+ - realtek,rtl8365mb+ - realtek,rtl8366+ - realtek,rtl8366rb+ - realtek,rtl8366s+ - realtek,rtl8367+ - realtek,rtl8367b+ - realtek,rtl8368s+ - realtek,rtl8369+ - realtek,rtl8370+ description: |+ realtek,rtl8365mb: 4+1 ports+ realtek,rtl8366:+ realtek,rtl8366rb:+ realtek,rtl8366s: 4+1 ports+ realtek,rtl8367:+ realtek,rtl8367b:+ realtek,rtl8368s: 8 ports+ realtek,rtl8369:+ realtek,rtl8370: 8+2 ports+ reg:+ maxItems: 1++ mdc-gpios:+ description: GPIO line for the MDC clock line.+ maxItems: 1++ mdio-gpios:+ description: GPIO line for the MDIO data line.+ maxItems: 1++ reset-gpios:+ description: GPIO to be used to reset the whole device+ maxItems: 1++ realtek,disable-leds:+ type: boolean+ description: |+ if the LED drivers are not used in the+ hardware design this will disable them so they are not turned on+ and wasting power.++ interrupt-controller:+ type: object+ description: |+ This defines an interrupt controller with an IRQ line (typically+ a GPIO) that will demultiplex and handle the interrupt from the single+ interrupt line coming out of one of the SMI-based chips. It most+ importantly provides link up/down interrupts to the PHY blocks inside+ the ASIC.++ properties:++ interrupt-controller:+ description: see interrupt-controller/interrupts.txt
Don't need generic descriptions. Just 'true' here is fine.
quoted hunk
++ interrupts:+ description: TODO
You have to define how many interrupts and what they are.
'interrupt-parent' is never required. It's valid for the
'interrupt-parent' to be in any parent node.
quoted hunk
+ - interrupt-controller+ - '#address-cells'+ - '#interrupt-cells'++ mdio:+ type: object+ description:+ This defines the internal MDIO bus of the SMI device, mostly for the+ purpose of being able to hook the interrupts to the right PHY and+ the right PHY to the corresponding port.++ properties:+ compatible:+ const: "realtek,smi-mdio"
This is applied to the wrong level. It should be applied to 'mdio' node.
You also need to drop 'http://devicetree.org'. With that, you can drop
most of the above. IOW, just this:
mdio:
$ref: /schemas/net/mdio.yaml#
unevaluatedProperties: false
properties:
compatible:
const: realtek,smi-mdio
On Wed, Jan 5, 2022 at 12:44 AM Luiz Angelo Daros de Luca
[off-list ref] wrote:
BTW, I couldn't find a datasheet for rtl8366rb.
There is none... all I have is a code dump from realtek.
The custom header had to be reverse engineered.
The commit message
says it is from a DIR-685 but wikidevi days that device has a
RTL8366SR, which is described as "SINGLE-CHIP 5+1-PORT 10/100/1000
MBPS SWITCH CONTROLLER WITH DUAL MAC INTERFACES".
The DIR-685 has WAN + 4 x LAN and the WAN port is handled in a separate
register from the LAN ports (suggesting it can also do an optical
line) so I think
it's 4 + 1.
Yours,
Linus Walleij
From: Luiz Angelo Daros de Luca <luizluca@gmail.com> Date: 2022-01-29 16:02:57
Thanks Rob, now that the code side is merged, I'm back to docs.
quoted
+ interrupt-controller:
+ description: see interrupt-controller/interrupts.txt
Don't need generic descriptions. Just 'true' here is fine.
Do you really mean quoted true, like in "description: 'true' "?
Without quotes it will fail
quoted
++ interrupts:+ description: TODO
You have to define how many interrupts and what they are.
I didn't write the interruption code and Linus and Alvin might help here.
The switch has a single interrupt pin that signals an interruption happened.
The code reads a register to multiplex to these interruptions:
INT_TYPE_LINK_STATUS = 0,
INT_TYPE_METER_EXCEED,
INT_TYPE_LEARN_LIMIT,
INT_TYPE_LINK_SPEED,
INT_TYPE_CONGEST,
INT_TYPE_GREEN_FEATURE,
INT_TYPE_LOOP_DETECT,
INT_TYPE_8051,
INT_TYPE_CABLE_DIAG,
INT_TYPE_ACL,
INT_TYPE_RESERVED, /* Unused */
INT_TYPE_SLIENT,
And most of them, but not all, multiplex again to each port.
However, the linux driver today does not care about any of these
interruptions but INT_TYPE_LINK_STATUS. So it simply multiplex only
this the interruption to each port, in a n-cell map (n being number of
ports).
I don't know what to describe here as device-tree should be something
independent of a particular OS or driver.
Anyway, I doubt someone might want to plug one of these interruptions
outside the switch driver. Could it be simple as this:
interrupts:
minItems: 3
maxItems: 10
description:
interrupt mapping one per switch port
Once realtek-smi.yaml settles, I'll also send the realtek-mdio.yaml.
Regards,
Luiz
On 29/01/2022 19:02, Luiz Angelo Daros de Luca wrote:
Thanks Rob, now that the code side is merged, I'm back to docs.
quoted
quoted
+ interrupt-controller:
+ description: see interrupt-controller/interrupts.txt
Don't need generic descriptions. Just 'true' here is fine.
Do you really mean quoted true, like in "description: 'true' "?
Without quotes it will fail
quoted
quoted
++ interrupts:+ description: TODO
You have to define how many interrupts and what they are.
I didn't write the interruption code and Linus and Alvin might help here.
The switch has a single interrupt pin that signals an interruption happened.
The code reads a register to multiplex to these interruptions:
INT_TYPE_LINK_STATUS = 0,
INT_TYPE_METER_EXCEED,
INT_TYPE_LEARN_LIMIT,
INT_TYPE_LINK_SPEED,
INT_TYPE_CONGEST,
INT_TYPE_GREEN_FEATURE,
INT_TYPE_LOOP_DETECT,
INT_TYPE_8051,
INT_TYPE_CABLE_DIAG,
INT_TYPE_ACL,
INT_TYPE_RESERVED, /* Unused */
INT_TYPE_SLIENT,
And most of them, but not all, multiplex again to each port.
However, the linux driver today does not care about any of these
interruptions but INT_TYPE_LINK_STATUS. So it simply multiplex only
this the interruption to each port, in a n-cell map (n being number of
ports).
I don't know what to describe here as device-tree should be something
independent of a particular OS or driver.
Anyway, I doubt someone might want to plug one of these interruptions
outside the switch driver. Could it be simple as this:
interrupts:
minItems: 3
maxItems: 10
description:
interrupt mapping one per switch port
Once realtek-smi.yaml settles, I'll also send the realtek-mdio.yaml.
Why not turn realtek-smi.yaml into realtek.yaml which would also contain information for the mdio interface? The things different with using MDIO are that we don't use the [mdc,mdio,reset]-gpios properties and don't handle the PHYs to the DSA ports. Couldn't you present these differences on a single YAML file?
Arınç
From: Luiz Angelo Daros de Luca <luizluca@gmail.com> Date: 2022-01-29 20:52:57
Why not turn realtek-smi.yaml into realtek.yaml which would also contain
information for the mdio interface? The things different with using MDIO
are that we don't use the [mdc,mdio,reset]-gpios properties and don't
handle the PHYs to the DSA ports. Couldn't you present these differences
on a single YAML file?
Hello, Arinç
realtek-mdio is an mdio driver with a couple of less properties. They
do share a lot of stuff. But I don't know if I can fit the schema
validation into a single file.
YAML files are not simply documentation. They are used to validate DTS
files. But that's still off-topic. Let's finish SMI version first and
then discuss
if the MDIO version should be standalone or merged with SMI.
Regards,
On 1/29/2022 12:52 PM, Luiz Angelo Daros de Luca wrote:
quoted
Why not turn realtek-smi.yaml into realtek.yaml which would also contain
information for the mdio interface? The things different with using MDIO
are that we don't use the [mdc,mdio,reset]-gpios properties and don't
handle the PHYs to the DSA ports. Couldn't you present these differences
on a single YAML file?
Hello, Arinç
realtek-mdio is an mdio driver with a couple of less properties. They
do share a lot of stuff. But I don't know if I can fit the schema
validation into a single file.
YAML files are not simply documentation. They are used to validate DTS
files. But that's still off-topic. Let's finish SMI version first and
then discuss
if the MDIO version should be standalone or merged with SMI.
Your YAML file can cover both types of electrical bus, what you are defining is the layout and the properties of the Ethernet switch Device Tree node which is exactly the same whether the switch is the children of a SPI controller or the children of a MDIO bus controller. If there are properties that only apply to SPI or MDIO, you can make use of conditionals within the YAML file to enforce those. Having a single binding file would be very helpful to make sure all eggs are in the same basket.
--
Florian
From: Luiz Angelo Daros de Luca <luizluca@gmail.com> Date: 2022-01-31 00:50:07
Your YAML file can cover both types of electrical bus, what you are
defining is the layout and the properties of the Ethernet switch Device
Tree node which is exactly the same whether the switch is the children
of a SPI controller or the children of a MDIO bus controller. If there
are properties that only apply to SPI or MDIO, you can make use of
conditionals within the YAML file to enforce those. Having a single
binding file would be very helpful to make sure all eggs are in the same
basket.
If you say it is possible, I'll give it a try. I'll just need a hand
with the interruption section.
Luiz
From: Luiz Angelo Daros de Luca <luizluca@gmail.com> Date: 2022-02-09 08:38:12
If there
are properties that only apply to SPI or MDIO, you can make use of
conditionals within the YAML file to enforce those. Having a single
binding file would be very helpful to make sure all eggs are in the same
basket.
Sorry Florian but I failed to find a way to test the parent node
(platform or mdio) and conditionally offer properties.
What I did was to guess if it is an mdio driver or not by checking the
"reg" property. Is there a better way to solve it?
Luiz
PS: I might post the merged v2 doc soon.
From: Rob Herring <robh@kernel.org> Date: 2022-02-09 15:29:32
On Sat, Jan 29, 2022 at 10:02 AM Luiz Angelo Daros de Luca
[off-list ref] wrote:
Thanks Rob, now that the code side is merged, I'm back to docs.
Sigh, bindings are supposed to be accepted first...
quoted
quoted
+ interrupt-controller:
+ description: see interrupt-controller/interrupts.txt
Don't need generic descriptions. Just 'true' here is fine.
Do you really mean quoted true, like in "description: 'true' "?
Without quotes it will fail
interrupt-controller: true
quoted
quoted
++ interrupts:+ description: TODO
You have to define how many interrupts and what they are.
I didn't write the interruption code and Linus and Alvin might help here.
The switch has a single interrupt pin that signals an interruption happened.
Then it's 1 interrupt?
The code reads a register to multiplex to these interruptions:
INT_TYPE_LINK_STATUS = 0,
INT_TYPE_METER_EXCEED,
INT_TYPE_LEARN_LIMIT,
INT_TYPE_LINK_SPEED,
INT_TYPE_CONGEST,
INT_TYPE_GREEN_FEATURE,
INT_TYPE_LOOP_DETECT,
INT_TYPE_8051,
INT_TYPE_CABLE_DIAG,
INT_TYPE_ACL,
INT_TYPE_RESERVED, /* Unused */
INT_TYPE_SLIENT,
Unless the DT needs to route all these interrupts to multiple nodes,
then the switch needs to be an interrupt-controller.
And most of them, but not all, multiplex again to each port.
Now I'm lost. So it's 1 per port, not 1 for the switch?
However, the linux driver today does not care about any of these
interruptions but INT_TYPE_LINK_STATUS. So it simply multiplex only
this the interruption to each port, in a n-cell map (n being number of
ports).
I don't know what to describe here as device-tree should be something
independent of a particular OS or driver.
You shouldn't need to know what Linux does to figure this out.
Anyway, I doubt someone might want to plug one of these interruptions
outside the switch driver. Could it be simple as this:
interrupts:
minItems: 3
maxItems: 10
description:
interrupt mapping one per switch port
Once realtek-smi.yaml settles, I'll also send the realtek-mdio.yaml.
Regards,
Luiz