On Mon, Nov 04, 2024 at 10:27:04AM +0100, Krzysztof Kozlowski wrote:
> On 04/11/2024 09:56, Sean Nyekjaer wrote:
> > Convert binding doc tcan4x5x.txt to yaml.
> >
> > Signed-off-by: Sean Nyekjaer <sean@xxxxxxxxxx>
> > ---
> >
> > 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.
diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
index 9ff52b8b3063..0fc37b10e899 100644
--- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
+++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml
@@ -50,7 +50,7 @@ properties:
maxItems: 1
bosch,mram-cfg:
- $ref: bosch,m_can.yaml#
+ $ref: /schemas/net/can/bosch,m_can.yaml#/properties/bosch,mram-cfg
spi-max-frequency:
description:
Still results in:
% make dt_binding_check DT_SCHEMA_FILES=ti,tcan4x5x.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
CHKDT Documentation/devicetree/bindings
Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml: properties:bosch,mram-cfg: 'anyOf' conditional failed, one must be fixed:
'description' is a dependency of '$ref'
'/schemas/net/can/bosch,m_can.yaml#/properties/bosch,mram-cfg' does not match 'types.yaml#/definitions/'
hint: A vendor property needs a $ref to types.yaml
'/schemas/net/can/bosch,m_can.yaml#/properties/bosch,mram-cfg' does not match '^#/(definitions|\\$defs)/'
hint: A vendor property can have a $ref to a a $defs schema
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>
> >
> > 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@xxxxxxxxxxxxxx>
> > +
> > +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.
>
>
I would prefer to not change anything other that doing the conversion to
DT schema.
> > +
> > + 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.
>
Is this a change? It existed in the old doc aswell...
> > +
> > + 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?
>
Good catch.
> > +
> > + 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.
>
>
Added for v2.
> > +additionalProperties: false
>
> And this becomes unevaluatedProperties: false
>
> Best regards,
> Krzysztof
>
Added for v2.
Br,
/Sean
