| Message ID | 20230118074828.66155-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1,1/2] iio: core: Replace iio_sysfs_match_string_with_gaps() by __sysfs_match_string() | expand |
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; >
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.
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.
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á
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.
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á
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á >
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;
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(-)