2019-07-08 17:56:30

by Pawel Laszczak

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

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 <[email protected]>
---
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


2019-07-09 06:16:13

by Felipe Balbi

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


Hi,

Pawel Laszczak <[email protected]> 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

2019-07-09 06:25:41

by Pawel Laszczak

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

Hi,
>
>Hi,
>
>Pawel Laszczak <[email protected]> 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

2019-07-09 06:33:26

by Felipe Balbi

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


Hi,

Pawel Laszczak <[email protected]> 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

2019-07-09 06:39:13

by Pawel Laszczak

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



>-----Original Message-----
>From: Felipe Balbi <[email protected]>
>Sent: Tuesday, July 9, 2019 8:30 AM
>To: Pawel Laszczak <[email protected]>
>Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; Suresh Punnoose <[email protected]>; Jayshri Dajiram Pawar <[email protected]>; Rahul Kumar
><[email protected]>; Anil Joy Varughese <[email protected]>
>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 <[email protected]> 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

2019-07-21 14:55:48

by Pawel Laszczak

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

Hi,

>>Pawel Laszczak <[email protected]> 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

2019-07-31 05:44:25

by Pawel Laszczak

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

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 <[email protected]> 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.
>