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

[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.

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

[Xie Yisheng]

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.
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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.

[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
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Xie Yisheng]

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);