| Message ID | 20180201202012.36524-3-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, 2018-02-01 at 22:20 +0200, Andy Shevchenko wrote: > As well as its sibling of_device_get_match_data() has no such checks, > no need to do it in acpi_get_match_data(). > > First of all, we are not supposed to call fwnode API like this without > driver attached. > > Second, since __acpi_match_device() does check input parameter there > is > no need to duplicate it outside. > > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() > function") > Cc: Sinan Kaya <okaya@codeaurora.org> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Vinod Koul <vinod.koul@intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: rebase on top of new patches, rephrase commit message > drivers/acpi/bus.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index f3a7c29e9190..413e4b1cb1be 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -791,12 +791,6 @@ void *acpi_get_match_data(const struct device > *dev) > { > const struct acpi_device_id *match; > > - if (!dev->driver) > - return NULL; > - > - if (!dev->driver->acpi_match_table) > - return NULL; > - Perhaps I have to add that this conditional prevents device_get_match_data() to work on PRP0001 devices AFAIU. > match = acpi_match_device(dev->driver->acpi_match_table, > dev); > if (!match) > return NULL;
On Thu, Feb 1, 2018 at 9:46 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, 2018-02-01 at 22:20 +0200, Andy Shevchenko wrote: >> As well as its sibling of_device_get_match_data() has no such checks, >> no need to do it in acpi_get_match_data(). >> >> First of all, we are not supposed to call fwnode API like this without >> driver attached. >> >> Second, since __acpi_match_device() does check input parameter there >> is >> no need to duplicate it outside. >> >> Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() >> function") >> Cc: Sinan Kaya <okaya@codeaurora.org> >> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> >> Cc: Vinod Koul <vinod.koul@intel.com> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> v2: rebase on top of new patches, rephrase commit message >> drivers/acpi/bus.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >> index f3a7c29e9190..413e4b1cb1be 100644 >> --- a/drivers/acpi/bus.c >> +++ b/drivers/acpi/bus.c >> @@ -791,12 +791,6 @@ void *acpi_get_match_data(const struct device >> *dev) >> { >> const struct acpi_device_id *match; >> >> - if (!dev->driver) >> - return NULL; >> - > >> - if (!dev->driver->acpi_match_table) >> - return NULL; >> - > > Perhaps I have to add that this conditional prevents > device_get_match_data() to work on PRP0001 devices AFAIU. Yes, please. I'm also not sure if the Fixes: tag is really applicable to this. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2018-02-04 at 08:21 +0100, Rafael J. Wysocki wrote: > On Thu, Feb 1, 2018 at 9:46 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, 2018-02-01 at 22:20 +0200, Andy Shevchenko wrote: > > > As well as its sibling of_device_get_match_data() has no such > > > checks, > > > no need to do it in acpi_get_match_data(). > > > > > > First of all, we are not supposed to call fwnode API like this > > > without > > > driver attached. > > > > > > Second, since __acpi_match_device() does check input parameter > > > there > > > is > > > no need to duplicate it outside. > > > > > > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() > > > function") > > > Cc: Sinan Kaya <okaya@codeaurora.org> > > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: Vinod Koul <vinod.koul@intel.com> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > rhaps I have to add that this conditional prevents > > device_get_match_data() to work on PRP0001 devices AFAIU. > > Yes, please. OK. > I'm also not sure if the Fixes: tag is really applicable to this. We can of course drop it, I put it here in a hope that this series would be material to v4.16-rc2 to fix an introduced API.
On Monday, February 5, 2018 5:04:22 PM CET Andy Shevchenko wrote: > On Sun, 2018-02-04 at 08:21 +0100, Rafael J. Wysocki wrote: > > On Thu, Feb 1, 2018 at 9:46 PM, Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, 2018-02-01 at 22:20 +0200, Andy Shevchenko wrote: > > > > As well as its sibling of_device_get_match_data() has no such > > > > checks, > > > > no need to do it in acpi_get_match_data(). > > > > > > > > First of all, we are not supposed to call fwnode API like this > > > > without > > > > driver attached. > > > > > > > > Second, since __acpi_match_device() does check input parameter > > > > there > > > > is > > > > no need to duplicate it outside. > > > > > > > > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() > > > > function") > > > > Cc: Sinan Kaya <okaya@codeaurora.org> > > > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Cc: Vinod Koul <vinod.koul@intel.com> > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > > > rhaps I have to add that this conditional prevents > > > device_get_match_data() to work on PRP0001 devices AFAIU. > > > > Yes, please. > > OK. > > > I'm also not sure if the Fixes: tag is really applicable to this. > > We can of course drop it, I put it here in a hope that this series would > be material to v4.16-rc2 to fix an introduced API. I can still take it for -rc1 even if ready, these tags aren't necessary for that. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2018-02-05 at 17:54 +0100, Rafael J. Wysocki wrote: > On Monday, February 5, 2018 5:04:22 PM CET Andy Shevchenko wrote: > > On Sun, 2018-02-04 at 08:21 +0100, Rafael J. Wysocki wrote: > > > On Thu, Feb 1, 2018 at 9:46 PM, Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > I'm also not sure if the Fixes: tag is really applicable to this. > > > > We can of course drop it, I put it here in a hope that this series > > would > > be material to v4.16-rc2 to fix an introduced API. > > I can still take it for -rc1 even if ready, these tags aren't > necessary for that. Noted. I'll remove tags and resend.
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index f3a7c29e9190..413e4b1cb1be 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -791,12 +791,6 @@ void *acpi_get_match_data(const struct device *dev) { const struct acpi_device_id *match; - if (!dev->driver) - return NULL; - - if (!dev->driver->acpi_match_table) - return NULL; - match = acpi_match_device(dev->driver->acpi_match_table, dev); if (!match) return NULL;
As well as its sibling of_device_get_match_data() has no such checks, no need to do it in acpi_get_match_data(). First of all, we are not supposed to call fwnode API like this without driver attached. Second, since __acpi_match_device() does check input parameter there is no need to duplicate it outside. Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function") Cc: Sinan Kaya <okaya@codeaurora.org> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Vinod Koul <vinod.koul@intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v2: rebase on top of new patches, rephrase commit message drivers/acpi/bus.c | 6 ------ 1 file changed, 6 deletions(-)