Re: [PATCH v5 2/3] memory: omap-gpmc: add support for wait pin polarity

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 20/09/2022 12:12, Niedermayr, BENEDIKT wrote:
>> I commented exactly below the line which I question. I don't question
>> other lines. So let me be a bit more specific:
>>
>> Why do you need
>> "p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT"
>> ? Can you write a scenario where this is useful?
>>
> Ok. I think I got you now. Sorry I'm relatively new to OSS contributions, so please be patient with me...
> 
> If I remove that part of the if clause, then an error message would be printed in case "p->wait_pin_polarity == WAITPINPOLARITY_DEFAULT".

Exactly this will happen. As expected. This value cannot appear in DTS,
therefore I would expect error message.

Now you allow such value in DTS which is not the same as your bindings.


> But this is a not an error case. WAITPINPOLARITY_DEFAULT is a valid value, is assigned right before the if clause as an init value(not extracted from DT),
> and leads to not touching the GPMC_CONFIG register in gpmc_cs_program_settings().
> So in gpmc_cs_program_settings() if:
>     p->wait_pin_polarity != WAITPINPOLARITY_ACTIVE_HIGH -> Issue a write to the GPMC_CONFIG register
>     p->wait_pin_polarity != WAITPINPOLARITY_ACTIVE_LOW  -> Issua a write to the GPMC_CONFIG register
>     p->wait_pin_polarity != WAITPINPOLARITY_DEFAULT     -> Do not touch the GPMC_CONFIG register
> 
> We want to preserve the reset value of the GPMC_CONFIG register in case the DT does not use the "gpmc,wait-pin-polarity" property. Otherwise
> we might break platforms which rely on these reset values. 

Best regards,
Krzysztof