netdev - Re: [PATCH net-next] dt-bindings: net: dsa: sja1105: convert to YAML schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210601213409.GA1031105@robh.at.kernel.org>
Date:   Tue, 1 Jun 2021 16:34:09 -0500
From:   Rob Herring <robh@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        devicetree@...r.kernel.org, netdev@...r.kernel.org,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [PATCH net-next] dt-bindings: net: dsa: sja1105: convert to YAML
 schema

On Tue, Jun 01, 2021 at 02:47:35AM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> The following issues exist with the device-specific sja1105,role-mac and
> sja1105,role-phy:
> 
> (a) the "sja1105" is not a valid vendor prefix and should probably have
>     been "nxp", but
> (b) as per the discussion with Florian here:
>     https://lore.kernel.org/netdev/20210201214515.cx6ivvme2tlquge2@skbuf/
>     more phy-mode values similar to "revmii" can be added which denote
>     that the port is in the role of a PHY (such as "revrmii"), making
>     the sja1105,role-phy redundant. Because there are no upstream users
>     (or any users at all, to my knowledge) of these properties, they
>     could even be removed in a future commit as far as I am concerned.
> (c) when I force-add sja1105,role-phy to a device tree for testing, the
>     patternProperties matching does not work, it results in the following
>     error:
> 
> ethernet-switch@2: ethernet-ports:port@1: 'sja1105,role-phy' does not match any of the regexes: 'pinctrl-[0-9]+'
>         From schema: Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml

I believe that would be from 'additionalProperties: false' under 
"^(ethernet-)?port@[0-9]+$" in dsa.yaml. If additional properties need 
to be allowed, then it needs to be changed to 'true'. But if the 
properties aren't really used, just removing them would be better. But 
maybe there's other DSA users with custom properties.

> 
> But what's even more interesting is that if I remove the
> "additionalProperties: true" that dsa.yaml has, I get even more
> validation errors coming from patternProperties not matching either,
> from spi-controller.yaml:

Why would you do that?

> 
> ethernet-switch@2: 'compatible', 'mdio', 'reg', 'spi-cpol', 'spi-max-frequency' do not match any of the regexes: '^(ethernet-)?ports$', 'pinctrl-[0-9]+'
> 
> So... it is probably broken. Rob Herring says here:
> https://lore.kernel.org/linux-spi/20210324181037.GB3320002@robh.at.kernel.org/
> 
>   I'm aware of the issue, but I don't have a solution for this situation.
>   It's a problem anywhere we have a parent or bus binding defining
>   properties for child nodes. For now, I'd just avoid it in the examples
>   and we'll figure out how to deal with actual dts files later.

That was mainly in reference to vendor specific SPI master properties. 

For 'spi-cpol', that generally shouldn't be needed. A given device 
generally only supports 1 mode and the driver should know that. IOW, it 
can be implied from the compatible. There's of course some exceptions.

For 'spi-max-frequency', just add 'spi-max-frequency: true' (or provide 
some constraints as to what the max is.

Rob