[PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Andy Shevchenko]

There are several places in the kernel where this kind of functionality is
being used. Provide a generic helper for such cases.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/core.c    | 24 ++++++++++++++++++++++++
 include/linux/device.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 7cd789c4985d..972bfe975cd0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3832,6 +3832,30 @@ struct device *device_find_child_by_name(struct device *parent,
 }
 EXPORT_SYMBOL_GPL(device_find_child_by_name);
 
+/**
+ * device_find_first_child - device iterator for locating the fist child device.
+ * @parent: parent struct device
+ *
+ * This is similar to the device_find_child() function above, but it
+ * returns a reference to the first child device.
+ *
+ * NOTE: you will need to drop the reference with put_device() after use.
+ */
+struct device *device_find_first_child(struct device *parent)
+{
+	struct klist_iter i;
+	struct device *child;
+
+	if (!parent)
+		return NULL;
+
+	klist_iter_init(&parent->p->klist_children, &i);
+	child = get_device(next_device(&i));
+	klist_iter_exit(&i);
+	return child;
+}
+EXPORT_SYMBOL_GPL(device_find_first_child);
+
 int __init devices_init(void)
 {
 	devices_kset = kset_create_and_add("devices", &device_uevent_ops, NULL);
diff --git a/include/linux/device.h b/include/linux/device.h
index dc941997795c..20171a4358df 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -905,6 +905,7 @@ struct device *device_find_child(struct device *dev, void *data,
 				 int (*match)(struct device *dev, void *data));
 struct device *device_find_child_by_name(struct device *parent,
 					 const char *name);
+struct device *device_find_first_child(struct device *parent);
 int device_rename(struct device *dev, const char *new_name);
 int device_move(struct device *dev, struct device *new_parent,
 		enum dpm_order dpm_order);
-- 
2.35.1

[PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Andy Shevchenko]

We have already a helper to get the first child device, use it and
drop custom approach.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ea09d1b42bf6..87dc8773108b 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2613,11 +2613,6 @@ int spi_slave_abort(struct spi_device *spi)
 }
 EXPORT_SYMBOL_GPL(spi_slave_abort);
 
-static int match_true(struct device *dev, void *data)
-{
-	return 1;
-}
-
 static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
@@ -2625,7 +2620,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
 						   dev);
 	struct device *child;
 
-	child = device_find_child(&ctlr->dev, NULL, match_true);
+	child = device_find_first_child(&ctlr->dev);
 	return sprintf(buf, "%s\n",
 		       child ? to_spi_device(child)->modalias : NULL);
 }
@@ -2644,7 +2639,7 @@ static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
 	if (rc != 1 || !name[0])
 		return -EINVAL;
 
-	child = device_find_child(&ctlr->dev, NULL, match_true);
+	child = device_find_first_child(&ctlr->dev);
 	if (child) {
 		/* Remove registered slave */
 		device_unregister(child);
-- 
2.35.1

Re: [PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Rafael J. Wysocki]

On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> We have already a helper to get the first child device, use it and
> drop custom approach.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Nice cleanup.

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

> ---
>  drivers/spi/spi.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ea09d1b42bf6..87dc8773108b 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2613,11 +2613,6 @@ int spi_slave_abort(struct spi_device *spi)
>  }
>  EXPORT_SYMBOL_GPL(spi_slave_abort);
>
> -static int match_true(struct device *dev, void *data)
> -{
> -       return 1;
> -}
> -
>  static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
>                           char *buf)
>  {
> @@ -2625,7 +2620,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
>                                                    dev);
>         struct device *child;
>
> -       child = device_find_child(&ctlr->dev, NULL, match_true);
> +       child = device_find_first_child(&ctlr->dev);
>         return sprintf(buf, "%s\n",
>                        child ? to_spi_device(child)->modalias : NULL);
>  }
> @@ -2644,7 +2639,7 @@ static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
>         if (rc != 1 || !name[0])
>                 return -EINVAL;
>
> -       child = device_find_child(&ctlr->dev, NULL, match_true);
> +       child = device_find_first_child(&ctlr->dev);
>         if (child) {
>                 /* Remove registered slave */
>                 device_unregister(child);
> --
> 2.35.1
>

Re: [PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Greg Kroah-Hartman]

On Tue, Jun 07, 2022 at 11:20:58PM +0300, Andy Shevchenko wrote:
> We have already a helper to get the first child device, use it and
> drop custom approach.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/spi/spi.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ea09d1b42bf6..87dc8773108b 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2613,11 +2613,6 @@ int spi_slave_abort(struct spi_device *spi)
>  }
>  EXPORT_SYMBOL_GPL(spi_slave_abort);
>  
> -static int match_true(struct device *dev, void *data)
> -{
> -	return 1;
> -}
> -
>  static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
>  			  char *buf)
>  {
> @@ -2625,7 +2620,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
>  						   dev);
>  	struct device *child;
>  
> -	child = device_find_child(&ctlr->dev, NULL, match_true);
> +	child = device_find_first_child(&ctlr->dev);
>  	return sprintf(buf, "%s\n",
>  		       child ? to_spi_device(child)->modalias : NULL);
>  }

Horrible naming convention asside, what is this really showing?  I do
not see this documented in Documentation/ABI/ anywhere, so can it just
be dropped entirely?

Ah, it's in Documentation/spi/spi-summary.rst not where it belongs...

Looks like "any" of the child devices could match here, so it's just
finding the first one by default.  So you aren't explicitly asking for
the real first device, you could return the last one as well, and it
would still work as there is just "one" device in this list from what I
can tell.

So is does this really deserve a new driver core api call?

thanks,

greg k-h

Re: [PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Andy Shevchenko]

On Wed, Jun 08, 2022 at 01:36:17PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 07, 2022 at 11:20:58PM +0300, Andy Shevchenko wrote:
> > We have already a helper to get the first child device, use it and
> > drop custom approach.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/spi/spi.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index ea09d1b42bf6..87dc8773108b 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -2613,11 +2613,6 @@ int spi_slave_abort(struct spi_device *spi)
> >  }
> >  EXPORT_SYMBOL_GPL(spi_slave_abort);
> >  
> > -static int match_true(struct device *dev, void *data)
> > -{
> > -	return 1;
> > -}
> > -
> >  static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
> >  			  char *buf)
> >  {
> > @@ -2625,7 +2620,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
> >  						   dev);
> >  	struct device *child;
> >  
> > -	child = device_find_child(&ctlr->dev, NULL, match_true);
> > +	child = device_find_first_child(&ctlr->dev);
> >  	return sprintf(buf, "%s\n",
> >  		       child ? to_spi_device(child)->modalias : NULL);
> >  }
> 
> Horrible naming convention asside, what is this really showing?  I do
> not see this documented in Documentation/ABI/ anywhere, so can it just
> be dropped entirely?
> 
> Ah, it's in Documentation/spi/spi-summary.rst not where it belongs...
> 
> Looks like "any" of the child devices could match here, so it's just
> finding the first one by default.  So you aren't explicitly asking for
> the real first device, you could return the last one as well, and it
> would still work as there is just "one" device in this list from what I
> can tell.
> 
> So is does this really deserve a new driver core api call?

As I said I noticed more places like this (*) and the problem is that I can't
simply use device_match_any() because of the different prototype.

I agree that all thing should be using _any instead of _first.

*) e.g. ptp_ocp.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Greg Kroah-Hartman]

On Wed, Jun 08, 2022 at 02:49:14PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 08, 2022 at 01:36:17PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 07, 2022 at 11:20:58PM +0300, Andy Shevchenko wrote:
> > > We have already a helper to get the first child device, use it and
> > > drop custom approach.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/spi/spi.c | 9 ++-------
> > >  1 file changed, 2 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > > index ea09d1b42bf6..87dc8773108b 100644
> > > --- a/drivers/spi/spi.c
> > > +++ b/drivers/spi/spi.c
> > > @@ -2613,11 +2613,6 @@ int spi_slave_abort(struct spi_device *spi)
> > >  }
> > >  EXPORT_SYMBOL_GPL(spi_slave_abort);
> > >  
> > > -static int match_true(struct device *dev, void *data)
> > > -{
> > > -	return 1;
> > > -}
> > > -
> > >  static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
> > >  			  char *buf)
> > >  {
> > > @@ -2625,7 +2620,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
> > >  						   dev);
> > >  	struct device *child;
> > >  
> > > -	child = device_find_child(&ctlr->dev, NULL, match_true);
> > > +	child = device_find_first_child(&ctlr->dev);
> > >  	return sprintf(buf, "%s\n",
> > >  		       child ? to_spi_device(child)->modalias : NULL);
> > >  }
> > 
> > Horrible naming convention asside, what is this really showing?  I do
> > not see this documented in Documentation/ABI/ anywhere, so can it just
> > be dropped entirely?
> > 
> > Ah, it's in Documentation/spi/spi-summary.rst not where it belongs...
> > 
> > Looks like "any" of the child devices could match here, so it's just
> > finding the first one by default.  So you aren't explicitly asking for
> > the real first device, you could return the last one as well, and it
> > would still work as there is just "one" device in this list from what I
> > can tell.
> > 
> > So is does this really deserve a new driver core api call?
> 
> As I said I noticed more places like this (*) and the problem is that I can't
> simply use device_match_any() because of the different prototype.

Why not exactly?  match_true() above and device_match_any() have the
same signature from what I can tell:
	static int match_true(struct device *dev, void *data)
	int device_match_any(struct device *dev, const void *unused)

What am I missing, the const?

> I agree that all thing should be using _any instead of _first.

Yes, so let's fix it please, don't propagate bad patterns.

thanks,

greg k-h

Re: [PATCH v1 2/2] spi: Use device_find_first_child() instead of custom approach

[Andy Shevchenko]

On Wed, Jun 08, 2022 at 02:03:32PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 08, 2022 at 02:49:14PM +0300, Andy Shevchenko wrote:

...

> Why not exactly?  match_true() above and device_match_any() have the
> same signature from what I can tell:
> 	static int match_true(struct device *dev, void *data)
> 	int device_match_any(struct device *dev, const void *unused)
> 
> What am I missing, the const?

Yep! Compiler is very unhappy about it.

> > I agree that all thing should be using _any instead of _first.
> 
> Yes, so let's fix it please, don't propagate bad patterns.

Will do, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Rafael J. Wysocki]

On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> There are several places in the kernel where this kind of functionality is
> being used. Provide a generic helper for such cases.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/core.c    | 24 ++++++++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 7cd789c4985d..972bfe975cd0 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3832,6 +3832,30 @@ struct device *device_find_child_by_name(struct device *parent,
>  }
>  EXPORT_SYMBOL_GPL(device_find_child_by_name);
>
> +/**
> + * device_find_first_child - device iterator for locating the fist child device.
> + * @parent: parent struct device
> + *
> + * This is similar to the device_find_child() function above, but it
> + * returns a reference to the first child device.
> + *
> + * NOTE: you will need to drop the reference with put_device() after use.
> + */
> +struct device *device_find_first_child(struct device *parent)
> +{
> +       struct klist_iter i;
> +       struct device *child;
> +
> +       if (!parent)
> +               return NULL;
> +
> +       klist_iter_init(&parent->p->klist_children, &i);
> +       child = get_device(next_device(&i));
> +       klist_iter_exit(&i);
> +       return child;
> +}
> +EXPORT_SYMBOL_GPL(device_find_first_child);

I would define it as

static int match_first(struct device *dev, void *)
{
       return 1;
}

struct device *device_find_first_child(struct device *parent)
{
        return device_find_first_child(parent, NULL, match_first);
}
EXPORT_SYMBOL_GPL(device_find_first_child);

which is not that much more overhead.

> +
>  int __init devices_init(void)
>  {
>         devices_kset = kset_create_and_add("devices", &device_uevent_ops, NULL);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index dc941997795c..20171a4358df 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -905,6 +905,7 @@ struct device *device_find_child(struct device *dev, void *data,
>                                  int (*match)(struct device *dev, void *data));
>  struct device *device_find_child_by_name(struct device *parent,
>                                          const char *name);
> +struct device *device_find_first_child(struct device *parent);
>  int device_rename(struct device *dev, const char *new_name);
>  int device_move(struct device *dev, struct device *new_parent,
>                 enum dpm_order dpm_order);
> --
> 2.35.1
>

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Andy Shevchenko]

On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> I would define it as
> 
> static int match_first(struct device *dev, void *)
> {
>        return 1;
> }
> 
> struct device *device_find_first_child(struct device *parent)
> {
>         return device_find_first_child(parent, NULL, match_first);
> }
> EXPORT_SYMBOL_GPL(device_find_first_child);
> 
> which is not that much more overhead.

With this we actually may simply provide a match function and it will make the
clean ups (like patch 2 in the series) almost the same without introducing a
device core call.

Something like

int device_match_any_for_find(struct device *dev, void *unused)
{
	return 1;
}

As I replied to Greg it's pity we can't use device_match_any()...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Greg Kroah-Hartman]

On Wed, Jun 08, 2022 at 02:53:28PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> 
> ...
> 
> > I would define it as
> > 
> > static int match_first(struct device *dev, void *)
> > {
> >        return 1;
> > }
> > 
> > struct device *device_find_first_child(struct device *parent)
> > {
> >         return device_find_first_child(parent, NULL, match_first);
> > }
> > EXPORT_SYMBOL_GPL(device_find_first_child);
> > 
> > which is not that much more overhead.
> 
> With this we actually may simply provide a match function and it will make the
> clean ups (like patch 2 in the series) almost the same without introducing a
> device core call.
> 
> Something like
> 
> int device_match_any_for_find(struct device *dev, void *unused)
> {
> 	return 1;
> }
> 
> As I replied to Greg it's pity we can't use device_match_any()...

	int device_match_any(struct device *dev, const void *unused)

How is that not ok to use here?

thanks,

greg k-h

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 2:04 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 08, 2022 at 02:53:28PM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> > > On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> >
> > ...
> >
> > > I would define it as
> > >
> > > static int match_first(struct device *dev, void *)
> > > {
> > >        return 1;
> > > }
> > >
> > > struct device *device_find_first_child(struct device *parent)
> > > {
> > >         return device_find_first_child(parent, NULL, match_first);
> > > }
> > > EXPORT_SYMBOL_GPL(device_find_first_child);
> > >
> > > which is not that much more overhead.
> >
> > With this we actually may simply provide a match function and it will make the
> > clean ups (like patch 2 in the series) almost the same without introducing a
> > device core call.
> >
> > Something like
> >
> > int device_match_any_for_find(struct device *dev, void *unused)
> > {
> >       return 1;
> > }
> >
> > As I replied to Greg it's pity we can't use device_match_any()...
>
>         int device_match_any(struct device *dev, const void *unused)
>
> How is that not ok to use here?

Because of the const that will be frowned upon by the compiler.

We need to define another device_match_any_relaxed() taking (void *)
as the second argument for this.

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Greg Kroah-Hartman]

On Wed, Jun 08, 2022 at 02:15:19PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 8, 2022 at 2:04 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 08, 2022 at 02:53:28PM +0300, Andy Shevchenko wrote:
> > > On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> > > > On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > ...
> > >
> > > > I would define it as
> > > >
> > > > static int match_first(struct device *dev, void *)
> > > > {
> > > >        return 1;
> > > > }
> > > >
> > > > struct device *device_find_first_child(struct device *parent)
> > > > {
> > > >         return device_find_first_child(parent, NULL, match_first);
> > > > }
> > > > EXPORT_SYMBOL_GPL(device_find_first_child);
> > > >
> > > > which is not that much more overhead.
> > >
> > > With this we actually may simply provide a match function and it will make the
> > > clean ups (like patch 2 in the series) almost the same without introducing a
> > > device core call.
> > >
> > > Something like
> > >
> > > int device_match_any_for_find(struct device *dev, void *unused)
> > > {
> > >       return 1;
> > > }
> > >
> > > As I replied to Greg it's pity we can't use device_match_any()...
> >
> >         int device_match_any(struct device *dev, const void *unused)
> >
> > How is that not ok to use here?
> 
> Because of the const that will be frowned upon by the compiler.
> 
> We need to define another device_match_any_relaxed() taking (void *)
> as the second argument for this.

Or we could cast it away :)

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 2:23 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 08, 2022 at 02:15:19PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Jun 8, 2022 at 2:04 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Jun 08, 2022 at 02:53:28PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> > > > > On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > ...
> > > >
> > > > > I would define it as
> > > > >
> > > > > static int match_first(struct device *dev, void *)
> > > > > {
> > > > >        return 1;
> > > > > }
> > > > >
> > > > > struct device *device_find_first_child(struct device *parent)
> > > > > {
> > > > >         return device_find_first_child(parent, NULL, match_first);
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(device_find_first_child);
> > > > >
> > > > > which is not that much more overhead.
> > > >
> > > > With this we actually may simply provide a match function and it will make the
> > > > clean ups (like patch 2 in the series) almost the same without introducing a
> > > > device core call.
> > > >
> > > > Something like
> > > >
> > > > int device_match_any_for_find(struct device *dev, void *unused)
> > > > {
> > > >       return 1;
> > > > }
> > > >
> > > > As I replied to Greg it's pity we can't use device_match_any()...
> > >
> > >         int device_match_any(struct device *dev, const void *unused)
> > >
> > > How is that not ok to use here?
> >
> > Because of the const that will be frowned upon by the compiler.
> >
> > We need to define another device_match_any_relaxed() taking (void *)
> > as the second argument for this.
>
> Or we could cast it away :)

I'm not sure what exactly you mean.  The function pointer signature
doesn't match here.

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 1:54 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jun 08, 2022 at 01:29:08PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Jun 7, 2022 at 10:22 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > I would define it as
> >
> > static int match_first(struct device *dev, void *)
> > {
> >        return 1;
> > }
> >
> > struct device *device_find_first_child(struct device *parent)
> > {
> >         return device_find_first_child(parent, NULL, match_first);
> > }
> > EXPORT_SYMBOL_GPL(device_find_first_child);
> >
> > which is not that much more overhead.
>
> With this we actually may simply provide a match function and it will make the
> clean ups (like patch 2 in the series) almost the same without introducing a
> device core call.

That works too, but IMO it would be a bit cleaner to have the wrapper
defined as a proper function.

>
> Something like
>
> int device_match_any_for_find(struct device *dev, void *unused)
> {
>         return 1;
> }
>
> As I replied to Greg it's pity we can't use device_match_any()...

Well, that only is a matter of adding one more variant of _match_any_ ...

Re: [PATCH v1 1/2] driver core: Introduce device_find_first_child() helper

[Greg Kroah-Hartman]

On Tue, Jun 07, 2022 at 11:20:57PM +0300, Andy Shevchenko wrote:
> There are several places in the kernel where this kind of functionality is
> being used. Provide a generic helper for such cases.

This feels really wrong/broken.

There should not be any specific ordering of children in the tree.  What
subsystem relies on this such that they require this?

thanks,

greg k-h