Hi Krzysztof, On Mon, Aug 22, 2022 at 7:39 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > 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". Ok, I got it. > > > > >> > >>> --- > >>> > >>> .../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? STM32F4 > > > > >> > >>> + > >>> + 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? IMHO, I think so. Thanks and regards, Dario > > > Best regards, > Krzysztof -- Dario Binacchi Embedded Linux Developer dario.binacchi@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com