Maxime Ripard <[email protected]> 于2026年6月26日周五 15:49写道: > > On Fri, Jun 26, 2026 at 10:15:03AM +0800, Sunyun Yang wrote: > > Krzysztof Kozlowski <[email protected]> 于2026年6月25日周四 21:51写道: > > > > > > On 25/06/2026 15:40, Sunyun Yang wrote: > > > > Sunyun Yang <[email protected]> 于2026年6月25日周四 21:26写道: > > > >> > > > >> Krzysztof Kozlowski <[email protected]> 于2026年6月25日周四 21:17写道: > > > >>> > > > >>> On 25/06/2026 15:14, Sunyun Yang wrote: > > > >>>> Krzysztof Kozlowski <[email protected]> 于2026年6月25日周四 20:54写道: > > > >>>>> > > > >>>>> On 08/05/2026 15:40, [email protected] wrote: > > > >>>>>> + > > > >>>>>> +static void lt9611c_reset(struct lt9611c *lt9611c) > > > >>>>>> +{ > > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); > > > >>>>>> + msleep(20); > > > >>>>>> + > > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); > > > >>>>>> + msleep(20); > > > >>>>>> + > > > >>>>>> + gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); > > > >>>>> > > > >>>>> This is just plain wrong. Why do you assert, then de-assert and then > > > >>>>> finally assert AGAIN the reset leaving the device in powerdown > > > >>>>> stage? > > > >>>>> > > > >>>> I am using software to emulate the hardware RESET button on our EVB. > > > >>>> When the hardware RESET button is pressed while our chip is running, > > > >>>> the signal level changes from HIGH to LOW and then back to HIGH. > > > >>>> > > > >>>> Of course, we can also use the following: > > > >>>> static void lt9611c_reset(struct lt9611c *lt9611c) > > > >>>> { > > > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); > > > >>>> msleep(50); > > > >>>> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); > > > >>>> msleep(20); > > > >>>> } > > > >>> > > > >>> Makes no sense either and you just did not get the point and did not > > > >>> answer my question. I asked WHY you leave asserted. Answer "we > > > >>> emulate" > > > >>> is just plain wrong. > > > >>> > > > >>> So again please answer: > > > >>> > > > >>> Why do you leave device with reset asserted? > > > >>> > > > >> > > > >> devicetree: reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>; > > > >> > > > >> GPIO_ACTIVE_HIGH: > > > >> > > > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 0); ------ reset pin > > > >> is Low level : Clear the register configuration in the chip to stop > > > >> the chip from working. > > > >> > > > >> gpiod_set_value_cansleep(lt9611c->reset_gpio, 1); ------ reset pin > > > >> is high level: The chip resumes operation. > > > >> > > > >> > > > > > > > > Our purpose is: pull the level low to clear the register configuration > > > > in the chip, and then pull it high to allow the MCU inside the chip to > > > > re‑initialize the registers. > > > > > > > > > And you do completely opposite... so that confirms your code is just > > > wrong. > > > > > > > The lontium-lt9611.yaml uses GPIO_ACTIVE_HIGH. I am just following the > > rule of this device tree. If I modify the device tree to use > > GPIO_ACTIVE_LOW, > > and use the following code in my driver, then my driver would be correct. > > However, would the existing kernel drivers lontium-lt9611uxc.c and > > lontium-lt9611.c be affected? > > It might, but then it's a DT problem. The GPIO API for drivers always > considers the logical state of a GPIO, so if you need to assert a > signal, you'll always need to set 1. That's what Krzysztof was trying to > explain. > > The DT will provide with GPIO_ACTIVE_* how that logical state translates > to a physical GPIO state. > > If the DT says that this particular GPIO is active-high, then it means > that we need to set the GPIO to 1 to assert reset. Now of course, it > might not make sense for the controller itself, but it might for the > board if there's a GPIO inverter in the middle for example. > > Anyway, in the case you're raising, the issue definitely lies in the DT, > and that's what would need to be fixed. > > I also wouldn't be too concerned about lontium-lt9611.yaml, it's just an > example. > > Maxime
thanks Maxime, I will modify this code in the next version of the driver, and I hope you can accept these changes. Maxime: I have another question I would like to ask you regarding [email protected]. Since sashiko-bot sometimes has opinions that differ from yours, whose advice should I follow? If I do not adopt sashiko-bot's suggestions, will my patches still be accepted into the upstream Linux kernel?
