Re: [PATCH v6 2/2] drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3937 bytes --]

On Fri, Jun 26, 2026 at 10:15:03AM +0800, Sunyun Yang wrote:
> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:51写道：
> >
> > On 25/06/2026 15:40, Sunyun Yang wrote:
> > > Sunyun Yang <syyang@lontium.com> 于2026年6月25日周四 21:26写道：
> > >>
> > >> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 21:17写道：
> > >>>
> > >>> On 25/06/2026 15:14, Sunyun Yang wrote:
> > >>>> Krzysztof Kozlowski <krzk@kernel.org> 于2026年6月25日周四 20:54写道：
> > >>>>>
> > >>>>> On 08/05/2026 15:40, syyang@lontium.com 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]