On 26.11.2024 08:19:04, Krzysztof Kozlowski wrote: > On Mon, Nov 25, 2024 at 06:31:00PM +0200, Ciprian Costea wrote: > > From: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx> > > > > Add S32G2/S32G3 SoCs compatible strings. > > > > A particularity for these SoCs is the presence of separate interrupts for > > state change, bus errors, MBs 0-7 and MBs 8-127 respectively. > > > > Increase maxItems of 'interrupts' to 4 for S32G based SoCs and keep the > > same restriction for other SoCs. > > > > Also, as part of this commit, move the 'allOf' after the required > > properties to make the documentation easier to read. > > > > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx> > > Reviewed-by: Frank Li <Frank.Li@xxxxxxx> > > You made multiple changes afterwards, which invalidated the review. See > submitting-patches which explain what to do in such case. > > > --- > > .../bindings/net/can/fsl,flexcan.yaml | 46 +++++++++++++++++-- > > 1 file changed, 42 insertions(+), 4 deletions(-) > > ... > > > maxItems: 2 > > @@ -136,6 +143,37 @@ required: > > - reg > > - interrupts > > > > +allOf: > > + - $ref: can-controller.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: nxp,s32g2-flexcan > > + then: > > + properties: > > + interrupts: > > + items: > > + - description: > > + Message Buffer interrupt for mailboxes 0-7 > > Keep it in one line. According to the excel sheet the IRQ is also for the enhanced RX FIFO. > > > + - description: > > + Interrupt indicating that the CAN bus went to Buss Off state > > s/Interrupt indicating that// > Buss Off state status? What about: "Device went into Bus Off state" However from the excel sheet I read it as a device changes state, to Bus Off, finished Bus Off or transition from error counters from < 96 to >= 96. So "Device state change" would be a more complete description? > > + - description: > > + Interrupt indicating that errors were detected on the CAN bus > > Error detection? > > > + - description: > > + Message Buffer interrupt for mailboxes 8-127 (ored) nitpick: all these different events for the other interrupts are ored, so IMHO you can omit the "(ored)". > > + interrupt-names: > > + items: > > + - const: mb_0-7 > > Choose one: either underscores or hyphens. Keep it consistent in your > bindings. > > + - const: state > > + - const: berr The order of IRQ names is not consistent with the description. > > + - const: mb_8-127 > > Choose one: either underscores or hyphens. Keep it consistent in your > bindings. > > > + required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-names > > What happened to "else:"? Why all other devices now have up to 4 interrupts? Do you already have a dtsi snippet for the flexcan nodes? Please make sure that the interrupts are correctly mapped. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
