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: <518b1420-a356-4e4b-8422-c2689bc54794@redhat.com>
Date: Mon, 14 Apr 2025 14:21:56 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.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

Hi Sakari,

On 14-Apr-25 13:43, Sakari Ailus wrote:
> 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.

The GPIO being interpreted as active-low is a thing specific to
the chip used though. Where as in the future the HPD pin type
in the INT3472 device might be used with other chips...

Anyways either way is fine with me bu, as mentioned, using GPIO_ACTIVE_LOW
will invert the values returned by gpiod_get_value(), for which the driver
likely needs to be adjusted.

Regards,

Hans