Re: [PATCH v2 1/3] dt-bindings: can: fsl,flexcan: add S32G2/S32G3 SoC support
From: Krzysztof Kozlowski
Date: Tue Nov 26 2024 - 02:19:15 EST
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.
> + - description:
> + Interrupt indicating that the CAN bus went to Buss Off state
s/Interrupt indicating that//
Buss Off state status?
> + - description:
> + Interrupt indicating that errors were detected on the CAN bus
Error detection?
> + - description:
> + Message Buffer interrupt for mailboxes 8-127 (ored)
> + interrupt-names:
> + items:
> + - const: mb_0-7
Choose one: either underscores or hyphens. Keep it consistent in your
bindings.
> + - const: state
> + - const: berr
> + - 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?
Best regards,
Krzysztof