I am introducing these patches for the patch for ethernet driver which I
will send later.
In Patch 1, I abstract `match_string` to `__match_string` with a comparison
function, make the original name calling it with `strcmp` and add
`match_string_nocase` calling it with `strcasecmp`.
In Patch 2 & 3, I implement `{device,fwnode}_property_match_string_nocase`
and `fwnode_is_compatible` for compatible property matching.
Soha Jin (3):
string: add match_string_nocase() for case-insensitive match
device property: add {device,fwnode}_property_match_string_nocase()
device property: add fwnode_is_compatible() for compatible match
drivers/base/property.c | 92 ++++++++++++++++++++++++++++++++--------
include/linux/property.h | 13 ++++++
include/linux/string.h | 31 +++++++++++++-
lib/string_helpers.c | 10 +++--
4 files changed, 123 insertions(+), 23 deletions(-)
--
2.30.2
fwnode_is_compatible is a shortcut to check if a device is compatible with
a compat string in fwnode property "compatible". This function is similar
to of_device_is_compatible.
Signed-off-by: Soha Jin <[email protected]>
---
include/linux/property.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/property.h b/include/linux/property.h
index dbe747f3e3be..776e4a8bc379 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -252,6 +252,13 @@ fwnode_property_string_array_count(const struct fwnode_handle *fwnode,
return fwnode_property_read_string_array(fwnode, propname, NULL, 0);
}
+static inline bool fwnode_is_compatible(const struct fwnode_handle *fwnode,
+ const char *compat)
+{
+ return fwnode_property_match_string_nocase(fwnode, "compatible",
+ compat) >= 0;
+}
+
struct software_node;
/**
--
2.30.2
device_property_match_string_nocase and fwnode_property_match_string_nocase
are used to do case-insensitive match with match_string_nocase.
Signed-off-by: Soha Jin <[email protected]>
---
drivers/base/property.c | 92 ++++++++++++++++++++++++++++++++--------
include/linux/property.h | 6 +++
2 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 4d6278a84868..5e579b915e77 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -242,6 +242,30 @@ int device_property_match_string(struct device *dev, const char *propname,
}
EXPORT_SYMBOL_GPL(device_property_match_string);
+/**
+ * device_property_match_string_nocase - find a string in an array and return
+ * index with case-insensitive comparison
+ * @dev: Device to get the property of
+ * @propname: Name of the property holding the array
+ * @string: String to look for
+ *
+ * Find a given string in a string array and if it is found return the
+ * index back.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not have a value,
+ * %-EPROTO if the property is not an array of strings,
+ * %-ENXIO if no suitable firmware interface is present.
+ */
+int device_property_match_string_nocase(struct device *dev,
+ const char *propname, const char *string)
+{
+ return fwnode_property_match_string_nocase(dev_fwnode(dev), propname,
+ string);
+}
+EXPORT_SYMBOL_GPL(device_property_match_string_nocase);
+
static int fwnode_property_read_int_array(const struct fwnode_handle *fwnode,
const char *propname,
unsigned int elem_size, void *val,
@@ -441,23 +465,9 @@ int fwnode_property_read_string(const struct fwnode_handle *fwnode,
}
EXPORT_SYMBOL_GPL(fwnode_property_read_string);
-/**
- * fwnode_property_match_string - find a string in an array and return index
- * @fwnode: Firmware node to get the property of
- * @propname: Name of the property holding the array
- * @string: String to look for
- *
- * Find a given string in a string array and if it is found return the
- * index back.
- *
- * Return: %0 if the property was found (success),
- * %-EINVAL if given arguments are not valid,
- * %-ENODATA if the property does not have a value,
- * %-EPROTO if the property is not an array of strings,
- * %-ENXIO if no suitable firmware interface is present.
- */
-int fwnode_property_match_string(const struct fwnode_handle *fwnode,
- const char *propname, const char *string)
+int fwnode_property_do_match_string(const struct fwnode_handle *fwnode,
+ const char *propname, const char *string,
+ int (*cmp)(const char *, const char *))
{
const char **values;
int nval, ret;
@@ -477,15 +487,61 @@ int fwnode_property_match_string(const struct fwnode_handle *fwnode,
if (ret < 0)
goto out;
- ret = match_string(values, nval, string);
+ ret = __match_string(values, nval, string, cmp);
if (ret < 0)
ret = -ENODATA;
out:
kfree(values);
return ret;
}
+
+/**
+ * fwnode_property_match_string - find a string in an array and return index
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property holding the array
+ * @string: String to look for
+ *
+ * Find a given string in a string array and if it is found return the
+ * index back.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not have a value,
+ * %-EPROTO if the property is not an array of strings,
+ * %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_match_string(const struct fwnode_handle *fwnode,
+ const char *propname, const char *string)
+{
+ return fwnode_property_do_match_string(fwnode, propname, string,
+ strcmp);
+}
EXPORT_SYMBOL_GPL(fwnode_property_match_string);
+/**
+ * fwnode_property_match_string_nocase - find a string in an array and return
+ * index with case-insensitive comparison
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property holding the array
+ * @string: String to look for
+ *
+ * Find a given string in a string array and if it is found return the
+ * index back.
+ *
+ * Return: %0 if the property was found (success),
+ * %-EINVAL if given arguments are not valid,
+ * %-ENODATA if the property does not have a value,
+ * %-EPROTO if the property is not an array of strings,
+ * %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_match_string_nocase(const struct fwnode_handle *fwnode,
+ const char *propname, const char *string)
+{
+ return fwnode_property_do_match_string(fwnode, propname, string,
+ strcasecmp);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_match_string_nocase);
+
/**
* fwnode_property_get_reference_args() - Find a reference with arguments
* @fwnode: Firmware node where to look for the reference
diff --git a/include/linux/property.h b/include/linux/property.h
index 117cc200c656..dbe747f3e3be 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -49,6 +49,9 @@ int device_property_read_string(struct device *dev, const char *propname,
const char **val);
int device_property_match_string(struct device *dev,
const char *propname, const char *string);
+int device_property_match_string_nocase(struct device *dev,
+ const char *propname,
+ const char *string);
bool fwnode_device_is_available(const struct fwnode_handle *fwnode);
bool fwnode_property_present(const struct fwnode_handle *fwnode,
@@ -72,6 +75,9 @@ int fwnode_property_read_string(const struct fwnode_handle *fwnode,
const char *propname, const char **val);
int fwnode_property_match_string(const struct fwnode_handle *fwnode,
const char *propname, const char *string);
+int fwnode_property_match_string_nocase(const struct fwnode_handle *fwnode,
+ const char *propname,
+ const char *string);
int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
const char *prop, const char *nargs_prop,
unsigned int nargs, unsigned int index,
--
2.30.2
On Mon, Oct 10, 2022 at 12:21:52AM +0800, Soha Jin wrote:
> I am introducing these patches for the patch for ethernet driver which I
> will send later.
We can't take functions that have no real users. Please send these
patches as part of your driver submission so we can see how they are
used and if they are even needed at all.
thanks,
greg k-h
Hi Soha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on sailus-media-tree/streams linus/master v6.0 next-20221007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Soha-Jin/Case-insensitive-match_string-and-fwnode_is_compatible/20221010-002529
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
config: um-i386_defconfig
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/436baf20c2b3f0a9f4eb7f4c11df3b2c4488d060
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Soha-Jin/Case-insensitive-match_string-and-fwnode_is_compatible/20221010-002529
git checkout 436baf20c2b3f0a9f4eb7f4c11df3b2c4488d060
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash drivers/base/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/base/property.c:468:5: warning: no previous prototype for 'fwnode_property_do_match_string' [-Wmissing-prototypes]
468 | int fwnode_property_do_match_string(const struct fwnode_handle *fwnode,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/fwnode_property_do_match_string +468 drivers/base/property.c
467
> 468 int fwnode_property_do_match_string(const struct fwnode_handle *fwnode,
469 const char *propname, const char *string,
470 int (*cmp)(const char *, const char *))
471 {
472 const char **values;
473 int nval, ret;
474
475 nval = fwnode_property_read_string_array(fwnode, propname, NULL, 0);
476 if (nval < 0)
477 return nval;
478
479 if (nval == 0)
480 return -ENODATA;
481
482 values = kcalloc(nval, sizeof(*values), GFP_KERNEL);
483 if (!values)
484 return -ENOMEM;
485
486 ret = fwnode_property_read_string_array(fwnode, propname, values, nval);
487 if (ret < 0)
488 goto out;
489
490 ret = __match_string(values, nval, string, cmp);
491 if (ret < 0)
492 ret = -ENODATA;
493 out:
494 kfree(values);
495 return ret;
496 }
497
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Soha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on linus/master v6.0 next-20221007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Soha-Jin/Case-insensitive-match_string-and-fwnode_is_compatible/20221010-002529
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
config: i386-randconfig-a001
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/436baf20c2b3f0a9f4eb7f4c11df3b2c4488d060
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Soha-Jin/Case-insensitive-match_string-and-fwnode_is_compatible/20221010-002529
git checkout 436baf20c2b3f0a9f4eb7f4c11df3b2c4488d060
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/base/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/base/property.c:468:5: warning: no previous prototype for 'fwnode_property_do_match_string' [-Wmissing-prototypes]
468 | int fwnode_property_do_match_string(const struct fwnode_handle *fwnode,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/fwnode_property_do_match_string +468 drivers/base/property.c
467
> 468 int fwnode_property_do_match_string(const struct fwnode_handle *fwnode,
469 const char *propname, const char *string,
470 int (*cmp)(const char *, const char *))
471 {
472 const char **values;
473 int nval, ret;
474
475 nval = fwnode_property_read_string_array(fwnode, propname, NULL, 0);
476 if (nval < 0)
477 return nval;
478
479 if (nval == 0)
480 return -ENODATA;
481
482 values = kcalloc(nval, sizeof(*values), GFP_KERNEL);
483 if (!values)
484 return -ENOMEM;
485
486 ret = fwnode_property_read_string_array(fwnode, propname, values, nval);
487 if (ret < 0)
488 goto out;
489
490 ret = __match_string(values, nval, string, cmp);
491 if (ret < 0)
492 ret = -ENODATA;
493 out:
494 kfree(values);
495 return ret;
496 }
497
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Mon, Oct 10, 2022 at 12:21:54AM +0800, Soha Jin wrote:
> device_property_match_string_nocase and fwnode_property_match_string_nocase
> are used to do case-insensitive match with match_string_nocase.
Again, why? What platform is broken that needs this?
thanks,
greg k-h
On Mon, Oct 10, 2022 at 12:21:55AM +0800, Soha Jin wrote:
> fwnode_is_compatible is a shortcut to check if a device is compatible with
> a compat string in fwnode property "compatible". This function is similar
> to of_device_is_compatible.
>
> Signed-off-by: Soha Jin <[email protected]>
> ---
> include/linux/property.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index dbe747f3e3be..776e4a8bc379 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -252,6 +252,13 @@ fwnode_property_string_array_count(const struct fwnode_handle *fwnode,
> return fwnode_property_read_string_array(fwnode, propname, NULL, 0);
> }
>
> +static inline bool fwnode_is_compatible(const struct fwnode_handle *fwnode,
> + const char *compat)
> +{
> + return fwnode_property_match_string_nocase(fwnode, "compatible",
> + compat) >= 0;
Who would mistype "compatible" in a case insensitive way?
Why can't you fix the firmware to be correct instead of forcing the
operating system to fix it for them?
thanks,
greg k-h
Hi Greg,
>
> On Mon, Oct 10, 2022 at 12:21:52AM +0800, Soha Jin wrote:
> > I am introducing these patches for the patch for ethernet driver which
> > I will send later.
>
> We can't take functions that have no real users. Please send these patches
> as part of your driver submission so we can see how they are used and if they
> are even needed at all.
>
This is the first time I submit patches to kernel, I am sorry for anything
wrong I did.
I am just thinking these patches and the driver patches are in different
trees, so I split the patches into different parts. The driver patch is at
https://lore.kernel.org/all/[email protected]/.
Regards,
Soha
On Mon, Oct 10, 2022 at 11:07:13AM +0800, Soha Jin wrote:
> Hi Greg,
>
> >
> > On Mon, Oct 10, 2022 at 12:21:52AM +0800, Soha Jin wrote:
> > > I am introducing these patches for the patch for ethernet driver which
> > > I will send later.
> >
> > We can't take functions that have no real users. Please send these patches
> > as part of your driver submission so we can see how they are used and if they
> > are even needed at all.
> >
>
> This is the first time I submit patches to kernel, I am sorry for anything
> wrong I did.
>
> I am just thinking these patches and the driver patches are in different
> trees, so I split the patches into different parts. The driver patch is at
> https://lore.kernel.org/all/[email protected]/.
As you can see, the kernel test robot reported that these changes are
broken as it does not know about this separate series you submitted.
Please make them all one series so we can properly review them together
and the testing infrastructure can correctly run.
thanks,
greg k-h
On Mon, Oct 10, 2022 at 12:21:52AM +0800, Soha Jin wrote:
> I am introducing these patches for the patch for ethernet driver which I
> will send later.
>
> In Patch 1, I abstract `match_string` to `__match_string` with a comparison
> function, make the original name calling it with `strcmp` and add
> `match_string_nocase` calling it with `strcasecmp`.
>
> In Patch 2 & 3, I implement `{device,fwnode}_property_match_string_nocase`
> and `fwnode_is_compatible` for compatible property matching.
Let's ask Rob about usage of case-insensitive comparator for compatible
strings.
So, why do we have such in the OF code and do we really need it in the modern
world?
--
With Best Regards,
Andy Shevchenko
Hi Greg,
> From: 'Greg Kroah-Hartman' <[email protected]>
> Sent: Monday, October 10, 2022 2:26 PM
>
> On Mon, Oct 10, 2022 at 11:07:13AM +0800, Soha Jin wrote:
> > Hi Greg,
> >
> > This is the first time I submit patches to kernel, I am sorry for
> > anything wrong I did.
> >
> > I am just thinking these patches and the driver patches are in
> > different trees, so I split the patches into different parts. The
> > driver patch is at
> https://lore.kernel.org/all/[email protected]/.
>
> As you can see, the kernel test robot reported that these changes are broken
> as it does not know about this separate series you submitted.
>
> Please make them all one series so we can properly review them together
> and the testing infrastructure can correctly run.
I see. I will put this patch series into that thread as patch v2 after the
letter case problem is settled.
Regards,
Soha