Re: [PATCH v6 2/2] dt-bindings: memory-controllers: gpmc-child: add wait-pin polarity

[Rob Herring <robh@kernel.org>]

On Thu, Sep 29, 2022 at 02:56:39PM +0200, B. Niedermayr wrote:
> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> 
> The GPMC controller has the ability to configure the polarity for the
> wait pin. The current properties do not allow this configuration.
> This binding directly configures the WAITPIN<X>POLARITY bit
> in the GPMC_CONFIG register by setting the gpmc,wait-pin-polarity
> dt-property.
> 
> Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
> ---
>  .../bindings/memory-controllers/ti,gpmc-child.yaml         | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
> index 6e3995bb1630..477189973334 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc-child.yaml
> @@ -230,6 +230,13 @@ properties:
>        Wait-pin used by client. Must be less than "gpmc,num-waitpins".
>      $ref: /schemas/types.yaml#/definitions/uint32
>  
> +  gpmc,wait-pin-polarity:

'gpmc' is not a vendor. Don't continue this bad pattern, use 'ti'.

> +    description: |
> +      Set the desired polarity for the selected wait pin.
> +      1 for active low, 0 for active high.

Well that looks backwards. I assume from the commit msg above, it's the 
register value, but that's not what the description says. Please go with 
the logical state here and do the inversion in the driver.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +
>    gpmc,wait-on-read:
>      description: Enables wait monitoring on reads.
>      type: boolean
> -- 
> 2.25.1
> 
>