netdev - Re: [RFC PATCH] dt-bindings: can: convert tcan4x5x.txt to DT 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ee47c6d7-4197-4f5d-b39e-aab70a9337d6@kernel.org>
Date: Mon, 4 Nov 2024 10:27:04 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Sean Nyekjaer <sean@...nix.com>, Marc Kleine-Budde <mkl@...gutronix.de>,
 Vincent Mailhol <mailhol.vincent@...adoo.fr>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: linux-can@...r.kernel.org, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] dt-bindings: can: convert tcan4x5x.txt to DT schema

On 04/11/2024 09:56, Sean Nyekjaer wrote:
> Convert binding doc tcan4x5x.txt to yaml.
> 
> Signed-off-by: Sean Nyekjaer <sean@...nix.com>
> ---
> 
> Can we somehow reference bosch,mram-cfg from the bosch,m_can.yaml?
> I have searched for yaml files that tries the same, but it's usually
> includes a whole node.
> 
> I have also tried:
> $ref: /schema/bosch,m_can.yaml#/properties/bosch,mram-cfg

Yes, this would work just with full path, so /schemas/net/can/...

See:
Documentation/devicetree/bindings/pinctrl/starfive,jh7100-pinctrl.yaml

But you can also just copy it. Ideally this should be moved to common
schema or replaced with more generic property, but these do not have to
be part of this conversion.

> 
> Any hints to share a property?
> 
>  .../devicetree/bindings/net/can/tcan4x5x.txt  | 48 ---------
>  .../bindings/net/can/ti,tcan4x5x.yaml         | 97 +++++++++++++++++++
>  2 files changed, 97 insertions(+), 48 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
>  create mode 100644 Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> 

...

> diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> new file mode 100644
> index 000000000000..62c108fac6b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/ti,tcan4x5x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments TCAN4x5x CAN Controller
> +
> +maintainers:
> +  - Marc Kleine-Budde <mkl@...gutronix.de>
> +
> +allOf:
> +  - $ref: can-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - ti,tcan4552
> +          - ti,tcan4553
> +          - ti,tcan4x5x

That's not really what old binding said.

It said for example:
"ti,tcan4552", "ti,tcan4x5x"

Which is not allowed above. You need list. Considering there are no
in-tree users of ti,tcan4x5x alone, I would allow only lists followed by
ti,tcan4x5x. IOW: disallow ti,tcan4x5x alone.

Mention this change to the binding in the commit message.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: Regulator that powers the CAN controller.
> +
> +  xceiver-supply:
> +    description: Regulator that powers the CAN transceiver.

You need to mention all changes done to the binding in the commit msg.

> +
> +  reset-gpios:
> +    description: Hardwired output GPIO. If not defined then software reset.
> +    maxItems: 1
> +
> +  device-state-gpios:
> +    description: Input GPIO that indicates if the device is in a sleep state or if the device is active.
> +      Not available with tcan4552/4553.
> +    maxItems: 1
> +
> +  device-wake-gpios:
> +    description: Wake up GPIO to wake up the TCAN device. Not available with tcan4552/4553.
> +    maxItems: 1
> +
> +  bosch,mram-cfg:
> +    $ref: bosch,m_can.yaml#
> +
> +  spi-max-frequency:
> +    description:
> +      Must be half or less of "clocks" frequency.
> +    maximum: 10000000

Old binding said 18 MHz?

> +
> +  wakeup-source:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Enable CAN remote wakeup.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - bosch,mram-cfg
> +

Missing allOf: with $ref to spi-peripheral-props. See other SPI devices.

> +additionalProperties: false

And this becomes unevaluatedProperties: false

Best regards,
Krzysztof