| Message ID | 20220221200102.6290-1-luizluca@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3,1/2] dt-bindings: net: dsa: add new mdio property | expand |
On Mon, Feb 21, 2022 at 05:01:02PM -0300, Luiz Angelo Daros de Luca wrote: > The optional mdio property will be used by dsa switch to configure > slave_mii_bus when the driver does not allocate it during setup. > > Some drivers already offer/require a similar property but, in some > cases, they rely on a compatible string to identify the mdio bus node. > Each subdriver might decide to keep existing approach or migrate to this > new common property. Your threading of these two patches is broken. The usual way to do this is git format-patch HEAD~2 git send-email *.patch You will then get uniform subject lines and the two emails threaded together. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
> Your threading of these two patches is broken. The usual way to do this is > > git format-patch HEAD~2 > git send-email *.patch > > You will then get uniform subject lines and the two emails threaded > together. Thanks, Andrew, I did something like that. However, bindings and net-next have different requirements. One needs the mail to go to devicetree@vger.kernel.org and the other a different prefix. So, I used send-email twice. Or should I use net-next for both and send both also to devicetree@? I did forget to set the "In-Reply-To:" in the second message. Luiz
On Mon, Feb 21, 2022 at 09:08:05PM -0300, Luiz Angelo Daros de Luca wrote: > > Your threading of these two patches is broken. The usual way to do this is > > > > git format-patch HEAD~2 > > git send-email *.patch > > > > You will then get uniform subject lines and the two emails threaded > > together. > > Thanks, Andrew, I did something like that. However, bindings and > net-next have different requirements. One needs the mail to go to > devicetree@vger.kernel.org and the other a different prefix. So, I > used send-email twice. Or should I use net-next for both and send both > also to devicetree@? I did forget to set the "In-Reply-To:" in the > second message. Binding need to be cc: to device tree, however they are generally merged by some other subsystem, where ever the driver belongs. Rob will review the binding, give his reviewed-by: and then Jakub or David will merge both to net-next. You can send both to device tree, or you can add a Cc: line to the binding patch, before your own signed-off. git send-email will see it and extend the list of recipients with whatever email address you put there. Andrew
On Mon, Feb 21, 2022 at 05:01:02PM -0300, Luiz Angelo Daros de Luca wrote: > The optional mdio property will be used by dsa switch to configure > slave_mii_bus when the driver does not allocate it during setup. > > Some drivers already offer/require a similar property but, in some > cases, they rely on a compatible string to identify the mdio bus node. That case will fail with this change. It precludes any binding referencing dsa.yaml from defining a 'mdio' node with properties other than what mdio.yaml defines. The rule is becoming any common schema should not define more than one level of nodes if those levels can be extended. > Each subdriver might decide to keep existing approach or migrate to this > new common property. > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > --- > Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > index b9d48e357e77..f9aa09052785 100644 > --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > @@ -32,6 +32,12 @@ properties: > (single device hanging off a CPU port) must not specify this property > $ref: /schemas/types.yaml#/definitions/uint32-array > > + mdio: > + unevaluatedProperties: false > + description: > + Container of PHY and devices on the switches MDIO bus. > + $ref: /schemas/net/mdio.yaml# > + > patternProperties: > "^(ethernet-)?ports$": > type: object > -- > 2.35.1 > >
> On Mon, Feb 21, 2022 at 05:01:02PM -0300, Luiz Angelo Daros de Luca wrote: > > The optional mdio property will be used by dsa switch to configure > > slave_mii_bus when the driver does not allocate it during setup. > > > > Some drivers already offer/require a similar property but, in some > > cases, they rely on a compatible string to identify the mdio bus node. > > That case will fail with this change. It precludes any binding > referencing dsa.yaml from defining a 'mdio' node with properties other > than what mdio.yaml defines. Thanks Rob. I believe any DSA drivers mdio subnodes will reference mdio.yaml. It doesn't make sense to not to. So, it is fine to restrict that from dsa.yaml. Are you saying that it will also prevent extra properties inside that mdio, either declared in dsa.yaml or in any other dsa driver yaml? During my tests, it looks like both "mdio", from dsa.yaml or the <driver>.yaml are validated independently. I can include a new property or even a "required" on either level and the checks seem to work fine. I didn't try it harder but I believe I cannot negate/drop a property or a requirement declared in one YAML file using the other YAML. But, in this case, dsa.yaml mdio is only referencing mdio.yaml, just like any dsa driver would do. And about the compatibile string usage, it is more about code than schema. I did checked every driver and got these results: These already uses mdio name: - drivers/net/dsa/qca8k.c: of_get_child_by_name(priv->dev->of_node, "mdio"); - drivers/net/dsa/mv88e6xxx/chip.c: of_get_child_by_name(np, "mdio"); - drivers/net/dsa/qca/ar9331.c: of_get_child_by_name(np, "mdio") This one creates a new mdios container for its two mdio nodes. So, they are at a different level: - drivers/net/dsa/sja1105/sja1105_mdio.c: of_get_compatible_child(mdio_node, "nxp,sja1110-base-tx-mdio"); - drivers/net/dsa/sja1105/sja1105_mdio.c: of_get_compatible_child(mdio_node, "nxp,sja1110-base-t1-mdio"); realtek-smi, even before yaml conversion required a mdio node (although code only looks for the compatible strings), - drivers/net/dsa/realtek/realtek-smi.c of_get_compatible_child(priv->dev->of_node, "realtek,smi-mdio"); This one is the most problematic one. It only has a TXT that does not mention an "mdio" node name, although examples do use it. - drivers/net/dsa/lantiq_gswip.c: of_get_compatible_child(dev->of_node, "lantiq,xrx200-mdio"); I still don't get why the compatible string is in use when there is a finite number of (or only one) mdio nodes. Wouldn't it be easier to use of_get_child_by_name? Anyway, all of them might not collide with dsa mdio definition. > The rule is becoming any common schema should not define more than one > level of nodes if those levels can be extended. Sorry but I didn't understand what you are suggesting. Should I simply drop the dsa/mdio doc? > > Each subdriver might decide to keep existing approach or migrate to this > > new common property. > > > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > > --- > > Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > index b9d48e357e77..f9aa09052785 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > > @@ -32,6 +32,12 @@ properties: > > (single device hanging off a CPU port) must not specify this property > > $ref: /schemas/types.yaml#/definitions/uint32-array > > > > + mdio: > > + unevaluatedProperties: false > > + description: > > + Container of PHY and devices on the switches MDIO bus. > > + $ref: /schemas/net/mdio.yaml# > > + > > patternProperties: > > "^(ethernet-)?ports$": > > type: object > > -- > > 2.35.1 > > > >
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml index b9d48e357e77..f9aa09052785 100644 --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml @@ -32,6 +32,12 @@ properties: (single device hanging off a CPU port) must not specify this property $ref: /schemas/types.yaml#/definitions/uint32-array + mdio: + unevaluatedProperties: false + description: + Container of PHY and devices on the switches MDIO bus. + $ref: /schemas/net/mdio.yaml# + patternProperties: "^(ethernet-)?ports$": type: object
The optional mdio property will be used by dsa switch to configure slave_mii_bus when the driver does not allocate it during setup. Some drivers already offer/require a similar property but, in some cases, they rely on a compatible string to identify the mdio bus node. Each subdriver might decide to keep existing approach or migrate to this new common property. Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> --- Documentation/devicetree/bindings/net/dsa/dsa.yaml | 6 ++++++ 1 file changed, 6 insertions(+)