On Fri, 2023-12-01 at 11:01 -0600, David Lechner wrote: > On Fri, Dec 1, 2023 at 2:47 AM Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > > > > On Thu, 2023-11-30 at 15:41 -0600, David Lechner wrote: > > > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay > > > <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote: > > > > > > > > From: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > > > > > > The reset gpio was being requested with GPIOD_OUT_LOW which means, not > > > > asserted. Then it was being asserted but never de-asserted which means > > > > the devices was left in reset. Fix it by de-asserting the gpio. > > > > > > It could be helpful to update the devicetree bindings to state the > > > expected active-high or active-low setting for this gpio so it is > > > clear which state means asserted. > > > > > > > You could state that the chip is active low but I don't see that change that > > important for now. Not sure if this is clear and maybe that's why your comment. > > GPIOD_OUT_HIGH has nothing to do with active high or low. It just means, "get me > > the > > pin in the asserted state". > > > > I would assume that this bug happened in the first place because > someone forgot GPIOD_OUT_LOW in the devicetree when they were > developing the driver. So this is why I suggested that updating the > devicetree binding docs so that future users are less likely to make > the same mistake. Currently, the bindings don't even have reset-gpios > in the examples. Hmm, I think you're missing the point... The bug has nothing to do with devicetree. This is what was happening: 1) We were calling devm_gpiod_get_optional() with GPIOD_OUT_LOW. What this means is that you get an output gpio deasserted. Hence the device is out of reset. And here is the important part... what you have in dts does not matter. If you have active low, it means the pin level will be 1. If you have high, the pin level is 0. And this is all handled by gpiolib for you. 2) Then, we called gpiod_direction_output(..., 1), which means set the direction out (which is actually not needed since it was already done when getting the pin) and assert the pin. Hence, reset the device. And we were never de-asserting the pin so the device would be left in reset. - Nuno Sá
