[RFC,v2,09/30] Documentation/devicetree: Add renesas,sh7751-pci binding document.

Commit Message

Yoshinori Sato Sept. 13, 2023, 9:23 a.m. UTC

Signed-off-by: Yoshinori Sato <[e-mail]>
---
 .../bindings/pci/renesas,sh7751-pci.yaml      | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml

Comments

Krzysztof Kozlowski Sept. 13, 2023, 10:42 a.m. UTC | #1

On 13/09/2023 11:23, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato <[e-mail]>

You did not resolve several comments from previous version. I don't
understand why, so I just assume this is not ready for review. Just
quick look tells this wasn't tested and has multiple issues. Maybe that
was the intention, but nothing is described in commit log confirming
such intention.

Therefore to be clear: that's a NAK.

Also, standard template:

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.


Best regards,
Krzysztof

Geert Uytterhoeven Sept. 18, 2023, 3:41 p.m. UTC | #2

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato
<[e-mail]> wrote:
> Signed-off-by: Yoshinori Sato <[e-mail]>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/renesas,sh7751-pci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7751 PCI Host controller
> +
> +maintainers:
> +  - Yoshinori Sato <[e-mail]>
> +
> +properties:
> +  compatible:
> +      - items:

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:14:7:
[warning] wrong indentation: expected 4 but found 6 (indentation)

> +          - enum:
> +              - renesas,r2d-pci                 # Renesas RTS7751R2D board

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:16:32:
[error] syntax error: found character '\t' that cannot start any token
(syntax)

> +              - iodata,julian-pci        # IO DATA DEVICE Julian board

Please drop the two board-specific compatible values, they do not
represent different implementations of the PCI core in the SH7751 SoC.

> +          - const: renesas,sh7751-pci
> +
> +  reg:
> +    minItems: 3
> +
> +  interrupt-cells:
> +    const: 1
> +
> +  address-cells:
> +    const: 3
> +
> +  size-cells:
> +    const: 2;
> +
> +  range:
> +    description: |
> +      The PCI bus memory area and I/O area.
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-cells
> +  - address-cells
> +  - size-cells
> +  - range
> +
> +examples:
> +  - |
> +        pci@fe200000 {

Please align "pci" with the "|" above

> +                compatible = "renesas,sh7751-pci";

Please indent by 4 spaces

> +                #address-cells = <3>;
> +                #size-cells = <2>;
> +                ranges = <0x02000000 0 0xfd000000 0xfd000000 0 0x01000000>,
> +                         <0x01000000 0 0xfe240000 0xfe240000 0 0x00040000>;
> +                reg = <0xfe200000 0x0400>,
> +                      <0x0c000000 0x04000000>,
> +                      <0xff800000 0x0030>;
> +                #interrupt-cells = <1>;

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.example.dtb:
pci@fe200000: 'device_type' is a required property
        from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#

> +        };

Gr{oetje,eeting}s,

                        Geert

Patch

diff --git a/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
new file mode 100644
index 000000000000..17a24e24c393
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
@@ -0,0 +1,57 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/renesas,sh7751-pci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas SH7751 PCI Host controller
+
+maintainers:
+  - Yoshinori Sato <[e-mail]>
+
+properties:
+  compatible:
+      - items:
+          - enum:
+              - renesas,r2d-pci	         # Renesas RTS7751R2D board
+              - iodata,julian-pci        # IO DATA DEVICE Julian board
+          - const: renesas,sh7751-pci
+
+  reg:
+    minItems: 3
+
+  interrupt-cells:
+    const: 1
+
+  address-cells:
+    const: 3
+
+  size-cells:
+    const: 2;
+
+  range:
+    description: |
+      The PCI bus memory area and I/O area.
+
+
+required:
+  - compatible
+  - reg
+  - interrupt-cells
+  - address-cells
+  - size-cells
+  - range
+
+examples:
+  - |
+        pci@fe200000 {
+                compatible = "renesas,sh7751-pci";
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges = <0x02000000 0 0xfd000000 0xfd000000 0 0x01000000>,
+                         <0x01000000 0 0xfe240000 0xfe240000 0 0x00040000>;
+                reg = <0xfe200000 0x0400>,
+                      <0x0c000000 0x04000000>,
+                      <0xff800000 0x0030>;
+                #interrupt-cells = <1>;
+        };