| Message ID | 20210325124225.2760-2-linux.amoon@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Amlogic Soc - Add missing ethernet mdio compatible string | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Thu, Mar 25, 2021 at 12:42:20PM +0000, Anand Moon wrote: > Fix the parsing of check of pattern ethernet-phy-ieee802.3 used > by the device tree to initialize the mdio phy. > > As per the of_mdio below 2 are valid compatible string > "ethernet-phy-ieee802.3-c22" > "ethernet-phy-ieee802.3-c45" Nope, this is not the full story. Yes, you can have these compatible strings. But you can also use the PHY ID, e.g. ethernet-phy-idAAAA.BBBB, where AAAA and BBBB are what you find in registers 2 and 3 of the PHY. > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > index 2766fe45bb98..cfc7909d3e56 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > @@ -33,7 +33,7 @@ properties: > description: PHYs that implement IEEE802.3 clause 22 > - const: ethernet-phy-ieee802.3-c45 > description: PHYs that implement IEEE802.3 clause 45 > - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" So here you need, in addition to, not instead of. Please test you change on for example imx6ul-14x14-evk.dtsi Andrew
Hi Andrew, On Thu, 25 Mar 2021 at 18:27, Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, Mar 25, 2021 at 12:42:20PM +0000, Anand Moon wrote: > > Fix the parsing of check of pattern ethernet-phy-ieee802.3 used > > by the device tree to initialize the mdio phy. > > > > As per the of_mdio below 2 are valid compatible string > > "ethernet-phy-ieee802.3-c22" > > "ethernet-phy-ieee802.3-c45" > > Nope, this is not the full story. Yes, you can have these compatible > strings. But you can also use the PHY ID, > e.g. ethernet-phy-idAAAA.BBBB, where AAAA and BBBB are what you find in > registers 2 and 3 of the PHY. > Oops I did not read the drivers/net/mdio/of_mdio.c completely. Thanks for letting me know so in the next series, I will try to add the below compatible string as per the description in the dts. compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22"; > > Cc: Rob Herring <robh@kernel.org> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > index 2766fe45bb98..cfc7909d3e56 100644 > > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > @@ -33,7 +33,7 @@ properties: > > description: PHYs that implement IEEE802.3 clause 22 > > - const: ethernet-phy-ieee802.3-c45 > > description: PHYs that implement IEEE802.3 clause 45 > > - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > > + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" > > So here you need, in addition to, not instead of. > > Please test you change on for example imx6ul-14x14-evk.dtsi > Yes I have gone through the test case. > Andrew - Anand
On 25.03.2021 14:33, Anand Moon wrote: > Hi Andrew, > > On Thu, 25 Mar 2021 at 18:27, Andrew Lunn <andrew@lunn.ch> wrote: >> >> On Thu, Mar 25, 2021 at 12:42:20PM +0000, Anand Moon wrote: >>> Fix the parsing of check of pattern ethernet-phy-ieee802.3 used >>> by the device tree to initialize the mdio phy. >>> >>> As per the of_mdio below 2 are valid compatible string >>> "ethernet-phy-ieee802.3-c22" >>> "ethernet-phy-ieee802.3-c45" >> >> Nope, this is not the full story. Yes, you can have these compatible >> strings. But you can also use the PHY ID, >> e.g. ethernet-phy-idAAAA.BBBB, where AAAA and BBBB are what you find in >> registers 2 and 3 of the PHY. >> > > Oops I did not read the drivers/net/mdio/of_mdio.c completely. > Thanks for letting me know so in the next series, > I will try to add the below compatible string as per the description in the dts. That's not needed, typically the PHY ID is auto-detected. Before sending a new series, please describe in detail what your problem is. Simply there shouldn't be a need for such a series. As I said: e.g. Odroid-C2 worked fine for me with a mainline kernel. > > compatible = "ethernet-phy-id001c.c916", > "ethernet-phy-ieee802.3-c22"; > >>> Cc: Rob Herring <robh@kernel.org> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml >>> index 2766fe45bb98..cfc7909d3e56 100644 >>> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml >>> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml >>> @@ -33,7 +33,7 @@ properties: >>> description: PHYs that implement IEEE802.3 clause 22 >>> - const: ethernet-phy-ieee802.3-c45 >>> description: PHYs that implement IEEE802.3 clause 45 >>> - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" >>> + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" >> >> So here you need, in addition to, not instead of. >> >> Please test you change on for example imx6ul-14x14-evk.dtsi >> > > Yes I have gone through the test case. > >> Andrew > > - Anand >
On Thu, 25 Mar 2021 12:42:20 +0000, Anand Moon wrote: > Fix the parsing of check of pattern ethernet-phy-ieee802.3 used > by the device tree to initialize the mdio phy. > > As per the of_mdio below 2 are valid compatible string > "ethernet-phy-ieee802.3-c22" > "ethernet-phy-ieee802.3-c45" > > Cc: Rob Herring <robh@kernel.org> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-phy.example.dt.yaml: ethernet-phy@0: compatible: 'oneOf' conditional failed, one must be fixed: ['ethernet-phy-id0141.0e90', 'ethernet-phy-ieee802.3-c45'] is too long Additional items are not allowed ('ethernet-phy-ieee802.3-c45' was unexpected) 'ethernet-phy-ieee802.3-c22' was expected 'ethernet-phy-ieee802.3-c45' was expected 'ethernet-phy-id0141.0e90' does not match '^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-phy.yaml See https://patchwork.ozlabs.org/patch/1458341 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
Hi Rob On Thu, 25 Mar 2021 at 22:26, Rob Herring <robh@kernel.org> wrote: > > On Thu, 25 Mar 2021 12:42:20 +0000, Anand Moon wrote: > > Fix the parsing of check of pattern ethernet-phy-ieee802.3 used > > by the device tree to initialize the mdio phy. > > > > As per the of_mdio below 2 are valid compatible string > > "ethernet-phy-ieee802.3-c22" > > "ethernet-phy-ieee802.3-c45" > > > > Cc: Rob Herring <robh@kernel.org> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-phy.example.dt.yaml: ethernet-phy@0: compatible: 'oneOf' conditional failed, one must be fixed: > ['ethernet-phy-id0141.0e90', 'ethernet-phy-ieee802.3-c45'] is too long > Additional items are not allowed ('ethernet-phy-ieee802.3-c45' was unexpected) > 'ethernet-phy-ieee802.3-c22' was expected > 'ethernet-phy-ieee802.3-c45' was expected > 'ethernet-phy-id0141.0e90' does not match '^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$' > From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > See https://patchwork.ozlabs.org/patch/1458341 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > 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. > Now I have a better understanding on device tree shema in Documentation/devicetree/bindings/net/ethernet-phy.yaml changes it meant to parse *ethernet-phy-id0181.4400* for example and not ethernet-phy-ieee802.3-c22 and ethernet-phy-ieee802.3-c45. So please dicard these changes. -Anand
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index 2766fe45bb98..cfc7909d3e56 100644 --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml @@ -33,7 +33,7 @@ properties: description: PHYs that implement IEEE802.3 clause 22 - const: ethernet-phy-ieee802.3-c45 description: PHYs that implement IEEE802.3 clause 45 - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" description: If the PHY reports an incorrect ID (or none at all) then the compatible list may contain an entry with the correct PHY ID @@ -44,10 +44,10 @@ properties: this is the chip vendor OUI bits 19:24, followed by 10 bits of a vendor specific ID. - items: - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" - const: ethernet-phy-ieee802.3-c22 - items: - - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" + - pattern: "^ethernet-phy-ieee[0-9]{3}\\.[0-9][-][a-f0-9]{4}$" - const: ethernet-phy-ieee802.3-c45 reg:
Fix the parsing of check of pattern ethernet-phy-ieee802.3 used by the device tree to initialize the mdio phy. As per the of_mdio below 2 are valid compatible string "ethernet-phy-ieee802.3-c22" "ethernet-phy-ieee802.3-c45" Cc: Rob Herring <robh@kernel.org> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)