On Mon, Feb 25, 2019 at 04:29:00PM +0200, Mika Westerberg wrote:
> On Mon, Feb 25, 2019 at 05:12:47PM +0300, Andy Shevchenko wrote:
> > In case of PRP0001 the compatible string may have additional data affiliated
> > with the device. When we call device_get_match_data() on such device, we will
> > get nothing since currently acpi_device_get_match_data() doesn't respect
> > PRP0001.
> >
> > To fix above, try acpi_of_match_device() if there is no ACPI table in the
> > driver.
> >
> > Anyway, note that the device is supposed to get its own proper ACPI ID.
> > +static const void *acpi_device_get_match_data_of(const struct device *dev)
>
> Maybe call it acpi_of_device_get_match_data() instead following
> acpi_of_match device and the like?
Makes sense.
> > +{
> > + struct acpi_device *adev = ACPI_COMPANION(dev);
> > + const struct of_device_id *match = NULL;
> > +
> > + acpi_of_match_device(adev, dev->driver->of_match_table, &match);
> > + if (!match)
> > + return NULL;
> > +
> > + return match->data;
>
> Hm, why not write it like:
>
> if (!acpi_of_match_device(adev, dev->driver->of_match_table, &match))
> return NULL;
> return match->data;
>
> instead?
Sadly the acpi_of_match_device() description doesn't say what return value
means. Nevertheless, reading its code shows that your suggestion will work.
> > +}
> > +
> > const void *acpi_device_get_match_data(const struct device *dev)
> > {
> > const struct acpi_device_id *match;
> >
> > + if (!dev->driver->acpi_match_table)
>
> If a driver does not have acpi_match_table is it expected to be
> compatible with PRP0001 or should you check for adev->data.of_compatible
> here as well?
It's checked anyway in acpi_of_match_device(). I see no point for the check
duplication.
>
> > + return acpi_device_get_match_data_of(dev);
> > +
> > match = acpi_match_device(dev->driver->acpi_match_table, dev);
> > if (!match)
> > return NULL;
> > --
> > 2.20.1
--
With Best Regards,
Andy Shevchenko
