Hi Krzysztof, On Tue, 2022-09-20 at 13:23 +0200, Krzysztof Kozlowski wrote: > 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. > And now I completely got it... With this implementation it's even possible to set WAITPINPOLARITY_DEFAULT in the DT... Ok, changing this will lead to an error message if the "gpmc,wait-pin-polarity" is not set in DT. Means the DT property is more orless not an optional property anymore. If one defines the wait-pin without defining the polarity the driver probes successfully but and error message is printed. Is this an acceptable solution for you? > > > 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 Cheers, benedikt
