Hi Krzysztof,

On Tue, Aug 23, 2022 at 3:41 PM Krzysztof Kozlowski
[off-list ref] wrote:

On 20/08/2022 11:29, 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>

---

Changes in v2:
- Change the file name into 'st,stm32-bxcan-core.yaml'.
- Rename compatibles:
  - st,stm32-bxcan-core -> st,stm32f4-bxcan-core
  - st,stm32-bxcan -> st,stm32f4-bxcan
- Rename master property to st,can-master.
- Remove the status property from the example.
- Put the node child properties as required.

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

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

@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/st,stm32-bxcan.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics bxCAN controller
+
+description: STMicroelectronics BxCAN controller for CAN bus
+
+maintainers:
+  - Dario Binacchi <dario.binacchi@amarulasolutions.com>
+
+allOf:
+  - $ref: can-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - st,stm32f4-bxcan-core
+
+  reg:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  clocks:
+    description:
+      Input clock for registers access
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - resets
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+patternProperties:

No improvements here, so my comment stay. Please fix it.

Sorry, I'ff fix it in version 3.

quoted

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

I still do not understand why you need children. You did not CC me on
driver change, so difficult to say. You did not describe the parent

On the next submissions I'll send you all the series patches.

device - there is no description.

Ok, I'll do it.

Why do you need parent device at all?
This looks like some driver-driven-bindings instead of just real
hardware description.

The two devices are not independent.
As described in the reference manual RM0386 (STM32F469xx and STM32F479xx
advanced Arm®-based 32-bit MCUs) in paragraph 34.2, the bxCAN controller is a
dual CAN peripheral configuration:

• CAN1: Master bxCAN for managing the communication between a Slave bxCAN and
the 512-byte SRAM memory
• CAN2: Slave bxCAN, with no direct access to the SRAM memory.

So, if I want to use CAN2 only (and not CAN1), I need to be able to use shared
resources with CAN1 without having to probe the CAN1 driver. IMHO here is the
justification of the parent node.

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