[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

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, if pure OF driver calls this function, it's weird to have ACPI
companion without ACPI ID in this case.

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>
---
 drivers/acpi/bus.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index f87ed3be779a..b271eb16341d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -789,12 +789,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;

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

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, if pure OF driver calls this function, it's weird to have ACPI
companion without ACPI ID in this case.

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>
---
 drivers/acpi/bus.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index f87ed3be779a..b271eb16341d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -789,12 +789,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;

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Sinan Kaya]

On 1/31/2018 4:29 PM, 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, if pure OF driver calls this function, it's weird to have ACPI
> companion without ACPI ID in this case.

We talked about this during review. 

of_match_device() does all the checking for the OF part. ACPI doesn't have
any checks.

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Rafael J. Wysocki]

On Wed, Jan 31, 2018 at 11:17 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 1/31/2018 4:29 PM, 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, if pure OF driver calls this function, it's weird to have ACPI
>> companion without ACPI ID in this case.
>
> We talked about this during review.
>
> of_match_device() does all the checking for the OF part. ACPI doesn't have
> any checks.

Yeah, this patch is just plain incorrect AFAICS.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

On Thu, 2018-02-01 at 08:27 +0100, Rafael J. Wysocki wrote:
> On Wed, Jan 31, 2018 at 11:17 PM, Sinan Kaya <okaya@codeaurora.org>
> wrote:
> > On 1/31/2018 4:29 PM, 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, if pure OF driver calls this function, it's weird to have
> > > ACPI
> > > companion without ACPI ID in this case.
> > 
> > We talked about this during review.
> > 
> > of_match_device() does all the checking for the OF part. ACPI
> > doesn't have
> > any checks.
> 
> Yeah, this patch is just plain incorrect AFAICS.

I don't see how check dev->driver is implemented on OF side then


of_device_get_match_data() which is called by
of_fwnode_device_get_match_data() has dereferenced dev->driver w/o any
check.

I can't agree that the patch is plain incorrect, if I didn't miss
anything.

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Sinan Kaya]

On 2018-02-01 07:24, Andy Shevchenko wrote:
> On Thu, 2018-02-01 at 08:27 +0100, Rafael J. Wysocki wrote:
>> On Wed, Jan 31, 2018 at 11:17 PM, Sinan Kaya <okaya@codeaurora.org>
>> wrote:
>> > On 1/31/2018 4:29 PM, 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, if pure OF driver calls this function, it's weird to have
>> > > ACPI
>> > > companion without ACPI ID in this case.
>> >
>> > We talked about this during review.
>> >
>> > of_match_device() does all the checking for the OF part. ACPI
>> > doesn't have
>> > any checks.
>> 
>> Yeah, this patch is just plain incorrect AFAICS.
> 
> I don't see how check dev->driver is implemented on OF side then
> 
> 
> of_device_get_match_data() which is called by
> of_fwnode_device_get_match_data() has dereferenced dev->driver w/o any
> check.
> 
> I can't agree that the patch is plain incorrect, if I didn't miss
> anything.


Sorry, i should have been more specific. I was talkimg about match_data 
not driver.

I agree that driver check is redundant.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v1,1/4] ACPI / bus: Remove checks in acpi_get_match_data()

[Rafael J. Wysocki]

On Thursday, February 1, 2018 1:24:53 PM CET Andy Shevchenko wrote:
> On Thu, 2018-02-01 at 08:27 +0100, Rafael J. Wysocki wrote:
> > On Wed, Jan 31, 2018 at 11:17 PM, Sinan Kaya <okaya@codeaurora.org>
> > wrote:
> > > On 1/31/2018 4:29 PM, 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, if pure OF driver calls this function, it's weird to have
> > > > ACPI
> > > > companion without ACPI ID in this case.
> > > 
> > > We talked about this during review.
> > > 
> > > of_match_device() does all the checking for the OF part. ACPI
> > > doesn't have
> > > any checks.
> > 
> > Yeah, this patch is just plain incorrect AFAICS.
> 
> I don't see how check dev->driver is implemented on OF side then
> 
> 
> of_device_get_match_data() which is called by
> of_fwnode_device_get_match_data() has dereferenced dev->driver w/o any
> check.
> 
> I can't agree that the patch is plain incorrect, if I didn't miss
> anything.

OK, you're right, sorry.
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html