Hi Krzysztof,

On Mon, Aug 22, 2022 at 7:39 PM Krzysztof Kozlowski
[off-list ref] wrote:

On 20/08/2022 11:08, Dario Binacchi wrote:

quoted

Hi Krzysztof,

On Thu, Aug 18, 2022 at 10:22 AM Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 17/08/2022 17:35, Dario Binacchi wrote:

quoted

Add documentation of device tree bindings for the STM32 basic extended
CAN (bxcan) controller.

Signed-off-by: Dario Binacchi <redacted>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

You do not need two SoBs. Keep only one, matching the From field.

I started implementing this driver in my spare time, so my intention
was to keep track of it.

SoB is not related to copyrights. Keep personal copyrights (with/next to
work ones), but SoB is coming from a person and that's only one. Choose
one "person".

Ok, I got it.

quoted

quoted

quoted

---

 .../devicetree/bindings/net/can/st,bxcan.yaml | 139 ++++++++++++++++++
 1 file changed, 139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/st,bxcan.yaml

diff --git a/Documentation/devicetree/bindings/net/can/st,bxcan.yaml b/Documentation/devicetree/bindings/net/can/st,bxcan.yaml
new file mode 100644
index 000000000000..f4cfd26e4785
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/st,bxcan.yaml

File name like compatible, so st,stm32-bxcan-core.yaml (or some other
name, see comment later)

quoted

quoted

@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/st,bxcan.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics bxCAN controller Device Tree Bindings

s/Device Tree Bindings//

quoted

quoted

+
+description: STMicroelectronics BxCAN controller for CAN bus
+
+maintainers:
+  - Dario Binacchi [off-list ref]
+
+allOf:
+  - $ref: can-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - st,stm32-bxcan-core

compatibles are supposed to be specific. If this is some type of
micro-SoC, then it should have its name/number. If it is dedicated
device, is the final name bxcan core? Google says  the first is true, so
you miss specific device part.

I don't know if I understand correctly, I hope the change in version 2
is what you requested.

What is the name of the SoC, where this is in?

STM32F4

quoted

quoted

quoted

+
+  reg:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  clocks:
+    description:
+      Input clock for registers access
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - resets
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+additionalProperties: false
+
+patternProperties:

This goes after "properties: in top level (before "required").

quoted

+  "^can@[0-9]+$":
+    type: object
+    description:
+      A CAN block node contains two subnodes, representing each one a CAN
+      instance available on the machine.
+
+    properties:
+      compatible:
+        enum:
+          - st,stm32-bxcan

Why exactly do you need compatible for the child? Is it an entierly
separate device?

I took inspiration from other drivers for ST microcontroller
peripherals (e. g. drivers/iio/adc/stm32-adc-core.c,
drivers/iio/adc/stm32-adc.c) where
some resources are shared between the peripheral instances. In the
case of CAN, master (CAN1) and slave (CAN2) share the registers for
configuring the filters and the clock.
In the core module you can find the functions about the shared
resources, while the childrens implement the driver.

In both cases you refer to the driver, but we talk here about bindings
which are rather not related. So I repeat the question - is the child
entirely separate device which can be used in other devices?

IMHO, I think so.

Thanks and regards,
Dario

Best regards,
Krzysztof

-- 

Dario Binacchi

Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com