diff mbox

[U-Boot,v1,1/2] usb: dfu: add config option to use in dfu mode fullspeed only

Message ID 1410266224-4051-2-git-send-email-hs@denx.de
State Superseded
Headers show

Commit Message

Heiko Schocher Sept. 9, 2014, 12:37 p.m. UTC
add the new config option CONFIG_DFU_FULLSPEED. With this
option enabled, DFU uses fullspeed only.

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Tom Rini <trini@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Liu Bin <b-liu@ti.com>
Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
---
 README                     | 3 +++
 drivers/usb/gadget/f_dfu.c | 3 +++
 drivers/usb/gadget/g_dnl.c | 3 +++
 3 files changed, 9 insertions(+)

Comments

Bin Liu Sept. 9, 2014, 2:09 p.m. UTC | #1
Heiko,

On 09/09/2014 07:37 AM, Heiko Schocher wrote:
> add the new config option CONFIG_DFU_FULLSPEED. With this
> option enabled, DFU uses fullspeed only.

Can we not introduce the new config option but check the gadget driver 
speed in runtime as what the ether gadget driver does?

I don't want two config options (CONFIG_USB_GADGET_DUALSPEED and 
CONFIG_DFU_FULLSPEED) to control one feature, as in your patch 2/2.

Regards,
-Bin.

>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Liu Bin <b-liu@ti.com>
> Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
> ---
>   README                     | 3 +++
>   drivers/usb/gadget/f_dfu.c | 3 +++
>   drivers/usb/gadget/g_dnl.c | 3 +++
>   3 files changed, 9 insertions(+)
>
> diff --git a/README b/README
> index 0a0f528..1413392 100644
> --- a/README
> +++ b/README
> @@ -1607,6 +1607,9 @@ The following options need to be configured:
>   		entering dfuMANIFEST state. Host waits this timeout, before
>   		sending again an USB request to the device.
>
> +		CONFIG_DFU_FULLSPEED
> +		use for the dfu functionality fullspeed only.
> +
>   - USB Device Android Fastboot support:
>   		CONFIG_CMD_FASTBOOT
>   		This enables the command "fastboot" which enables the Android
> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
> index 1145aab..dfa9f3b 100644
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -238,6 +238,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu)
>   {
>   	f_dfu->usb_function.strings = dfu_strings;
>   	f_dfu->usb_function.hs_descriptors = f_dfu->function;
> +	f_dfu->usb_function.descriptors = f_dfu->function;
>   	f_dfu->dfu_state = DFU_STATE_dfuIDLE;
>   }
>
> @@ -245,6 +246,7 @@ static inline void to_runtime_mode(struct f_dfu *f_dfu)
>   {
>   	f_dfu->usb_function.strings = NULL;
>   	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
> +	f_dfu->usb_function.descriptors = dfu_runtime_descs;
>   }
>
>   static int handle_upload(struct usb_request *req, u16 len)
> @@ -809,6 +811,7 @@ static int dfu_bind_config(struct usb_configuration *c)
>   		return -ENOMEM;
>   	f_dfu->usb_function.name = "dfu";
>   	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
> +	f_dfu->usb_function.descriptors = dfu_runtime_descs;
>   	f_dfu->usb_function.bind = dfu_bind;
>   	f_dfu->usb_function.unbind = dfu_unbind;
>   	f_dfu->usb_function.set_alt = dfu_set_alt;
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> index 58cad11..79e0c9c 100644
> --- a/drivers/usb/gadget/g_dnl.c
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -143,6 +143,9 @@ static int g_dnl_config_register(struct usb_composite_dev *cdev)
>   	config->bConfigurationValue = CONFIGURATION_NUMBER;
>   	config->iConfiguration = STRING_USBDOWN;
>   	config->bind = g_dnl_do_config;
> +#if defined(CONFIG_DFU_FULLSPEED)
> +	config->fullspeed = 1;
> +#endif
>
>   	return usb_add_config(cdev, config);
>   }
>
Ɓukasz Majewski Sept. 9, 2014, 2:25 p.m. UTC | #2
Hi Bin,

> Heiko,
> 
> On 09/09/2014 07:37 AM, Heiko Schocher wrote:
> > add the new config option CONFIG_DFU_FULLSPEED. With this
> > option enabled, DFU uses fullspeed only.
> 
> Can we not introduce the new config option but check the gadget
> driver speed in runtime as what the ether gadget driver does?
> 
> I don't want two config options (CONFIG_USB_GADGET_DUALSPEED and 
> CONFIG_DFU_FULLSPEED) to control one feature, as in your patch 2/2.

+1

> 
> Regards,
> -Bin.
> 
> >
> > Signed-off-by: Heiko Schocher <hs@denx.de>
> > Cc: Tom Rini <trini@ti.com>
> > Cc: Lukasz Majewski <l.majewski@samsung.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Liu Bin <b-liu@ti.com>
> > Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
> > ---
> >   README                     | 3 +++
> >   drivers/usb/gadget/f_dfu.c | 3 +++
> >   drivers/usb/gadget/g_dnl.c | 3 +++
> >   3 files changed, 9 insertions(+)
> >
> > diff --git a/README b/README
> > index 0a0f528..1413392 100644
> > --- a/README
> > +++ b/README
> > @@ -1607,6 +1607,9 @@ The following options need to be configured:
> >   		entering dfuMANIFEST state. Host waits this
> > timeout, before sending again an USB request to the device.
> >
> > +		CONFIG_DFU_FULLSPEED
> > +		use for the dfu functionality fullspeed only.
> > +
> >   - USB Device Android Fastboot support:
> >   		CONFIG_CMD_FASTBOOT
> >   		This enables the command "fastboot" which enables
> > the Android diff --git a/drivers/usb/gadget/f_dfu.c
> > b/drivers/usb/gadget/f_dfu.c index 1145aab..dfa9f3b 100644
> > --- a/drivers/usb/gadget/f_dfu.c
> > +++ b/drivers/usb/gadget/f_dfu.c
> > @@ -238,6 +238,7 @@ static inline void to_dfu_mode(struct f_dfu
> > *f_dfu) {
> >   	f_dfu->usb_function.strings = dfu_strings;
> >   	f_dfu->usb_function.hs_descriptors = f_dfu->function;
> > +	f_dfu->usb_function.descriptors = f_dfu->function;
> >   	f_dfu->dfu_state = DFU_STATE_dfuIDLE;
> >   }
> >
> > @@ -245,6 +246,7 @@ static inline void to_runtime_mode(struct f_dfu
> > *f_dfu) {
> >   	f_dfu->usb_function.strings = NULL;
> >   	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
> > +	f_dfu->usb_function.descriptors = dfu_runtime_descs;
> >   }
> >
> >   static int handle_upload(struct usb_request *req, u16 len)
> > @@ -809,6 +811,7 @@ static int dfu_bind_config(struct
> > usb_configuration *c) return -ENOMEM;
> >   	f_dfu->usb_function.name = "dfu";
> >   	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
> > +	f_dfu->usb_function.descriptors = dfu_runtime_descs;
> >   	f_dfu->usb_function.bind = dfu_bind;
> >   	f_dfu->usb_function.unbind = dfu_unbind;
> >   	f_dfu->usb_function.set_alt = dfu_set_alt;
> > diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> > index 58cad11..79e0c9c 100644
> > --- a/drivers/usb/gadget/g_dnl.c
> > +++ b/drivers/usb/gadget/g_dnl.c
> > @@ -143,6 +143,9 @@ static int g_dnl_config_register(struct
> > usb_composite_dev *cdev) config->bConfigurationValue =
> > CONFIGURATION_NUMBER; config->iConfiguration = STRING_USBDOWN;
> >   	config->bind = g_dnl_do_config;
> > +#if defined(CONFIG_DFU_FULLSPEED)
> > +	config->fullspeed = 1;
> > +#endif
> >
> >   	return usb_add_config(cdev, config);
> >   }
> >
>
Heiko Schocher Sept. 9, 2014, 2:37 p.m. UTC | #3
Hello Bin Liu,

Am 09.09.2014 16:09, schrieb Bin Liu:
> Heiko,
>
> On 09/09/2014 07:37 AM, Heiko Schocher wrote:
>> add the new config option CONFIG_DFU_FULLSPEED. With this
>> option enabled, DFU uses fullspeed only.
>
> Can we not introduce the new config option but check the gadget driver speed in runtime as what the ether gadget driver does?
> I don't want two config options (CONFIG_USB_GADGET_DUALSPEED and CONFIG_DFU_FULLSPEED) to control one feature, as in your patch 2/2.

Hmm.. I am not sure, if I understand you correct. I could use
CONFIG_USB_GADGET_DUALSPEED instead the new config option
for setting "config->fullspeed = 1;" in g_dnl_config_register()

If you mean this, yes, thats would be a good change...

bye,
Heiko
>
> Regards,
> -Bin.
>
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> Cc: Tom Rini <trini@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Liu Bin <b-liu@ti.com>
>> Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
>> ---
>> README | 3 +++
>> drivers/usb/gadget/f_dfu.c | 3 +++
>> drivers/usb/gadget/g_dnl.c | 3 +++
>> 3 files changed, 9 insertions(+)
>>
>> diff --git a/README b/README
>> index 0a0f528..1413392 100644
>> --- a/README
>> +++ b/README
>> @@ -1607,6 +1607,9 @@ The following options need to be configured:
>> entering dfuMANIFEST state. Host waits this timeout, before
>> sending again an USB request to the device.
>>
>> + CONFIG_DFU_FULLSPEED
>> + use for the dfu functionality fullspeed only.
>> +
>> - USB Device Android Fastboot support:
>> CONFIG_CMD_FASTBOOT
>> This enables the command "fastboot" which enables the Android
>> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
>> index 1145aab..dfa9f3b 100644
>> --- a/drivers/usb/gadget/f_dfu.c
>> +++ b/drivers/usb/gadget/f_dfu.c
>> @@ -238,6 +238,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu)
>> {
>> f_dfu->usb_function.strings = dfu_strings;
>> f_dfu->usb_function.hs_descriptors = f_dfu->function;
>> + f_dfu->usb_function.descriptors = f_dfu->function;
>> f_dfu->dfu_state = DFU_STATE_dfuIDLE;
>> }
>>
>> @@ -245,6 +246,7 @@ static inline void to_runtime_mode(struct f_dfu *f_dfu)
>> {
>> f_dfu->usb_function.strings = NULL;
>> f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
>> + f_dfu->usb_function.descriptors = dfu_runtime_descs;
>> }
>>
>> static int handle_upload(struct usb_request *req, u16 len)
>> @@ -809,6 +811,7 @@ static int dfu_bind_config(struct usb_configuration *c)
>> return -ENOMEM;
>> f_dfu->usb_function.name = "dfu";
>> f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
>> + f_dfu->usb_function.descriptors = dfu_runtime_descs;
>> f_dfu->usb_function.bind = dfu_bind;
>> f_dfu->usb_function.unbind = dfu_unbind;
>> f_dfu->usb_function.set_alt = dfu_set_alt;
>> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
>> index 58cad11..79e0c9c 100644
>> --- a/drivers/usb/gadget/g_dnl.c
>> +++ b/drivers/usb/gadget/g_dnl.c
>> @@ -143,6 +143,9 @@ static int g_dnl_config_register(struct usb_composite_dev *cdev)
>> config->bConfigurationValue = CONFIGURATION_NUMBER;
>> config->iConfiguration = STRING_USBDOWN;
>> config->bind = g_dnl_do_config;
>> +#if defined(CONFIG_DFU_FULLSPEED)
>> + config->fullspeed = 1;
>> +#endif
>>
>> return usb_add_config(cdev, config);
>> }
>>
>
>
>
Bin Liu Sept. 9, 2014, 2:42 p.m. UTC | #4
Heiko,

On 09/09/2014 09:37 AM, Heiko Schocher wrote:
> Hello Bin Liu,
>
> Am 09.09.2014 16:09, schrieb Bin Liu:
>> Heiko,
>>
>> On 09/09/2014 07:37 AM, Heiko Schocher wrote:
>>> add the new config option CONFIG_DFU_FULLSPEED. With this
>>> option enabled, DFU uses fullspeed only.
>>
>> Can we not introduce the new config option but check the gadget driver
>> speed in runtime as what the ether gadget driver does?
>> I don't want two config options (CONFIG_USB_GADGET_DUALSPEED and
>> CONFIG_DFU_FULLSPEED) to control one feature, as in your patch 2/2.
>
> Hmm.. I am not sure, if I understand you correct. I could use
> CONFIG_USB_GADGET_DUALSPEED instead the new config option
> for setting "config->fullspeed = 1;" in g_dnl_config_register()
>
> If you mean this, yes, thats would be a good change...

No, I did not mean this build time macro, but was thinking you can check 
g->speed at the same place in runtime.

I think you can refer to ether.c, which supports full-speed but does not 
use any macro.

Regards,
-Bin.

>
> bye,
> Heiko
>>
>> Regards,
>> -Bin.
>>
>>>
>>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>> Cc: Tom Rini <trini@ti.com>
>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Liu Bin <b-liu@ti.com>
>>> Cc: Lukas Stockmann <lukas.stockmann@siemens.com>
>>> ---
>>> README | 3 +++
>>> drivers/usb/gadget/f_dfu.c | 3 +++
>>> drivers/usb/gadget/g_dnl.c | 3 +++
>>> 3 files changed, 9 insertions(+)
>>>
>>> diff --git a/README b/README
>>> index 0a0f528..1413392 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -1607,6 +1607,9 @@ The following options need to be configured:
>>> entering dfuMANIFEST state. Host waits this timeout, before
>>> sending again an USB request to the device.
>>>
>>> + CONFIG_DFU_FULLSPEED
>>> + use for the dfu functionality fullspeed only.
>>> +
>>> - USB Device Android Fastboot support:
>>> CONFIG_CMD_FASTBOOT
>>> This enables the command "fastboot" which enables the Android
>>> diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
>>> index 1145aab..dfa9f3b 100644
>>> --- a/drivers/usb/gadget/f_dfu.c
>>> +++ b/drivers/usb/gadget/f_dfu.c
>>> @@ -238,6 +238,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu)
>>> {
>>> f_dfu->usb_function.strings = dfu_strings;
>>> f_dfu->usb_function.hs_descriptors = f_dfu->function;
>>> + f_dfu->usb_function.descriptors = f_dfu->function;
>>> f_dfu->dfu_state = DFU_STATE_dfuIDLE;
>>> }
>>>
>>> @@ -245,6 +246,7 @@ static inline void to_runtime_mode(struct f_dfu
>>> *f_dfu)
>>> {
>>> f_dfu->usb_function.strings = NULL;
>>> f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
>>> + f_dfu->usb_function.descriptors = dfu_runtime_descs;
>>> }
>>>
>>> static int handle_upload(struct usb_request *req, u16 len)
>>> @@ -809,6 +811,7 @@ static int dfu_bind_config(struct
>>> usb_configuration *c)
>>> return -ENOMEM;
>>> f_dfu->usb_function.name = "dfu";
>>> f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
>>> + f_dfu->usb_function.descriptors = dfu_runtime_descs;
>>> f_dfu->usb_function.bind = dfu_bind;
>>> f_dfu->usb_function.unbind = dfu_unbind;
>>> f_dfu->usb_function.set_alt = dfu_set_alt;
>>> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
>>> index 58cad11..79e0c9c 100644
>>> --- a/drivers/usb/gadget/g_dnl.c
>>> +++ b/drivers/usb/gadget/g_dnl.c
>>> @@ -143,6 +143,9 @@ static int g_dnl_config_register(struct
>>> usb_composite_dev *cdev)
>>> config->bConfigurationValue = CONFIGURATION_NUMBER;
>>> config->iConfiguration = STRING_USBDOWN;
>>> config->bind = g_dnl_do_config;
>>> +#if defined(CONFIG_DFU_FULLSPEED)
>>> + config->fullspeed = 1;
>>> +#endif
>>>
>>> return usb_add_config(cdev, config);
>>> }
>>>
>>
>>
>>
>
Heiko Schocher Sept. 10, 2014, 6:10 a.m. UTC | #5
Hello Bin Liu,

Am 09.09.2014 16:42, schrieb Bin Liu:
> Heiko,
>
> On 09/09/2014 09:37 AM, Heiko Schocher wrote:
>> Hello Bin Liu,
>>
>> Am 09.09.2014 16:09, schrieb Bin Liu:
>>> Heiko,
>>>
>>> On 09/09/2014 07:37 AM, Heiko Schocher wrote:
>>>> add the new config option CONFIG_DFU_FULLSPEED. With this
>>>> option enabled, DFU uses fullspeed only.
>>>
>>> Can we not introduce the new config option but check the gadget driver
>>> speed in runtime as what the ether gadget driver does?
>>> I don't want two config options (CONFIG_USB_GADGET_DUALSPEED and
>>> CONFIG_DFU_FULLSPEED) to control one feature, as in your patch 2/2.
>>
>> Hmm.. I am not sure, if I understand you correct. I could use
>> CONFIG_USB_GADGET_DUALSPEED instead the new config option
>> for setting "config->fullspeed = 1;" in g_dnl_config_register()
>>
>> If you mean this, yes, thats would be a good change...
>
> No, I did not mean this build time macro, but was thinking you can check g->speed at the same place in runtime.
>
> I think you can refer to ether.c, which supports full-speed but does not use any macro.

Yep, you are right, I can get rid completely of this new define!
The speed selection is done in drivers/usb/gadget/composite.c

I removed it, do some tests, and send a v2. Thanks!

bye,
Heiko
diff mbox

Patch

diff --git a/README b/README
index 0a0f528..1413392 100644
--- a/README
+++ b/README
@@ -1607,6 +1607,9 @@  The following options need to be configured:
 		entering dfuMANIFEST state. Host waits this timeout, before
 		sending again an USB request to the device.
 
+		CONFIG_DFU_FULLSPEED
+		use for the dfu functionality fullspeed only.
+
 - USB Device Android Fastboot support:
 		CONFIG_CMD_FASTBOOT
 		This enables the command "fastboot" which enables the Android
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 1145aab..dfa9f3b 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -238,6 +238,7 @@  static inline void to_dfu_mode(struct f_dfu *f_dfu)
 {
 	f_dfu->usb_function.strings = dfu_strings;
 	f_dfu->usb_function.hs_descriptors = f_dfu->function;
+	f_dfu->usb_function.descriptors = f_dfu->function;
 	f_dfu->dfu_state = DFU_STATE_dfuIDLE;
 }
 
@@ -245,6 +246,7 @@  static inline void to_runtime_mode(struct f_dfu *f_dfu)
 {
 	f_dfu->usb_function.strings = NULL;
 	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
+	f_dfu->usb_function.descriptors = dfu_runtime_descs;
 }
 
 static int handle_upload(struct usb_request *req, u16 len)
@@ -809,6 +811,7 @@  static int dfu_bind_config(struct usb_configuration *c)
 		return -ENOMEM;
 	f_dfu->usb_function.name = "dfu";
 	f_dfu->usb_function.hs_descriptors = dfu_runtime_descs;
+	f_dfu->usb_function.descriptors = dfu_runtime_descs;
 	f_dfu->usb_function.bind = dfu_bind;
 	f_dfu->usb_function.unbind = dfu_unbind;
 	f_dfu->usb_function.set_alt = dfu_set_alt;
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 58cad11..79e0c9c 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -143,6 +143,9 @@  static int g_dnl_config_register(struct usb_composite_dev *cdev)
 	config->bConfigurationValue = CONFIGURATION_NUMBER;
 	config->iConfiguration = STRING_USBDOWN;
 	config->bind = g_dnl_do_config;
+#if defined(CONFIG_DFU_FULLSPEED)
+	config->fullspeed = 1;
+#endif
 
 	return usb_add_config(cdev, config);
 }