diff mbox series

[05/11] dt-bindings: net: can: Add serdev LIN bus dt bindings

Message ID 20240422065114.3185505-6-christoph.fritz@hexdev.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series LIN Bus support for Linux | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 21 this patch: 21
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang fail Errors and warnings before: 20 this patch: 20
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 22 this patch: 22
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Christoph Fritz April 22, 2024, 6:51 a.m. UTC 
Add documentation of device tree bindings for serdev UART LIN-Bus
devices equipped with LIN transceivers.

Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
---
 .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml

Comments

Krzysztof Kozlowski April 22, 2024, 7:54 a.m. UTC | #1 
On 22/04/2024 08:51, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.

A nit, subject: drop second/last, redundant "dt bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> new file mode 100644
> index 0000000000000..cb4e932ff249c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linux serdev LIN-Bus Support

This looks like Linux binding, but we expect here description of hardware.


> +
> +description: |
> +  LIN-Bus support for UART devices equipped with LIN transceivers,
> +  utilizing the Serial Device Bus (serdev) interface.

serdev is Linux thingy, AFAIR. Please describe the hardware.

> +
> +  For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> +
> +properties:
> +  compatible:
> +    const: linux,lin-serdev

Feels confusing. Your link describes real hardware, but you wrote
bindings for software construct.

If you add this to DT, then it is hard-wired on the board, right? If so,
how this could be a software construct?


> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    &uart2 {

& does not make much sense here. I think you wanted it to be serial bus,
so serial.

> +      status = "okay";

Drop, it was not disabled anywhere.


> +      linbus {
> +        compatible = "linux,lin-serdev";
> +      };
> +    };

Best regards,
Krzysztof
Rob Herring (Arm) April 22, 2024, 8:26 a.m. UTC | #2 
On Mon, 22 Apr 2024 08:51:08 +0200, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---
>  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml: 'oneOf' conditional failed, one must be fixed:
	'unevaluatedProperties' is a required property
	'additionalProperties' is a required property
	hint: Either unevaluatedProperties or additionalProperties must be present
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Error: Documentation/devicetree/bindings/net/can/linux,lin-serdev.example.dts:18.9-15 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:427: Documentation/devicetree/bindings/net/can/linux,lin-serdev.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240422065114.3185505-6-christoph.fritz@hexdev.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski April 23, 2024, 6:55 a.m. UTC | #3 
On 22/04/2024 08:51, Christoph Fritz wrote:
> Add documentation of device tree bindings for serdev UART LIN-Bus
> devices equipped with LIN transceivers.
> 
> Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> ---

Please test patches before sending...

Best regards,
Krzysztof
Christoph Fritz May 2, 2024, 5:26 a.m. UTC | #4 
Hello Krzysztof,

 thanks for your feedback, please see my answers below.

On Mon, 2024-04-22 at 09:54 +0200, Krzysztof Kozlowski wrote:
> On 22/04/2024 08:51, Christoph Fritz wrote:
> > Add documentation of device tree bindings for serdev UART LIN-Bus
> > devices equipped with LIN transceivers.
> 
> A nit, subject: drop second/last, redundant "dt bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

OK

> 
> > 
> > Signed-off-by: Christoph Fritz <christoph.fritz@hexdev.de>
> > ---
> >  .../bindings/net/can/linux,lin-serdev.yaml    | 29 +++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > new file mode 100644
> > index 0000000000000..cb4e932ff249c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
> > @@ -0,0 +1,29 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Linux serdev LIN-Bus Support
> 
> This looks like Linux binding, but we expect here description of hardware.

OK


> > +
> > +description: |
> > +  LIN-Bus support for UART devices equipped with LIN transceivers,
> > +  utilizing the Serial Device Bus (serdev) interface.
> 
> serdev is Linux thingy, AFAIR. Please describe the hardware.

OK, in v2 it will get changed to:

"""
LIN transceiver, mostly hard-wired to a serial device, used for
communication on a LIN bus.
"""

> 
> > +
> > +  For more details on an adapter, visit: https://hexdev.de/hexlin#tty
> > +
> > +properties:
> > +  compatible:
> > +    const: linux,lin-serdev
> 
> Feels confusing. Your link describes real hardware, but you wrote
> bindings for software construct.
> 
> If you add this to DT, then it is hard-wired on the board, right?

Yes, it is hard-wired.

>  If so, how this could be a software construct?

It's not, but fairly generic, that's why I used 'linux,lin-serdev' as
compatible string. In v2 I'll change it to 'hexdev,lin-serdev'.

> > +
> > +required:
> > +  - compatible
> > +
> > +examples:
> > +  - |
> > +    &uart2 {
> 
> & does not make much sense here. I think you wanted it to be serial bus,
> so serial.

OK

> 
> > +      status = "okay";
> 
> Drop, it was not disabled anywhere.

OK

> 
> 
> > +      linbus {
> > +        compatible = "linux,lin-serdev";
> > +      };
> > +    };

Let me address these points, fix warnings from dt_binding_check and
reroll in v2.

Thanks
  -- Christoph
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
new file mode 100644
index 0000000000000..cb4e932ff249c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/linux,lin-serdev.yaml
@@ -0,0 +1,29 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/linux,lin-serdev.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linux serdev LIN-Bus Support
+
+description: |
+  LIN-Bus support for UART devices equipped with LIN transceivers,
+  utilizing the Serial Device Bus (serdev) interface.
+
+  For more details on an adapter, visit: https://hexdev.de/hexlin#tty
+
+properties:
+  compatible:
+    const: linux,lin-serdev
+
+required:
+  - compatible
+
+examples:
+  - |
+    &uart2 {
+      status = "okay";
+      linbus {
+        compatible = "linux,lin-serdev";
+      };
+    };