Re: [PATCH v4] iio: accel: adxl345: Add ACPI HID table

[Andy Shevchenko]

On Fri, Feb 18, 2022 at 4:46 AM Kai-Heng Feng
<kai.heng.feng@xxxxxxxxxxxxx> wrote:
> On Thu, Feb 17, 2022 at 6:57 PM Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
> > On Thursday, February 17, 2022, Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote:

...

> >> +               acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
> >> +               if (acpi_id) {
> >> +                       type = acpi_id->driver_data;
> >> +                       name = acpi_id->id;
> >> +               } else
> >> +                       return -ENODEV;
> >
> > Thanks, but can we do this in ACPI agnostic way?
> >
> > Can be as simple as
> >
> > if (id)
> >   ...
> > else {
> >   match = device_get_match_data(dev);
> >   if (!match)
> >     return -ENODEV;
> > }
> >
> > Note, it might require to reconsider what is put in the driver data (either convert to pointers, or be sure that valid type is never a 0/NULL).
>
> Unlike acpi_match_device(), device_get_match_data() only get
> driver_data, so we need a new struct to provide both name and type.

It's unfortunate. Let me think about it a bit more.

> > Also note, in both cases using ID name for name us fragile. Probably we have to fix that first. Let me check today’s evening.
>
> Can you please explain more on this? How does ID name make it fragile?

I thought this one is used somehow by userspace to distinguish the
instance of the device, but looking into the rest of the IIO drivers
it seems more or less  a field for part number. That said, the ID is
okay to use. I hope Jonathan may correct me.

-- 
With Best Regards,
Andy Shevchenko