On 20/08/2022 11:08, Dario Binacchi wrote: > Hi Krzysztof, > > On Thu, Aug 18, 2022 at 10:22 AM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 17/08/2022 17:35, Dario Binacchi wrote: >>> Add documentation of device tree bindings for the STM32 basic extended >>> CAN (bxcan) controller. >>> >>> Signed-off-by: Dario Binacchi <dariobin@xxxxxxxxx> >>> Signed-off-by: Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> >> >> 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". > >> >>> --- >>> >>> .../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) > >> >>> @@ -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// > >> >>> + >>> +description: STMicroelectronics BxCAN controller for CAN bus >>> + >>> +maintainers: >>> + - Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> >>> + >>> +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? > >> >>> + >>> + 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"). >> >>> + "^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? Best regards, Krzysztof
