[PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Andy Shevchenko]

None of the current users is using gaps in the list of the items.
No need to have a specific function for that, just replace it by
library available __sysfs_match_string().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/industrialio-core.c | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 52e690f031cb..26e357f14db8 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -205,36 +205,6 @@ bool iio_buffer_enabled(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(iio_buffer_enabled);
 
-/**
- * iio_sysfs_match_string_with_gaps - matches given string in an array with gaps
- * @array: array of strings
- * @n: number of strings in the array
- * @str: string to match with
- *
- * Returns index of @str in the @array or -EINVAL, similar to match_string().
- * Uses sysfs_streq instead of strcmp for matching.
- *
- * This routine will look for a string in an array of strings.
- * The search will continue until the element is found or the n-th element
- * is reached, regardless of any NULL elements in the array.
- */
-static int iio_sysfs_match_string_with_gaps(const char * const *array, size_t n,
-					    const char *str)
-{
-	const char *item;
-	int index;
-
-	for (index = 0; index < n; index++) {
-		item = array[index];
-		if (!item)
-			continue;
-		if (sysfs_streq(item, str))
-			return index;
-	}
-
-	return -EINVAL;
-}
-
 #if defined(CONFIG_DEBUG_FS)
 /*
  * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for
@@ -569,7 +539,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
 	if (!e->set)
 		return -EINVAL;
 
-	ret = iio_sysfs_match_string_with_gaps(e->items, e->num_items, buf);
+	ret = __sysfs_match_string(e->items, e->num_items, buf);
 	if (ret < 0)
 		return ret;
 
-- 
2.39.0

[PATCH v1 2/2] iio: core: Sort headers

[Andy Shevchenko]

Sort the headers in alphabetic order in order to ease
the maintenance for this part.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/industrialio-core.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 26e357f14db8..c117f50d0cf3 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -8,30 +8,32 @@
 
 #define pr_fmt(fmt) "iio-core: " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/idr.h>
-#include <linux/kdev_t.h>
-#include <linux/err.h>
+#include <linux/anon_inodes.h>
+#include <linux/cdev.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/fs.h>
+#include <linux/idr.h>
+#include <linux/kdev_t.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/property.h>
 #include <linux/sched.h>
-#include <linux/wait.h>
-#include <linux/cdev.h>
 #include <linux/slab.h>
-#include <linux/anon_inodes.h>
-#include <linux/debugfs.h>
-#include <linux/mutex.h>
-#include <linux/iio/iio.h>
+#include <linux/wait.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/buffer_impl.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio-opaque.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
 #include "iio_core.h"
 #include "iio_core_trigger.h"
-#include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/buffer_impl.h>
 
 /* IDA to assign each registered device a unique id */
 static DEFINE_IDA(iio_ida);
-- 
2.39.0

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Lars-Peter Clausen]

On 1/17/23 23:48, Andy Shevchenko wrote:
> None of the current users is using gaps in the list of the items.
> No need to have a specific function for that, just replace it by
> library available __sysfs_match_string().

Hm, I specifically remember adding this for a driver where there were 
gaps. One of the DACs. But it might be that the driver itself never made 
it upstream.

>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/iio/industrialio-core.c | 32 +-------------------------------
>   1 file changed, 1 insertion(+), 31 deletions(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 52e690f031cb..26e357f14db8 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -205,36 +205,6 @@ bool iio_buffer_enabled(struct iio_dev *indio_dev)
>   }
>   EXPORT_SYMBOL_GPL(iio_buffer_enabled);
>   
> -/**
> - * iio_sysfs_match_string_with_gaps - matches given string in an array with gaps
> - * @array: array of strings
> - * @n: number of strings in the array
> - * @str: string to match with
> - *
> - * Returns index of @str in the @array or -EINVAL, similar to match_string().
> - * Uses sysfs_streq instead of strcmp for matching.
> - *
> - * This routine will look for a string in an array of strings.
> - * The search will continue until the element is found or the n-th element
> - * is reached, regardless of any NULL elements in the array.
> - */
> -static int iio_sysfs_match_string_with_gaps(const char * const *array, size_t n,
> -					    const char *str)
> -{
> -	const char *item;
> -	int index;
> -
> -	for (index = 0; index < n; index++) {
> -		item = array[index];
> -		if (!item)
> -			continue;
> -		if (sysfs_streq(item, str))
> -			return index;
> -	}
> -
> -	return -EINVAL;
> -}
> -
>   #if defined(CONFIG_DEBUG_FS)
>   /*
>    * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for
> @@ -569,7 +539,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
>   	if (!e->set)
>   		return -EINVAL;
>   
> -	ret = iio_sysfs_match_string_with_gaps(e->items, e->num_items, buf);
> +	ret = __sysfs_match_string(e->items, e->num_items, buf);
>   	if (ret < 0)
>   		return ret;
>   

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Andy Shevchenko]

On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen wrote:
> On 1/17/23 23:48, Andy Shevchenko wrote:
> > None of the current users is using gaps in the list of the items.
> > No need to have a specific function for that, just replace it by
> > library available __sysfs_match_string().
> 
> Hm, I specifically remember adding this for a driver where there were gaps.
> One of the DACs. But it might be that the driver itself never made it
> upstream.

I have checked all modules that have struct iio_enum and/or ("or" probably may
not happen) IIO_ENUM() in them.

It might be that I missed something.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Lars-Peter Clausen]

On 1/18/23 07:49, Andy Shevchenko wrote:
> On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen wrote:
>> On 1/17/23 23:48, Andy Shevchenko wrote:
>>> None of the current users is using gaps in the list of the items.
>>> No need to have a specific function for that, just replace it by
>>> library available __sysfs_match_string().
>> Hm, I specifically remember adding this for a driver where there were gaps.
>> One of the DACs. But it might be that the driver itself never made it
>> upstream.
> I have checked all modules that have struct iio_enum and/or ("or" probably may
> not happen) IIO_ENUM() in them.
>
> It might be that I missed something.
I checked too, I can't find it either. The driver probably never made it 
upstream.

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Nuno Sá]

On Wed, 2023-01-18 at 08:37 -0800, Lars-Peter Clausen wrote:
> On 1/18/23 07:49, Andy Shevchenko wrote:
> > On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen wrote:
> > > On 1/17/23 23:48, Andy Shevchenko wrote:
> > > > None of the current users is using gaps in the list of the
> > > > items.
> > > > No need to have a specific function for that, just replace it
> > > > by
> > > > library available __sysfs_match_string().
> > > Hm, I specifically remember adding this for a driver where there
> > > were gaps.
> > > One of the DACs. But it might be that the driver itself never
> > > made it
> > > upstream.
> > I have checked all modules that have struct iio_enum and/or ("or"
> > probably may
> > not happen) IIO_ENUM() in them.
> > 
> > It might be that I missed something.
> I checked too, I can't find it either. The driver probably never made
> it 
> upstream.

Yeah, I also did a quick check and I could find it in one adc (most
likely we have more downstream users of this) that did not make it
upstream. Eventually, we want to have it upstream but the ABI using the
gaps can arguably be dropped...

Anyways, from my side I'm fine with this change. We can revert it if we
ever have a real user for this. I'll just have to be careful when
updating ADI tree (but that is our problem :)).

- Nuno Sá

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Andy Shevchenko]

On Thu, Jan 19, 2023 at 09:00:45AM +0100, Nuno Sá wrote:
> On Wed, 2023-01-18 at 08:37 -0800, Lars-Peter Clausen wrote:
> > On 1/18/23 07:49, Andy Shevchenko wrote:
> > > On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen wrote:
> > > > On 1/17/23 23:48, Andy Shevchenko wrote:
> > > > > None of the current users is using gaps in the list of the
> > > > > items.
> > > > > No need to have a specific function for that, just replace it
> > > > > by
> > > > > library available __sysfs_match_string().
> > > > Hm, I specifically remember adding this for a driver where there
> > > > were gaps.
> > > > One of the DACs. But it might be that the driver itself never
> > > > made it
> > > > upstream.
> > > I have checked all modules that have struct iio_enum and/or ("or"
> > > probably may
> > > not happen) IIO_ENUM() in them.
> > > 
> > > It might be that I missed something.
> > I checked too, I can't find it either. The driver probably never made
> > it 
> > upstream.
> 
> Yeah, I also did a quick check and I could find it in one adc (most
> likely we have more downstream users of this) that did not make it
> upstream. Eventually, we want to have it upstream but the ABI using the
> gaps can arguably be dropped...
> 
> Anyways, from my side I'm fine with this change. We can revert it if we
> ever have a real user for this. I'll just have to be careful when
> updating ADI tree (but that is our problem :)).

We usually do not keep a dead code in the kernel, and handling gaps is
a dead code. And yes, we always can return to that when we have a user,
most likely as a part of the generic library and not just IIO.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Nuno Sá]

On Thu, 2023-01-19 at 13:23 +0200, Andy Shevchenko wrote:
> On Thu, Jan 19, 2023 at 09:00:45AM +0100, Nuno Sá wrote:
> > On Wed, 2023-01-18 at 08:37 -0800, Lars-Peter Clausen wrote:
> > > On 1/18/23 07:49, Andy Shevchenko wrote:
> > > > On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen
> > > > wrote:
> > > > > On 1/17/23 23:48, Andy Shevchenko wrote:
> > > > > > None of the current users is using gaps in the list of the
> > > > > > items.
> > > > > > No need to have a specific function for that, just replace
> > > > > > it
> > > > > > by
> > > > > > library available __sysfs_match_string().
> > > > > Hm, I specifically remember adding this for a driver where
> > > > > there
> > > > > were gaps.
> > > > > One of the DACs. But it might be that the driver itself never
> > > > > made it
> > > > > upstream.
> > > > I have checked all modules that have struct iio_enum and/or
> > > > ("or"
> > > > probably may
> > > > not happen) IIO_ENUM() in them.
> > > > 
> > > > It might be that I missed something.
> > > I checked too, I can't find it either. The driver probably never
> > > made
> > > it 
> > > upstream.
> > 
> > Yeah, I also did a quick check and I could find it in one adc (most
> > likely we have more downstream users of this) that did not make it
> > upstream. Eventually, we want to have it upstream but the ABI using
> > the
> > gaps can arguably be dropped...
> > 
> > Anyways, from my side I'm fine with this change. We can revert it
> > if we
> > ever have a real user for this. I'll just have to be careful when
> > updating ADI tree (but that is our problem :)).
> 
> We usually do not keep a dead code in the kernel, and handling gaps
> is a dead code.

Yes, I know... That is why I cannot really complain about this
change :)

- Nuno Sá

Re: [PATCH v1 1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string()

[Jonathan Cameron]

On Thu, 19 Jan 2023 14:24:07 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2023-01-19 at 13:23 +0200, Andy Shevchenko wrote:
> > On Thu, Jan 19, 2023 at 09:00:45AM +0100, Nuno Sá wrote:  
> > > On Wed, 2023-01-18 at 08:37 -0800, Lars-Peter Clausen wrote:  
> > > > On 1/18/23 07:49, Andy Shevchenko wrote:  
> > > > > On Wed, Jan 18, 2023 at 07:22:30AM -0800, Lars-Peter Clausen
> > > > > wrote:  
> > > > > > On 1/17/23 23:48, Andy Shevchenko wrote:  
> > > > > > > None of the current users is using gaps in the list of the
> > > > > > > items.
> > > > > > > No need to have a specific function for that, just replace
> > > > > > > it
> > > > > > > by
> > > > > > > library available __sysfs_match_string().  
> > > > > > Hm, I specifically remember adding this for a driver where
> > > > > > there
> > > > > > were gaps.
> > > > > > One of the DACs. But it might be that the driver itself never
> > > > > > made it
> > > > > > upstream.  
> > > > > I have checked all modules that have struct iio_enum and/or
> > > > > ("or"
> > > > > probably may
> > > > > not happen) IIO_ENUM() in them.
> > > > > 
> > > > > It might be that I missed something.  
> > > > I checked too, I can't find it either. The driver probably never
> > > > made
> > > > it 
> > > > upstream.  
> > > 
> > > Yeah, I also did a quick check and I could find it in one adc (most
> > > likely we have more downstream users of this) that did not make it
> > > upstream. Eventually, we want to have it upstream but the ABI using
> > > the
> > > gaps can arguably be dropped...
> > > 
> > > Anyways, from my side I'm fine with this change. We can revert it
> > > if we
> > > ever have a real user for this. I'll just have to be careful when
> > > updating ADI tree (but that is our problem :)).

You could always upstream the problematic drivers :)

> > 
> > We usually do not keep a dead code in the kernel, and handling gaps
> > is a dead code.  
> 
> Yes, I know... That is why I cannot really complain about this
> change :)

I joined in because I was really really sure we had a user of this
at somepoint. However, despite there having been a bunch of users
in the counter stuff before that spun out as a separate subsystem
looks like they were contiguous as well. Ah well the reasoning behind
this dance may remain lost to history :)

Series applied,

Jonathan

> 
> - Nuno Sá
>