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

[Sunyun Yang]

Krzysztof Kozlowski <krzk@xxxxxxxxxx> 于2026年6月26日周五 15:38写道：


>
> On 26/06/2026 04:15, Sunyun Yang wrote:
> > Krzysztof Kozlowski <krzk@xxxxxxxxxx> 于2026年6月25日周四 21:51写道：
> >>
> >> On 25/06/2026 15:40, Sunyun Yang wrote:
> >>> Sunyun Yang <syyang@xxxxxxxxxxx> 于2026年6月25日周四 21:26写道：
> >>>>
> >>>> Krzysztof Kozlowski <krzk@xxxxxxxxxx> 于2026年6月25日周四 21:17写道：
> >>>>>
> >>>>> On 25/06/2026 15:14, Sunyun Yang wrote:
> >>>>>> Krzysztof Kozlowski <krzk@xxxxxxxxxx> 于2026年6月25日周四 20:54写道：
> >>>>>>>
> >>>>>>> On 08/05/2026 15:40, syyang@xxxxxxxxxxx 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?
>
> DT has nothing to do here. 1 is assert, 0 is de-assert. Your code does
> things opposite to any logic, because you finish function with reset
> asserted.
>

I understand your point, and I will make the changes in the next version.

Additionally, I have another question I would like to ask you
regarding sashiko-bot@xxxxxxxxxx. 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?

>
> Best regards,
> Krzysztof