Re: [PATCH v4 1/3] dt-bindings: w1: UART 1-Wire bus

[Christoph Winklhofer]

On Fri, Jan 12, 2024 at 07:39:17PM -0600, Rob Herring wrote:
> On Sat, Jan 06, 2024 at 05:02:24PM +0100, Christoph Winklhofer wrote:
> > Add device tree binding for UART 1-Wire bus.
> > 
> > Signed-off-by: Christoph Winklhofer <cj.winklhofer@xxxxxxxxx>
> > ---
> >  Documentation/devicetree/bindings/w1/w1-uart.yaml | 62 +++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/w1/w1-uart.yaml b/Documentation/devicetree/bindings/w1/w1-uart.yaml
> > new file mode 100644
> > index 000000000000..6b90693b2ca0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/w1/w1-uart.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/w1/w1-uart.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
..
> > +properties:
> > +  compatible:
> > +    const: w1-uart
> > +
> > +  reset-speed:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 9600
> > +    description: |
> 
> Don't need '|' if no formatting
> 

Ok.

> > +      The baud rate for the 1-Wire reset and presence detect.
> > +
> > +  touch_0-speed:
> 
> Don't use '_' in property names.
> 
> I'm somewhat familar with 1-wire, but I don't get what 'touch' means 
> here. I assume these are low and high times which are a function of the 
> baudrate.
> 

I change the name to 'write-0-speed' and 'write-1-speed'. The function
in the w1-framework is named 'touch_bit' - therefore the previous
naming. 

It is the baud-rate used in the 1-Wire cycle to write a 0-Bit
(write-0-speed) and to perform a 1-Wire cycle that writes a 1-Bit and
reads a 0-Bit or 1-Bit (write-1-speed).

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 115200
> > +    description: |
> > +      The baud rate for the 1-Wire write-0 cycle (touch bit 0).
> > +
> > +  touch_1-speed:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    default: 115200
> > +    description: |
> > +      The baud rate for the 1-Wire write-1 and read cycle (touch bit 1).
> > +
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: false
> 
> w1.txt says there can be a child node for the 1-wire device. You need 
> 'type: object'. Or w1.txt needs to be converted to schema and referenced 
> here (along with using unevaluatedProperties here instead).
> 
> Rob

Will change it to type object. However, I also tried to convert w1.txt
to w1.yaml - I am not quite sure, so I add an excerpt of my attempt.
The example in w1.txt builds fine. Is oneOf allowed as a top-level
property, the documentation only mentions allOf?
Only one child should be allowed, but I did not find a way to restrict
this.

# w1-uart.yaml:
unevaluatedProperties:
  $ref: w1.yaml

# w1.yaml:
properties:
  compatible:
    enum:
      - maxim,ds2760

oneOf:
  - $ref: /schemas/power/supply/maxim,ds2760.yaml

unevaluatedProperties: false
#

Thank you for the review!

Kind regards,
Christoph