On Mon, Nov 30, 2020 at 07:09:55PM +0200, Laurent Pinchart wrote:
> On Mon, Nov 30, 2020 at 01:31:24PM +0000, Daniel Scally wrote:

I agree with most of Laurent's comments. S

...

> > +     Say Y here if your device is a detachable / hybrid laptop that comes
> > +     with Windows installed by the OEM, for example:
> > +

> > +           - Microsoft Surface models (except Surface Pro 3)

In this line mixed TABs and spaces. Not sure if it's only in Laurent's reply.

> > +           - The Lenovo Miix line (for example the 510, 520, 710 and 720)
> > +           - Dell 7285

...

> > +   for (i = 0; i < ARRAY_SIZE(cio2_supported_devices); i++) {
> > +           const char *this_device = cio2_supported_devices[i];
> 
> s/this_device/name/ (or sensor_name, ...) ?

I would go with hid.

...

> > +           for_each_acpi_dev_match(adev, this_device, NULL, -1) {
> > +                   if (!adev || !(adev->status.present && 
> > adev->status.enabled))
> 
>                       if (!adev || !adev->status.present || 
> !adev->status.enabled))
> 
> may be a bit more readable. Does for_each_acpi_dev_match() return NULL
> devices though ? If no, you could drop the !adev check. You may also be
> able to drop the !present check, as I don't think ACPI allows !present
> && enabled.

I think this should be rather

        if (acpi_bus_get_status(adev) || !adev->status.present)

-- 
With Best Regards,
Andy Shevchenko


Reply via email to