Re: [PATCH] driver: gadget: fastboot: Link endpoint and descriptors

Re: [PATCH] driver: gadget: fastboot: Link endpoint and descriptors

[qianfan]

在 2021/11/4 0:53, Sean Anderson 写道:

> On 9/16/21 3:02 AM, qianfanguijin@163.com wrote:
>> From: qianfan Zhao <qianfanguijin@163.com>
>>
>> If the downloading file size is equal to the partition size, "fastboot
>> flash" can't work, at least in sunxi platform, because used an
>> uninitalized point: ep->desc.
>
> Hm, I think that usb_ep_ops->enable needs to set ep->desc = desc on 
> success.
>
> Of the existing drivers, only musb-new and mtu3 skip this.

I checked the udc driver and found that not all the udc driver set 
"ep->desc = desc", such as atmel_usba_udc, dwc2_udc_otg, musb-new and 
mtu3. Those drivers save "desc" to bsp driver's private data only, such 
as "msub_ep->desc = desc", but the common usb_ep's desc is an invalid point.

And I'm not find any documents declare the behaves how 
usb_ep_ops->enable did, save desc or not.  So I'd prefect save desc in 
the fastboot level, and the udc driver can also save desc again if they 
want.

>
> --Sean

Re: [PATCH] driver: gadget: fastboot: Link endpoint and descriptors

[Sean Anderson]

On 11/5/21 2:28 AM, qianfan wrote:
> 在 2021/11/4 0:53, Sean Anderson 写道:
> 
>> On 9/16/21 3:02 AM, qianfanguijin@163.com wrote:
>>> From: qianfan Zhao <qianfanguijin@163.com>
>>>
>>> If the downloading file size is equal to the partition size, "fastboot
>>> flash" can't work, at least in sunxi platform, because used an
>>> uninitalized point: ep->desc.
>>
>> Hm, I think that usb_ep_ops->enable needs to set ep->desc = desc on success.
>>
>> Of the existing drivers, only musb-new and mtu3 skip this.
> 
> I checked the udc driver and found that not all the udc driver set "ep->desc = desc", such as atmel_usba_udc, dwc2_udc_otg, musb-new and mtu3. Those drivers save "desc" to bsp driver's private data only, such as "msub_ep->desc = desc", but the common usb_ep's desc is an invalid point.
> 
> And I'm not find any documents declare the behaves how usb_ep_ops->enable did, save desc or not.  So I'd prefect save desc in the fastboot level, and the udc driver can also save desc again if they want.

This idiom is also present in f_thor.c, f_rockusb.c, and f_mass_storage.c

Given that a majority of otg implementations set this, I think the other should be changed to match.

--Sean

Re: [PATCH] driver: gadget: fastboot: Link endpoint and descriptors

[qianfan]

Hi:

I had pushed a new version and you can read it from 
http://patchwork.ozlabs.org/project/uboot/patch/tencent_AECDF2ED56A768BA5AFB470F1B5350EBA90A@qq.com/

This also fixed fastboot data abort bug on am335x platform such as:

=> fastboot 0
musb-hdrc: peripheral reset irq lost!
** Bad device specification mmc bootloader_a **
Couldn't find partition mmc bootloader_a
Starting download of 108152 bytes
data abort
pc : [<9ff9b198>]          lr : [<9ff9b195>]
reloc pc : [<80830198>]    lr : [<80830195>]
sp : 9df35f30  ip : 00000000     fp : 00000002
r10: 00000000  r9 : 9df4aeb0     r8 : 00000001
r7 : 9df574c0  r6 : 9df4fea4     r5 : 00000001  r4 : 0001a678
r3 : 00000000  r2 : 9df35f48     r1 : 9df35f48  r0 : 0001a678
Flags: nzCv  IRQs off  FIQs on  Mode SVC_32 (T)
Code: f7fd fdaa 6823 1e04 (791d) 795b
Resetting CPU ...

resetting ...
CCCCCCCC
U-Boot SPL 2022.01-rc1-01417-gcf8017ac80 (Nov 15 2021 - 13:35:51 +0800)
Trying to boot from USB eth
eth0: eth_cpsw, eth1: usb_ether
using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
MAC de:ad:be:ef:00:01
HOST MAC de:ad:be:ef:00:00

在 2021/11/5 21:29, Sean Anderson 写道:
> On 11/5/21 2:28 AM, qianfan wrote:
>> 在 2021/11/4 0:53, Sean Anderson 写道:
>>
>>> On 9/16/21 3:02 AM, qianfanguijin@163.com wrote:
>>>> From: qianfan Zhao <qianfanguijin@163.com>
>>>>
>>>> If the downloading file size is equal to the partition size, "fastboot
>>>> flash" can't work, at least in sunxi platform, because used an
>>>> uninitalized point: ep->desc.
>>>
>>> Hm, I think that usb_ep_ops->enable needs to set ep->desc = desc on 
>>> success.
>>>
>>> Of the existing drivers, only musb-new and mtu3 skip this.
>>
>> I checked the udc driver and found that not all the udc driver set 
>> "ep->desc = desc", such as atmel_usba_udc, dwc2_udc_otg, musb-new and 
>> mtu3. Those drivers save "desc" to bsp driver's private data only, 
>> such as "msub_ep->desc = desc", but the common usb_ep's desc is an 
>> invalid point.
>>
>> And I'm not find any documents declare the behaves how 
>> usb_ep_ops->enable did, save desc or not.  So I'd prefect save desc 
>> in the fastboot level, and the udc driver can also save desc again if 
>> they want.
>
> This idiom is also present in f_thor.c, f_rockusb.c, and f_mass_storage.c
>
> Given that a majority of otg implementations set this, I think the 
> other should be changed to match.
>
> --Sean
>
>

[PATCH] driver: gadget: fastboot: Link endpoint and descriptors

[qianfanguijin]

From: qianfan Zhao <qianfanguijin@163.com>

If the downloading file size is equal to the partition size, "fastboot
flash" can't work, at least in sunxi platform, because used an
uninitalized point: ep->desc.

Reproduce: fastboot flash loader1 spl/sunxi-spl.bin.

Fix it.

Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
 drivers/usb/gadget/f_fastboot.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 8ba55aab9f..45bb3aba66 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -319,13 +319,13 @@ static int fastboot_set_alt(struct usb_function *f,
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct usb_gadget *gadget = cdev->gadget;
 	struct f_fastboot *f_fb = func_to_fastboot(f);
-	const struct usb_endpoint_descriptor *d;
 
 	debug("%s: func: %s intf: %d alt: %d\n",
 	      __func__, f->name, interface, alt);
 
-	d = fb_ep_desc(gadget, &fs_ep_out, &hs_ep_out, &ss_ep_out);
-	ret = usb_ep_enable(f_fb->out_ep, d);
+	f_fb->out_ep->desc = fb_ep_desc(gadget, &fs_ep_out, &hs_ep_out,
+					&ss_ep_out);
+	ret = usb_ep_enable(f_fb->out_ep, f_fb->out_ep->desc);
 	if (ret) {
 		puts("failed to enable out ep\n");
 		return ret;
@@ -339,8 +339,8 @@ static int fastboot_set_alt(struct usb_function *f,
 	}
 	f_fb->out_req->complete = rx_handler_command;
 
-	d = fb_ep_desc(gadget, &fs_ep_in, &hs_ep_in, &ss_ep_in);
-	ret = usb_ep_enable(f_fb->in_ep, d);
+	f_fb->in_ep->desc = fb_ep_desc(gadget, &fs_ep_in, &hs_ep_in, &ss_ep_in);
+	ret = usb_ep_enable(f_fb->in_ep, f_fb->in_ep->desc);
 	if (ret) {
 		puts("failed to enable in ep\n");
 		goto err;
-- 
2.17.1

Re: [PATCH] driver: gadget: fastboot: Link endpoint and descriptors

[Sean Anderson]

On 9/16/21 3:02 AM, qianfanguijin@163.com wrote:
> From: qianfan Zhao <qianfanguijin@163.com>
> 
> If the downloading file size is equal to the partition size, "fastboot
> flash" can't work, at least in sunxi platform, because used an
> uninitalized point: ep->desc.

Hm, I think that usb_ep_ops->enable needs to set ep->desc = desc on success.

Of the existing drivers, only musb-new and mtu3 skip this.

--Sean