On Wed, Apr 29, 2020 at 1:33 PM Daniel Thompson
<daniel.thompson@xxxxxxxxxx> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
