Re: [PATCH 2/3] platform/x86: int3472/discrete: Get the polarity from the _DSM entry

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/24/22 21:13, Andy Shevchenko wrote:
> On Thu, Nov 24, 2022 at 10:00 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The out of tree IPU6 driver has moved to also using the in kernel INT3472
>> code for doing power-ctrl rather then doing their own thing (good!).
>>
>> On IPU6 the polarity is encoded in the _DSM entry rather then being
>> hardcoded to GPIO_ACTIVE_LOW.
>>
>> Using the _DSM entry for this on IPU3 leads to regressions, so only
>> use the _DSM entry for this on non IPU3 devices.
>>
>> Note there is a whole bunch of PCI-ids for the IPU6 which is why
>> the check is for the IPU3-CIO2, because the CIO2 there has a unique
>> PCI-id which can be used for this.
> 
> ...
> 
>> +/* IPU3 vs IPU6 needs to be handled differently */
>> +#define IPU3_CIO2_PCI_ID                               0x9d32
> 
> If it makes more than a single driver, perhaps pci_ids.h is a good
> place to keep it there?

Yes, I've added a note to my TODO to clean this up in a follow-up
patch (the pci-ids.h maintainers want to see multiple users of
an id before accepting new ids in there).

> 
> ...
> 
>> +       dev_dbg(int3472->dev, "%s %s pin %d active-%s\n", func,
>> +               agpio->resource_source.string_ptr, agpio->pin_table[0],
> 
>> +               (polarity == GPIO_ACTIVE_HIGH) ? "high" : "low");
> 
> Parentheses are not needed, right?

Right, but I prefer to use them in conditional statements like these,
because I personally find that they make things easier to read.

Regards,

Hans