diff mbox series

[3/7] dt-bindings: net: mdio: Add child nodes

Message ID 20190703095513.12340-3-maxime.ripard@bootlin.com
State Accepted, archived
Headers show
Series [1/7] dt-bindings: net: mdio: Add a nodename pattern | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Maxime Ripard July 3, 2019, 9:55 a.m. UTC
The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
property. Make sure that's validated as well.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/net/mdio.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Rob Herring July 3, 2019, 1:53 p.m. UTC | #1
On Wed, Jul 3, 2019 at 3:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
> property. Make sure that's validated as well.

I don't think this is always true. I seem to recall there's some
timestamping devices connected via mdio.

In any case, it's not a long list of names, so we can probably just
enumerate them as needed. Does this generate any warnings?

>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/net/mdio.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/mdio.yaml b/Documentation/devicetree/bindings/net/mdio.yaml
> index 24d67074d494..5d08d2ffd4eb 100644
> --- a/Documentation/devicetree/bindings/net/mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/mdio.yaml
> @@ -39,6 +39,20 @@ properties:
>        and must therefore be appropriately determined based on all PHY
>        requirements (maximum value of all per-PHY RESET pulse widths).
>
> +patternProperties:
> +  "^ethernet-phy@[0-9a-f]+$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 31
> +        description:
> +          The ID number for the PHY.
> +
> +    required:
> +      - reg
> +
>  examples:
>    - |
>      davinci_mdio: mdio@5c030000 {
> --
> 2.21.0
>
Maxime Ripard July 3, 2019, 2:13 p.m. UTC | #2
On Wed, Jul 03, 2019 at 07:53:43AM -0600, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 3:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
> > property. Make sure that's validated as well.
>
> I don't think this is always true. I seem to recall there's some
> timestamping devices connected via mdio.
>
> In any case, it's not a long list of names, so we can probably just
> enumerate them as needed. Does this generate any warnings?

I did a run on both arm and arm64 Allwinner DTS, and it doesn't.

I can do one on multi_v7 / arm64's defconfig, but that's probably
going to be a bit hard to tell from the noise of warnings.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Rob Herring July 3, 2019, 2:20 p.m. UTC | #3
On Wed, Jul 3, 2019 at 8:13 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Jul 03, 2019 at 07:53:43AM -0600, Rob Herring wrote:
> > On Wed, Jul 3, 2019 at 3:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
> > > property. Make sure that's validated as well.
> >
> > I don't think this is always true. I seem to recall there's some
> > timestamping devices connected via mdio.
> >
> > In any case, it's not a long list of names, so we can probably just
> > enumerate them as needed. Does this generate any warnings?
>
> I did a run on both arm and arm64 Allwinner DTS, and it doesn't.
>
> I can do one on multi_v7 / arm64's defconfig, but that's probably
> going to be a bit hard to tell from the noise of warnings.

I do allmodconfig because that will build all dtbs. You can run checks
with a single schema like this:

make dtbs_check DT_SCHEMA_FILES=a-single-schema.yaml

Rob
Maxime Ripard July 3, 2019, 7:57 p.m. UTC | #4
Hi Rob,

On Wed, Jul 03, 2019 at 08:20:32AM -0600, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 8:13 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Wed, Jul 03, 2019 at 07:53:43AM -0600, Rob Herring wrote:
> > > On Wed, Jul 3, 2019 at 3:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
> > > > property. Make sure that's validated as well.
> > >
> > > I don't think this is always true. I seem to recall there's some
> > > timestamping devices connected via mdio.
> > >
> > > In any case, it's not a long list of names, so we can probably just
> > > enumerate them as needed. Does this generate any warnings?
> >
> > I did a run on both arm and arm64 Allwinner DTS, and it doesn't.
> >
> > I can do one on multi_v7 / arm64's defconfig, but that's probably
> > going to be a bit hard to tell from the noise of warnings.
>
> I do allmodconfig because that will build all dtbs. You can run checks
> with a single schema like this:
>
> make dtbs_check DT_SCHEMA_FILES=a-single-schema.yaml

Right, of course :)

I just did it, and apart from a few arm32 broadcom boards that don't
have the right address-cells / size-cells (probably false positive due
to the fact they use mdio@something as node name), there's no other
warnings.

I'm not sure what you were on about though. If there's another node
than an ethernet phy, we won' have any warning since we don't have
additionalProperties to false.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Rob Herring July 3, 2019, 8:14 p.m. UTC | #5
On Wed, Jul 3, 2019 at 1:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi Rob,
>
> On Wed, Jul 03, 2019 at 08:20:32AM -0600, Rob Herring wrote:
> > On Wed, Jul 3, 2019 at 8:13 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Wed, Jul 03, 2019 at 07:53:43AM -0600, Rob Herring wrote:
> > > > On Wed, Jul 3, 2019 at 3:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > The child nodes of a mdio bus are supposed to be ethernet PHYs, with a reg
> > > > > property. Make sure that's validated as well.
> > > >
> > > > I don't think this is always true. I seem to recall there's some
> > > > timestamping devices connected via mdio.
> > > >
> > > > In any case, it's not a long list of names, so we can probably just
> > > > enumerate them as needed. Does this generate any warnings?
> > >
> > > I did a run on both arm and arm64 Allwinner DTS, and it doesn't.
> > >
> > > I can do one on multi_v7 / arm64's defconfig, but that's probably
> > > going to be a bit hard to tell from the noise of warnings.
> >
> > I do allmodconfig because that will build all dtbs. You can run checks
> > with a single schema like this:
> >
> > make dtbs_check DT_SCHEMA_FILES=a-single-schema.yaml
>
> Right, of course :)
>
> I just did it, and apart from a few arm32 broadcom boards that don't
> have the right address-cells / size-cells (probably false positive due
> to the fact they use mdio@something as node name), there's no other
> warnings.

Actually, they look like real errors in the DT with the values of
address-cells / size-cells swapped. But the node is disabled in the
ones I looked at.

> I'm not sure what you were on about though. If there's another node
> than an ethernet phy, we won' have any warning since we don't have
> additionalProperties to false.

True. I guess I wasn't thinking through it... Anyways, I applied it.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/mdio.yaml b/Documentation/devicetree/bindings/net/mdio.yaml
index 24d67074d494..5d08d2ffd4eb 100644
--- a/Documentation/devicetree/bindings/net/mdio.yaml
+++ b/Documentation/devicetree/bindings/net/mdio.yaml
@@ -39,6 +39,20 @@  properties:
       and must therefore be appropriately determined based on all PHY
       requirements (maximum value of all per-PHY RESET pulse widths).
 
+patternProperties:
+  "^ethernet-phy@[0-9a-f]+$":
+    type: object
+
+    properties:
+      reg:
+        minimum: 0
+        maximum: 31
+        description:
+          The ID number for the PHY.
+
+    required:
+      - reg
+
 examples:
   - |
     davinci_mdio: mdio@5c030000 {