[PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Pawel Laszczak]

Patch introduces new function usb_dr_mode_to_string for converting dual
role mod to string and removes static from usb_dr_mode_to_string
definition.

Both changes have made to avoid duplication of code by cdns3 driver.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/common/common.c | 12 +++++++++++-
 include/linux/usb/otg.h     | 16 ++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 18f5dcf58b0d..a485071325b3 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -118,13 +118,14 @@ static const char *const usb_dr_modes[] = {
 	[USB_DR_MODE_OTG]		= "otg",
 };
 
-static enum usb_dr_mode usb_get_dr_mode_from_string(const char *str)
+enum usb_dr_mode usb_get_dr_mode_from_string(const char *str)
 {
 	int ret;
 
 	ret = match_string(usb_dr_modes, ARRAY_SIZE(usb_dr_modes), str);
 	return (ret < 0) ? USB_DR_MODE_UNKNOWN : ret;
 }
+EXPORT_SYMBOL_GPL(usb_get_dr_mode_from_string);
 
 enum usb_dr_mode usb_get_dr_mode(struct device *dev)
 {
@@ -139,6 +140,15 @@ enum usb_dr_mode usb_get_dr_mode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(usb_get_dr_mode);
 
+const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode)
+{
+	if (dr_mode > USB_DR_MODE_UNKNOWN || dr_mode <= USB_DR_MODE_PERIPHERAL)
+		return usb_dr_modes[dr_mode];
+
+	return usb_dr_modes[USB_DR_MODE_UNKNOWN];
+}
+EXPORT_SYMBOL_GPL(usb_dr_mode_to_string);
+
 #ifdef CONFIG_OF
 /**
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index 69f1b6328532..c156817672c4 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -129,4 +129,20 @@ enum usb_dr_mode {
  */
 extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
 
+/**
+ * usb_get_dr_mode_from_string - Convert string to dual role mode.
+ * @str: Pointer to the given string
+ *
+ * The function gets string and returns the correspondig enum usb_dr_mode.
+ */
+extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
+
+/**
+ * usb_dr_mode_to_string - Convert dual role mode to string.
+ * @dr_mode: Pointer to the given dual role mode
+ *
+ * The function gets enum usb_dr_mode, and returns the correspondig string.
+ */
+extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
+
 #endif /* __LINUX_USB_OTG_H */
-- 
2.17.1

Re: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Felipe Balbi]

Hi,

Pawel Laszczak <pawell@cadence.com> writes:
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 69f1b6328532..c156817672c4 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>   */
>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>  
> +/**
> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
> + * @str: Pointer to the given string
> + *
> + * The function gets string and returns the correspondig enum usb_dr_mode.
> + */
> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
> +
> +/**
> + * usb_dr_mode_to_string - Convert dual role mode to string.
> + * @dr_mode: Pointer to the given dual role mode
> + *
> + * The function gets enum usb_dr_mode, and returns the correspondig string.
> + */
> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
> +
>  #endif /* __LINUX_USB_OTG_H */

Still missing the stubs I mentioned. Did you try compiling with and
without common enabled?

-- 
balbi

RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Pawel Laszczak]

Hi,
>
>Hi,
>
>Pawel Laszczak <pawell@cadence.com> writes:
>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>> index 69f1b6328532..c156817672c4 100644
>> --- a/include/linux/usb/otg.h
>> +++ b/include/linux/usb/otg.h
>> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>>   */
>>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>>
>> +/**
>> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
>> + * @str: Pointer to the given string
>> + *
>> + * The function gets string and returns the correspondig enum usb_dr_mode.
>> + */
>> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
>> +
>> +/**
>> + * usb_dr_mode_to_string - Convert dual role mode to string.
>> + * @dr_mode: Pointer to the given dual role mode
>> + *
>> + * The function gets enum usb_dr_mode, and returns the correspondig string.
>> + */
>> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
>> +
>>  #endif /* __LINUX_USB_OTG_H */
>
>Still missing the stubs I mentioned. Did you try compiling with and
>without common enabled?
>
Sorry, I thought that I send answer yesterday but it's look like I prepared the answer but 
I  forgot to send. 

In /drivers/usb/Kconfig we have: 

config USB
	tristate "Support for Host-side USB"
	depends on USB_ARCH_HAS_HCD
	select USB_COMMON

and in /drivers/usb/gadget/Kconfig we have:

menuconfig USB_GADGET
	tristate "USB Gadget Support"
	select USB_COMMON

I think that it should cover all cases. 

Am I right ?

Pawel

>--
>balbi

RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Felipe Balbi]

Hi,

Pawel Laszczak <pawell@cadence.com> writes:
>>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>>> index 69f1b6328532..c156817672c4 100644
>>> --- a/include/linux/usb/otg.h
>>> +++ b/include/linux/usb/otg.h
>>> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>>>   */
>>>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>>>
>>> +/**
>>> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
>>> + * @str: Pointer to the given string
>>> + *
>>> + * The function gets string and returns the correspondig enum usb_dr_mode.
>>> + */
>>> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
>>> +
>>> +/**
>>> + * usb_dr_mode_to_string - Convert dual role mode to string.
>>> + * @dr_mode: Pointer to the given dual role mode
>>> + *
>>> + * The function gets enum usb_dr_mode, and returns the correspondig string.
>>> + */
>>> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
>>> +
>>>  #endif /* __LINUX_USB_OTG_H */
>>
>>Still missing the stubs I mentioned. Did you try compiling with and
>>without common enabled?
>>
> Sorry, I thought that I send answer yesterday but it's look like I prepared the answer but 
> I  forgot to send. 
>
> In /drivers/usb/Kconfig we have: 
>
> config USB
> 	tristate "Support for Host-side USB"
> 	depends on USB_ARCH_HAS_HCD
> 	select USB_COMMON
>
> and in /drivers/usb/gadget/Kconfig we have:
>
> menuconfig USB_GADGET
> 	tristate "USB Gadget Support"
> 	select USB_COMMON
>
> I think that it should cover all cases. 
>
> Am I right ?

Run a few tens of randconfig builds and see if you ever catch any
problem. I think randconfig can produce a defconfig where USB=n
USB_GADGET=n and USB_COMMON=y.

-- 
balbi

RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Pawel Laszczak]

>-----Original Message-----
>From: Felipe Balbi <felipe.balbi@linux.intel.com>
>Sent: Tuesday, July 9, 2019 8:30 AM
>To: Pawel Laszczak <pawell@cadence.com>
>Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; rogerq@ti.com; linux-kernel@vger.kernel.org; jbergsagel@ti.com;
>nsekhar@ti.com; nm@ti.com; Suresh Punnoose <sureshp@cadence.com>; Jayshri Dajiram Pawar <jpawar@cadence.com>; Rahul Kumar
><kurahul@cadence.com>; Anil Joy Varughese <aniljoy@cadence.com>
>Subject: RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.
>
>EXTERNAL MAIL
>
>
>
>Hi,
>
>Pawel Laszczak <pawell@cadence.com> writes:
>>>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>>>> index 69f1b6328532..c156817672c4 100644
>>>> --- a/include/linux/usb/otg.h
>>>> +++ b/include/linux/usb/otg.h
>>>> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>>>>   */
>>>>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>>>>
>>>> +/**
>>>> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
>>>> + * @str: Pointer to the given string
>>>> + *
>>>> + * The function gets string and returns the correspondig enum usb_dr_mode.
>>>> + */
>>>> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
>>>> +
>>>> +/**
>>>> + * usb_dr_mode_to_string - Convert dual role mode to string.
>>>> + * @dr_mode: Pointer to the given dual role mode
>>>> + *
>>>> + * The function gets enum usb_dr_mode, and returns the correspondig string.
>>>> + */
>>>> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
>>>> +
>>>>  #endif /* __LINUX_USB_OTG_H */
>>>
>>>Still missing the stubs I mentioned. Did you try compiling with and
>>>without common enabled?
>>>
>> Sorry, I thought that I send answer yesterday but it's look like I prepared the answer but
>> I  forgot to send.
>>
>> In /drivers/usb/Kconfig we have:
>>
>> config USB
>> 	tristate "Support for Host-side USB"
>> 	depends on USB_ARCH_HAS_HCD
>> 	select USB_COMMON
>>
>> and in /drivers/usb/gadget/Kconfig we have:
>>
>> menuconfig USB_GADGET
>> 	tristate "USB Gadget Support"
>> 	select USB_COMMON
>>
>> I think that it should cover all cases.
>>
>> Am I right ?
>
>Run a few tens of randconfig builds and see if you ever catch any
>problem. I think randconfig can produce a defconfig where USB=n
>USB_GADGET=n and USB_COMMON=y.

Ok, I will test it, but I think that it should work. 
The same situation we have for example with: usb_otg_state_string or usb_ep_type_string.

>
>--
>balbi

RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Pawel Laszczak]

Hi,

>>Pawel Laszczak <pawell@cadence.com> writes:
>>>>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>>>>> index 69f1b6328532..c156817672c4 100644
>>>>> --- a/include/linux/usb/otg.h
>>>>> +++ b/include/linux/usb/otg.h
>>>>> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>>>>>   */
>>>>>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>>>>>
>>>>> +/**
>>>>> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
>>>>> + * @str: Pointer to the given string
>>>>> + *
>>>>> + * The function gets string and returns the correspondig enum usb_dr_mode.
>>>>> + */
>>>>> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
>>>>> +
>>>>> +/**
>>>>> + * usb_dr_mode_to_string - Convert dual role mode to string.
>>>>> + * @dr_mode: Pointer to the given dual role mode
>>>>> + *
>>>>> + * The function gets enum usb_dr_mode, and returns the correspondig string.
>>>>> + */
>>>>> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
>>>>> +
>>>>>  #endif /* __LINUX_USB_OTG_H */
>>>>
>>>>Still missing the stubs I mentioned. Did you try compiling with and
>>>>without common enabled?
>>>>
>>> Sorry, I thought that I send answer yesterday but it's look like I prepared the answer but
>>> I  forgot to send.
>>>
>>> In /drivers/usb/Kconfig we have:
>>>
>>> config USB
>>> 	tristate "Support for Host-side USB"
>>> 	depends on USB_ARCH_HAS_HCD
>>> 	select USB_COMMON
>>>
>>> and in /drivers/usb/gadget/Kconfig we have:
>>>
>>> menuconfig USB_GADGET
>>> 	tristate "USB Gadget Support"
>>> 	select USB_COMMON
>>>
>>> I think that it should cover all cases.
>>>
>>> Am I right ?
>>
>>Run a few tens of randconfig builds and see if you ever catch any
>>problem. I think randconfig can produce a defconfig where USB=n
>>USB_GADGET=n and USB_COMMON=y.
>
>Ok, I will test it, but I think that it should work.
>The same situation we have for example with: usb_otg_state_string or usb_ep_type_string.
>

I've been testing it with USB=n USB_GADGET=n and USB_COMMON=y and also only with CONFIG_USB_COMMON=y. 
Also I've tested this patch with different default configuration together with CDNS3 driver which use these functions. 
I've test It mainly with x86 and arm architecture. 
So far I've not found any issue. 

Cheers,
Pawell

RE: [PATCH 1/3] usb: common: Add usb_get_dr_mode_from_string and usb_dr_mode_to_string.

[Pawel Laszczak]

Hi Felipe,

What about this patch. I just noticed that prefix is incorrect "1/3". Can it stay or should I send it again ?

Cheers,
Pawel 

>>>Pawel Laszczak <pawell@cadence.com> writes:
>>>>>> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
>>>>>> index 69f1b6328532..c156817672c4 100644
>>>>>> --- a/include/linux/usb/otg.h
>>>>>> +++ b/include/linux/usb/otg.h
>>>>>> @@ -129,4 +129,20 @@ enum usb_dr_mode {
>>>>>>   */
>>>>>>  extern enum usb_dr_mode usb_get_dr_mode(struct device *dev);
>>>>>>
>>>>>> +/**
>>>>>> + * usb_get_dr_mode_from_string - Convert string to dual role mode.
>>>>>> + * @str: Pointer to the given string
>>>>>> + *
>>>>>> + * The function gets string and returns the correspondig enum usb_dr_mode.
>>>>>> + */
>>>>>> +extern enum usb_dr_mode usb_get_dr_mode_from_string(const char *str);
>>>>>> +
>>>>>> +/**
>>>>>> + * usb_dr_mode_to_string - Convert dual role mode to string.
>>>>>> + * @dr_mode: Pointer to the given dual role mode
>>>>>> + *
>>>>>> + * The function gets enum usb_dr_mode, and returns the correspondig string.
>>>>>> + */
>>>>>> +extern const char *usb_dr_mode_to_string(const enum usb_dr_mode dr_mode);
>>>>>> +
>>>>>>  #endif /* __LINUX_USB_OTG_H */
>>>>>
>>>>>Still missing the stubs I mentioned. Did you try compiling with and
>>>>>without common enabled?
>>>>>
>>>> Sorry, I thought that I send answer yesterday but it's look like I prepared the answer but
>>>> I  forgot to send.
>>>>
>>>> In /drivers/usb/Kconfig we have:
>>>>
>>>> config USB
>>>> 	tristate "Support for Host-side USB"
>>>> 	depends on USB_ARCH_HAS_HCD
>>>> 	select USB_COMMON
>>>>
>>>> and in /drivers/usb/gadget/Kconfig we have:
>>>>
>>>> menuconfig USB_GADGET
>>>> 	tristate "USB Gadget Support"
>>>> 	select USB_COMMON
>>>>
>>>> I think that it should cover all cases.
>>>>
>>>> Am I right ?
>>>
>>>Run a few tens of randconfig builds and see if you ever catch any
>>>problem. I think randconfig can produce a defconfig where USB=n
>>>USB_GADGET=n and USB_COMMON=y.
>>
>>Ok, I will test it, but I think that it should work.
>>The same situation we have for example with: usb_otg_state_string or usb_ep_type_string.
>>
>
>I've been testing it with USB=n USB_GADGET=n and USB_COMMON=y and also only with CONFIG_USB_COMMON=y.
>Also I've tested this patch with different default configuration together with CDNS3 driver which use these functions.
>I've test It mainly with x86 and arm architecture.
>So far I've not found any issue.
>