Hi, On 11/24/22 21:13, Andy Shevchenko wrote: > On Thu, Nov 24, 2022 at 10:00 PM Hans de Goede <hdegoede@xxxxxxxxxx> 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