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
