Re: [PATCH] Correct a typo which inverted the reset GPIO pin sequence.

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Fri, Dec 12, 2025 at 07:44:21AM -0800, Brad Kelley wrote:
> On 12/11/2025 2:19 AM, Richard Fitzgerald wrote:
> > Actually this patch looks incorrect and will break things.
> > The 2nd argument to gpiod_direction_output() is the initial state of
> > the GPIO. The kerneldoc for the function says "The initial value of the
> > output must be specified as the logical value of the GPIO"
> > 
> > So the original code is correct: first we assert it (logical state 1)
> > then below it is deasserted (logical state 0).
> > 
> > The problem is that originally the code set the raw signal level
> > (0 to reset, 1 to not-reset) but now that it uses gpiod you must
> > add the ACTIVE_LOW flag to the gpio definition if its electrical
> > signal level is inverse of its logical level.
> > 
> > See the code in gpiod_direction_output_nonotify() in
> > drivers/gpio/gpiolib.c, which inverts the value if FLAG_ACTIVE_LOW
> > is set.
> 
> Thanks to both of you for the explanations and patience.  That all 
> makes sense and I appreciate the guidance.  

Absolutely no problem, welcome to the kernel community.

> This my first patch. I think I need to send a withdrawal email.

No need for any separate withdrawal.

> The problem for my use seems to be in the superaudioboard overlay 
> which sets the GPIO_ACTIVE_HIGH flag.  A quick compile of the 
> edited dts file fixes the problem.  I changed the line to this:
>   reset-gpio = <&gpio 26 1>; /* Pin 26, active low */
> but maybe there's an updated method.

Suspected that might be the problem, glad we could help you
figuring it out.

> I'll do some more research and testing and submit a new patch to 
> the proper maintainers for that overlay.

Aye that makes sense, if the DT is in the kernel the process
should be pretty similar just as you say finding the right
maintainers.

Thanks,
Charles