On Thu, Sep 17, 2020 at 4:53 PM Dan Scally <djrscally@xxxxxxxxx> wrote: > > Hi Andy, thanks for input (as always) You're welcome! I'm really impressed by your activity in this area. > On 17/09/2020 13:45, Andy Shevchenko wrote: > > On Thu, Sep 17, 2020 at 11:52:28AM +0100, Dan Scally wrote: > >> On 17/09/2020 11:33, Sakari Ailus wrote: To the point of placement, I think this should go under drivers/platform/x86 (Adding Hans and Mark, who can express their opinions). ... > > Ah, I think you misinterpreted the meaning of above. The above is a switch how > > camera device appears either as PCI or an ACPI. So, it effectively means you > > should *not* have any relation for this HID until you find a platform where the > > device is for real enumerated via ACPI. > > > Ah, ok. So that was never going to work. Thanks. That does raise another > question; we have had some testers report failure, which turns out to be > because on their platforms the definition of their cameras in ACPI is > never translated into an i2c_client so the cio2-bridge doesn't bind. > Those have a similar conditional in the _STA method, see CAM1 in this > DSDT for example: > https://raw.githubusercontent.com/linux-surface/acpidumps/master/surface_go/dsdt.dsl. > Is there anything we can do to enable those cameras to be discovered too? It means that this ... > >>>> +#define PROPERTY_ENTRY_NULL \ > >>>> +((const struct property_entry) { }) > >>> Alignment. Same appears to apply to other macros (please indent). > >> Yep > >>>> +#define SOFTWARE_NODE_NULL \ > >>>> +((const struct software_node) { }) > > Why?! > > > It felt ugly to have the other definitions be macros and not this one, > but I can change it. My point is that those macros are simply redundant. The point is to have a terminator record (all 0:s in the last entry of an array) which is usually being achieved by allocating memory with kcalloc() which does implicitly this for you. -- With Best Regards, Andy Shevchenko
