Received-SPF: pass (google.com: domain of linux-kernel+bounces-25486-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Sun, 14 Jan 2024 15:47:38 +0100
From: Christoph Winklhofer <cj.winklhofer@gmail.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Rob Herring <robh@kernel.org>, Conor Dooley <conor+dt@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v4 1/3] dt-bindings: w1: UART 1-Wire bus
Message-ID: <ZaP0CoCYLQxrT3VD@cjw-notebook>
References: <20240106-w1-uart-v4-0-7fe1378a8b3e@gmail.com>
 <20240106-w1-uart-v4-1-7fe1378a8b3e@gmail.com>
 <20240113013917.GA3795949-robh@kernel.org>
 <ZaLQxGjjmA_iKOv2@cjw-notebook>
 <81c79939-56cc-4d78-9552-56568999df09@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <81c79939-56cc-4d78-9552-56568999df09@linaro.org>

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