diff mbox series

[v2,03/28] acpi: utils: Cleanup acpi_dev_match_cb

Message ID 1560534863-15115-4-git-send-email-suzuki.poulose@arm.com
State Accepted
Commit e6374f6b2e9c9f9a7cf5418157ad7f30f3abd70e
Headers show
Series None | expand

Commit Message

Suzuki K Poulose June 14, 2019, 5:53 p.m. UTC
The prototype of bus_find_device() will be unified with that of
class_find_device() subsequently, but for this purpose the callback
functions passed to it need to take (const void *) as the second
argument.  Consequently, they cannot modify the memory pointed to by
that argument which currently is not the case for acpi_dev_match_cb().
However, acpi_dev_match_cb() really need not modify the "match" object
passed to it, because acpi_dev_get_first_match_dev() which uses it via
bus_find_device() can easily convert the result of bus_find_device()
into the pointer to return.

For this reason, update acpi_dev_match_cb() to avoid the redundant
memory updates.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

---
 drivers/acpi/utils.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

-- 
2.7.4

Comments

Rafael J. Wysocki June 17, 2019, 10:08 p.m. UTC | #1
On Fri, Jun 14, 2019 at 7:54 PM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>

> The prototype of bus_find_device() will be unified with that of

> class_find_device() subsequently, but for this purpose the callback

> functions passed to it need to take (const void *) as the second

> argument.  Consequently, they cannot modify the memory pointed to by

> that argument which currently is not the case for acpi_dev_match_cb().

> However, acpi_dev_match_cb() really need not modify the "match" object

> passed to it, because acpi_dev_get_first_match_dev() which uses it via

> bus_find_device() can easily convert the result of bus_find_device()

> into the pointer to return.

>

> For this reason, update acpi_dev_match_cb() to avoid the redundant

> memory updates.

>

> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> Cc: "Rafael J. Wysocki" <rafael@kernel.org>

> Cc: Len Brown <lenb@kernel.org>

> Cc: linux-acpi@vger.kernel.org

> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>


Or if you want me to take this patch, please let me know.

> ---

>  drivers/acpi/utils.c | 7 +------

>  1 file changed, 1 insertion(+), 6 deletions(-)

>

> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c

> index 7def63a..1391b63 100644

> --- a/drivers/acpi/utils.c

> +++ b/drivers/acpi/utils.c

> @@ -725,8 +725,6 @@ bool acpi_dev_found(const char *hid)

>  EXPORT_SYMBOL(acpi_dev_found);

>

>  struct acpi_dev_match_info {

> -       const char *dev_name;

> -       struct acpi_device *adev;

>         struct acpi_device_id hid[2];

>         const char *uid;

>         s64 hrv;

> @@ -746,9 +744,6 @@ static int acpi_dev_match_cb(struct device *dev, void *data)

>             strcmp(adev->pnp.unique_id, match->uid)))

>                 return 0;

>

> -       match->dev_name = acpi_dev_name(adev);

> -       match->adev = adev;

> -

>         if (match->hrv == -1)

>                 return 1;

>

> @@ -818,7 +813,7 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)

>         match.hrv = hrv;

>

>         dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);

> -       return dev ? match.adev : NULL;

> +       return dev ? to_acpi_device(dev) : NULL;

>  }

>  EXPORT_SYMBOL(acpi_dev_get_first_match_dev);

>

> --

> 2.7.4

>
Greg Kroah-Hartman June 24, 2019, 3:20 a.m. UTC | #2
On Tue, Jun 18, 2019 at 12:08:45AM +0200, Rafael J. Wysocki wrote:
> On Fri, Jun 14, 2019 at 7:54 PM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:

> >

> > The prototype of bus_find_device() will be unified with that of

> > class_find_device() subsequently, but for this purpose the callback

> > functions passed to it need to take (const void *) as the second

> > argument.  Consequently, they cannot modify the memory pointed to by

> > that argument which currently is not the case for acpi_dev_match_cb().

> > However, acpi_dev_match_cb() really need not modify the "match" object

> > passed to it, because acpi_dev_get_first_match_dev() which uses it via

> > bus_find_device() can easily convert the result of bus_find_device()

> > into the pointer to return.

> >

> > For this reason, update acpi_dev_match_cb() to avoid the redundant

> > memory updates.

> >

> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>

> > Cc: Len Brown <lenb@kernel.org>

> > Cc: linux-acpi@vger.kernel.org

> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

> 

> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> 

> Or if you want me to take this patch, please let me know.


I'll take it now, thanks.

greg k-h
diff mbox series

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 7def63a..1391b63 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -725,8 +725,6 @@  bool acpi_dev_found(const char *hid)
 EXPORT_SYMBOL(acpi_dev_found);
 
 struct acpi_dev_match_info {
-	const char *dev_name;
-	struct acpi_device *adev;
 	struct acpi_device_id hid[2];
 	const char *uid;
 	s64 hrv;
@@ -746,9 +744,6 @@  static int acpi_dev_match_cb(struct device *dev, void *data)
 	    strcmp(adev->pnp.unique_id, match->uid)))
 		return 0;
 
-	match->dev_name = acpi_dev_name(adev);
-	match->adev = adev;
-
 	if (match->hrv == -1)
 		return 1;
 
@@ -818,7 +813,7 @@  acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
 	match.hrv = hrv;
 
 	dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
-	return dev ? match.adev : NULL;
+	return dev ? to_acpi_device(dev) : NULL;
 }
 EXPORT_SYMBOL(acpi_dev_get_first_match_dev);