diff mbox series

[v3,2/3] dt-bindings: mmc: Add bindings for LiteSDCard

Message ID 20211208132042.3226275-3-gsomlo@gmail.com (mailing list archive)
State New, archived
Headers show
Series mmc: Add LiteSDCard mmc driver | expand

Commit Message

Gabriel L. Somlo Dec. 8, 2021, 1:20 p.m. UTC
LiteSDCard is a small footprint, configurable SDCard core for FPGA
based system on chips.

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
---

New in v3:
  - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
    bindings document (please let me know if that was premature, and
    happy to take further review if needed :)
  - add dedicated DT property for source clock frequency

 .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml

Comments

Rob Herring Dec. 8, 2021, 11:06 p.m. UTC | #1
On Wed, 08 Dec 2021 08:20:41 -0500, Gabriel Somlo wrote:
> LiteSDCard is a small footprint, configurable SDCard core for FPGA
> based system on chips.
> 
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> 
> New in v3:
>   - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
>     bindings document (please let me know if that was premature, and
>     happy to take further review if needed :)
>   - add dedicated DT property for source clock frequency
> 
>  .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names:items: 'oneOf' conditional failed, one must be fixed:
	[{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}] is not of type 'object'
	'irq (optional)' does not match '^[a-zA-Z0-9,.\\-_ #+/]+$'
	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg: {'items': [{'description': 'PHY registers'}, {'description': 'CORE registers'}, {'description': 'DMA Reader buffer'}, {'description': 'DMA Writer buffer'}, {'description': 'IRQ registers (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names: {'items': [{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: ignoring, error in schema: properties: reg-names: items
warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/litex,mmc.yaml
Documentation/devicetree/bindings/mmc/litex,mmc.example.dt.yaml:0:0: /example-0/mmc@12005000: failed to match any schema with compatible: ['litex,mmc']

doc reference errors (make refcheckdocs):


See https://patchwork.ozlabs.org/patch/1565210

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Gabriel L. Somlo Dec. 9, 2021, 1:08 a.m. UTC | #2
On Wed, Dec 08, 2021 at 05:06:46PM -0600, Rob Herring wrote:
> On Wed, 08 Dec 2021 08:20:41 -0500, Gabriel Somlo wrote:
> > LiteSDCard is a small footprint, configurable SDCard core for FPGA
> > based system on chips.
> > 
> > Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> > 
> > New in v3:
> >   - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
> >     bindings document (please let me know if that was premature, and
> >     happy to take further review if needed :)
> >   - add dedicated DT property for source clock frequency
> > 
> >  .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names:items: 'oneOf' conditional failed, one must be fixed:
> 	[{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}] is not of type 'object'
> 	'irq (optional)' does not match '^[a-zA-Z0-9,.\\-_ #+/]+$'
> 	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg: {'items': [{'description': 'PHY registers'}, {'description': 'CORE registers'}, {'description': 'DMA Reader buffer'}, {'description': 'DMA Writer buffer'}, {'description': 'IRQ registers (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
> 	hint: "maxItems" is not needed with an "items" list
> 	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names: {'items': [{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
> 	hint: "maxItems" is not needed with an "items" list
> 	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: ignoring, error in schema: properties: reg-names: items
> warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> Documentation/devicetree/bindings/mmc/litex,mmc.example.dt.yaml:0:0: /example-0/mmc@12005000: failed to match any schema with compatible: ['litex,mmc']
> 
> doc reference errors (make refcheckdocs):
> 
> 
> See https://patchwork.ozlabs.org/patch/1565210
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 

Thanks! 

I made the following changes:

--- a/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
+++ b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
@@ -29,9 +29,8 @@ properties:
       - description: CORE registers
       - description: DMA Reader buffer
       - description: DMA Writer buffer
-      - description: IRQ registers (optional)
+      - description: IRQ registers
     minItems: 4
-    maxItems: 5

   reg-names:
     items:
@@ -39,12 +38,13 @@ properties:
       - const: core
       - const: reader
       - const: writer
-      - const: irq (optional)
+      - const: irq
     minItems: 4
-    maxItems: 5

   clocks:
     maxItems: 1
+    description:
+      Handle to reference clock.

   interrupts:
     maxItems: 1

... which took care of the bulk of the error messages reported. However,
I'm still getting the one below, whether or not I leave the `maxItems 1`
line there under `clocks:`

$ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
/home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
/home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
  DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
  DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
  ...

It appears as though `make dt_binding_check` is trying to read from
`Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
does not exist. The clock reference I'm talking about could be *any*
clock elsewhere in the dts!

This wasn't part of the originally reported errors, not sure why I'm
seeing it now. Also, not sure what (if anything) I still need to do
about it, any advice much appreciated!

Thanks,
--Gabriel
Geert Uytterhoeven Dec. 9, 2021, 8:37 a.m. UTC | #3
Hi Gabriel,

On Thu, Dec 9, 2021 at 2:08 AM Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> ... which took care of the bulk of the error messages reported. However,
> I'm still getting the one below, whether or not I leave the `maxItems 1`
> line there under `clocks:`
>
> $ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
>         hint: "maxItems" is not needed with an "items" list
>         from schema $id: http://devicetree.org/meta-schemas/items.yaml#
>   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
> warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
>   DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
>   DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
>   ...

--- a/Documentation/devicetree/bindings/clock/litex,clock.yaml
+++ b/Documentation/devicetree/bindings/clock/litex,clock.yaml
@@ -45,7 +45,6 @@ properties:
       List of strings of clock output signal names indexed
       by the first cell in the clock specifier.
     minItems: 1
-    maxItems: 7
     items:
       - const: CLKOUT0
       - const: CLKOUT1

I have that in my local tree, but hadn't sent it to you yet, because
litex,clock definitely need more work.

> It appears as though `make dt_binding_check` is trying to read from
> `Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
> does not exist. The clock reference I'm talking about could be *any*

Oh, it does exist in your tree ;-)
To check the examples, it has to apply all other binding files that
might apply, hence some checks are always run.

You can avoid some (but not all) such checks by adding

    DT_SCHEMA_FILES=Documentation/devicetree/bindings/path/to/binding.yaml

> clock elsewhere in the dts!
>
> This wasn't part of the originally reported errors, not sure why I'm
> seeing it now. Also, not sure what (if anything) I still need to do
> about it, any advice much appreciated!

Of course, as Rob doesn't have that file in his tree.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Gabriel L. Somlo Dec. 9, 2021, 6:25 p.m. UTC | #4
On Thu, Dec 09, 2021 at 09:37:27AM +0100, Geert Uytterhoeven wrote:
> Hi Gabriel,
> 
> On Thu, Dec 9, 2021 at 2:08 AM Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> > ... which took care of the bulk of the error messages reported. However,
> > I'm still getting the one below, whether or not I leave the `maxItems 1`
> > line there under `clocks:`
> >
> > $ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> > /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
> >         hint: "maxItems" is not needed with an "items" list
> >         from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> > /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
> > warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
> >   DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
> >   DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
> >   ...
> 
> --- a/Documentation/devicetree/bindings/clock/litex,clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/litex,clock.yaml
> @@ -45,7 +45,6 @@ properties:
>        List of strings of clock output signal names indexed
>        by the first cell in the clock specifier.
>      minItems: 1
> -    maxItems: 7
>      items:
>        - const: CLKOUT0
>        - const: CLKOUT1
> 
> I have that in my local tree, but hadn't sent it to you yet, because
> litex,clock definitely need more work.
> 
> > It appears as though `make dt_binding_check` is trying to read from
> > `Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
> > does not exist. The clock reference I'm talking about could be *any*
> 
> Oh, it does exist in your tree ;-)
> To check the examples, it has to apply all other binding files that
> might apply, hence some checks are always run.
> 
> You can avoid some (but not all) such checks by adding
> 
>     DT_SCHEMA_FILES=Documentation/devicetree/bindings/path/to/binding.yaml
> 
> > clock elsewhere in the dts!
> >
> > This wasn't part of the originally reported errors, not sure why I'm
> > seeing it now. Also, not sure what (if anything) I still need to do
> > about it, any advice much appreciated!
> 
> Of course, as Rob doesn't have that file in his tree.

Oh, I'm working on the `litex-rebase` branch, which does have the
litex,clock file. Running the check on the Linus master with litex_mmc
v4 on top now passes the check without any errors or warnings. I'll
incorporate the fixes pointed out by Rob when I publish v4.

Sorry for the misunderstanding, thanks Geert for pointing out the
source of my confusion -- I think all's well now on the dt-bindings
front w.r.t. litex_mmc... :)

Cheers,
--Gabriel
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/litex,mmc.yaml b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
new file mode 100644
index 000000000000..9b8d4ec30375
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
@@ -0,0 +1,72 @@ 
+# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/litex,mmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LiteX LiteSDCard device
+
+maintainers:
+  - Gabriel Somlo <gsomlo@gmail.com>
+
+description: |
+  LiteSDCard is a small footprint, configurable SDCard core for FPGA based
+  system on chips.
+
+  The hardware source is Open Source and can be found on at
+  https://github.com/enjoy-digital/litesdcard/.
+
+allOf:
+  - $ref: mmc-controller.yaml#
+
+properties:
+  compatible:
+    const: litex,mmc
+
+  reg:
+    items:
+      - description: PHY registers
+      - description: CORE registers
+      - description: DMA Reader buffer
+      - description: DMA Writer buffer
+      - description: IRQ registers (optional)
+    minItems: 4
+    maxItems: 5
+
+  reg-names:
+    items:
+      - const: phy
+      - const: core
+      - const: reader
+      - const: writer
+      - const: irq (optional)
+    minItems: 4
+    maxItems: 5
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    mmc: mmc@12005000 {
+        compatible = "litex,mmc";
+        reg = <0x12005000 0x100>,
+              <0x12003800 0x100>,
+              <0x12003000 0x100>,
+              <0x12004800 0x100>,
+              <0x12004000 0x100>;
+        reg-names = "phy", "core", "reader", "writer", "irq";
+        clocks = <&reference_clk>;
+        interrupts = <4>;
+    };