[PATCH v2 00/21] use match_string() helper

[PATCH v2 00/21] use match_string() helper

[Yisheng Xie]

Andy introduce helper function match_string() which can be used to return
the index of array for a matching string. so we can use it in many places
instead of open coded variant.

I just try to make this API be used more commonly, sorry if this makes
too much big patchset.

v2:
 - Remove the Patches which Andy already sent out, or maintainer already
   picked up.
 - Add Reviewed-by or Acked-by tags for some patchs.
 - Add some new patches[19-21].

Yisheng Xie (21):
  usb: phy: use match_string() helper
  mfd: omap-usb-host: use match_string() helper
  Staging: gdm724x: use match_string() helper
  cxgb4: use match_string() helper
  hp100: use match_string() helper
  iwlwifi: mvm: use match_string() helper
  bus: fsl-mc: use match_string() helper
  clk: bcm2835: use match_string() helper
  clk: use match_string() helper
  cpufreq: intel_pstate: use match_string() helper
  drm/nouveau: use match_string() helper
  drm: i2c: ch7006: use match_string() helper
  ima: use match_string() helper
  sched/debug: use match_string() helper
  ALSA: oxygen: use match_string() helper
  ASoC: max98088: use match_string() helper
  ASoC: max98095: use match_string() helper
  ASoC: dapm: use match_string() helper
  bcache: use match_string() helper
  powerpc/xmon: use match_string() helper
  sparc64: use match_string() helper

 arch/powerpc/xmon/xmon.c                         | 23 +++++++++---------
 arch/sparc/kernel/setup_64.c                     |  7 +++---
 drivers/bus/fsl-mc/fsl-mc-allocator.c            | 24 ++++--------------
 drivers/clk/bcm/clk-bcm2835.c                    | 13 +++++-----
 drivers/clk/clk.c                                |  8 ++----
 drivers/cpufreq/intel_pstate.c                   | 15 +++++-------
 drivers/gpu/drm/i2c/ch7006_drv.c                 | 13 ++++------
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c        | 13 ++++------
 drivers/md/bcache/util.c                         |  9 ++-----
 drivers/mfd/omap-usb-host.c                      | 24 ++----------------
 drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c   | 14 +++--------
 drivers/net/ethernet/hp/hp100.c                  |  9 +------
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 13 +++-------
 drivers/staging/gdm724x/gdm_tty.c                | 18 ++++----------
 drivers/usb/phy/of.c                             | 10 ++++----
 kernel/sched/debug.c                             | 31 ++++++++++++------------
 security/integrity/ima/ima_main.c                | 11 +++------
 sound/pci/oxygen/oxygen_mixer.c                  | 14 +++++------
 sound/soc/codecs/max98088.c                      | 13 ++++------
 sound/soc/codecs/max98095.c                      | 13 ++++------
 sound/soc/soc-dapm.c                             | 18 ++++++--------
 21 files changed, 109 insertions(+), 204 deletions(-)

-- 
1.7.12.4

[PATCH v2 01/21] usb: phy: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: linux-usb@vger.kernel.org
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - donot rename err to ret  - per Andy

 drivers/usb/phy/of.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
index 1ab134f..9d74081 100644
--- a/drivers/usb/phy/of.c
+++ b/drivers/usb/phy/of.c
@@ -28,16 +28,16 @@
 enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
 {
 	const char *phy_type;
-	int err, i;
+	int err;
 
 	err = of_property_read_string(np, "phy_type", &phy_type);
 	if (err < 0)
 		return USBPHY_INTERFACE_MODE_UNKNOWN;
 
-	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
-		if (!strcmp(phy_type, usbphy_modes[i]))
-			return i;
+	err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
+	if (err < 0)
+		return USBPHY_INTERFACE_MODE_UNKNOWN;
 
-	return USBPHY_INTERFACE_MODE_UNKNOWN;
+	return err;
 }
 EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
-- 
1.7.12.4

Re: [PATCH v2 01/21] usb: phy: use match_string() helper

[Sergei Shtylyov]

Hello!

On 05/31/2018 02:11 PM, Yisheng Xie wrote:

> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: linux-usb@vger.kernel.org
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - donot rename err to ret  - per Andy

   Hm...

> 
>  drivers/usb/phy/of.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
> index 1ab134f..9d74081 100644
> --- a/drivers/usb/phy/of.c
> +++ b/drivers/usb/phy/of.c
> @@ -28,16 +28,16 @@
>  enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
>  {
>  	const char *phy_type;
> -	int err, i;
> +	int err;
>  
>  	err = of_property_read_string(np, "phy_type", &phy_type);
>  	if (err < 0)
>  		return USBPHY_INTERFACE_MODE_UNKNOWN;
>  
> -	for (i = 0; i < ARRAY_SIZE(usbphy_modes); i++)
> -		if (!strcmp(phy_type, usbphy_modes[i]))
> -			return i;
> +	err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
> +	if (err < 0)

   This is one of the few cases when 'err' is not the best name for such a
variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-)

> +		return USBPHY_INTERFACE_MODE_UNKNOWN;
>  
> -	return USBPHY_INTERFACE_MODE_UNKNOWN;
> +	return err;
>  }
>  EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
> 

MBR, Sergei

Re: [PATCH v2 01/21] usb: phy: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 7:55 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:

>>  - donot rename err to ret  - per Andy
>
>    Hm...

>> -     int err, i;

>> +     err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
>> +     if (err < 0)
>
>    This is one of the few cases when 'err' is not the best name for such a
> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-)

Then leaving i would make it?
I'm okay with either which just not renames err, b/c it's used with
something else in this function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 01/21] usb: phy: use match_string() helper

[Sergei Shtylyov]

On 05/31/2018 09:47 PM, Andy Shevchenko wrote:

>>>  - donot rename err to ret  - per Andy
>>
>>    Hm...
> 
>>> -     int err, i;
> 
>>> +     err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
>>> +     if (err < 0)
>>
>>    This is one of the few cases when 'err' is not the best name for such a
>> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-)
> 
> Then leaving i would make it?

   Yes. :-)

> I'm okay with either which just not renames err, b/c it's used with
> something else in this function.

   Looking at it again, 'err' seems equally bad for the result of 
of_property_read_string()... unless the check there is changed to just *if* (err) --
this function never returns positive values, 0 means success, others mean error.

MBR, Sergei 

Re: [PATCH v2 01/21] usb: phy: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 9:56 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 05/31/2018 09:47 PM, Andy Shevchenko wrote:

>>>> -     int err, i;

>>>> +     err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
>>>> +     if (err < 0)
>>>
>>>    This is one of the few cases when 'err' is not the best name for such a
>>> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-)
>>
>> Then leaving i would make it?
>    Yes. :-)

So, I leave it to Greg to decide either it's okay in this version, or
needs update with i left untouched.

>> I'm okay with either which just not renames err, b/c it's used with
>> something else in this function.
>
>    Looking at it again, 'err' seems equally bad for the result of
> of_property_read_string()... unless the check there is changed to just *if* (err) --
> this function never returns positive values, 0 means success, others mean error.

While you seems right, this is matter of another change which you are
welcome to propose.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 01/21] usb: phy: use match_string() helper

[Yisheng Xie]

On 2018/6/1 2:59, Andy Shevchenko wrote:
> On Thu, May 31, 2018 at 9:56 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> On 05/31/2018 09:47 PM, Andy Shevchenko wrote:
> 
>>>>> -     int err, i;
> 
>>>>> +     err = match_string(usbphy_modes, ARRAY_SIZE(usbphy_modes), phy_type);
>>>>> +     if (err < 0)
>>>>
>>>>    This is one of the few cases when 'err' is not the best name for such a
>>>> variable. I'd prefer to see something like 'match' or even 'rc' or 'ret'... :-)
>>>
>>> Then leaving i would make it?
>>    Yes. :-)
> 
> So, I leave it to Greg to decide either it's okay in this version, or
> needs update with i left untouched.
Hi Greg,

IIRC, you seems want to keep the err unchanged, right?

Please let me know if another version is need.

Thanks
Yisheng

> 
>>> I'm okay with either which just not renames err, b/c it's used with
>>> something else in this function.
>>
>>    Looking at it again, 'err' seems equally bad for the result of
>> of_property_read_string()... unless the check there is changed to just *if* (err) --
>> this function never returns positive values, 0 means success, others mean error.
> 
> While you seems right, this is matter of another change which you are
> welcome to propose.
> 

[PATCH v2 02/21] mfd: omap-usb-host: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Acked-by: Tony Lindgren <tony@atomide.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: linux-omap@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Acked-by and Reviewed-by tag.

 drivers/mfd/omap-usb-host.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7aab376..e11ab12 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -153,27 +153,6 @@ static inline u32 usbhs_read(void __iomem *base, u32 reg)
 	[OMAP_OHCI_PORT_MODE_TLL_2PIN_DPDM]	= "ohci-tll-2pin-dpdm",
 };
 
-/**
- * omap_usbhs_get_dt_port_mode - Get the 'enum usbhs_omap_port_mode'
- * from the port mode string.
- * @mode: The port mode string, usually obtained from device tree.
- *
- * The function returns the 'enum usbhs_omap_port_mode' that matches the
- * provided port mode string as per the port_modes table.
- * If no match is found it returns -ENODEV
- */
-static int omap_usbhs_get_dt_port_mode(const char *mode)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(port_modes); i++) {
-		if (!strcmp(mode, port_modes[i]))
-			return i;
-	}
-
-	return -ENODEV;
-}
-
 static struct platform_device *omap_usbhs_alloc_child(const char *name,
 			struct resource	*res, int num_resources, void *pdata,
 			size_t pdata_size, struct device *dev)
@@ -529,7 +508,8 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
 		if (ret < 0)
 			continue;
 
-		ret = omap_usbhs_get_dt_port_mode(mode);
+		/* get 'enum usbhs_omap_port_mode' from port mode string */
+		ret = match_string(port_modes, ARRAY_SIZE(port_modes), mode);
 		if (ret < 0) {
 			dev_warn(dev, "Invalid port%d-mode \"%s\" in device tree\n",
 					i, mode);
-- 
1.7.12.4

Re: [PATCH v2 02/21] mfd: omap-usb-host: use match_string() helper

[Lee Jones]

On Thu, 31 May 2018, Yisheng Xie wrote:

> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - add Acked-by and Reviewed-by tag.
> 
>  drivers/mfd/omap-usb-host.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)

I already applied this with the tags

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 02/21] mfd: omap-usb-host: use match_string() helper

[Yisheng Xie]

Hi Lee,

On 2018/6/4 15:44, Lee Jones wrote:
> On Thu, 31 May 2018, Yisheng Xie wrote:
> 
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
>> Acked-by: Tony Lindgren <tony@atomide.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: linux-omap@vger.kernel.org
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>> v2:
>>  - add Acked-by and Reviewed-by tag.
>>
>>  drivers/mfd/omap-usb-host.c | 24 ++----------------------
>>  1 file changed, 2 insertions(+), 22 deletions(-)
> 
> I already applied this with the tags

Thanks
Yisheng
> 

[PATCH v2 03/21] Staging: gdm724x: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Quytelda Kahja <quytelda@tamalin.org>
Cc: devel@driverdev.osuosl.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - const DRIVER_STRING instead  - per Andy

 drivers/staging/gdm724x/gdm_tty.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 3cdebb8..397ecaa 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -43,7 +43,7 @@
 static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
 static DEFINE_MUTEX(gdm_table_lock);
 
-static char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
+static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
 static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};
 
 static void gdm_port_destruct(struct tty_port *port)
@@ -65,22 +65,14 @@ static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	struct gdm *gdm = NULL;
 	int ret;
-	int i;
-	int j;
-
-	j = GDM_TTY_MINOR;
-	for (i = 0; i < TTY_MAX_COUNT; i++) {
-		if (!strcmp(tty->driver->driver_name, DRIVER_STRING[i])) {
-			j = tty->index;
-			break;
-		}
-	}
 
-	if (j == GDM_TTY_MINOR)
+	ret = match_string(DRIVER_STRING, TTY_MAX_COUNT,
+			   tty->driver->driver_name);
+	if (ret < 0 || tty->index == GDM_TTY_MINOR)
 		return -ENODEV;
 
 	mutex_lock(&gdm_table_lock);
-	gdm = gdm_table[i][j];
+	gdm = gdm_table[ret][tty->index];
 	if (!gdm) {
 		mutex_unlock(&gdm_table_lock);
 		return -ENODEV;
-- 
1.7.12.4

Re: [PATCH v2 03/21] Staging: gdm724x: use match_string() helper

[Greg Kroah-Hartman]

On Thu, May 31, 2018 at 07:11:08PM +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Quytelda Kahja <quytelda@tamalin.org>
> Cc: devel@driverdev.osuosl.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - const DRIVER_STRING instead  - per Andy
> 
>  drivers/staging/gdm724x/gdm_tty.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> index 3cdebb8..397ecaa 100644
> --- a/drivers/staging/gdm724x/gdm_tty.c
> +++ b/drivers/staging/gdm724x/gdm_tty.c
> @@ -43,7 +43,7 @@
>  static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
>  static DEFINE_MUTEX(gdm_table_lock);
>  
> -static char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
> +static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
>  static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};
>  
>  static void gdm_port_destruct(struct tty_port *port)
> @@ -65,22 +65,14 @@ static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>  {
>  	struct gdm *gdm = NULL;
>  	int ret;
> -	int i;
> -	int j;
> -
> -	j = GDM_TTY_MINOR;
> -	for (i = 0; i < TTY_MAX_COUNT; i++) {
> -		if (!strcmp(tty->driver->driver_name, DRIVER_STRING[i])) {
> -			j = tty->index;
> -			break;
> -		}
> -	}
>  
> -	if (j == GDM_TTY_MINOR)
> +	ret = match_string(DRIVER_STRING, TTY_MAX_COUNT,
> +			   tty->driver->driver_name);
> +	if (ret < 0 || tty->index == GDM_TTY_MINOR)
>  		return -ENODEV;

Very odd rewrite here.  Why call this function if you think the initial
parameters are not correct?  Are you sure about your test for
tty->index?

This should be cleaned up more please.

thanks,

greg k-h

Re: [PATCH v2 03/21] Staging: gdm724x: use match_string() helper

[Yisheng Xie]

Hi Greg,

On 2018/5/31 19:44, Greg Kroah-Hartman wrote:
> On Thu, May 31, 2018 at 07:11:08PM +0800, Yisheng Xie wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Quytelda Kahja <quytelda@tamalin.org>
>> Cc: devel@driverdev.osuosl.org
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>> v2:
>>  - const DRIVER_STRING instead  - per Andy
>>
>>  drivers/staging/gdm724x/gdm_tty.c | 18 +++++-------------
>>  1 file changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
>> index 3cdebb8..397ecaa 100644
>> --- a/drivers/staging/gdm724x/gdm_tty.c
>> +++ b/drivers/staging/gdm724x/gdm_tty.c
>> @@ -43,7 +43,7 @@
>>  static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
>>  static DEFINE_MUTEX(gdm_table_lock);
>>  
>> -static char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
>> +static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
>>  static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};
>>  
>>  static void gdm_port_destruct(struct tty_port *port)
>> @@ -65,22 +65,14 @@ static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>>  {
>>  	struct gdm *gdm = NULL;
>>  	int ret;
>> -	int i;
>> -	int j;
>> -
>> -	j = GDM_TTY_MINOR;
>> -	for (i = 0; i < TTY_MAX_COUNT; i++) {
>> -		if (!strcmp(tty->driver->driver_name, DRIVER_STRING[i])) {
>> -			j = tty->index;
>> -			break;
>> -		}
>> -	}
>>  
>> -	if (j == GDM_TTY_MINOR)
>> +	ret = match_string(DRIVER_STRING, TTY_MAX_COUNT,
>> +			   tty->driver->driver_name);
>> +	if (ret < 0 || tty->index == GDM_TTY_MINOR)
>>  		return -ENODEV;
> 
> Very odd rewrite here.  Why call this function if you think the initial
> parameters are not correct?  Are you sure about your test for
> tty->index?

Hmm, actually, I thought it no need to test tty->index here, but I not so sure
about that so I kept it, I will remove this check next version.

> 
> This should be cleaned up more please.
Sure!


Thanks
Yisheng
> 
> thanks,
> 
> greg k-h
> 
> .
> 

[PATCH v3 03/21] Staging: gdm724x: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Quytelda Kahja <quytelda@tamalin.org>
Cc: devel@driverdev.osuosl.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v3:
 - no need to check input tty's index - per Greg
v2:
 - const DRIVER_STRING instead  - per Andy

 drivers/staging/gdm724x/gdm_tty.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 3cdebb8..29ac6b5 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -43,7 +43,7 @@
 static struct gdm *gdm_table[TTY_MAX_COUNT][GDM_TTY_MINOR];
 static DEFINE_MUTEX(gdm_table_lock);

-static char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
+static const char *DRIVER_STRING[TTY_MAX_COUNT] = {"GCTATC", "GCTDM"};
 static char *DEVICE_STRING[TTY_MAX_COUNT] = {"GCT-ATC", "GCT-DM"};

 static void gdm_port_destruct(struct tty_port *port)
@@ -65,22 +65,14 @@ static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	struct gdm *gdm = NULL;
 	int ret;
-	int i;
-	int j;
-
-	j = GDM_TTY_MINOR;
-	for (i = 0; i < TTY_MAX_COUNT; i++) {
-		if (!strcmp(tty->driver->driver_name, DRIVER_STRING[i])) {
-			j = tty->index;
-			break;
-		}
-	}

-	if (j == GDM_TTY_MINOR)
+	ret = match_string(DRIVER_STRING, TTY_MAX_COUNT,
+			   tty->driver->driver_name);
+	if (ret < 0)
 		return -ENODEV;

 	mutex_lock(&gdm_table_lock);
-	gdm = gdm_table[i][j];
+	gdm = gdm_table[ret][tty->index];
 	if (!gdm) {
 		mutex_unlock(&gdm_table_lock);
 		return -ENODEV;
-- 
1.7.12.4

[PATCH v2 04/21] cxgb4: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Ganesh Goudar <ganeshgr@chelsio.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - no change from v1.

 drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
index 9da6f57..bd61610 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
@@ -782,17 +782,11 @@ static int cudbg_get_mem_region(struct adapter *padap,
 	if (rc)
 		return rc;
 
-	for (i = 0; i < ARRAY_SIZE(cudbg_region); i++) {
-		if (!strcmp(cudbg_region[i], region_name)) {
-			found = 1;
-			idx = i;
-			break;
-		}
-	}
-	if (!found)
-		return -EINVAL;
+	rc = match_string(cudbg_region, ARRAY_SIZE(cudbg_region), region_name);
+	if (rc < 0)
+		return rc;
 
-	found = 0;
+	idx = rc;
 	for (i = 0; i < meminfo->mem_c; i++) {
 		if (meminfo->mem[i].idx >= ARRAY_SIZE(cudbg_region))
 			continue; /* Skip holes */
-- 
1.7.12.4

Re: [PATCH v2 04/21] cxgb4: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: Ganesh Goudar <ganeshgr@chelsio.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - no change from v1.
>
>  drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> index 9da6f57..bd61610 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c
> @@ -782,17 +782,11 @@ static int cudbg_get_mem_region(struct adapter *padap,
>         if (rc)
>                 return rc;
>
> -       for (i = 0; i < ARRAY_SIZE(cudbg_region); i++) {
> -               if (!strcmp(cudbg_region[i], region_name)) {
> -                       found = 1;
> -                       idx = i;
> -                       break;
> -               }
> -       }
> -       if (!found)
> -               return -EINVAL;
> +       rc = match_string(cudbg_region, ARRAY_SIZE(cudbg_region), region_name);
> +       if (rc < 0)
> +               return rc;
>
> -       found = 0;
> +       idx = rc;
>         for (i = 0; i < meminfo->mem_c; i++) {
>                 if (meminfo->mem[i].idx >= ARRAY_SIZE(cudbg_region))
>                         continue; /* Skip holes */
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 05/21] hp100: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: netdev@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Reviewed-by tag.

 drivers/net/ethernet/hp/hp100.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c
index c8c7ad2..84501b3 100644
--- a/drivers/net/ethernet/hp/hp100.c
+++ b/drivers/net/ethernet/hp/hp100.c
@@ -335,7 +335,6 @@ static const char *hp100_read_id(int ioaddr)
 static __init int hp100_isa_probe1(struct net_device *dev, int ioaddr)
 {
 	const char *sig;
-	int i;
 
 	if (!request_region(ioaddr, HP100_REGION_SIZE, "hp100"))
 		goto err;
@@ -351,13 +350,7 @@ static __init int hp100_isa_probe1(struct net_device *dev, int ioaddr)
 	if (sig == NULL)
 		goto err;
 
-	for (i = 0; i < ARRAY_SIZE(hp100_isa_tbl); i++) {
-		if (!strcmp(hp100_isa_tbl[i], sig))
-			break;
-
-	}
-
-	if (i < ARRAY_SIZE(hp100_isa_tbl))
+	if (match_string(hp100_isa_tbl, ARRAY_SIZE(hp100_isa_tbl), sig) >= 0)
 		return hp100_probe1(dev, ioaddr, HP100_BUS_ISA, NULL);
  err:
 	return -ENODEV;
-- 
1.7.12.4

[PATCH v2 06/21] iwlwifi: mvm: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Intel Linux Wireless <linuxwifi@intel.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - let ret get return value of match_string  - per Andy

 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 0e6401c..d7ac511 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -671,16 +671,11 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file *file, char __user *user_buf,
 	};
 	int ret, bt_force_ant_mode;
 
-	for (bt_force_ant_mode = 0;
-	     bt_force_ant_mode < ARRAY_SIZE(modes_str);
-	     bt_force_ant_mode++) {
-		if (!strcmp(buf, modes_str[bt_force_ant_mode]))
-			break;
-	}
-
-	if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
-		return -EINVAL;
+	ret = match_string(modes_str, ARRAY_SIZE(modes_str), buf);
+	if (ret < 0)
+		return ret;
 
+	bt_force_ant_mode = ret;
 	ret = 0;
 	mutex_lock(&mvm->mutex);
 	if (mvm->bt_force_ant_mode == bt_force_ant_mode)
-- 
1.7.12.4

Re: [PATCH v2 06/21] iwlwifi: mvm: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Intel Linux Wireless <linuxwifi@intel.com>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - let ret get return value of match_string  - per Andy
>
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 0e6401c..d7ac511 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -671,16 +671,11 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file *file, char __user *user_buf,
>         };
>         int ret, bt_force_ant_mode;
>
> -       for (bt_force_ant_mode = 0;
> -            bt_force_ant_mode < ARRAY_SIZE(modes_str);
> -            bt_force_ant_mode++) {
> -               if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> -                       break;
> -       }
> -
> -       if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> -               return -EINVAL;
> +       ret = match_string(modes_str, ARRAY_SIZE(modes_str), buf);
> +       if (ret < 0)
> +               return ret;
>
> +       bt_force_ant_mode = ret;
>         ret = 0;
>         mutex_lock(&mvm->mutex);
>         if (mvm->bt_force_ant_mode == bt_force_ant_mode)
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 07/21] bus: fsl-mc: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Stuart Yoder <stuyoder@gmail.com>
Cc: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Reviewed-by tag.

 drivers/bus/fsl-mc/fsl-mc-allocator.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-allocator.c b/drivers/bus/fsl-mc/fsl-mc-allocator.c
index fb1442b..63c4735 100644
--- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
+++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
@@ -156,22 +156,6 @@ static int __must_check fsl_mc_resource_pool_remove_device(struct fsl_mc_device
 	[FSL_MC_POOL_IRQ] = "irq",
 };
 
-static int __must_check object_type_to_pool_type(const char *object_type,
-						 enum fsl_mc_pool_type
-								*pool_type)
-{
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(fsl_mc_pool_type_strings); i++) {
-		if (strcmp(object_type, fsl_mc_pool_type_strings[i]) == 0) {
-			*pool_type = i;
-			return 0;
-		}
-	}
-
-	return -EINVAL;
-}
-
 int __must_check fsl_mc_resource_allocate(struct fsl_mc_bus *mc_bus,
 					  enum fsl_mc_pool_type pool_type,
 					  struct fsl_mc_resource **new_resource)
@@ -581,9 +565,11 @@ static int fsl_mc_allocator_probe(struct fsl_mc_device *mc_dev)
 		return -EINVAL;
 
 	mc_bus = to_fsl_mc_bus(mc_bus_dev);
-	error = object_type_to_pool_type(mc_dev->obj_desc.type, &pool_type);
-	if (error < 0)
-		return error;
+	pool_type = match_string(fsl_mc_pool_type_strings,
+				 ARRAY_SIZE(fsl_mc_pool_type_strings),
+				 mc_dev->obj_desc.type);
+	if (pool_type < 0)
+		return pool_type;
 
 	error = fsl_mc_resource_pool_add_device(mc_bus, pool_type, mc_dev);
 	if (error < 0)
-- 
1.7.12.4

[PATCH v2 08/21] clk: bcm2835: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Eric Anholt <eric@anholt.net>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: linux-clk@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - donot change the type of i  - per Andy

 drivers/clk/bcm/clk-bcm2835.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index fa0d5c8..5e18433 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1395,7 +1395,7 @@ static struct clk_hw *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 	struct bcm2835_clock *clock;
 	struct clk_init_data init;
 	const char *parents[1 << CM_SRC_BITS];
-	size_t i, j;
+	size_t i;
 	int ret;
 
 	/*
@@ -1405,12 +1405,11 @@ static struct clk_hw *bcm2835_register_clock(struct bcm2835_cprman *cprman,
 	for (i = 0; i < data->num_mux_parents; i++) {
 		parents[i] = data->parents[i];
 
-		for (j = 0; j < ARRAY_SIZE(cprman_parent_names); j++) {
-			if (strcmp(parents[i], cprman_parent_names[j]) == 0) {
-				parents[i] = cprman->real_parent_names[j];
-				break;
-			}
-		}
+		ret = match_string(cprman_parent_names,
+				   ARRAY_SIZE(cprman_parent_names),
+				   parents[i]);
+		if (ret >= 0)
+			parents[i] = cprman->real_parent_names[ret];
 	}
 
 	memset(&init, 0, sizeof(init));
-- 
1.7.12.4

Re: [PATCH v2 08/21] clk: bcm2835: use match_string() helper

[Stephen Boyd]

Quoting Yisheng Xie (2018-05-31 04:11:13)
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: linux-clk@vger.kernel.org
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---

Applied to clk-next

[PATCH v2 09/21] clk: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - leave second parameter on the first line  - per Andy

 drivers/clk/clk.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7af555f..d01bdda 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2171,7 +2171,6 @@ void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent)
 bool clk_has_parent(struct clk *clk, struct clk *parent)
 {
 	struct clk_core *core, *parent_core;
-	unsigned int i;
 
 	/* NULL clocks should be nops, so return success if either is NULL. */
 	if (!clk || !parent)
@@ -2184,11 +2183,8 @@ bool clk_has_parent(struct clk *clk, struct clk *parent)
 	if (core->parent == parent_core)
 		return true;
 
-	for (i = 0; i < core->num_parents; i++)
-		if (strcmp(core->parent_names[i], parent_core->name) == 0)
-			return true;
-
-	return false;
+	return match_string(core->parent_names, core->num_parents,
+			    parent_core->name) >= 0;
 }
 EXPORT_SYMBOL_GPL(clk_has_parent);
 
-- 
1.7.12.4

Re: [PATCH v2 09/21] clk: use match_string() helper

[Stephen Boyd]

Quoting Yisheng Xie (2018-05-31 04:11:14)
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---

Applied to clk-next

[PATCH v2 10/21] cpufreq: intel_pstate: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Reviewed-by tag.

 drivers/cpufreq/intel_pstate.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 17e566af..d701e26 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -645,21 +645,18 @@ static ssize_t store_energy_performance_preference(
 {
 	struct cpudata *cpu_data = all_cpu_data[policy->cpu];
 	char str_preference[21];
-	int ret, i = 0;
+	int ret;
 
 	ret = sscanf(buf, "%20s", str_preference);
 	if (ret != 1)
 		return -EINVAL;
 
-	while (energy_perf_strings[i] != NULL) {
-		if (!strcmp(str_preference, energy_perf_strings[i])) {
-			intel_pstate_set_energy_pref_index(cpu_data, i);
-			return count;
-		}
-		++i;
-	}
+	ret = match_string(energy_perf_strings, -1, str_preference);
+	if (ret < 0)
+		return ret;
 
-	return -EINVAL;
+	intel_pstate_set_energy_pref_index(cpu_data, ret);
+	return count;
 }
 
 static ssize_t show_energy_performance_preference(
-- 
1.7.12.4

Re: [PATCH v2 10/21] cpufreq: intel_pstate: use match_string() helper

[Rafael J. Wysocki]

On Thursday, May 31, 2018 1:11:15 PM CEST Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - add Reviewed-by tag.
> 
>  drivers/cpufreq/intel_pstate.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 17e566af..d701e26 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -645,21 +645,18 @@ static ssize_t store_energy_performance_preference(
>  {
>  	struct cpudata *cpu_data = all_cpu_data[policy->cpu];
>  	char str_preference[21];
> -	int ret, i = 0;
> +	int ret;
>  
>  	ret = sscanf(buf, "%20s", str_preference);
>  	if (ret != 1)
>  		return -EINVAL;
>  
> -	while (energy_perf_strings[i] != NULL) {
> -		if (!strcmp(str_preference, energy_perf_strings[i])) {
> -			intel_pstate_set_energy_pref_index(cpu_data, i);
> -			return count;
> -		}
> -		++i;
> -	}
> +	ret = match_string(energy_perf_strings, -1, str_preference);
> +	if (ret < 0)
> +		return ret;
>  
> -	return -EINVAL;
> +	intel_pstate_set_energy_pref_index(cpu_data, ret);
> +	return count;
>  }
>  
>  static ssize_t show_energy_performance_preference(
> 

Srinivas, any concerns?

Re: [PATCH v2 10/21] cpufreq: intel_pstate: use match_string() helper

[Srinivas Pandruvada]

On Tue, 2018-06-26 at 17:23 +0200, Rafael J. Wysocki wrote:
> On Thursday, May 31, 2018 1:11:15 PM CEST Yisheng Xie wrote:
> > match_string() returns the index of an array for a matching string,
> > which can be used instead of open coded variant.
> > 
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > Cc: linux-pm@vger.kernel.org
> > Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> > ---
> > v2:
> >  - add Reviewed-by tag.
> > 
> >  drivers/cpufreq/intel_pstate.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > performance
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index 17e566af..d701e26 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -645,21 +645,18 @@ static ssize_t
> > store_energy_performance_preference(
> >  {
> >  	struct cpudata *cpu_data = all_cpu_data[policy->cpu];
> >  	char str_preference[21];
> > -	int ret, i = 0;
> > +	int ret;
> >  
> >  	ret = sscanf(buf, "%20s", str_preference);
> >  	if (ret != 1)
> >  		return -EINVAL;
> >  
> > -	while (energy_perf_strings[i] != NULL) {
> > -		if (!strcmp(str_preference,
> > energy_perf_strings[i])) {
> > -			intel_pstate_set_energy_pref_index(cpu_dat
> > a, i);
> > -			return count;
> > -		}
> > -		++i;
> > -	}
> > +	ret = match_string(energy_perf_strings, -1,
> > str_preference);
> > +	if (ret < 0)
> > +		return ret;
> >  
> > -	return -EINVAL;
> > +	intel_pstate_set_energy_pref_index(cpu_data, ret);
> > +	return count;
> >  }
> >  
> >  static ssize_t show_energy_performance_preference(
> > 
> 
> Srinivas, any concerns?
> 

Re: [PATCH v2 10/21] cpufreq: intel_pstate: use match_string() helper

[Rafael J. Wysocki]

On Wednesday, June 27, 2018 1:56:30 AM CEST Srinivas Pandruvada wrote:
> On Tue, 2018-06-26 at 17:23 +0200, Rafael J. Wysocki wrote:
> > On Thursday, May 31, 2018 1:11:15 PM CEST Yisheng Xie wrote:
> > > match_string() returns the index of an array for a matching string,
> > > which can be used instead of open coded variant.
> > > 
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Patch applied, thanks!

[PATCH v2 11/21] drm/nouveau: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - handle err case before normal case - per Andy

 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
index 6d99f11..67ba2ac 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
@@ -644,16 +644,13 @@ static int nv17_tv_create_resources(struct drm_encoder *encoder,
 	int i;
 
 	if (nouveau_tv_norm) {
-		for (i = 0; i < num_tv_norms; i++) {
-			if (!strcmp(nv17_tv_norm_names[i], nouveau_tv_norm)) {
-				tv_enc->tv_norm = i;
-				break;
-			}
-		}
-
-		if (i == num_tv_norms)
+		i = match_string(nv17_tv_norm_names,
+				 num_tv_norms, nouveau_tv_norm);
+		if (i < 0)
 			NV_WARN(drm, "Invalid TV norm setting \"%s\"\n",
 				nouveau_tv_norm);
+		else
+			tv_enc->tv_norm = i;
 	}
 
 	drm_mode_create_tv_properties(dev, num_tv_norms, nv17_tv_norm_names);
-- 
1.7.12.4

Re: [PATCH v2 11/21] drm/nouveau: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - handle err case before normal case - per Andy
>
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> index 6d99f11..67ba2ac 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> @@ -644,16 +644,13 @@ static int nv17_tv_create_resources(struct drm_encoder *encoder,
>         int i;
>
>         if (nouveau_tv_norm) {
> -               for (i = 0; i < num_tv_norms; i++) {
> -                       if (!strcmp(nv17_tv_norm_names[i], nouveau_tv_norm)) {
> -                               tv_enc->tv_norm = i;
> -                               break;
> -                       }
> -               }
> -
> -               if (i == num_tv_norms)
> +               i = match_string(nv17_tv_norm_names,
> +                                num_tv_norms, nouveau_tv_norm);
> +               if (i < 0)
>                         NV_WARN(drm, "Invalid TV norm setting \"%s\"\n",
>                                 nouveau_tv_norm);
> +               else
> +                       tv_enc->tv_norm = i;
>         }
>
>         drm_mode_create_tv_properties(dev, num_tv_norms, nv17_tv_norm_names);
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 12/21] drm: i2c: ch7006: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: David Airlie <airlied@linux.ie>
Cc: Yisheng Xie <xieyisheng1@huawei.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - handle err case before normal case.

 drivers/gpu/drm/i2c/ch7006_drv.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index 544a8a2..9eefdfe 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -464,16 +464,13 @@ static int ch7006_encoder_init(struct i2c_client *client,
 	priv->chip_version = ch7006_read(client, CH7006_VERSION_ID);
 
 	if (ch7006_tv_norm) {
-		for (i = 0; i < NUM_TV_NORMS; i++) {
-			if (!strcmp(ch7006_tv_norm_names[i], ch7006_tv_norm)) {
-				priv->norm = i;
-				break;
-			}
-		}
-
-		if (i == NUM_TV_NORMS)
+		i = match_string(ch7006_tv_norm_names,
+				 NUM_TV_NORMS, ch7006_tv_norm);
+		if (i < 0)
 			ch7006_err(client, "Invalid TV norm setting \"%s\".\n",
 				   ch7006_tv_norm);
+		else
+			priv->norm = i;
 	}
 
 	if (ch7006_scale >= 0 && ch7006_scale <= 2)
-- 
1.7.12.4

Re: [PATCH v2 12/21] drm: i2c: ch7006: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: David Airlie <airlied@linux.ie>
> Cc: Yisheng Xie <xieyisheng1@huawei.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - handle err case before normal case.
>
>  drivers/gpu/drm/i2c/ch7006_drv.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
> index 544a8a2..9eefdfe 100644
> --- a/drivers/gpu/drm/i2c/ch7006_drv.c
> +++ b/drivers/gpu/drm/i2c/ch7006_drv.c
> @@ -464,16 +464,13 @@ static int ch7006_encoder_init(struct i2c_client *client,
>         priv->chip_version = ch7006_read(client, CH7006_VERSION_ID);
>
>         if (ch7006_tv_norm) {
> -               for (i = 0; i < NUM_TV_NORMS; i++) {
> -                       if (!strcmp(ch7006_tv_norm_names[i], ch7006_tv_norm)) {
> -                               priv->norm = i;
> -                               break;
> -                       }
> -               }
> -
> -               if (i == NUM_TV_NORMS)
> +               i = match_string(ch7006_tv_norm_names,
> +                                NUM_TV_NORMS, ch7006_tv_norm);
> +               if (i < 0)
>                         ch7006_err(client, "Invalid TV norm setting \"%s\".\n",
>                                    ch7006_tv_norm);
> +               else
> +                       priv->norm = i;
>         }
>
>         if (ch7006_scale >= 0 && ch7006_scale <= 2)
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 13/21] ima: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Reviewed-by tag.

 security/integrity/ima/ima_main.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 74d0bd7..f807093 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -61,14 +61,11 @@ static int __init hash_setup(char *str)
 		goto out;
 	}
 
-	for (i = 0; i < HASH_ALGO__LAST; i++) {
-		if (strcmp(str, hash_algo_name[i]) == 0) {
-			ima_hash_algo = i;
-			break;
-		}
-	}
-	if (i == HASH_ALGO__LAST)
+	i = match_string(hash_algo_name, HASH_ALGO__LAST, str);
+	if (i < 0)
 		return 1;
+
+	ima_hash_algo = i;
 out:
 	hash_setup_done = 1;
 	return 1;
-- 
1.7.12.4

Re: [PATCH v2 13/21] ima: use match_string() helper

[Mimi Zohar]

On Thu, 2018-05-31 at 19:11 +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-integrity@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>

In the future, the patch's author Signed-off-by is always first.

Thanks, this patch is now queued in the next-integrity branch.

Mimi

> ---
> v2:
>  - add Reviewed-by tag.
> 
>  security/integrity/ima/ima_main.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 74d0bd7..f807093 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -61,14 +61,11 @@ static int __init hash_setup(char *str)
>  		goto out;
>  	}
> 
> -	for (i = 0; i < HASH_ALGO__LAST; i++) {
> -		if (strcmp(str, hash_algo_name[i]) == 0) {
> -			ima_hash_algo = i;
> -			break;
> -		}
> -	}
> -	if (i == HASH_ALGO__LAST)
> +	i = match_string(hash_algo_name, HASH_ALGO__LAST, str);
> +	if (i < 0)
>  		return 1;
> +
> +	ima_hash_algo = i;
>  out:
>  	hash_setup_done = 1;
>  	return 1;

Re: [PATCH v2 13/21] ima: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 6:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2018-05-31 at 19:11 +0800, Yisheng Xie wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
>> Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: "Serge E. Hallyn" <serge@hallyn.com>
>> Cc: linux-integrity@vger.kernel.org
>> Cc: linux-security-module@vger.kernel.org
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>
> In the future, the patch's author Signed-off-by is always first.
>
> Thanks, this patch is now queued in the next-integrity branch.

Hmm... It's interesting since git commit -s checks for author's SoB as
a _last_ in the bunch.

Is it configurable?


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 14/21] sched/debug: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - rename i to ret to show the change in returned value meaning - per Andy

 kernel/sched/debug.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 15b10e2..5591147 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -111,20 +111,19 @@ static int sched_feat_set(char *cmp)
 		cmp += 3;
 	}
 
-	for (i = 0; i < __SCHED_FEAT_NR; i++) {
-		if (strcmp(cmp, sched_feat_names[i]) == 0) {
-			if (neg) {
-				sysctl_sched_features &= ~(1UL << i);
-				sched_feat_disable(i);
-			} else {
-				sysctl_sched_features |= (1UL << i);
-				sched_feat_enable(i);
-			}
-			break;
-		}
+	i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp);
+	if (i < 0)
+		return i;
+
+	if (neg) {
+		sysctl_sched_features &= ~(1UL << i);
+		sched_feat_disable(i);
+	} else {
+		sysctl_sched_features |= (1UL << i);
+		sched_feat_enable(i);
 	}
 
-	return i;
+	return 0;
 }
 
 static ssize_t
@@ -133,7 +132,7 @@ static int sched_feat_set(char *cmp)
 {
 	char buf[64];
 	char *cmp;
-	int i;
+	int ret;
 	struct inode *inode;
 
 	if (cnt > 63)
@@ -148,10 +147,10 @@ static int sched_feat_set(char *cmp)
 	/* Ensure the static_key remains in a consistent state */
 	inode = file_inode(filp);
 	inode_lock(inode);
-	i = sched_feat_set(cmp);
+	ret = sched_feat_set(cmp);
 	inode_unlock(inode);
-	if (i == __SCHED_FEAT_NR)
-		return -EINVAL;
+	if (ret < 0)
+		return ret;
 
 	*ppos += cnt;
 
-- 
1.7.12.4

Re: [PATCH v2 14/21] sched/debug: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
>  - rename i to ret to show the change in returned value meaning - per Andy
>
>  kernel/sched/debug.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index 15b10e2..5591147 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -111,20 +111,19 @@ static int sched_feat_set(char *cmp)
>                 cmp += 3;
>         }
>
> -       for (i = 0; i < __SCHED_FEAT_NR; i++) {
> -               if (strcmp(cmp, sched_feat_names[i]) == 0) {
> -                       if (neg) {
> -                               sysctl_sched_features &= ~(1UL << i);
> -                               sched_feat_disable(i);
> -                       } else {
> -                               sysctl_sched_features |= (1UL << i);
> -                               sched_feat_enable(i);
> -                       }
> -                       break;
> -               }
> +       i = match_string(sched_feat_names, __SCHED_FEAT_NR, cmp);
> +       if (i < 0)
> +               return i;
> +
> +       if (neg) {
> +               sysctl_sched_features &= ~(1UL << i);
> +               sched_feat_disable(i);
> +       } else {
> +               sysctl_sched_features |= (1UL << i);
> +               sched_feat_enable(i);
>         }
>
> -       return i;
> +       return 0;
>  }
>
>  static ssize_t
> @@ -133,7 +132,7 @@ static int sched_feat_set(char *cmp)
>  {
>         char buf[64];
>         char *cmp;
> -       int i;
> +       int ret;
>         struct inode *inode;
>
>         if (cnt > 63)
> @@ -148,10 +147,10 @@ static int sched_feat_set(char *cmp)
>         /* Ensure the static_key remains in a consistent state */
>         inode = file_inode(filp);
>         inode_lock(inode);
> -       i = sched_feat_set(cmp);
> +       ret = sched_feat_set(cmp);
>         inode_unlock(inode);
> -       if (i == __SCHED_FEAT_NR)
> -               return -EINVAL;
> +       if (ret < 0)
> +               return ret;
>
>         *ppos += cnt;
>
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Clemens Ladisch <clemens@ladisch.de>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - do not change the type of i - per Andy

 sound/pci/oxygen/oxygen_mixer.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 4ca1266..81af21a 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1052,10 +1052,10 @@ static int add_controls(struct oxygen *chip,
 		[CONTROL_CD_CAPTURE_SWITCH] = "CD Capture Switch",
 		[CONTROL_AUX_CAPTURE_SWITCH] = "Aux Capture Switch",
 	};
-	unsigned int i, j;
+	unsigned int i;
 	struct snd_kcontrol_new template;
 	struct snd_kcontrol *ctl;
-	int err;
+	int j, err;
 
 	for (i = 0; i < count; ++i) {
 		template = controls[i];
@@ -1086,11 +1086,11 @@ static int add_controls(struct oxygen *chip,
 		err = snd_ctl_add(chip->card, ctl);
 		if (err < 0)
 			return err;
-		for (j = 0; j < CONTROL_COUNT; ++j)
-			if (!strcmp(ctl->id.name, known_ctl_names[j])) {
-				chip->controls[j] = ctl;
-				ctl->private_free = oxygen_any_ctl_free;
-			}
+		j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
+		if (j >= 0) {
+			chip->controls[j] = ctl;
+			ctl->private_free = oxygen_any_ctl_free;
+		}
 	}
 	return 0;
 }
-- 
1.7.12.4

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Takashi Iwai]

On Thu, 31 May 2018 13:11:20 +0200,
Yisheng Xie wrote:
> 
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: Clemens Ladisch <clemens@ladisch.de>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: alsa-devel@alsa-project.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>

Applied, thanks.


Takashi

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 31 May 2018 13:11:20 +0200,
> Yisheng Xie wrote:
>>
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
>> Cc: Clemens Ladisch <clemens@ladisch.de>
>> Cc: Jaroslav Kysela <perex@perex.cz>
>> Cc: Takashi Iwai <tiwai@suse.com>
>> Cc: alsa-devel@alsa-project.org
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>
> Applied, thanks.

Is it too late for nitpick?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Takashi Iwai]

On Thu, 31 May 2018 20:40:28 +0200,
Andy Shevchenko wrote:
> 
> On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Thu, 31 May 2018 13:11:20 +0200,
> > Yisheng Xie wrote:
> >>
> >> match_string() returns the index of an array for a matching string,
> >> which can be used instead of open coded variant.
> >>
> >> Cc: Clemens Ladisch <clemens@ladisch.de>
> >> Cc: Jaroslav Kysela <perex@perex.cz>
> >> Cc: Takashi Iwai <tiwai@suse.com>
> >> Cc: alsa-devel@alsa-project.org
> >> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> >
> > Applied, thanks.
> 
> Is it too late for nitpick?

Depends :)


Takashi

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 9:43 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 31 May 2018 20:40:28 +0200,
> Andy Shevchenko wrote:
>>
>> On Thu, May 31, 2018 at 9:39 PM, Takashi Iwai <tiwai@suse.de> wrote:

>> > Applied, thanks.
>>
>> Is it too late for nitpick?
>
> Depends :)

See my previous mail then.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.

Sorry, didn't notice before one thing:

> +               j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
> +               if (j >= 0) {
> +                       chip->controls[j] = ctl;
> +                       ctl->private_free = oxygen_any_ctl_free;
> +               }

It looks to me you may get rid of j completely by utilizing existing err.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Takashi Iwai]

On Thu, 31 May 2018 20:41:36 +0200,
Andy Shevchenko wrote:
> 
> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> > match_string() returns the index of an array for a matching string,
> > which can be used instead of open coded variant.
> 
> Sorry, didn't notice before one thing:
> 
> > +               j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
> > +               if (j >= 0) {
> > +                       chip->controls[j] = ctl;
> > +                       ctl->private_free = oxygen_any_ctl_free;
> > +               }
> 
> It looks to me you may get rid of j completely by utilizing existing err.

Well, err isn't ideal as it's referred as the actual index.
That is, the line below looks weird to me:
	chip->controls[err] = ctl;

Of course, j isn't the best name, either, but at least, keeping the
same variable makes the code conversion logic clearer.


thanks,

Takashi

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 9:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 31 May 2018 20:41:36 +0200,
> Andy Shevchenko wrote:
>> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:

>> > +               j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
>> > +               if (j >= 0) {
>> > +                       chip->controls[j] = ctl;
>> > +                       ctl->private_free = oxygen_any_ctl_free;
>> > +               }
>>
>> It looks to me you may get rid of j completely by utilizing existing err.
>
> Well, err isn't ideal as it's referred as the actual index.
> That is, the line below looks weird to me:
>         chip->controls[err] = ctl;
>
> Of course, j isn't the best name, either, but at least, keeping the
> same variable makes the code conversion logic clearer.

Works for me either way.
Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/21] ALSA: oxygen: use match_string() helper

[Takashi Iwai]

On Thu, 31 May 2018 21:02:05 +0200,
Andy Shevchenko wrote:
> 
> On Thu, May 31, 2018 at 9:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Thu, 31 May 2018 20:41:36 +0200,
> > Andy Shevchenko wrote:
> >> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> 
> >> > +               j = match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
> >> > +               if (j >= 0) {
> >> > +                       chip->controls[j] = ctl;
> >> > +                       ctl->private_free = oxygen_any_ctl_free;
> >> > +               }
> >>
> >> It looks to me you may get rid of j completely by utilizing existing err.
> >
> > Well, err isn't ideal as it's referred as the actual index.
> > That is, the line below looks weird to me:
> >         chip->controls[err] = ctl;
> >
> > Of course, j isn't the best name, either, but at least, keeping the
> > same variable makes the code conversion logic clearer.
> 
> Works for me either way.
> Thanks!

OK, let's take as is.


thanks,

Takashi

[PATCH v2 16/21] ASoC: max98088: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - split ret=xxx and move assignment to the line before 'if'
 - fix error return value - both per Andy

 sound/soc/codecs/max98088.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
index 865f64c..fb515aa 100644
--- a/sound/soc/codecs/max98088.c
+++ b/sound/soc/codecs/max98088.c
@@ -1382,15 +1382,12 @@ static int max98088_set_bias_level(struct snd_soc_component *component,
 
 static int max98088_get_channel(struct snd_soc_component *component, const char *name)
 {
-	int i;
+	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(eq_mode_name); i++)
-		if (strcmp(name, eq_mode_name[i]) == 0)
-			return i;
-
-	/* Shouldn't happen */
-	dev_err(component->dev, "Bad EQ channel name '%s'\n", name);
-	return -EINVAL;
+	ret = match_string(eq_mode_name, ARRAY_SIZE(eq_mode_name), name);
+	if (ret < 0)
+		dev_err(component->dev, "Bad EQ channel name '%s'\n", name);
+	return ret;
 }
 
 static void max98088_setup_eq1(struct snd_soc_component *component)
-- 
1.7.12.4

Re: [PATCH v2 16/21] ASoC: max98088: use match_string() helper

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

On Thu, May 31, 2018 at 07:11:21PM +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.

I don't have either the cover letter or the rest of the series here so
I've no context - what is the story here with regard to dependencies and
things?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 16/21] ASoC: max98088: use match_string() helper

[Yisheng Xie]

Hi Mark,

On 2018/5/31 19:49, Mark Brown wrote:
> On Thu, May 31, 2018 at 07:11:21PM +0800, Yisheng Xie wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
> 
> I don't have either the cover letter or the rest of the series here so
> I've no context - what is the story here with regard to dependencies and
> things?
Sorry about that I should have sent cover letter to you, but for too many maintainer
to sent, I ignore this to avoid make too much noisy.

Anyway, Here is the cover letter of v1 and I have add v2's cover letter to you:
  https://lkml.org/lkml/2018/5/21/303

Each patch in this patchset is a separate one, for what this patchset want to do
is use match_string() helper for echo subsystem.

Thanks
Yisheng
> 

Re: [PATCH v2 16/21] ASoC: max98088: use match_string() helper

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Thu, May 31, 2018 at 08:25:39PM +0800, Yisheng Xie wrote:

> Anyway, Here is the cover letter of v1 and I have add v2's cover letter to you:
>   https://lkml.org/lkml/2018/5/21/303

> Each patch in this patchset is a separate one, for what this patchset want to do
> is use match_string() helper for echo subsystem.

For something like this it's generally easier to not send everything as
one big series - it avoids any confusion about dependencies and there's
no actual relationship between the patches.  Instead just sending each
subsystem as a series or perhaps even just a bunch of separate patches
should make things smoother.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 16/21] ASoC: max98088: use match_string() helper

[Yisheng Xie]

Hi Mark,

On 2018/6/1 0:02, Mark Brown wrote:
> On Thu, May 31, 2018 at 08:25:39PM +0800, Yisheng Xie wrote:
> 
>> Anyway, Here is the cover letter of v1 and I have add v2's cover letter to you:
>>   https://lkml.org/lkml/2018/5/21/303
> 
>> Each patch in this patchset is a separate one, for what this patchset want to do
>> is use match_string() helper for echo subsystem.
> 
> For something like this it's generally easier to not send everything as
> one big series - it avoids any confusion about dependencies and there's
> no actual relationship between the patches.  Instead just sending each
> subsystem as a series or perhaps even just a bunch of separate patches
> should make things smoother.

Thanks, will take this suggestion maybe next time.

Thanks
Yisheng
> 

Applied "ASoC: max98088: use match_string() helper" to the asoc tree

[Mark Brown]

The patch

   ASoC: max98088: use match_string() helper

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 3470631510fa383feac5969b436499ca9bad03b8 Mon Sep 17 00:00:00 2001
From: Xie Yisheng <xieyisheng1@huawei.com>
Date: Thu, 31 May 2018 19:11:21 +0800
Subject: [PATCH] ASoC: max98088: use match_string() helper

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/max98088.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
index 865f64c40b79..fb515aaa54fc 100644
--- a/sound/soc/codecs/max98088.c
+++ b/sound/soc/codecs/max98088.c
@@ -1382,15 +1382,12 @@ static const char *eq_mode_name[] = {"EQ1 Mode", "EQ2 Mode"};
 
 static int max98088_get_channel(struct snd_soc_component *component, const char *name)
 {
-	int i;
+	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(eq_mode_name); i++)
-		if (strcmp(name, eq_mode_name[i]) == 0)
-			return i;
-
-	/* Shouldn't happen */
-	dev_err(component->dev, "Bad EQ channel name '%s'\n", name);
-	return -EINVAL;
+	ret = match_string(eq_mode_name, ARRAY_SIZE(eq_mode_name), name);
+	if (ret < 0)
+		dev_err(component->dev, "Bad EQ channel name '%s'\n", name);
+	return ret;
 }
 
 static void max98088_setup_eq1(struct snd_soc_component *component)
-- 
2.17.0

[PATCH v2 17/21] ASoC: max98095: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - split ret=xxxx and move assignment to the line before 'if' - per Andy

 sound/soc/codecs/max98095.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c
index 6bf2d0b..3b3a10d 100644
--- a/sound/soc/codecs/max98095.c
+++ b/sound/soc/codecs/max98095.c
@@ -1634,15 +1634,12 @@ static void max98095_handle_eq_pdata(struct snd_soc_component *component)
 static int max98095_get_bq_channel(struct snd_soc_component *component,
 				   const char *name)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(bq_mode_name); i++)
-		if (strcmp(name, bq_mode_name[i]) == 0)
-			return i;
+	int ret;
 
-	/* Shouldn't happen */
-	dev_err(component->dev, "Bad biquad channel name '%s'\n", name);
-	return -EINVAL;
+	ret = match_string(bq_mode_name, ARRAY_SIZE(bq_mode_name), name);
+	if (ret < 0)
+		dev_err(component->dev, "Bad biquad channel name '%s'\n", name);
+	return ret;
 }
 
 static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
-- 
1.7.12.4

Applied "ASoC: max98095: use match_string() helper" to the asoc tree

[Mark Brown]

The patch

   ASoC: max98095: use match_string() helper

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 1567062f17284637519d004ecc5d7ea0d6c754c3 Mon Sep 17 00:00:00 2001
From: Xie Yisheng <xieyisheng1@huawei.com>
Date: Thu, 31 May 2018 19:11:22 +0800
Subject: [PATCH] ASoC: max98095: use match_string() helper

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/max98095.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c
index 6bf2d0ba864f..3b3a10da7f40 100644
--- a/sound/soc/codecs/max98095.c
+++ b/sound/soc/codecs/max98095.c
@@ -1634,15 +1634,12 @@ static const char *bq_mode_name[] = {"Biquad1 Mode", "Biquad2 Mode"};
 static int max98095_get_bq_channel(struct snd_soc_component *component,
 				   const char *name)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(bq_mode_name); i++)
-		if (strcmp(name, bq_mode_name[i]) == 0)
-			return i;
+	int ret;
 
-	/* Shouldn't happen */
-	dev_err(component->dev, "Bad biquad channel name '%s'\n", name);
-	return -EINVAL;
+	ret = match_string(bq_mode_name, ARRAY_SIZE(bq_mode_name), name);
+	if (ret < 0)
+		dev_err(component->dev, "Bad biquad channel name '%s'\n", name);
+	return ret;
 }
 
 static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
-- 
2.17.0

[PATCH v2 18/21] ASoC: dapm: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
 - add Reviewed-by tag.

 sound/soc/soc-dapm.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 2d97091..1e9a363 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -724,18 +724,14 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 		item = 0;
 	}
 
-	for (i = 0; i < e->items; i++) {
-		if (!(strcmp(control_name, e->texts[i]))) {
-			path->name = e->texts[i];
-			if (i == item)
-				path->connect = 1;
-			else
-				path->connect = 0;
-			return 0;
-		}
-	}
+	i = match_string(e->texts, e->items, control_name);
+	if (i < 0)
+		return -ENODEV;
+
+	path->name = e->texts[i];
+	path->connect = (i == item);
+	return 0;
 
-	return -ENODEV;
 }
 
 /* set up initial codec paths */
-- 
1.7.12.4

[PATCH v2 19/21] bcache: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Kent Overstreet <kent.overstreet@gmail.com>
Cc: linux-bcache@vger.kernel.org 
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 drivers/md/bcache/util.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
index 74febd5..cd1f4fd 100644
--- a/drivers/md/bcache/util.c
+++ b/drivers/md/bcache/util.c
@@ -136,22 +136,17 @@ ssize_t bch_snprint_string_list(char *buf, size_t size, const char * const list[
 
 ssize_t bch_read_string_list(const char *buf, const char * const list[])
 {
-	size_t i;
+	ssize_t i;
 	char *s, *d = kstrndup(buf, PAGE_SIZE - 1, GFP_KERNEL);
 	if (!d)
 		return -ENOMEM;
 
 	s = strim(d);
 
-	for (i = 0; list[i]; i++)
-		if (!strcmp(list[i], s))
-			break;
+	i = match_string(list, -1, s);
 
 	kfree(d);
 
-	if (!list[i])
-		return -EINVAL;
-
 	return i;
 }
 
-- 
1.7.12.4

Re: [PATCH v2 19/21] bcache: use match_string() helper

[Coly Li]

On 2018/5/31 7:11 PM, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: Kent Overstreet <kent.overstreet@gmail.com>
> Cc: linux-bcache@vger.kernel.org 
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>

Hi Yishenng,

Andy Shevchenko <andriy.shevchenko@linux.intel.com> submitted a patch to
replace the whole bch_read_string_list() with __sysfs_match_string().
And this patch is applied in Jens' block tree, will go into mainline
kernel in v4.18.

If you search bcache mailing list, you may find a patch named with
"bcache: Replace bch_read_string_list() by __sysfs_match_string()".

That means this patch will conflict with existing changes.

Thanks.

Coly Li

> ---
>  drivers/md/bcache/util.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
> index 74febd5..cd1f4fd 100644
> --- a/drivers/md/bcache/util.c
> +++ b/drivers/md/bcache/util.c
> @@ -136,22 +136,17 @@ ssize_t bch_snprint_string_list(char *buf, size_t size, const char * const list[
>  
>  ssize_t bch_read_string_list(const char *buf, const char * const list[])
>  {
> -	size_t i;
> +	ssize_t i;
>  	char *s, *d = kstrndup(buf, PAGE_SIZE - 1, GFP_KERNEL);
>  	if (!d)
>  		return -ENOMEM;
>  
>  	s = strim(d);
>  
> -	for (i = 0; list[i]; i++)
> -		if (!strcmp(list[i], s))
> -			break;
> +	i = match_string(list, -1, s);
>  
>  	kfree(d);
>  
> -	if (!list[i])
> -		return -EINVAL;
> -
>  	return i;
>  }
>  
> 

Re: [PATCH v2 19/21] bcache: use match_string() helper

[Yisheng Xie]

Hi Coly,

On 2018/6/1 11:45, Coly Li wrote:
> On 2018/5/31 7:11 PM, Yisheng Xie wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
>> Cc: Kent Overstreet <kent.overstreet@gmail.com>
>> Cc: linux-bcache@vger.kernel.org 
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> 
> Hi Yishenng,
> 
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> submitted a patch to
> replace the whole bch_read_string_list() with __sysfs_match_string().
> And this patch is applied in Jens' block tree, will go into mainline
> kernel in v4.18.
> 
> If you search bcache mailing list, you may find a patch named with
> "bcache: Replace bch_read_string_list() by __sysfs_match_string()".
> 
> That means this patch will conflict with existing changes.

Get it, and thanks for this information.

Sorry Andy, for doing this once more.

Thanks
Yisheng
> 
> Thanks.
> 
> Coly Li
> 
>> ---
>>  drivers/md/bcache/util.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
>> index 74febd5..cd1f4fd 100644
>> --- a/drivers/md/bcache/util.c
>> +++ b/drivers/md/bcache/util.c
>> @@ -136,22 +136,17 @@ ssize_t bch_snprint_string_list(char *buf, size_t size, const char * const list[
>>  
>>  ssize_t bch_read_string_list(const char *buf, const char * const list[])
>>  {
>> -	size_t i;
>> +	ssize_t i;
>>  	char *s, *d = kstrndup(buf, PAGE_SIZE - 1, GFP_KERNEL);
>>  	if (!d)
>>  		return -ENOMEM;
>>  
>>  	s = strim(d);
>>  
>> -	for (i = 0; list[i]; i++)
>> -		if (!strcmp(list[i], s))
>> -			break;
>> +	i = match_string(list, -1, s);
>>  
>>  	kfree(d);
>>  
>> -	if (!list[i])
>> -		return -EINVAL;
>> -
>>  	return i;
>>  }
>>  
>>
> 
> 
> 

Re: [PATCH v2 19/21] bcache: use match_string() helper

[Coly Li]

On 2018/6/1 12:32 PM, Yisheng Xie wrote:
> Hi Coly,
> 
> On 2018/6/1 11:45, Coly Li wrote:
>> On 2018/5/31 7:11 PM, Yisheng Xie wrote:
>>> match_string() returns the index of an array for a matching string,
>>> which can be used instead of open coded variant.
>>>
>>> Cc: Kent Overstreet <kent.overstreet@gmail.com>
>>> Cc: linux-bcache@vger.kernel.org 
>>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>>
>> Hi Yishenng,
>>
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> submitted a patch to
>> replace the whole bch_read_string_list() with __sysfs_match_string().
>> And this patch is applied in Jens' block tree, will go into mainline
>> kernel in v4.18.
>>
>> If you search bcache mailing list, you may find a patch named with
>> "bcache: Replace bch_read_string_list() by __sysfs_match_string()".
>>
>> That means this patch will conflict with existing changes.
> 
> Get it, and thanks for this information.
> 
> Sorry Andy, for doing this once more.
> 
Hi Yisheng,

Thank you for the effort, hope to see more bcache patches from you :-)

Coly Li

[PATCH v2 20/21] powerpc/xmon: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 arch/powerpc/xmon/xmon.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0842f1..872ac8c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3161,7 +3161,7 @@ static void proccall(void)
 }
 
 #define N_PTREGS	44
-static char *regnames[N_PTREGS] = {
+static const char *regnames[N_PTREGS] = {
 	"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
 	"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
 	"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
@@ -3196,18 +3196,17 @@ static void proccall(void)
 			regname[i] = c;
 		}
 		regname[i] = 0;
-		for (i = 0; i < N_PTREGS; ++i) {
-			if (strcmp(regnames[i], regname) == 0) {
-				if (xmon_regs == NULL) {
-					printf("regs not available\n");
-					return 0;
-				}
-				*vp = ((unsigned long *)xmon_regs)[i];
-				return 1;
-			}
+		i = match_string(regnames, N_PTREGS, regname);
+		if (i < 0) {
+			printf("invalid register name '%%%s'\n", regname);
+			return 0;
 		}
-		printf("invalid register name '%%%s'\n", regname);
-		return 0;
+		if (xmon_regs == NULL) {
+			printf("regs not available\n");
+			return 0;
+		}
+		*vp = ((unsigned long *)xmon_regs)[i];
+		return 1;
 	}
 
 	/* skip leading "0x" if any */
-- 
1.7.12.4

Re: [v2,20/21] powerpc/xmon: use match_string() helper

[Michael Ellerman]

On Thu, 2018-05-31 at 11:11:25 UTC, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/0abbf2bfdc9dec32e9832aa8d4522a

cheers

[PATCH v2 21/21] sparc64: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: sparclinux@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 arch/sparc/kernel/setup_64.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
index 7944b3c..7af8c7e 100644
--- a/arch/sparc/kernel/setup_64.c
+++ b/arch/sparc/kernel/setup_64.c
@@ -512,10 +512,9 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
 				break;
 			}
 		}
-		for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
-			if (!strcmp(prop, crypto_hwcaps[i]))
-				caps |= HWCAP_SPARC_CRYPTO;
-		}
+		i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
+		if (i >= 0)
+			caps |= HWCAP_SPARC_CRYPTO;
 
 		plen = strlen(prop) + 1;
 		prop += plen;
-- 
1.7.12.4

Re: [PATCH v2 21/21] sparc64: use match_string() helper

[Andy Shevchenko]

On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.

> @@ -512,10 +512,9 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>                                 break;
>                         }
>                 }

It seems previous loop also can be replaced.

> -               for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
> -                       if (!strcmp(prop, crypto_hwcaps[i]))
> -                               caps |= HWCAP_SPARC_CRYPTO;
> -               }
> +               i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
> +               if (i >= 0)
> +                       caps |= HWCAP_SPARC_CRYPTO;
>
>                 plen = strlen(prop) + 1;
>                 prop += plen;
> --
> 1.7.12.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 21/21] sparc64: use match_string() helper

[Yisheng Xie]

Hi Andy,

On 2018/6/1 19:34, Andy Shevchenko wrote:
> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
> 
>> @@ -512,10 +512,9 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>>                                 break;
>>                         }
>>                 }
> 
> It seems previous loop also can be replaced.

No, because the there is an NULL in the middle of the array hwcaps:
 static const char *hwcaps[] = {
          "flush", "stbar", "swap", "muldiv", "v9",
          "ultra3", "blkinit", "n2",

          /* These strings are as they appear in the machine description
           * 'hwcap-list' property for cpu nodes.
           */
          "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
          "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
          "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
          "adp",
  };

Thanks
Yisheng
> 
>> -               for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
>> -                       if (!strcmp(prop, crypto_hwcaps[i]))
>> -                               caps |= HWCAP_SPARC_CRYPTO;
>> -               }
>> +               i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
>> +               if (i >= 0)
>> +                       caps |= HWCAP_SPARC_CRYPTO;
>>
>>                 plen = strlen(prop) + 1;
>>                 prop += plen;
>> --
>> 1.7.12.4
>>
> 
> 
> 

Re: [PATCH v2 21/21] sparc64: use match_string() helper

[Andy Shevchenko]

On Mon, Jun 4, 2018 at 4:06 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> Hi Andy,
>
> On 2018/6/1 19:34, Andy Shevchenko wrote:
>> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>>> match_string() returns the index of an array for a matching string,
>>> which can be used instead of open coded variant.
>>
>>> @@ -512,10 +512,9 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>>>                                 break;
>>>                         }
>>>                 }
>>
>> It seems previous loop also can be replaced.
>
> No, because the there is an NULL in the middle of the array hwcaps:
>  static const char *hwcaps[] = {
>           "flush", "stbar", "swap", "muldiv", "v9",
>           "ultra3", "blkinit", "n2",
>
>           /* These strings are as they appear in the machine description
>            * 'hwcap-list' property for cpu nodes.
>            */
>           "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
>           "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
>           "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
>           "adp",
>   };

Actually you can.
What you need is to add string literal instead of NULL and make an
additional condition after match_string() in all users (not to many),
something like

i = match_string();
if (i < 0)
 ...
if (BIT(i) == HWCAP_SPARC_CRYPTO) // or !=
 ...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 21/21] sparc64: use match_string() helper

[Yisheng Xie]

Hi Andy,

On 2018/6/4 18:06, Andy Shevchenko wrote:
> On Mon, Jun 4, 2018 at 4:06 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>> Hi Andy,
>>
>> On 2018/6/1 19:34, Andy Shevchenko wrote:
>>> On Thu, May 31, 2018 at 2:11 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>>>> match_string() returns the index of an array for a matching string,
>>>> which can be used instead of open coded variant.
>>>
>>>> @@ -512,10 +512,9 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>>>>                                 break;
>>>>                         }
>>>>                 }
>>>
>>> It seems previous loop also can be replaced.
>>
>> No, because the there is an NULL in the middle of the array hwcaps:
>>  static const char *hwcaps[] = {
>>           "flush", "stbar", "swap", "muldiv", "v9",
>>           "ultra3", "blkinit", "n2",
>>
>>           /* These strings are as they appear in the machine description
>>            * 'hwcap-list' property for cpu nodes.
>>            */
>>           "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
>>           "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
>>           "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
>>           "adp",
>>   };
> 
> Actually you can.
> What you need is to add string literal instead of NULL and make an
> additional condition after match_string() in all users (not to many),
> something like
> 
> i = match_string();
> if (i < 0)
>  ...
> if (BIT(i) == HWCAP_SPARC_CRYPTO) // or !=

OK, I get your point.

Thanks
Yisheng
>  ...
> 

[PATCH v3 21/21] sparc64: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: sparclinux@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v3:
 - add string literal instead of NULL for array hwcaps to make it
   can use match_string() too.  - per Andy
v2
 - new add for use match_string() helper patchset.

 arch/sparc/kernel/setup_64.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
index 7944b3c..4f0ec0c 100644
--- a/arch/sparc/kernel/setup_64.c
+++ b/arch/sparc/kernel/setup_64.c
@@ -401,7 +401,7 @@ void __init start_early_boot(void)
 	 */
 	"mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
 	"ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
-	"ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
+	"ima", "cspare", "pause", "cbcond", "resv" /*reserved for crypto */,
 	"adp",
 };

@@ -418,7 +418,7 @@ void cpucap_info(struct seq_file *m)
 	seq_puts(m, "cpucaps\t\t: ");
 	for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
 		unsigned long bit = 1UL << i;
-		if (hwcaps[i] && (caps & bit)) {
+		if (bit != HWCAP_SPARC_CRYPTO && (caps & bit)) {
 			seq_printf(m, "%s%s",
 				   printed ? "," : "", hwcaps[i]);
 			printed++;
@@ -472,7 +472,7 @@ static void __init report_hwcaps(unsigned long caps)

 	for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
 		unsigned long bit = 1UL << i;
-		if (hwcaps[i] && (caps & bit))
+		if (bit != HWCAP_SPARC_CRYPTO && (caps & bit))
 			report_one_hwcap(&printed, hwcaps[i]);
 	}
 	if (caps & HWCAP_SPARC_CRYPTO)
@@ -504,18 +504,13 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
 	while (len) {
 		int i, plen;

-		for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
-			unsigned long bit = 1UL << i;
+		i = match_string(hwcaps, ARRAY_SIZE(hwcaps), prop);
+		if (i >= 0)
+			caps |= (1UL << i);

-			if (hwcaps[i] && !strcmp(prop, hwcaps[i])) {
-				caps |= bit;
-				break;
-			}
-		}
-		for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
-			if (!strcmp(prop, crypto_hwcaps[i]))
-				caps |= HWCAP_SPARC_CRYPTO;
-		}
+		i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
+		if (i >= 0)
+			caps |= HWCAP_SPARC_CRYPTO;

 		plen = strlen(prop) + 1;
 		prop += plen;
-- 
1.7.12.4

Re: [PATCH v3 21/21] sparc64: use match_string() helper

[Andy Shevchenko]

On Wed, Jun 6, 2018 at 5:19 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>

Thanks for an update.
My comments below.

I think you need to mentioned the string literal change in the commit message.

> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: sparclinux@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v3:
>  - add string literal instead of NULL for array hwcaps to make it
>    can use match_string() too.  - per Andy
> v2
>  - new add for use match_string() helper patchset.
>
>  arch/sparc/kernel/setup_64.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
> index 7944b3c..4f0ec0c 100644
> --- a/arch/sparc/kernel/setup_64.c
> +++ b/arch/sparc/kernel/setup_64.c
> @@ -401,7 +401,7 @@ void __init start_early_boot(void)
>          */
>         "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
>         "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
> -       "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
> +       "ima", "cspare", "pause", "cbcond", "resv" /*reserved for crypto */,
>         "adp",

Why not to spell "crypto" explicitly and remove comment?

>  };
>
> @@ -418,7 +418,7 @@ void cpucap_info(struct seq_file *m)
>         seq_puts(m, "cpucaps\t\t: ");
>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>                 unsigned long bit = 1UL << i;
> -               if (hwcaps[i] && (caps & bit)) {
> +               if (bit != HWCAP_SPARC_CRYPTO && (caps & bit)) {

I would rather swap the order of subsonditions to check if caps has a
bit first, and then exclude CRYPTO.

>                         seq_printf(m, "%s%s",
>                                    printed ? "," : "", hwcaps[i]);
>                         printed++;
> @@ -472,7 +472,7 @@ static void __init report_hwcaps(unsigned long caps)
>
>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>                 unsigned long bit = 1UL << i;
> -               if (hwcaps[i] && (caps & bit))
> +               if (bit != HWCAP_SPARC_CRYPTO && (caps & bit))
>                         report_one_hwcap(&printed, hwcaps[i]);

Ditto.

>         }
>         if (caps & HWCAP_SPARC_CRYPTO)
> @@ -504,18 +504,13 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>         while (len) {
>                 int i, plen;
>
> -               for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
> -                       unsigned long bit = 1UL << i;
> +               i = match_string(hwcaps, ARRAY_SIZE(hwcaps), prop);
> +               if (i >= 0)
> +                       caps |= (1UL << i);

Parens are redundant (and actually didn't present in the original code above).

>
> -                       if (hwcaps[i] && !strcmp(prop, hwcaps[i])) {
> -                               caps |= bit;
> -                               break;
> -                       }
> -               }
> -               for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
> -                       if (!strcmp(prop, crypto_hwcaps[i]))
> -                               caps |= HWCAP_SPARC_CRYPTO;
> -               }
> +               i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
> +               if (i >= 0)
> +                       caps |= HWCAP_SPARC_CRYPTO;
>
>                 plen = strlen(prop) + 1;
>                 prop += plen;
> --
> 1.7.12.4
>
>
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 21/21] sparc64: use match_string() helper

[Yisheng Xie]

Hi Andy,

Sorry for late response. I will take your suggestion in next version.

Thanks
Yisheng

On 2018/6/6 13:01, Andy Shevchenko wrote:
> On Wed, Jun 6, 2018 at 5:19 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used instead of open coded variant.
>>
> 
> Thanks for an update.
> My comments below.
> 
> I think you need to mentioned the string literal change in the commit message.
> 
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
>> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
>> Cc: sparclinux@vger.kernel.org
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>> v3:
>>  - add string literal instead of NULL for array hwcaps to make it
>>    can use match_string() too.  - per Andy
>> v2
>>  - new add for use match_string() helper patchset.
>>
>>  arch/sparc/kernel/setup_64.c | 23 +++++++++--------------
>>  1 file changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
>> index 7944b3c..4f0ec0c 100644
>> --- a/arch/sparc/kernel/setup_64.c
>> +++ b/arch/sparc/kernel/setup_64.c
>> @@ -401,7 +401,7 @@ void __init start_early_boot(void)
>>          */
>>         "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
>>         "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
>> -       "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
>> +       "ima", "cspare", "pause", "cbcond", "resv" /*reserved for crypto */,
>>         "adp",
> 
> Why not to spell "crypto" explicitly and remove comment?
> 
>>  };
>>
>> @@ -418,7 +418,7 @@ void cpucap_info(struct seq_file *m)
>>         seq_puts(m, "cpucaps\t\t: ");
>>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>>                 unsigned long bit = 1UL << i;
>> -               if (hwcaps[i] && (caps & bit)) {
>> +               if (bit != HWCAP_SPARC_CRYPTO && (caps & bit)) {
> 
> I would rather swap the order of subsonditions to check if caps has a
> bit first, and then exclude CRYPTO.
> 
>>                         seq_printf(m, "%s%s",
>>                                    printed ? "," : "", hwcaps[i]);
>>                         printed++;
>> @@ -472,7 +472,7 @@ static void __init report_hwcaps(unsigned long caps)
>>
>>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>>                 unsigned long bit = 1UL << i;
>> -               if (hwcaps[i] && (caps & bit))
>> +               if (bit != HWCAP_SPARC_CRYPTO && (caps & bit))
>>                         report_one_hwcap(&printed, hwcaps[i]);
> 
> Ditto.
> 
>>         }
>>         if (caps & HWCAP_SPARC_CRYPTO)
>> @@ -504,18 +504,13 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>>         while (len) {
>>                 int i, plen;
>>
>> -               for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>> -                       unsigned long bit = 1UL << i;
>> +               i = match_string(hwcaps, ARRAY_SIZE(hwcaps), prop);
>> +               if (i >= 0)
>> +                       caps |= (1UL << i);
> 
> Parens are redundant (and actually didn't present in the original code above).
> 
>>
>> -                       if (hwcaps[i] && !strcmp(prop, hwcaps[i])) {
>> -                               caps |= bit;
>> -                               break;
>> -                       }
>> -               }
>> -               for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
>> -                       if (!strcmp(prop, crypto_hwcaps[i]))
>> -                               caps |= HWCAP_SPARC_CRYPTO;
>> -               }
>> +               i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
>> +               if (i >= 0)
>> +                       caps |= HWCAP_SPARC_CRYPTO;
>>
>>                 plen = strlen(prop) + 1;
>>                 prop += plen;
>> --
>> 1.7.12.4
>>
>>
>>
> 
> 
> 

[PATCH v4 21/21] sparc64: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

As Andy's suggestion, this patch add string literal instead of NULL
for crypto in array of hwcaps and make an additional condition after
match_string() in all users of it.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: sparclinux@vger.kernel.org
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
 arch/sparc/kernel/setup_64.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
index 7944b3c..6fa0c78 100644
--- a/arch/sparc/kernel/setup_64.c
+++ b/arch/sparc/kernel/setup_64.c
@@ -401,8 +401,7 @@ void __init start_early_boot(void)
 	 */
 	"mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
 	"ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
-	"ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
-	"adp",
+	"ima", "cspare", "pause", "cbcond", "crypto", "adp",
 };

 static const char *crypto_hwcaps[] = {
@@ -418,7 +417,7 @@ void cpucap_info(struct seq_file *m)
 	seq_puts(m, "cpucaps\t\t: ");
 	for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
 		unsigned long bit = 1UL << i;
-		if (hwcaps[i] && (caps & bit)) {
+		if ((caps & bit) && bit != HWCAP_SPARC_CRYPTO) {
 			seq_printf(m, "%s%s",
 				   printed ? "," : "", hwcaps[i]);
 			printed++;
@@ -472,7 +471,7 @@ static void __init report_hwcaps(unsigned long caps)

 	for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
 		unsigned long bit = 1UL << i;
-		if (hwcaps[i] && (caps & bit))
+		if ((caps & bit) && bit != HWCAP_SPARC_CRYPTO)
 			report_one_hwcap(&printed, hwcaps[i]);
 	}
 	if (caps & HWCAP_SPARC_CRYPTO)
@@ -504,18 +503,13 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
 	while (len) {
 		int i, plen;

-		for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
-			unsigned long bit = 1UL << i;
+		i = match_string(hwcaps, ARRAY_SIZE(hwcaps), prop);
+		if (i >= 0)
+			caps |= 1UL << i;

-			if (hwcaps[i] && !strcmp(prop, hwcaps[i])) {
-				caps |= bit;
-				break;
-			}
-		}
-		for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
-			if (!strcmp(prop, crypto_hwcaps[i]))
-				caps |= HWCAP_SPARC_CRYPTO;
-		}
+		i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
+		if (i >= 0)
+			caps |= HWCAP_SPARC_CRYPTO;

 		plen = strlen(prop) + 1;
 		prop += plen;
-- 
1.7.12.4

Re: [PATCH v4 21/21] sparc64: use match_string() helper

[Andy Shevchenko]

On Thu, Jun 21, 2018 at 4:39 AM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
> match_string() returns the index of an array for a matching string,
> which can be used instead of open coded variant.
>
> As Andy's suggestion, this patch add string literal instead of NULL
> for crypto in array of hwcaps and make an additional condition after
> match_string() in all users of it.
>

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Anthony Yznaga <anthony.yznaga@oracle.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: sparclinux@vger.kernel.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
>  arch/sparc/kernel/setup_64.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c
> index 7944b3c..6fa0c78 100644
> --- a/arch/sparc/kernel/setup_64.c
> +++ b/arch/sparc/kernel/setup_64.c
> @@ -401,8 +401,7 @@ void __init start_early_boot(void)
>          */
>         "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
>         "ASIBlkInit", "fmaf", "vis3", "hpc", "random", "trans", "fjfmau",
> -       "ima", "cspare", "pause", "cbcond", NULL /*reserved for crypto */,
> -       "adp",
> +       "ima", "cspare", "pause", "cbcond", "crypto", "adp",
>  };
>
>  static const char *crypto_hwcaps[] = {
> @@ -418,7 +417,7 @@ void cpucap_info(struct seq_file *m)
>         seq_puts(m, "cpucaps\t\t: ");
>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>                 unsigned long bit = 1UL << i;
> -               if (hwcaps[i] && (caps & bit)) {
> +               if ((caps & bit) && bit != HWCAP_SPARC_CRYPTO) {
>                         seq_printf(m, "%s%s",
>                                    printed ? "," : "", hwcaps[i]);
>                         printed++;
> @@ -472,7 +471,7 @@ static void __init report_hwcaps(unsigned long caps)
>
>         for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
>                 unsigned long bit = 1UL << i;
> -               if (hwcaps[i] && (caps & bit))
> +               if ((caps & bit) && bit != HWCAP_SPARC_CRYPTO)
>                         report_one_hwcap(&printed, hwcaps[i]);
>         }
>         if (caps & HWCAP_SPARC_CRYPTO)
> @@ -504,18 +503,13 @@ static unsigned long __init mdesc_cpu_hwcap_list(void)
>         while (len) {
>                 int i, plen;
>
> -               for (i = 0; i < ARRAY_SIZE(hwcaps); i++) {
> -                       unsigned long bit = 1UL << i;
> +               i = match_string(hwcaps, ARRAY_SIZE(hwcaps), prop);
> +               if (i >= 0)
> +                       caps |= 1UL << i;
>
> -                       if (hwcaps[i] && !strcmp(prop, hwcaps[i])) {
> -                               caps |= bit;
> -                               break;
> -                       }
> -               }
> -               for (i = 0; i < ARRAY_SIZE(crypto_hwcaps); i++) {
> -                       if (!strcmp(prop, crypto_hwcaps[i]))
> -                               caps |= HWCAP_SPARC_CRYPTO;
> -               }
> +               i = match_string(crypto_hwcaps, ARRAY_SIZE(crypto_hwcaps), prop);
> +               if (i >= 0)
> +                       caps |= HWCAP_SPARC_CRYPTO;
>
>                 plen = strlen(prop) + 1;
>                 prop += plen;
> --
> 1.7.12.4
>
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/21] use match_string() helper

[Yisheng Xie]

+ Mark

On 2018/5/31 19:11, Yisheng Xie wrote:
> Andy introduce helper function match_string() which can be used to return
> the index of array for a matching string. so we can use it in many places
> instead of open coded variant.
> 
> I just try to make this API be used more commonly, sorry if this makes
> too much big patchset.
> 
> v2:
>  - Remove the Patches which Andy already sent out, or maintainer already
>    picked up.
>  - Add Reviewed-by or Acked-by tags for some patchs.
>  - Add some new patches[19-21].
> 
> Yisheng Xie (21):
>   usb: phy: use match_string() helper
>   mfd: omap-usb-host: use match_string() helper
>   Staging: gdm724x: use match_string() helper
>   cxgb4: use match_string() helper
>   hp100: use match_string() helper
>   iwlwifi: mvm: use match_string() helper
>   bus: fsl-mc: use match_string() helper
>   clk: bcm2835: use match_string() helper
>   clk: use match_string() helper
>   cpufreq: intel_pstate: use match_string() helper
>   drm/nouveau: use match_string() helper
>   drm: i2c: ch7006: use match_string() helper
>   ima: use match_string() helper
>   sched/debug: use match_string() helper
>   ALSA: oxygen: use match_string() helper
>   ASoC: max98088: use match_string() helper
>   ASoC: max98095: use match_string() helper
>   ASoC: dapm: use match_string() helper
>   bcache: use match_string() helper
>   powerpc/xmon: use match_string() helper
>   sparc64: use match_string() helper
> 
>  arch/powerpc/xmon/xmon.c                         | 23 +++++++++---------
>  arch/sparc/kernel/setup_64.c                     |  7 +++---
>  drivers/bus/fsl-mc/fsl-mc-allocator.c            | 24 ++++--------------
>  drivers/clk/bcm/clk-bcm2835.c                    | 13 +++++-----
>  drivers/clk/clk.c                                |  8 ++----
>  drivers/cpufreq/intel_pstate.c                   | 15 +++++-------
>  drivers/gpu/drm/i2c/ch7006_drv.c                 | 13 ++++------
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c        | 13 ++++------
>  drivers/md/bcache/util.c                         |  9 ++-----
>  drivers/mfd/omap-usb-host.c                      | 24 ++----------------
>  drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c   | 14 +++--------
>  drivers/net/ethernet/hp/hp100.c                  |  9 +------
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 13 +++-------
>  drivers/staging/gdm724x/gdm_tty.c                | 18 ++++----------
>  drivers/usb/phy/of.c                             | 10 ++++----
>  kernel/sched/debug.c                             | 31 ++++++++++++------------
>  security/integrity/ima/ima_main.c                | 11 +++------
>  sound/pci/oxygen/oxygen_mixer.c                  | 14 +++++------
>  sound/soc/codecs/max98088.c                      | 13 ++++------
>  sound/soc/codecs/max98095.c                      | 13 ++++------
>  sound/soc/soc-dapm.c                             | 18 ++++++--------
>  21 files changed, 109 insertions(+), 204 deletions(-)
>