Hey William, On Mon, Jan 29, 2024 at 11:12:37AM +0800, William Qiu wrote: > Add bindings for CAST CAN Controller > > Signed-off-by: William Qiu <william.qiu@xxxxxxxxxxxxxxxx> > --- > .../devicetree/bindings/net/can/cast,can.yaml | 95 +++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/can/cast,can.yaml > > diff --git a/Documentation/devicetree/bindings/net/can/cast,can.yaml b/Documentation/devicetree/bindings/net/can/cast,can.yaml > new file mode 100644 > index 000000000000..ea52132d9b1c > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/can/cast,can.yaml > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/can/cast,can.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: CAST CAN controller > + > +maintainers: > + - William Qiu <william.qiu@xxxxxxxxxxxxxxxx> > + > +allOf: > + - $ref: can-controller.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: starfive,can > + then: > + required: > + - starfive,syscon If you've got property related stuff in the allOf, move it down after the property definitions. > +properties: > + compatible: > + enum: > + - cast,can > + - cast,canfd I don't like these uber generic compatibles that have no users as a fallback. Allowing them in the binding only really discourages people from creating device specific compatibles. Secondly, this is some purchased IP that I am sure has a versioning scheme and the compatibles that you have created do not reflect that. If they were being used as a fallback, I would request some versioning. That's not going to really work though since the canfd features on the jh7110 require setting u0_can_ctrl_can_fd_enable, so neither of these compatibles really has a use right now. > + - starfive,can Just "starfive,can"? Can you please add device specific compatibles for the SoCs on which this is used? > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 3 > + > + clock-names: > + items: > + - const: apb_clk > + - const: timer_clk > + - const: can_clk Drop _clk, they're all clocks! > + > + resets: > + minItems: 3 > + > + reset-names: > + items: > + - const: rst_apb > + - const: rst_core > + - const: rst_timer Same here, drop rst_ > + > + starfive,syscon: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to System Register Controller syscon node > + - description: offset of SYS_SYSCON_NE__SAIF__SYSCFG register for CAN controller The docs I have call this register "SYS_SYSCONSAIF__SYSCFG". Did the names change since the TRM I have was written? > + - description: shift of SYS_SYSCON_NE__SAIF__SYSCFG register for CAN controller > + - description: mask of SYS_SYSCON_NE__SAIF__SYSCFG register for CAN controller > + description: > + Should be four parameters, the phandle to System Register Controller > + syscon node and the offset/shift/mask of SYS_SYSCON_NE__SAIF__SYSCFG register > + for CAN controller. Cheers, Conor.
Attachment:
signature.asc
Description: PGP signature
