On Wed, Jun 5, 2024 at 2:19 PM Maarten Brock <Maarten.Brock@xxxxxxxx> wrote: > > On 6/5/24 18:30, Maarten Brock wrote: > > >> From: Hugo Villeneuve <hugo@xxxxxxxxxxx> > > >> Sent: Tuesday, 4 June 2024 16:23 <...> > > >> Add function description from original comment "Reset device, > > >> purging any pending irq / data", since the comment applies to both > > >> hardware and software reset, > > > No it does not (at this moment). See below. ... > > > The problem here is that this only deasserts the reset if it were asserted before. > > > Nothing happens if it already was deasserted. This makes the delay also pretty > > > useless. > > > > > > To make this a proper reset pulse for the device you must first assert the reset, > > > then wait >3us, and finally deassert the reset. > > > > > > Maarten Brock > > Hi Maarten, > > > > My understanding is when calling devm_gpiod_get_optional(dev, "reset", > > GPIOD_OUT_LOW) and returning a valid (gpio_desc *), the flag > > GPIOD_OUT_LOW guarantees the GPIO is set to output and low (assert the > > reset pin). > > Ah, right. Sorry, I missed that. > So GPIOD_OUT_LOW disregards the inversion from GPIO_ACTIVE_LOW. > And gpiod_set_value_cansleep(reset_gpiod, 0) uses the inversion to make the pin high. > Looks fine to me now. No, you used the correct terms "assert"/"deassert", the parameter there is logical, means "deassert". It was differently in the legacy GPIO APIs, which are almost gone. -- With Best Regards, Andy Shevchenko
