Message ID | 20241111-tcan-wkrqv-v2-2-9763519b5252@geanix.com |
---|---|
State | Changes Requested |
Headers | show |
Series | can: tcan4x5x: add option for selecting nWKRQ voltage | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/patch-applied | fail | build log |
On 11.11.2024 09:54:50, Sean Nyekjaer wrote: > nWKRQ supports an output voltage of either the internal reference voltage > (3.6V) or the reference voltage of the digital interface 0 - 6V. > Add the devicetree option ti,nwkrq-voltage-sel to be able to select > between them. > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- > Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > index f1d18a5461e05296998ae9bf09bdfa1226580131..a77c560868d689e92ded08b9deb43e5a2b89bf2b 100644 > --- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > @@ -106,6 +106,18 @@ properties: > Must be half or less of "clocks" frequency. > maximum: 18000000 > > + ti,nwkrq-voltage-sel: > + $ref: /schemas/types.yaml#/definitions/uint8 > + description: > + nWKRQ Pin GPO buffer voltage rail configuration. > + The option of this properties will tell which > + voltage rail is used for the nWKRQ Pin. > + oneOf: > + - description: Internal voltage rail > + const: 0 > + - description: VIO voltage rail > + const: 1 We usually don't want to put register values into the DT. Is 0, i.e. the internal voltage rail the default? Is using a boolean better here? regards, Marc
Hi Marc, On Tue, Nov 12, 2024 at 08:35:43AM +0100, Marc Kleine-Budde wrote: > On 11.11.2024 09:54:50, Sean Nyekjaer wrote: > > nWKRQ supports an output voltage of either the internal reference voltage > > (3.6V) or the reference voltage of the digital interface 0 - 6V. > > Add the devicetree option ti,nwkrq-voltage-sel to be able to select > > between them. > > > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > > --- > > Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > index f1d18a5461e05296998ae9bf09bdfa1226580131..a77c560868d689e92ded08b9deb43e5a2b89bf2b 100644 > > --- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > @@ -106,6 +106,18 @@ properties: > > Must be half or less of "clocks" frequency. > > maximum: 18000000 > > > > + ti,nwkrq-voltage-sel: > > + $ref: /schemas/types.yaml#/definitions/uint8 > > + description: > > + nWKRQ Pin GPO buffer voltage rail configuration. > > + The option of this properties will tell which > > + voltage rail is used for the nWKRQ Pin. > > + oneOf: > > + - description: Internal voltage rail > > + const: 0 > > + - description: VIO voltage rail > > + const: 1 > > We usually don't want to put register values into the DT. Is 0, i.e. the > internal voltage rail the default? Is using a boolean better here? > > regards, > Marc > Thanks for the review :) Can you come up with a sane naming? A boolean that equals true when it's set to VIO voltage? Or the other way around? /Sean
On Tue, Nov 12, 2024 at 08:40:56AM +0100, Sean Nyekjaer wrote: > Hi Marc, > > On Tue, Nov 12, 2024 at 08:35:43AM +0100, Marc Kleine-Budde wrote: > > On 11.11.2024 09:54:50, Sean Nyekjaer wrote: > > > nWKRQ supports an output voltage of either the internal reference voltage > > > (3.6V) or the reference voltage of the digital interface 0 - 6V. > > > Add the devicetree option ti,nwkrq-voltage-sel to be able to select > > > between them. > > > > > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > > > --- > > > Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > > index f1d18a5461e05296998ae9bf09bdfa1226580131..a77c560868d689e92ded08b9deb43e5a2b89bf2b 100644 > > > --- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > > +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml > > > @@ -106,6 +106,18 @@ properties: > > > Must be half or less of "clocks" frequency. > > > maximum: 18000000 > > > > > > + ti,nwkrq-voltage-sel: > > > + $ref: /schemas/types.yaml#/definitions/uint8 > > > + description: > > > + nWKRQ Pin GPO buffer voltage rail configuration. > > > + The option of this properties will tell which > > > + voltage rail is used for the nWKRQ Pin. > > > + oneOf: > > > + - description: Internal voltage rail > > > + const: 0 > > > + - description: VIO voltage rail > > > + const: 1 > > > > We usually don't want to put register values into the DT. Is 0, i.e. the > > internal voltage rail the default? Is using a boolean better here? > > > > regards, > > Marc > > > > Thanks for the review :) > > Can you come up with a sane naming? > A boolean that equals true when it's set to VIO voltage? Or the other > way around? Make the property named/present for the less common case if there is one. That might not be known here. Rob
diff --git a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml index f1d18a5461e05296998ae9bf09bdfa1226580131..a77c560868d689e92ded08b9deb43e5a2b89bf2b 100644 --- a/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml +++ b/Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml @@ -106,6 +106,18 @@ properties: Must be half or less of "clocks" frequency. maximum: 18000000 + ti,nwkrq-voltage-sel: + $ref: /schemas/types.yaml#/definitions/uint8 + description: + nWKRQ Pin GPO buffer voltage rail configuration. + The option of this properties will tell which + voltage rail is used for the nWKRQ Pin. + oneOf: + - description: Internal voltage rail + const: 0 + - description: VIO voltage rail + const: 1 + wakeup-source: $ref: /schemas/types.yaml#/definitions/flag description: @@ -157,6 +169,7 @@ examples: device-state-gpios = <&gpio3 21 GPIO_ACTIVE_HIGH>; device-wake-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>; reset-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>; + ti,nwkrq-voltage-sel = /bits/ 8 <0>; wakeup-source; }; };
nWKRQ supports an output voltage of either the internal reference voltage (3.6V) or the reference voltage of the digital interface 0 - 6V. Add the devicetree option ti,nwkrq-voltage-sel to be able to select between them. Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- Documentation/devicetree/bindings/net/can/ti,tcan4x5x.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+)