[v2,01/12] spi: dt-bindings: introduce FIFO depth properties

Message ID 20240212140331.915498-2-tudor.ambarus@linaro.org
State New
Headers
Series spi: s3c64xx: remove OF alias ID dependency |

Commit Message

Tudor Ambarus Feb. 12, 2024, 2:03 p.m. UTC
  There are SPI IPs that can be configured by the integrator with a
specific FIFO depth depending on the system's capabilities. For example,
the samsung USI SPI IP can be configured by the integrator with a TX/RX
FIFO from 8 byte to 256 bytes.

Introduce the ``fifo-depth`` property for such instances of IPs where the
same FIFO depth is used for both RX and TX. Introduce ``rx-fifo-depth``
and ``tx-fifo-depth`` properties for cases where the RX FIFO depth is
different from the TX FIFO depth.

Make the dedicated RX/TX properties dependent on each other and mutual
exclusive with the other.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 .../bindings/spi/spi-controller.yaml          | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
  

Comments

Rob Herring Feb. 12, 2024, 3:36 p.m. UTC | #1
On Mon, 12 Feb 2024 14:03:20 +0000, Tudor Ambarus wrote:
> There are SPI IPs that can be configured by the integrator with a
> specific FIFO depth depending on the system's capabilities. For example,
> the samsung USI SPI IP can be configured by the integrator with a TX/RX
> FIFO from 8 byte to 256 bytes.
> 
> Introduce the ``fifo-depth`` property for such instances of IPs where the
> same FIFO depth is used for both RX and TX. Introduce ``rx-fifo-depth``
> and ``tx-fifo-depth`` properties for cases where the RX FIFO depth is
> different from the TX FIFO depth.
> 
> Make the dedicated RX/TX properties dependent on each other and mutual
> exclusive with the other.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  .../bindings/spi/spi-controller.yaml          | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 

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:
/Documentation/devicetree/bindings/spi/spi-controller.yaml:152:9: [warning] wrong indentation: expected 6 but found 8 (indentation)
/Documentation/devicetree/bindings/spi/spi-controller.yaml:156:9: [warning] wrong indentation: expected 6 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240212140331.915498-2-tudor.ambarus@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  
Tudor Ambarus Feb. 13, 2024, 8:16 a.m. UTC | #2
On 12.02.2024 17:36, Rob Herring wrote:
> 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:
> ./Documentation/devicetree/bindings/spi/spi-controller.yaml:152:9: 
> [warning] wrong indentation: expected 6 but found 8 (indentation)
> ./Documentation/devicetree/bindings/spi/spi-controller.yaml:156:9: 
> [warning] wrong indentation: expected 6 but found 8 (indentation)
> 
> dtschema/dtc warnings/errors:

oh, the horror, I missed these. I'll re-check and resend.

Thanks,
ta
  
Rob Herring Feb. 13, 2024, 1:14 p.m. UTC | #3
On Mon, Feb 12, 2024 at 02:03:20PM +0000, Tudor Ambarus wrote:
> There are SPI IPs that can be configured by the integrator with a
> specific FIFO depth depending on the system's capabilities. For example,
> the samsung USI SPI IP can be configured by the integrator with a TX/RX
> FIFO from 8 byte to 256 bytes.
> 
> Introduce the ``fifo-depth`` property for such instances of IPs where the
> same FIFO depth is used for both RX and TX. Introduce ``rx-fifo-depth``
> and ``tx-fifo-depth`` properties for cases where the RX FIFO depth is
> different from the TX FIFO depth.
> 
> Make the dedicated RX/TX properties dependent on each other and mutual
> exclusive with the other.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  .../bindings/spi/spi-controller.yaml          | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)

With the indentation fixed,

Reviewed-by: Rob Herring <robh@kernel.org>
  
kernel test robot Feb. 16, 2024, 7:41 a.m. UTC | #4
Hi Tudor,

kernel test robot noticed the following build warnings:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on robh/for-next linus/master v6.8-rc4 next-20240216]
[cannot apply to krzk/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tudor-Ambarus/spi-dt-bindings-introduce-FIFO-depth-properties/20240212-221427
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link:    https://lore.kernel.org/r/20240212140331.915498-2-tudor.ambarus%40linaro.org
patch subject: [PATCH v2 01/12] spi: dt-bindings: introduce FIFO depth properties
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240216/202402161543.5JdIODY4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402161543.5JdIODY4-lkp@intel.com/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/spi/spi-controller.yaml:152:9: [warning] wrong indentation: expected 6 but found 8 (indentation)
   Documentation/devicetree/bindings/spi/spi-controller.yaml:156:9: [warning] wrong indentation: expected 6 but found 8 (indentation)

vim +152 Documentation/devicetree/bindings/spi/spi-controller.yaml

     8	
     9	maintainers:
    10	  - Mark Brown <broonie@kernel.org>
    11	
    12	description: |
    13	  SPI busses can be described with a node for the SPI controller device
    14	  and a set of child nodes for each SPI slave on the bus. The system SPI
    15	  controller may be described for use in SPI master mode or in SPI slave mode,
    16	  but not for both at the same time.
    17	
    18	properties:
    19	  $nodename:
    20	    pattern: "^spi(@.*|-([0-9]|[1-9][0-9]+))?$"
    21	
    22	  "#address-cells":
    23	    enum: [0, 1]
    24	
    25	  "#size-cells":
    26	    const: 0
    27	
    28	  cs-gpios:
    29	    description: |
    30	      GPIOs used as chip selects.
    31	      If that property is used, the number of chip selects will be
    32	      increased automatically with max(cs-gpios, hardware chip selects).
    33	
    34	      So if, for example, the controller has 4 CS lines, and the
    35	      cs-gpios looks like this
    36	        cs-gpios = <&gpio1 0 0>, <0>, <&gpio1 1 0>, <&gpio1 2 0>;
    37	
    38	      Then it should be configured so that num_chipselect = 4, with
    39	      the following mapping
    40	        cs0 : &gpio1 0 0
    41	        cs1 : native
    42	        cs2 : &gpio1 1 0
    43	        cs3 : &gpio1 2 0
    44	
    45	      The second flag of a gpio descriptor can be GPIO_ACTIVE_HIGH (0)
    46	      or GPIO_ACTIVE_LOW(1). Legacy device trees often use 0.
    47	
    48	      There is a special rule set for combining the second flag of an
    49	      cs-gpio with the optional spi-cs-high flag for SPI slaves.
    50	
    51	      Each table entry defines how the CS pin is to be physically
    52	      driven (not considering potential gpio inversions by pinmux):
    53	
    54	      device node     | cs-gpio       | CS pin state active | Note
    55	      ================+===============+=====================+=====
    56	      spi-cs-high     | -             | H                   |
    57	      -               | -             | L                   |
    58	      spi-cs-high     | ACTIVE_HIGH   | H                   |
    59	      -               | ACTIVE_HIGH   | L                   | 1
    60	      spi-cs-high     | ACTIVE_LOW    | H                   | 2
    61	      -               | ACTIVE_LOW    | L                   |
    62	
    63	      Notes:
    64	      1) Should print a warning about polarity inversion.
    65	         Here it would be wise to avoid and define the gpio as
    66	         ACTIVE_LOW.
    67	      2) Should print a warning about polarity inversion
    68	         because ACTIVE_LOW is overridden by spi-cs-high.
    69	         Should be generally avoided and be replaced by
    70	         spi-cs-high + ACTIVE_HIGH.
    71	
    72	  fifo-depth:
    73	    $ref: /schemas/types.yaml#/definitions/uint32
    74	    description:
    75	      Size of the RX and TX data FIFOs in bytes.
    76	
    77	  rx-fifo-depth:
    78	    $ref: /schemas/types.yaml#/definitions/uint32
    79	    description:
    80	      Size of the RX data FIFO in bytes.
    81	
    82	  tx-fifo-depth:
    83	    $ref: /schemas/types.yaml#/definitions/uint32
    84	    description:
    85	      Size of the TX data FIFO in bytes.
    86	
    87	  num-cs:
    88	    $ref: /schemas/types.yaml#/definitions/uint32
    89	    description:
    90	      Total number of chip selects.
    91	
    92	  spi-slave:
    93	    $ref: /schemas/types.yaml#/definitions/flag
    94	    description:
    95	      The SPI controller acts as a slave, instead of a master.
    96	
    97	  slave:
    98	    type: object
    99	
   100	    properties:
   101	      compatible:
   102	        description:
   103	          Compatible of the SPI device.
   104	
   105	    required:
   106	      - compatible
   107	
   108	patternProperties:
   109	  "^.*@[0-9a-f]+$":
   110	    type: object
   111	    $ref: spi-peripheral-props.yaml
   112	    additionalProperties: true
   113	
   114	    properties:
   115	      spi-3wire:
   116	        $ref: /schemas/types.yaml#/definitions/flag
   117	        description:
   118	          The device requires 3-wire mode.
   119	
   120	      spi-cpha:
   121	        $ref: /schemas/types.yaml#/definitions/flag
   122	        description:
   123	          The device requires shifted clock phase (CPHA) mode.
   124	
   125	      spi-cpol:
   126	        $ref: /schemas/types.yaml#/definitions/flag
   127	        description:
   128	          The device requires inverse clock polarity (CPOL) mode.
   129	
   130	    required:
   131	      - compatible
   132	      - reg
   133	
   134	dependencies:
   135	  rx-fifo-depth: [ tx-fifo-depth ]
   136	  tx-fifo-depth: [ rx-fifo-depth ]
   137	
   138	allOf:
   139	  - if:
   140	      not:
   141	        required:
   142	          - spi-slave
   143	    then:
   144	      properties:
   145	        "#address-cells":
   146	          const: 1
   147	    else:
   148	      properties:
   149	        "#address-cells":
   150	          const: 0
   151	  - not:
 > 152	        required:
   153	          - fifo-depth
   154	          - rx-fifo-depth
   155	  - not:
   156	        required:
   157	          - fifo-depth
   158	          - tx-fifo-depth
   159
  

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
index 524f6fe8c27b..add39884d226 100644
--- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
@@ -69,6 +69,21 @@  properties:
          Should be generally avoided and be replaced by
          spi-cs-high + ACTIVE_HIGH.
 
+  fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Size of the RX and TX data FIFOs in bytes.
+
+  rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Size of the RX data FIFO in bytes.
+
+  tx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Size of the TX data FIFO in bytes.
+
   num-cs:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
@@ -116,6 +131,10 @@  patternProperties:
       - compatible
       - reg
 
+dependencies:
+  rx-fifo-depth: [ tx-fifo-depth ]
+  tx-fifo-depth: [ rx-fifo-depth ]
+
 allOf:
   - if:
       not:
@@ -129,6 +148,14 @@  allOf:
       properties:
         "#address-cells":
           const: 0
+  - not:
+        required:
+          - fifo-depth
+          - rx-fifo-depth
+  - not:
+        required:
+          - fifo-depth
+          - tx-fifo-depth
 
 additionalProperties: true