Re: [PATCH v1 1/2] platform/x86: int3472: add hpd pin support

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hans, Dongcheng,

On Mon, Apr 14, 2025 at 01:09:47PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 14-Apr-25 13:04, Hans de Goede wrote:
> > Hi,
> > 
> > On 14-Apr-25 11:59, Yan, Dongcheng wrote:
> >> Hi Andy and Hans,
> >>
> >> I found the description of lt6911uxe's GPIO in the spec:
> >> GPIO5 is used as the interrupt signal (50ms low level) to inform SOC
> >> start reading registers from 6911UXE;
> >>
> >> So setting the polarity as GPIO_ACTIVE_LOW is acceptable?
> > 
> > Yes that is acceptable, thank you for looking this up.
> 
> p.s.
> 
> Note that setting GPIO_ACTIVE_LOW will invert the values returned
> by gpiod_get_value(), so if the driver uses that you will need
> to fix this in the driver.
> 
> Hmm, thinking more about this, I just realized that this is an
> input pin to the CPU, not an output pin like all other pins
> described by the INT3472 device. I missed that as first.
> 
> In that case using GPIO_LOOKUP_FLAGS_DEFAULT as before probably
> makes the most sense. Please add a comment that this is an input
> pin to the INT3472 patch and keep GPIO_LOOKUP_FLAGS_DEFAULT for
> the next version.

The GPIO_LOOKUP_FLAGS_DEFAULT is the "Linux default", not the hardware or
firmware default so I don't think it's relevant in this context. There's a
single non-GPIO bank driver using it, probably mistakenly.

I'd also use GPIO_ACTIVE_LOW, for the reason Dongcheng pointed to above.

-- 
Regards,

Sakari Ailus