| Message ID | 20241007163623.3274510-2-markus.stockhausen@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phy: Realtek Otto SerDes: add new driver | expand |
On Mon, 07 Oct 2024 12:36:21 -0400, Markus Stockhausen wrote: > Add bindings for the SerDes of the Realtek Otto platform. These are > network Switch SoCs with up to 52 ports divided into four different > model lines. > > Changes in v2: > - new subject > - removed patch command sequences > - renamed parameter controlled-ports to realtek,controlled-ports > > Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de> > --- > .../bindings/phy/realtek,otto-serdes.yaml | 95 +++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml: 'reguired' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'not', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref'] from schema $id: http://devicetree.org/meta-schemas/base.yaml# Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:41.33-46.11: ERROR (duplicate_label): /example-1/serdes@1b00a000: Duplicate label 'serdes' on /example-1/serdes@1b00a000 and /example-0/serdes@1b00e780 Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:64.33-69.11: ERROR (duplicate_label): /example-2/serdes@1b0003b0: Duplicate label 'serdes' on /example-2/serdes@1b0003b0 and /example-0/serdes@1b00e780 Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dts:87.33-92.11: ERROR (duplicate_label): /example-3/serdes@1b005638: Duplicate label 'serdes' on /example-3/serdes@1b005638 and /example-0/serdes@1b00e780 ERROR: Input tree has errors, aborting (use -f to force output) make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/phy/realtek,otto-serdes.example.dtb] Error 2 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2 make: *** [Makefile:224: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241007163623.3274510-2-markus.stockhausen@gmx.de The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 07/10/2024 18:36, Markus Stockhausen wrote: > Add bindings for the SerDes of the Realtek Otto platform. These are > network Switch SoCs with up to 52 ports divided into four different > model lines. > > Changes in v2: > - new subject > - removed patch command sequences > - renamed parameter controlled-ports to realtek,controlled-ports Changelog goes under ---. > > Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de> > --- > .../bindings/phy/realtek,otto-serdes.yaml | 95 +++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > > diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > new file mode 100644 > index 000000000000..a72ac206b35f > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml Nothing improved. > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek Otto SerDes controller > + > +maintainers: > + - Markus Stockhausen <markus.stockhausen@gmx.de> > + > +description: > + The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series > + have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B, > + RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture. > + > + Although these SerDes controllers have common basics they behave differently on the SoC families > + and rely on heavy register magic. To keep the driver clean it can load patch sequences from > + devictree and execute them during the controller actions like phy_init(), ... > + > + The driver exposes the SerDes registers different from the hardware but instead gives a > + consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the > + RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has > + 14 ports and 192 pages. Totally messed wrapping. Please wrap your code as Linux coding style. > + > +properties: > + $nodename: > + pattern: "^serdes@[0-9a-f]+$" > + > + compatible: > + items: > + - enum: > + - realtek,rtl8380-serdes > + - realtek,rtl8390-serdes > + - realtek,rtl9300-serdes > + - realtek,rtl9310-serdes > + > + reg: > + items: > + description: > + The primary SerDes paged register memory location. Other SerDes control and management > + registers are distributed all over the I/O memory space and are identified by the driver. What happened here? I asked only about |. Why are you adding unrelated changes? Anyway, still not tested and still does not look any other binding. > + > + "#phy-cells": > + const: 4 > + description: > + The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa > + 1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this > + SerDes is working for, the fourth number is the last switch port the SerDes is working for. > + > + realtek,controlled-ports: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + A bit mask defining the ports that are actively controlled by the driver. In case a bit is Driver? Bindings are not about drivers. Drop. I don't think you implemented my feedback. > + not set the driver will only process read operations on the SerDes. This is just in case the > + SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver > + will control all SerDes. > + > +reguired: > + - compatible > + - reg > + - "#phy-cells" > + > +additionalProperties: > + false Please open any existing binding and do it like there. Or start from scratch from example-schema. > + > +examples: > + - | > + serdes: serdes@1b00e780 { > + compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes"; > + reg = <0x1b00e780 0x1200>; > + controlled-ports = <0x003f>; > + #phy-cells = <4>; > + }; One example is enough. ... and still not tested. Sending untested code is waste of our time. Best regards, Krzysztof
On Mon, Oct 07, 2024 at 12:36:21PM -0400, Markus Stockhausen wrote: > Add bindings for the SerDes of the Realtek Otto platform. These are > network Switch SoCs with up to 52 ports divided into four different > model lines. > > Changes in v2: > - new subject > - removed patch command sequences > - renamed parameter controlled-ports to realtek,controlled-ports > > Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de> > --- > .../bindings/phy/realtek,otto-serdes.yaml | 95 +++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > > diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > new file mode 100644 > index 000000000000..a72ac206b35f > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek Otto SerDes controller > + > +maintainers: > + - Markus Stockhausen <markus.stockhausen@gmx.de> > + > +description: You need '>' on the end if you want to preserve paragraphs. > + The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series > + have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B, > + RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture. > + > + Although these SerDes controllers have common basics they behave differently on the SoC families > + and rely on heavy register magic. To keep the driver clean it can load patch sequences from > + devictree and execute them during the controller actions like phy_init(), ... > + > + The driver exposes the SerDes registers different from the hardware but instead gives a > + consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the > + RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has > + 14 ports and 192 pages. > + > +properties: > + $nodename: > + pattern: "^serdes@[0-9a-f]+$" The node name for phy providers is 'phy'. > + > + compatible: > + items: > + - enum: > + - realtek,rtl8380-serdes > + - realtek,rtl8390-serdes > + - realtek,rtl9300-serdes > + - realtek,rtl9310-serdes > + > + reg: > + items: > + description: > + The primary SerDes paged register memory location. Other SerDes control and management > + registers are distributed all over the I/O memory space and are identified by the driver. > + > + "#phy-cells": > + const: 4 > + description: > + The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa > + 1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this > + SerDes is working for, the fourth number is the last switch port the SerDes is working for. > + > + realtek,controlled-ports: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + A bit mask defining the ports that are actively controlled by the driver. In case a bit is > + not set the driver will only process read operations on the SerDes. This is just in case the > + SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver > + will control all SerDes. > + > +reguired: > + - compatible > + - reg > + - "#phy-cells" > + > +additionalProperties: > + false One line like *everywhere* else. > + > +examples: > + - | > + serdes: serdes@1b00e780 { Drop unused labels. > + compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes"; > + reg = <0x1b00e780 0x1200>; > + controlled-ports = <0x003f>; > + #phy-cells = <4>; > + }; > + - | > + serdes: serdes@1b00a000 { > + compatible = "realtek,rtl8390-serdes", "realtek,otto-serdes"; > + reg = <0x1b00a000 0x1c00>; > + controlled-ports = <0x3fff>; > + #phy-cells = <4>; > + }; > + - | > + serdes: serdes@1b0003b0 { > + compatible = "realtek,rtl9300-serdes", "realtek,otto-serdes"; > + reg = <0x1b0003b0 0x8>; > + controlled-ports = <0x0fff>; > + #phy-cells = <4>; > + }; > + - | > + serdes: serdes@1b005638 { > + compatible = "realtek,rtl9310-serdes", "realtek,otto-serdes"; > + reg = <0x1b005638 0x8>; > + controlled-ports = <0x3fff>; > + #phy-cells = <4>; > + }; You don't need 4 examples that are essentially the same. > -- > 2.46.2 >
> -----Ursprüngliche Nachricht----- > Von: Krzysztof Kozlowski <krzk@kernel.org> > Gesendet: Montag, 7. Oktober 2024 21:26 > An: Markus Stockhausen <markus.stockhausen@gmx.de>; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org > Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding > > ... and still not tested. Sending untested code is waste of our time. Hi Krzysztof, appreciate your feedback and I do not want to waste your time. My fixes where a mix of your feedback and some half-baked "make dt_binding_check" feedbacks (because packages where missing). My fault and sorry fort he noise. To get next version in better shape two questions regarding your feedback: 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. So I designed the comments in the driver. Does devicetree differ from that? 2 "Bindings vs drivers". The idea about controlled ports came from other bindings. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/st,stih407-irq-syscfg.yaml?h=v6.12-rc2 E.g. st,invert-ext. Something like this will be needed in the future because the SerDes allow to swap polarity which must be changed depending on the switch design. How to do this? Best regards. Markus
On 08/10/2024 07:38, markus.stockhausen@gmx.de wrote: >> -----Ursprüngliche Nachricht----- >> Von: Krzysztof Kozlowski <krzk@kernel.org> >> Gesendet: Montag, 7. Oktober 2024 21:26 >> An: Markus Stockhausen <markus.stockhausen@gmx.de>; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org >> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding >> >> ... and still not tested. Sending untested code is waste of our time. > > Hi Krzysztof, > > appreciate your feedback and I do not want to waste your time. My fixes where a mix > of your feedback and some half-baked "make dt_binding_check" feedbacks (because > packages where missing). My fault and sorry fort he noise. > > To get next version in better shape two questions regarding your feedback: > > 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. > So I designed the comments in the driver. Does devicetree differ from that? checkpatch is not a coding style. I asked to follow coding style, please read entire document in Documentation/process. > > 2 "Bindings vs drivers". The idea about controlled ports came from other bindings. Entire property description speaks about driver, not bindings. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/st,stih407-irq-syscfg.yaml?h=v6.12-rc2 stih is rather poor example to use. The property was added in 2015 (!) without review (!!!). > E.g. st,invert-ext. Something like this will be needed in the future because the > SerDes allow to swap polarity which must be changed depending on the switch > design. How to do this? I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver. I do not understand how polarity has anything to do with U-Boot configuring serdes. Best regards, Krzysztof
> -----Ursprüngliche Nachricht----- > Von: Krzysztof Kozlowski <krzk@kernel.org> > Gesendet: Dienstag, 8. Oktober 2024 08:17 > An: markus.stockhausen@gmx.de; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org > Betreff: Re: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding > > On 08/10/2024 07:38, markus.stockhausen@gmx.de wrote: > >> -----Ursprüngliche Nachricht----- > >> Von: Krzysztof Kozlowski <krzk@kernel.org> > >> Gesendet: Montag, 7. Oktober 2024 21:26 > >> An: Markus Stockhausen <markus.stockhausen@gmx.de>; > >> linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; > >> devicetree@vger.kernel.org > >> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes > >> PHY binding > >> > >> ... and still not tested. Sending untested code is waste of our time. > > > > Hi Krzysztof, > > > > appreciate your feedback and I do not want to waste your time. My > > fixes where a mix of your feedback and some half-baked "make > > dt_binding_check" feedbacks (because packages where missing). My fault and sorry fort he noise. > > > > To get next version in better shape two questions regarding your feedback: > > > > 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. > > So I designed the comments in the driver. Does devicetree differ from that? > > checkpatch is not a coding style. I asked to follow coding style, please read entire document in Documentation/process. Understood. > > > > 2 "Bindings vs drivers". The idea about controlled ports came from other bindings. > > Entire property description speaks about driver, not bindings. > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre > > e/Documentation/devicetree/bindings/interrupt-controller/st,stih407-ir > > q-syscfg.yaml?h=v6.12-rc2 > > stih is rather poor example to use. The property was added in 2015 (!) without review (!!!). > > > > E.g. st,invert-ext. Something like this will be needed in the future > > because the SerDes allow to swap polarity which must be changed > > depending on the switch design. How to do this? > > I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver. > > I do not understand how polarity has anything to do with U-Boot configuring serdes. Maybe my lack of knowledge in platform driver programming or the naming conventions leads to confusion. I'm searching for knobs to control the behaviour of the SerDes depending on the hardware. Two examples are (more may come): - "ignore SerDes X": because the provided patch sequence confuses the SerDes and overwrites registers with wrong values that vendor patched U-Boot has setup correctly before. - "reverse polarity of SerDes X": same goes here. Some boards need inverted signalling on some of the SerDes to work properly. This must be configurable somehow. Looking at some more modern implementation/documentation I need soemthing like in realtek,usb2phy.yaml - e.g. realtek,driving-level-compensate. Should I just leave "driver" out of the description? Best regards. Markus
On 07/10/2024 18:36, Markus Stockhausen wrote: > Add bindings for the SerDes of the Realtek Otto platform. These are > network Switch SoCs with up to 52 ports divided into four different > model lines. > > Changes in v2: > - new subject > - removed patch command sequences > - renamed parameter controlled-ports to realtek,controlled-ports <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument, so you will not CC people just because they made one commit years ago). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. </form letter> I already commented on this. The Cc list is still wrong. Please start using tools which do it automatically and you do not have to perform any choice of addresses. Best regards, Krzysztof
On 07/10/2024 18:36, Markus Stockhausen wrote: > + compatible: > + items: > + - enum: > + - realtek,rtl8380-serdes > + - realtek,rtl8390-serdes > + - realtek,rtl9300-serdes It turns out there is no chip like 9300. Align with Chris Packham, because it is not a superposition-SoC (exists and does not exist the same time, unless you check in given binding). Best regards, Krzysztof
On 08/10/2024 08:56, markus.stockhausen@gmx.de wrote: >> >>> E.g. st,invert-ext. Something like this will be needed in the future >>> because the SerDes allow to swap polarity which must be changed >>> depending on the switch design. How to do this? >> >> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver. >> >> I do not understand how polarity has anything to do with U-Boot configuring serdes. > > Maybe my lack of knowledge in platform driver programming or the naming > conventions leads to confusion. I'm searching for knobs to control the behaviour > of the SerDes depending on the hardware. Two examples are (more may come): > > - "ignore SerDes X": because the provided patch sequence confuses the SerDes > and overwrites registers with wrong values that vendor patched U-Boot has setup > correctly before. And if someone updates the bootloader to a bit different one, the DTS becomes wrong? How do you handle then same board with two different bootloaders requiring two different DTS? DTS is software-independent description of the hardware, so this does not look like DTS property. > > - "reverse polarity of SerDes X": same goes here. Some boards need inverted > signalling on some of the SerDes to work properly. This must be configurable > somehow. I do not see how this is related to "control ports" property. There are few bindings which already do this, so look at them. Best regards, Krzysztof
> -----Ursprüngliche Nachricht----- > Von: Krzysztof Kozlowski <krzk@kernel.org> > Gesendet: Dienstag, 8. Oktober 2024 10:32 > An: markus.stockhausen@gmx.de; linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org > Betreff: Re: AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding > > On 08/10/2024 08:56, markus.stockhausen@gmx.de wrote: > >> > >>> E.g. st,invert-ext. Something like this will be needed in the future > >>> because the SerDes allow to swap polarity which must be changed > >>> depending on the switch design. How to do this? > >> > >> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver. > >> > >> I do not understand how polarity has anything to do with U-Boot configuring serdes. > > > > Maybe my lack of knowledge in platform driver programming or the > > naming conventions leads to confusion. I'm searching for knobs to > > control the behaviour of the SerDes depending on the hardware. Two examples are (more may come): > > > > - "ignore SerDes X": because the provided patch sequence confuses the > > SerDes and overwrites registers with wrong values that vendor patched > > U-Boot has setup correctly before. > > And if someone updates the bootloader to a bit different one, the DTS > becomes wrong? How do you handle then same board with two different > bootloaders requiring two different DTS? DTS is software-independent > description of the hardware, so this does not look like DTS property. Good point. So the initial idea to provide dynamic patch sequences was the right direction but storing in devicetree is wrong. Like Chris mentioned I would change the code to make use of a dynamically loaded firmware file. - if exist: run sequences from there - if not exist: keep registers as is. Reasonable idea? Best regards. Markus
> -----Ursprüngliche Nachricht----- > Von: Rob Herring <robh@kernel.org> > Gesendet: Montag, 7. Oktober 2024 21:31 > An: Markus Stockhausen <markus.stockhausen@gmx.de> > Cc: linux-phy@lists.infradead.org; chris.packham@alliedtelesis.co.nz; devicetree@vger.kernel.org; krzk@kernel.org > Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding > ... > > + > > +properties: > > + $nodename: > > + pattern: "^serdes@[0-9a-f]+$" > > The node name for phy providers is 'phy'. Hi Rob, I found different configs in other files. E.g. - torrent-phy@f0fb500000 - serdes: serdes@e2004010 - serdes_phy: phy@8901000 Do I understand correctly that I should go with "serdes: phy@1b00e780"? If yes, adapt the $nodename pattern accordingly or drop it as in most other files? Best regards. Markus
Hi Krzysztof, with your feedback on the latest version I will take up the issues from v2 once again. To be sure that I do not miss anything in upcoming v5 I will comment on all your feedback. > > .... > > Changes in v2: > > - new subject > > - removed patch command sequences > > - renamed parameter controlled-ports to realtek,controlled-ports > > Changelog goes under ---. After reading this another 4 times now I think I understand. You mean "put changelog below signed-off-by". Will do with next patch. > > .... > > diff --git > > a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > > b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > > new file mode 100644 > > index 000000000000..a72ac206b35f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml > > Nothing improved. In between renamed to compatible "realtek,rtl8380m-serdes.yaml". I hope that fits the requested naming convention. > > + The driver exposes the SerDes registers different from the hardware > > + but instead gives a consistent view and programming interface. So > > + the RTL838x series has 6 ports and 4 pages, the RTL839x has 14 > > + ports and 12 pages, the RTL930x has 12 ports and 64 pages and the > > + RTL931x has > > + 14 ports and 192 pages. > > Totally messed wrapping. Please wrap your code as Linux coding style. Was restyled in between. If this is still an issue in latest version, please advise. > > + reg: > > + items: > > + description: > > + The primary SerDes paged register memory location. Other SerDes control and management > > + registers are distributed all over the I/O memory space and are identified by the driver. > > What happened here? I asked only about |. Why are you adding unrelated changes? > > Anyway, still not tested and still does not look any other binding. Has been tested in between with "make dt_binding_check". > > + realtek,controlled-ports: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + A bit mask defining the ports that are actively controlled by > > + the driver. In case a bit is > > Driver? Bindings are not about drivers. Drop. > > I don't think you implemented my feedback. All these have been removed. > > +additionalProperties: > > + false > > Please open any existing binding and do it like there. Or start from scratch from example-schema. Was converted to one line. > > + > > +examples: > > + - | > > + serdes: serdes@1b00e780 { > > + compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes"; > > + reg = <0x1b00e780 0x1200>; > > + controlled-ports = <0x003f>; > > + #phy-cells = <4>; > > + }; > > One example is enough. Only one example left. Best regards. Markus
On 16/10/2024 17:30, markus.stockhausen@gmx.de wrote: > Hi Krzysztof, > > with your feedback on the latest version I will take up the issues from > v2 once again. To be sure that I do not miss anything in upcoming v5 > I will comment on all your feedback. > >>> .... >>> Changes in v2: >>> - new subject >>> - removed patch command sequences >>> - renamed parameter controlled-ports to realtek,controlled-ports >> >> Changelog goes under ---. > > After reading this another 4 times now I think I understand. You mean > "put changelog below signed-off-by". Will do with next patch. --- is under Signed-off-by, so yes, but more importantly under ---. > >>> .... >>> diff --git >>> a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml >>> b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml >>> new file mode 100644 >>> index 000000000000..a72ac206b35f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml >> >> Nothing improved. > > In between renamed to compatible "realtek,rtl8380m-serdes.yaml". I hope > that fits the requested naming convention. Yes. > >>> + The driver exposes the SerDes registers different from the hardware >>> + but instead gives a consistent view and programming interface. So >>> + the RTL838x series has 6 ports and 4 pages, the RTL839x has 14 >>> + ports and 12 pages, the RTL930x has 12 ports and 64 pages and the >>> + RTL931x has >>> + 14 ports and 192 pages. >> >> Totally messed wrapping. Please wrap your code as Linux coding style. > > Was restyled in between. If this is still an issue in latest version, please advise. It's ok, the rest as well. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml new file mode 100644 index 000000000000..a72ac206b35f --- /dev/null +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Realtek Otto SerDes controller + +maintainers: + - Markus Stockhausen <markus.stockhausen@gmx.de> + +description: + The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series + have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B, + RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture. + + Although these SerDes controllers have common basics they behave differently on the SoC families + and rely on heavy register magic. To keep the driver clean it can load patch sequences from + devictree and execute them during the controller actions like phy_init(), ... + + The driver exposes the SerDes registers different from the hardware but instead gives a + consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the + RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has + 14 ports and 192 pages. + +properties: + $nodename: + pattern: "^serdes@[0-9a-f]+$" + + compatible: + items: + - enum: + - realtek,rtl8380-serdes + - realtek,rtl8390-serdes + - realtek,rtl9300-serdes + - realtek,rtl9310-serdes + + reg: + items: + description: + The primary SerDes paged register memory location. Other SerDes control and management + registers are distributed all over the I/O memory space and are identified by the driver. + + "#phy-cells": + const: 4 + description: + The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa + 1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this + SerDes is working for, the fourth number is the last switch port the SerDes is working for. + + realtek,controlled-ports: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + A bit mask defining the ports that are actively controlled by the driver. In case a bit is + not set the driver will only process read operations on the SerDes. This is just in case the + SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver + will control all SerDes. + +reguired: + - compatible + - reg + - "#phy-cells" + +additionalProperties: + false + +examples: + - | + serdes: serdes@1b00e780 { + compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes"; + reg = <0x1b00e780 0x1200>; + controlled-ports = <0x003f>; + #phy-cells = <4>; + }; + - | + serdes: serdes@1b00a000 { + compatible = "realtek,rtl8390-serdes", "realtek,otto-serdes"; + reg = <0x1b00a000 0x1c00>; + controlled-ports = <0x3fff>; + #phy-cells = <4>; + }; + - | + serdes: serdes@1b0003b0 { + compatible = "realtek,rtl9300-serdes", "realtek,otto-serdes"; + reg = <0x1b0003b0 0x8>; + controlled-ports = <0x0fff>; + #phy-cells = <4>; + }; + - | + serdes: serdes@1b005638 { + compatible = "realtek,rtl9310-serdes", "realtek,otto-serdes"; + reg = <0x1b005638 0x8>; + controlled-ports = <0x3fff>; + #phy-cells = <4>; + };
Add bindings for the SerDes of the Realtek Otto platform. These are network Switch SoCs with up to 52 ports divided into four different model lines. Changes in v2: - new subject - removed patch command sequences - renamed parameter controlled-ports to realtek,controlled-ports Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de> --- .../bindings/phy/realtek,otto-serdes.yaml | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml -- 2.46.2