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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_z04jMiTg_xW-c2@kekkonen.localdomain>
Date: Mon, 14 Apr 2025 11:43:30 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Hans de Goede <hdegoede@...hat.com>
Cc: "Yan, Dongcheng" <dongcheng.yan@...el.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	"Yan, Dongcheng" <dongcheng.yan@...ux.intel.com>,
	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
	hverkuil@...all.nl, u.kleine-koenig@...libre.com,
	ricardo.ribalda@...il.com, bingbu.cao@...ux.intel.com,
	hao.yao@...el.com
Subject: Re: [PATCH v1 1/2] platform/x86: int3472: add hpd pin support

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