Re: [RFC PATCH v2 17/30] Documentation/devicetree: Add renesas,sh7751-irl-ext binding document.

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:27 AM Yoshinori Sato
<[email protected]> wrote:
> Signed-off-by: Yoshinori Sato <[email protected]>

Thanks for your patch!

> ---
>  .../renesas,sh7751-irl-ext.yaml               | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml
> new file mode 100644
> index 000000000000..dfc35717b92a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,sh7751-irl-ext.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7751 IRL external encoder with enable regs.
> +
> +maintainers:
> +  - Yoshinori Sato <[email protected]>
> +
> +description: |
> +  This is the general used external interrupt encoder on SH7751 based boards.

generally

> +
> +properties:
> +  compatible:
> +    items:
> +      - const: renesas,sh7751-irl-ext
> +
> +  '#interrupt-cells':
> +    # an interrupt index and flags, as defined in interrupts.txt in
> +    # this directory
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  reg:
> +    minItems: 1
> +
> +  sh7751irl,width:

Vendor prefix should be "renesas" (everywhere).

> +    description: Enable register width

Specify

> +    $ref: /schemas/types.yaml#/definitions/uint32

Perhaps this should be specified implicitly, by using a
different compatible value depending on the register width?


> +
> +  sh7751irl,polarity:
> +    description: Enable register polarity

Specify

> +    $ref: /schemas/types.yaml#/definitions/uint32

Is this the interrupt polarity?  If yes, that should be specified
by the consumer through the last cell of the interrupts property
(IRQ_TYPE_*).

> +
> +  sh7751irl,irqbit:
> +    description: IRQ to enable register bit mapping
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +
> +required:
> +  - compatible
> +  - '#interrupt-cells'
> +  - interrupt-controller
> +  - reg
> +  - sh7751irl,width
> +  - sh7751irl,polarity
> +  - sh7751,irqbit

Vendor prefix does not match above.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        r2dintc: sh7751irl_encoder@a4000000 {

Please align "r2dintc" with the "|" above.

> +                compatible = "renesas,sh7751-irl-ext";
> +                reg = <0xa4000000 0x02>;
> +                interrupt-controller;
> +                #address-cells = <1>;

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml

    sh7751irl_encoder@a4000000: '#address-cells' does not match any of
the regexes: 'pinctrl-[0-9]+'

> +                #interrupt-cells = <2>;
> +                sh7751irl,width = <16>;
> +                sh7751irl,polarity = <0>;
> +                sh7751irl,irqbit =<11>,         /* PCI INTD */
> +                                  <9>,          /* CF IDE */
> +                                  <8>,          /* CF CD */
> +                                  <12>,         /* PCI INTC */
> +                                  <10>,         /* SM501 */
> +                                  <6>,          /* KEY */
> +                                  <5>,          /* RTC ALARM */
> +                                  <4>,          /* RTC T */
> +                                  <7>,          /* SDCARD */
> +                                  <14>,         /* PCI INTA */
> +                                  <13>,         /* PCI INTB */
> +                                  <0>,          /* EXT */
> +                                  <15>;         /* TP */

    'sh7751irl,irqbit', 'sh7751irl,polarity', 'sh7751irl,width' do not
match any of the regexes ...

> +        };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds