[PATCH v2 1/6] ACPI / bus: Return error code from __acpi_match_device() in one case

[PATCH v2 1/6] ACPI / bus: Return error code from __acpi_match_device() in one case

[Andy Shevchenko]

Instead of playing tricks with last invalid entry,
return simple -ENODATA error code casted to pointer.

It would be good for future in case caller passes NULL pointer for
ID table. Moreover, caller can check the code to be sure what happened
inside callee.

Fixes: 2b9c698efa58 ("ACPI / scan: Take the PRP0001 position in the list of IDs into account")
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: new patch
 drivers/acpi/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index f87ed3be779a..a87a97bf75f8 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -762,7 +762,7 @@ static const struct acpi_device_id *__acpi_match_device(
 		 */
 		if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)
 		    && acpi_of_match_device(device, of_ids))
-			return id;
+			return ERR_PTR(-ENODATA);
 	}
 	return NULL;
 }
-- 
2.15.1

[PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko]

When __acpi_match_device() is called it would be possible to have
ACPI ID table a MULL pointer. To avoid potential dereference,
check for this before traverse.

While here, remove redundant 'else'.

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: new patch
 drivers/acpi/bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a87a97bf75f8..f3a7c29e9190 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -745,11 +745,13 @@ static const struct acpi_device_id *__acpi_match_device(
 
 	list_for_each_entry(hwid, &device->pnp.ids, list) {
 		/* First, check the ACPI/PNP IDs provided by the caller. */
-		for (id = ids; id->id[0] || id->cls; id++) {
-			if (id->id[0] && !strcmp((char *) id->id, hwid->id))
-				return id;
-			else if (id->cls && __acpi_match_device_cls(id, hwid))
-				return id;
+		if (ids) {
+			for (id = ids; id->id[0] || id->cls; id++) {
+				if (id->id[0] && !strcmp((char *)id->id, hwid->id))
+					return id;
+				if (id->cls && __acpi_match_device_cls(id, hwid))
+					return id;
+			}
 		}
 
 		/*
-- 
2.15.1

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Sinan Kaya]

On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> When __acpi_match_device() is called it would be possible to have
> ACPI ID table a MULL pointer. To avoid potential dereference,

NULL

> check for this before traverse.
> 
> While here, remove redundant 'else'.
> 
> 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: new patch
>  drivers/acpi/bus.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a87a97bf75f8..f3a7c29e9190 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -745,11 +745,13 @@ static const struct acpi_device_id *__acpi_match_device(
>  
>  	list_for_each_entry(hwid, &device->pnp.ids, list) {
>  		/* First, check the ACPI/PNP IDs provided by the caller. */
> -		for (id = ids; id->id[0] || id->cls; id++) {
> -			if (id->id[0] && !strcmp((char *) id->id, hwid->id))
> -				return id;
> -			else if (id->cls && __acpi_match_device_cls(id, hwid))
> -				return id;
> +		if (ids) {
> +			for (id = ids; id->id[0] || id->cls; id++) {
> +				if (id->id[0] && !strcmp((char *)id->id, hwid->id))
> +					return id;
> +				if (id->cls && __acpi_match_device_cls(id, hwid))
> +					return id;
> +			}
>  		}
>  
>  		/*
> 

Why not bail out here immediately if ids is null?

__acpi_match_device()
{
	/*
	 * If the device is not present, it is unnecessary to load device
	 * driver for it.
	 */
	if (!device || !device->status.present)
		return NULL;
}

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko]

On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote:
> On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> > When __acpi_match_device() is called it would be possible to have
> > ACPI ID table a MULL pointer. To avoid potential dereference,
> 
> NULL

Thanks, will fix later.

> Why not bail out here immediately if ids is null?

Because of the code which wasn't in context of this patch.

See also patch 1 in the series.

It's about how acpi_driver_match_device() uses it.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko]

On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote:
> On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote:
> > On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> > > When __acpi_match_device() is called it would be possible to have
> > > ACPI ID table a MULL pointer. To avoid potential dereference,
> > 
> > NULL
> 
> Thanks, will fix later.
> 
> > Why not bail out here immediately if ids is null?
> 
> Because of the code which wasn't in context of this patch.
> 
> See also patch 1 in the series.
> 
> It's about how acpi_driver_match_device() uses it.

Btw, it makes device_get_match_data() to work properly on PRP0001 kind
of devices.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Rafael J. Wysocki]

On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote:
>> On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote:
>> > On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
>> > > When __acpi_match_device() is called it would be possible to have
>> > > ACPI ID table a MULL pointer. To avoid potential dereference,
>> >
>> > NULL
>>
>> Thanks, will fix later.
>>
>> > Why not bail out here immediately if ids is null?
>>
>> Because of the code which wasn't in context of this patch.
>>
>> See also patch 1 in the series.
>>
>> It's about how acpi_driver_match_device() uses it.
>
> Btw, it makes device_get_match_data() to work properly on PRP0001 kind
> of devices.

Maybe combine this one with the [1/6] then, it would make it somewhat
easier to follow what's going on.

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko]

On Sun, 2018-02-04 at 08:18 +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, 2018-02-01 at 22:40 +0200, Andy Shevchenko wrote:
> > > On Thu, 2018-02-01 at 15:32 -0500, Sinan Kaya wrote:
> > > > On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> > > > > When __acpi_match_device() is called it would be possible to
> > > > > have
> > > > > ACPI ID table a MULL pointer. To avoid potential dereference,
> > > > 
> > > > NULL
> > > 
> > > Thanks, will fix later.
> > > 
> > > > Why not bail out here immediately if ids is null?
> > > 
> > > Because of the code which wasn't in context of this patch.
> > > 
> > > See also patch 1 in the series.
> > > 
> > > It's about how acpi_driver_match_device() uses it.
> > 
> > Btw, it makes device_get_match_data() to work properly on PRP0001
> > kind
> > of devices.
> 
> Maybe combine this one with the [1/6] then, it would make it somewhat
> easier to follow what's going on.

Sure, I will fold in.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 2/6] ACPI / bus: Do not traverse through non-existed device table

[Andy Shevchenko]

On Mon, 2018-02-05 at 17:56 +0200, Andy Shevchenko wrote:
> On Sun, 2018-02-04 at 08:18 +0100, Rafael J. Wysocki wrote:
> > On Thu, Feb 1, 2018 at 9:45 PM, Andy Shevchenko
> > 

> > Maybe combine this one with the [1/6] then, it would make it
> > somewhat
> > easier to follow what's going on.
> 
> Sure, I will fold in.

Ah, wait, they have different Fixes tag. One goes deeply down in the
history.

So, I can instead put more explanations in patch 1.
Would it work?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH v2 3/6] 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, 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;
-
 	match = acpi_match_device(dev->driver->acpi_match_table, dev);
 	if (!match)
 		return NULL;
-- 
2.15.1

Re: [PATCH v2 3/6] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

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;

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 3/6] ACPI / bus: Remove checks in acpi_get_match_data()

[Rafael J. Wysocki]

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.

Re: [PATCH v2 3/6] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

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.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 3/6] ACPI / bus: Remove checks in acpi_get_match_data()

[Rafael J. Wysocki]

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.

Re: [PATCH v2 3/6] ACPI / bus: Remove checks in acpi_get_match_data()

[Andy Shevchenko]

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.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH v2 4/6] ACPI / bus: Rename acpi_get_match_data() to acpi_device_get_match_data()

[Andy Shevchenko]

Do the renaming to be consistent with its sibling, i.e.
of_device_get_match_data().

No functional change.

Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function")
Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data() callback")
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      | 4 ++--
 drivers/acpi/property.c | 2 +-
 include/linux/acpi.h    | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 413e4b1cb1be..0f3f38a4e0c5 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -787,7 +787,7 @@ const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 }
 EXPORT_SYMBOL_GPL(acpi_match_device);
 
-void *acpi_get_match_data(const struct device *dev)
+void *acpi_device_get_match_data(const struct device *dev)
 {
 	const struct acpi_device_id *match;
 
@@ -797,7 +797,7 @@ void *acpi_get_match_data(const struct device *dev)
 
 	return (void *)match->driver_data;
 }
-EXPORT_SYMBOL_GPL(acpi_get_match_data);
+EXPORT_SYMBOL_GPL(acpi_device_get_match_data);
 
 int acpi_match_device_ids(struct acpi_device *device,
 			  const struct acpi_device_id *ids)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 466d1503aba0..f9b5fa230a86 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1275,7 +1275,7 @@ static void *
 acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 				  const struct device *dev)
 {
-	return acpi_get_match_data(dev);
+	return acpi_device_get_match_data(dev);
 }
 
 #define DECLARE_ACPI_FWNODE_OPS(ops) \
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 246845da3f84..d0cbbbd88e0e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -587,7 +587,7 @@ extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
 const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 					       const struct device *dev);
 
-void *acpi_get_match_data(const struct device *dev);
+void *acpi_device_get_match_data(const struct device *dev);
 extern bool acpi_driver_match_device(struct device *dev,
 				     const struct device_driver *drv);
 int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
@@ -766,7 +766,7 @@ static inline const struct acpi_device_id *acpi_match_device(
 	return NULL;
 }
 
-static inline void *acpi_get_match_data(const struct device *dev)
+static inline void *acpi_device_get_match_data(const struct device *dev)
 {
 	return NULL;
 }
-- 
2.15.1

Re: [PATCH v2 4/6] ACPI / bus: Rename acpi_get_match_data() to acpi_device_get_match_data()

[Rafael J. Wysocki]

On Thu, Feb 1, 2018 at 9:20 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Do the renaming to be consistent with its sibling, i.e.
> of_device_get_match_data().
>
> No functional change.
>
> Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function")
> Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data() callback")

Why are you adding the Fixes: tags to a patch that doesn't make any
functional changes?

What are they useful for?

Re: [PATCH v2 4/6] ACPI / bus: Rename acpi_get_match_data() to acpi_device_get_match_data()

[Andy Shevchenko]

On Sun, 2018-02-04 at 08:23 +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 1, 2018 at 9:20 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > Do the renaming to be consistent with its sibling, i.e.
> > of_device_get_match_data().
> > 
> > No functional change.
> > 
> > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data()
> > function")
> > Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data()
> > callback")
> 
> Why are you adding the Fixes: tags to a patch that doesn't make any
> functional changes?
> 
> What are they useful for?

Because next patch depends on this one and fixes API. 
So, here the Fixes tags to be sure that next will apply cleanly.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 4/6] ACPI / bus: Rename acpi_get_match_data() to acpi_device_get_match_data()

[Rafael J. Wysocki]

On Monday, February 5, 2018 5:12:02 PM CET Andy Shevchenko wrote:
> On Sun, 2018-02-04 at 08:23 +0100, Rafael J. Wysocki wrote:
> > On Thu, Feb 1, 2018 at 9:20 PM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > Do the renaming to be consistent with its sibling, i.e.
> > > of_device_get_match_data().
> > > 
> > > No functional change.
> > > 
> > > Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data()
> > > function")
> > > Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data()
> > > callback")
> > 
> > Why are you adding the Fixes: tags to a patch that doesn't make any
> > functional changes?
> > 
> > What are they useful for?
> 
> Because next patch depends on this one and fixes API. 
> So, here the Fixes tags to be sure that next will apply cleanly.

They may mislead people to belive that the patch needs to be backported,
so I'd rather not add them.

[PATCH v2 5/6] dmaengine: hidma: Constify returned by device_get_match_data() value

[Andy Shevchenko]

The value under the hood is constant in any case.
It makes sense to show this explicitly.

Fixes: 95fbfb7aa28d ("dmaengine: qcom_hidma: Add support for the new revision")
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/dma/qcom/hidma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
index 963cc5228d05..24e163cefb7c 100644
--- a/drivers/dma/qcom/hidma.c
+++ b/drivers/dma/qcom/hidma.c
@@ -743,9 +743,9 @@ static int hidma_request_msi(struct hidma_dev *dmadev,
 
 static bool hidma_test_capability(struct device *dev, enum hidma_cap test_cap)
 {
-	enum hidma_cap cap;
+	const enum hidma_cap cap;
 
-	cap = (enum hidma_cap) device_get_match_data(dev);
+	cap = (const enum hidma_cap) device_get_match_data(dev);
 	return cap ? ((cap & test_cap) > 0) : 0;
 }
 
-- 
2.15.1

Re: [PATCH v2 5/6] dmaengine: hidma: Constify returned by device_get_match_data() value

[Andy Shevchenko]

On Thu, 2018-02-01 at 22:20 +0200, Andy Shevchenko wrote:
> The value under the hood is constant in any case.
> It makes sense to show this explicitly.
> 

This patch is wrong, sorry.

> Fixes: 95fbfb7aa28d ("dmaengine: qcom_hidma: Add support for the new
> revision")
> 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/dma/qcom/hidma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index 963cc5228d05..24e163cefb7c 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -743,9 +743,9 @@ static int hidma_request_msi(struct hidma_dev
> *dmadev,
>  
>  static bool hidma_test_capability(struct device *dev, enum hidma_cap
> test_cap)
>  {
> -	enum hidma_cap cap;
> +	const enum hidma_cap cap;
>  
> -	cap = (enum hidma_cap) device_get_match_data(dev);
> +	cap = (const enum hidma_cap) device_get_match_data(dev);
>  	return cap ? ((cap & test_cap) > 0) : 0;
>  }
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH v2 6/6] device property: Constify device_get_match_data()

[Andy Shevchenko]

Constify device_get_match_data() as OF and ACPI variants return
constant value.

Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function")
Fixes: b283f157611f ("device property: Introduce a common API to fetch device match data")
Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data() callback")
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       | 4 ++--
 drivers/acpi/property.c  | 2 +-
 drivers/base/property.c  | 5 ++---
 drivers/of/property.c    | 4 ++--
 include/linux/acpi.h     | 4 ++--
 include/linux/fwnode.h   | 4 ++--
 include/linux/property.h | 2 +-
 7 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 0f3f38a4e0c5..31f1f79fb3c3 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -787,7 +787,7 @@ const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 }
 EXPORT_SYMBOL_GPL(acpi_match_device);
 
-void *acpi_device_get_match_data(const struct device *dev)
+const void *acpi_device_get_match_data(const struct device *dev)
 {
 	const struct acpi_device_id *match;
 
@@ -795,7 +795,7 @@ void *acpi_device_get_match_data(const struct device *dev)
 	if (!match)
 		return NULL;
 
-	return (void *)match->driver_data;
+	return (const void *)match->driver_data;
 }
 EXPORT_SYMBOL_GPL(acpi_device_get_match_data);
 
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index f9b5fa230a86..5815356ea6ad 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1271,7 +1271,7 @@ static int acpi_fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 	return 0;
 }
 
-static void *
+static const void *
 acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 				  const struct device *dev)
 {
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 302236281d83..8f205f6461ed 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1410,9 +1410,8 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
 
-void *device_get_match_data(struct device *dev)
+const void *device_get_match_data(struct device *dev)
 {
-	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data,
-				  dev);
+	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
 }
 EXPORT_SYMBOL_GPL(device_get_match_data);
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 36ed84e26d9c..f46828e3b082 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -977,11 +977,11 @@ static int of_fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 	return 0;
 }
 
-static void *
+static const void *
 of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 				const struct device *dev)
 {
-	return (void *)of_device_get_match_data(dev);
+	return of_device_get_match_data(dev);
 }
 
 const struct fwnode_operations of_fwnode_ops = {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d0cbbbd88e0e..9a618204aba4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -587,7 +587,7 @@ extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
 const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 					       const struct device *dev);
 
-void *acpi_device_get_match_data(const struct device *dev);
+const void *acpi_device_get_match_data(const struct device *dev);
 extern bool acpi_driver_match_device(struct device *dev,
 				     const struct device_driver *drv);
 int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
@@ -766,7 +766,7 @@ static inline const struct acpi_device_id *acpi_match_device(
 	return NULL;
 }
 
-static inline void *acpi_device_get_match_data(const struct device *dev)
+static inline const void *acpi_device_get_match_data(const struct device *dev)
 {
 	return NULL;
 }
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 4fa1a489efe4..4fe8f289b3f6 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -73,8 +73,8 @@ struct fwnode_operations {
 	struct fwnode_handle *(*get)(struct fwnode_handle *fwnode);
 	void (*put)(struct fwnode_handle *fwnode);
 	bool (*device_is_available)(const struct fwnode_handle *fwnode);
-	void *(*device_get_match_data)(const struct fwnode_handle *fwnode,
-				       const struct device *dev);
+	const void *(*device_get_match_data)(const struct fwnode_handle *fwnode,
+					     const struct device *dev);
 	bool (*property_present)(const struct fwnode_handle *fwnode,
 				 const char *propname);
 	int (*property_read_int_array)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index 769d372c1edf..2eea4b310fc2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -283,7 +283,7 @@ bool device_dma_supported(struct device *dev);
 
 enum dev_dma_attr device_get_dma_attr(struct device *dev);
 
-void *device_get_match_data(struct device *dev);
+const void *device_get_match_data(struct device *dev);
 
 int device_get_phy_mode(struct device *dev);
 
-- 
2.15.1

Re: [PATCH v2 6/6] device property: Constify device_get_match_data()

[Sinan Kaya]

On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> -void *device_get_match_data(struct device *dev)
> +const void *device_get_match_data(struct device *dev)
>  {
> -	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data,
> -				  dev);
> +	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
>  }

I remember having a checkpatch problem here.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 6/6] device property: Constify device_get_match_data()

[Andy Shevchenko]

On Thu, 2018-02-01 at 15:35 -0500, Sinan Kaya wrote:
> On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
> > -void *device_get_match_data(struct device *dev)
> > +const void *device_get_match_data(struct device *dev)
> >  {
> > -	return fwnode_call_ptr_op(dev_fwnode(dev),
> > device_get_match_data,
> > -				  dev);
> > +	return fwnode_call_ptr_op(dev_fwnode(dev),
> > device_get_match_data, dev);
> >  }
> 
> I remember having a checkpatch problem here.

It's exactly 79 characters. 
What problem?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 6/6] device property: Constify device_get_match_data()

[Sinan Kaya]

On 2/1/2018 3:38 PM, Andy Shevchenko wrote:
> On Thu, 2018-02-01 at 15:35 -0500, Sinan Kaya wrote:
>> On 2/1/2018 3:20 PM, Andy Shevchenko wrote:
>>> -void *device_get_match_data(struct device *dev)
>>> +const void *device_get_match_data(struct device *dev)
>>>  {
>>> -	return fwnode_call_ptr_op(dev_fwnode(dev),
>>> device_get_match_data,
>>> -				  dev);
>>> +	return fwnode_call_ptr_op(dev_fwnode(dev),
>>> device_get_match_data, dev);
>>>  }
>>
>> I remember having a checkpatch problem here.
> 
> It's exactly 79 characters. 
> What problem?
> 

OK. Maybe with one of the versions I posted. Just checking.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 6/6] device property: Constify device_get_match_data()

[Sakari Ailus]

Hi Andy,

On Thu, Feb 01, 2018 at 10:20:12PM +0200, Andy Shevchenko wrote:
> Constify device_get_match_data() as OF and ACPI variants return
> constant value.
> 
> Fixes: 80212a162329 ("ACPI / bus: Introduce acpi_get_match_data() function")
> Fixes: b283f157611f ("device property: Introduce a common API to fetch device match data")
> Fixes: 146b4dbb0eef ("ACPI: properties: Implement get_match_data() callback")
> 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>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> (fwnode changes)

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH v2 1/6] ACPI / bus: Return error code from __acpi_match_device() in one case

[Rafael J. Wysocki]

On Thu, Feb 1, 2018 at 9:20 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Instead of playing tricks with last invalid entry,
> return simple -ENODATA error code casted to pointer.
>
> It would be good for future in case caller passes NULL pointer for
> ID table. Moreover, caller can check the code to be sure what happened
> inside callee.
>
> Fixes: 2b9c698efa58 ("ACPI / scan: Take the PRP0001 position in the list of IDs into account")
> 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: new patch
>  drivers/acpi/bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index f87ed3be779a..a87a97bf75f8 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -762,7 +762,7 @@ static const struct acpi_device_id *__acpi_match_device(
>                  */
>                 if (!strcmp(ACPI_DT_NAMESPACE_HID, hwid->id)
>                     && acpi_of_match_device(device, of_ids))
> -                       return id;
> +                       return ERR_PTR(-ENODATA);

So why is returning id from here a problem?

>         }
>         return NULL;
>  }
> --

Re: [PATCH v2 1/6] ACPI / bus: Return error code from __acpi_match_device() in one case

[Andy Shevchenko]

On Sun, 2018-02-04 at 08:16 +0100, Rafael J. Wysocki wrote:
> On Thu, Feb 1, 2018 at 9:20 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > Instead of playing tricks with last invalid entry,
> > return simple -ENODATA error code casted to pointer.
> > 
> > It would be good for future in case caller passes NULL pointer for
> > ID table. Moreover, caller can check the code to be sure what
> > happened
> > inside callee.

> > -                       return id;
> > +                       return ERR_PTR(-ENODATA);
> 
> So why is returning id from here a problem?

I think you already noticed that in the following patch it becomes a
problem if user supply ids == NULL.

That's why I added a second paragraph to the commit message to explain.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy