Re: [PATCH v2 3/3] dt-bindings: memory-controllers: gpmc-child: Add binding for wait-pin-polarity

[Roger Quadros <rogerq@kernel.org>]

On 05/09/2022 12:14, Niedermayr, BENEDIKT wrote:
> On Mon, 2022-09-05 at 11:56 +0300, Roger Quadros wrote:
>> Hi Benedikt,
>>
>> On 05/09/2022 10:17, B. Niedermayr wrote:
>>> From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com>
>>>
>>> Add a new dt-binding for the wait-pin-polarity 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..7c721206f10b 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:
>>> +    description: |
>>> +      Wait-pin polarity used by the clien. It relates to the pin
>>> defined
>>
>> did you mean "client?"
>> Can you please specify what value is for Active Low vs Active High?
> 
> Yes, that makes sense. And yes I meant "client". My typo.....
>>
>>> +      with "gpmc,wait-pin".
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Why can't type be boolean?
> 
> Of course we can use the boolean there. In that case I should give the
> property a more meaningful name e.g. wait-pin-active-high or wait-pin-
> active-low. 
> Since the default behavour of this pin is Active High,
> a bool property "gpmc,wait-pin-active-low" would make more sense for
> backwards compatibility. 
> If the property is missing, than the polarity stays on Active High like
> before.
> 

OK, in that case you don't have to clarify the polarity in description.

>>
>>> +    default: 0
>>> +
>>>    gpmc,wait-on-read:
>>>      description: Enables wait monitoring on reads.
>>>      type: boolean
>>
>> cheers,
>> -roger
> 
> cheers,
> benedikt
> 

cheers,
-roger