On Wed, Apr 29, 2020 at 1:33 PM Daniel Thompson daniel.thompson@linaro.org wrote:
On Wed, Apr 29, 2020 at 10:26:31AM +0200, Linus Walleij wrote:
if (pdata != NULL) {ret = devm_gpio_request_one(&spi->dev, pdata->reset_gpio,GPIOF_DIR_OUT | (!pdata->reset_inverted ?GPIOF_INIT_HIGH : GPIOF_INIT_LOW),"LMS283GF05 RESET");if (ret)return ret;}
st->reset = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_HIGH);Isn't this a change of behaviour w.r.t. to the initial state of the pin?
Yeah you're right. The original author intended reset to be de-asserted here so it should be GPIOD_OUT_LOW.
To be honest I suspect it is harmless because we launch into the reset sequence shortly after anyway. More that that I think I prefer it this way since it is better aligned with the behaviour of lms283gf05_power_set().
However if it is an intentional change of behaviour then it would be good to spell that out in the description for the benefit of future archaeologists.
Hm I'd rather not change semantics actually, you never know. I'll switch it back. If we decide to change it I'd use GPIOD_ASIS and not touch the hardware here.
Yours, Linus Walleij