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

  From: Christoph Winklhofer <cj.winklhofer@gmail.com>

Add device tree binding for UART 1-Wire bus.

Signed-off-by: Christoph Winklhofer <cj.winklhofer@gmail.com>
---
 Documentation/devicetree/bindings/w1/w1-uart.yaml | 62 +++++++++++++++++++++++
 1 file changed, 62 insertions(+)
  

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@gmail.com>
> ---
>  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#
> +
> +title: UART 1-Wire Bus
> +
> +maintainers:
> +  - Christoph Winklhofer <cj.winklhofer@gmail.com>
> +
> +description: |
> +  UART 1-wire bus. Utilizes the UART interface via the Serial Device Bus
> +  to create the 1-Wire timing patterns.
> +
> +  The UART peripheral must support full-duplex and operate in open-drain
> +  mode. The timing patterns are generated by a specific combination of
> +  baud-rate and transmitted byte, which corresponds to a 1-Wire read bit,
> +  write bit or reset pulse.
> +
> +  The default baud-rate for reset and presence detection is 9600 and for
> +  a 1-Wire read or write operation 115200. In case the actual baud-rate
> +  is different from the requested one, the transmitted byte is adapted
> +  to generate the 1-Wire timing patterns.
> +
> +  https://www.analog.com/en/technical-articles/using-a-uart-to-implement-a-1wire-bus-master.html
> +
> +
> +properties:
> +  compatible:
> +    const: w1-uart
> +
> +  reset-speed:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 9600
> +    description: |

Don't need '|' if no formatting

> +      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.


> +    $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
  

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@gmail.com>
> > ---
> >  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
  

On 13/01/2024 19:04, Christoph Winklhofer wrote:
> 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@gmail.com>
>>> ---
>>>  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).


Then probably -bps:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml


> 
>>> +    $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

w1 describes entire class of the devices, so it should not have
compatibles. Look how other buses do it.
> 
> oneOf:
>   - $ref: /schemas/power/supply/maxim,ds2760.yaml
> 
> unevaluatedProperties: false


Best regards,
Krzysztof
  

On Sun, Jan 14, 2024 at 11:54:48AM +0100, Krzysztof Kozlowski wrote:
> On 13/01/2024 19:04, Christoph Winklhofer wrote:
> > 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@gmail.com>
> >>> ---
> >>>  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).
> 
> 
> Then probably -bps:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

The serial.yaml uses prefix -speed for the baud rate but I can change it
to -bps.

> 
> > 
> >>> +    $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
> 
> w1 describes entire class of the devices, so it should not have
> compatibles. Look how other buses do it.

Thanks for the hint, looked at serial.yaml and usb.yaml. Hence w1.yaml
should describe the properties common to all w1 bus controllers
(property for child device) and w1-uart.yaml references it in allOf,
like:

# w1.yaml
patternProperties:
  ".*":
    if:
      type: object
    then:
      description: Devices are listed...
      properties:
	compatible:
      required:
	- compatible
additionalProperties: true

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

Kind regards,
Christoph
  

On 14/01/2024 15:47, Christoph Winklhofer wrote:
> On Sun, Jan 14, 2024 at 11:54:48AM +0100, Krzysztof Kozlowski wrote:
>> On 13/01/2024 19:04, Christoph Winklhofer wrote:
>>> 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@gmail.com>
>>>>> ---
>>>>>  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).
>>
>>
>> Then probably -bps:
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> The serial.yaml uses prefix -speed for the baud rate but I can change it
> to -bps.

Do you reference serial.yaml?

> 


Best regards,
Krzysztof
  

On Sun, Jan 14, 2024 at 04:55:42PM +0100, Krzysztof Kozlowski wrote:
> On 14/01/2024 15:47, Christoph Winklhofer wrote:
> > On Sun, Jan 14, 2024 at 11:54:48AM +0100, Krzysztof Kozlowski wrote:
> >> On 13/01/2024 19:04, Christoph Winklhofer wrote:
> >>> 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@gmail.com>
> >>>>> ---
> >>>>>  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).
> >>
> >>
> >> Then probably -bps:
> >> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> > 
> > The serial.yaml uses prefix -speed for the baud rate but I can change it
> > to -bps.
> 
> Do you reference serial.yaml?
> 

No, serial.yaml is not referenced but 'onewire' will be a child-node of
a serial-device which already defines baud rate related properties
with -speed (e.g. max-speed although not used in w1-uart). Hence, I
thought -speed is typically used for baud rates.

Thanks,
Christoph
  

On 15/01/2024 18:36, Christoph Winklhofer wrote:
> On Sun, Jan 14, 2024 at 04:55:42PM +0100, Krzysztof Kozlowski wrote:
>> On 14/01/2024 15:47, Christoph Winklhofer wrote:
>>> On Sun, Jan 14, 2024 at 11:54:48AM +0100, Krzysztof Kozlowski wrote:
>>>> On 13/01/2024 19:04, Christoph Winklhofer wrote:
>>>>> 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@gmail.com>
>>>>>>> ---
>>>>>>>  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).
>>>>
>>>>
>>>> Then probably -bps:
>>>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>>
>>> The serial.yaml uses prefix -speed for the baud rate but I can change it
>>> to -bps.
>>
>> Do you reference serial.yaml?
>>
> 
> No, serial.yaml is not referenced but 'onewire' will be a child-node of
> a serial-device which already defines baud rate related properties
> with -speed (e.g. max-speed although not used in w1-uart). Hence, I
> thought -speed is typically used for baud rates.

Ah, it defines max-speed for childre, so for onewire. Re-using that
property would make sense, but since you are defining completely new
properties, let's use proper (-bps) naming.

I still wonder, why would you use different baud rates for these three
different operations?

Best regards,
Krzysztof
  

On Mon, Jan 15, 2024 at 07:02:55PM +0100, Krzysztof Kozlowski wrote:
> On 15/01/2024 18:36, Christoph Winklhofer wrote:
> > On Sun, Jan 14, 2024 at 04:55:42PM +0100, Krzysztof Kozlowski wrote:
> >> On 14/01/2024 15:47, Christoph Winklhofer wrote:
> >>> On Sun, Jan 14, 2024 at 11:54:48AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 13/01/2024 19:04, Christoph Winklhofer wrote:
> >>>>> 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@gmail.com>
> >>>>>>> ---
> >>>>>>>  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).
> >>>>
> >>>>
> >>>> Then probably -bps:
> >>>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> >>>
> >>> The serial.yaml uses prefix -speed for the baud rate but I can change it
> >>> to -bps.
> >>
> >> Do you reference serial.yaml?
> >>
> > 
> > No, serial.yaml is not referenced but 'onewire' will be a child-node of
> > a serial-device which already defines baud rate related properties
> > with -speed (e.g. max-speed although not used in w1-uart). Hence, I
> > thought -speed is typically used for baud rates.
> 
> Ah, it defines max-speed for childre, so for onewire. Re-using that
> property would make sense, but since you are defining completely new
> properties, let's use proper (-bps) naming.
> 

Ok.

> I still wonder, why would you use different baud rates for these three
> different operations?

The timing for the pulse on the 1-Wire bus, to perform a 1-Wire reset or
read/write is too different to use the same baud rate for them. The
pulse can only be generated with the transmission of a single byte (to
initiate the operation and to evaluate the response), since every UART
packet starts low and ends high, only a limited set of baud rates are
possible.

Read-0/1 and Write-1 have the same low-time and therefore share the same
baud rate setting, actually this baud could be higher. Otherwise, for
Write-0 the maximum baud-rate is somewhere near 115200.

The table lists the low-time range for the 1-Wire standard speed
(overdrive is currently not implemented by the driver):

  Operation | Low-Time (usec) | Baud rate (default)
  ----------+-----------------+--------------------
  Reset     | 480 - 640       | 9600
  Read-0/1  |   5 -  15       | 115200
  Write-1   |   5 -  15       | 115200
  Write-0   |  60 - 120       | 115200

I thought it makes sense to have the option to overwrite the default
baud rates. Alternatively, the driver could probe a fixed set of
different baud rates that are able to generate the timing patterns.
Initially, the driver always used 9600 and 115200 - however some UART
devices may not support these bauds. Now the driver requests a baud rate
and checks if it can create the timing pattern with the returned one.

Kind regards,
Christoph