* [PATCH v3] usb: gadget: composite: Show warning if function driver's descriptors are incomplete.
@ 2021-11-09 10:19 Qihang Hu
2021-11-09 12:31 ` Peter Chen
0 siblings, 1 reply; 2+ messages in thread
From: Qihang Hu @ 2021-11-09 10:19 UTC (permalink / raw)
To: balbi, gregkh, peter.chen; +Cc: linux-usb, linux-kernel, Qihang Hu
In the config_ep_by_speed_and_alt function, select the corresponding
descriptor through g->speed. But some unsound function drivers may
not support the corresponding speed. So, we can directly display
warnings instead of panicking the kernel. At the same time, it
indicates a problem with the function in the warning message.
Signed-off-by: Qihang Hu <huqihang@oppo.com>
---
Changes in v2:
-Add warning message
Changes in v3:
-Change commit log
-Delete extra blank lines
-Modify 'incomplete_desc' to bool type
---
drivers/usb/gadget/composite.c | 39 ++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 72a9797dbbae..cb9c7edf9bbf 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -159,6 +159,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
int want_comp_desc = 0;
struct usb_descriptor_header **d_spd; /* cursor for speed desc */
+ struct usb_composite_dev *cdev;
+ bool incomplete_desc = false;
if (!g || !f || !_ep)
return -EIO;
@@ -167,28 +169,43 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
switch (g->speed) {
case USB_SPEED_SUPER_PLUS:
if (gadget_is_superspeed_plus(g)) {
- speed_desc = f->ssp_descriptors;
- want_comp_desc = 1;
- break;
+ if (f->ssp_descriptors) {
+ speed_desc = f->ssp_descriptors;
+ want_comp_desc = 1;
+ break;
+ }
+ incomplete_desc = true;
}
fallthrough;
case USB_SPEED_SUPER:
if (gadget_is_superspeed(g)) {
- speed_desc = f->ss_descriptors;
- want_comp_desc = 1;
- break;
+ if (f->ss_descriptors) {
+ speed_desc = f->ss_descriptors;
+ want_comp_desc = 1;
+ break;
+ }
+ incomplete_desc = true;
}
fallthrough;
case USB_SPEED_HIGH:
if (gadget_is_dualspeed(g)) {
- speed_desc = f->hs_descriptors;
- break;
+ if (f->hs_descriptors) {
+ speed_desc = f->hs_descriptors;
+ break;
+ }
+ incomplete_desc = true;
}
fallthrough;
default:
speed_desc = f->fs_descriptors;
}
+ cdev = get_gadget_data(g);
+ if (incomplete_desc)
+ WARNING(cdev,
+ "%s doesn't hold the descriptors for current speed\n",
+ f->name);
+
/* find correct alternate setting descriptor */
for_each_desc(speed_desc, d_spd, USB_DT_INTERFACE) {
int_desc = (struct usb_interface_descriptor *)*d_spd;
@@ -244,12 +261,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
_ep->maxburst = comp_desc->bMaxBurst + 1;
break;
default:
- if (comp_desc->bMaxBurst != 0) {
- struct usb_composite_dev *cdev;
-
- cdev = get_gadget_data(g);
+ if (comp_desc->bMaxBurst != 0)
ERROR(cdev, "ep0 bMaxBurst must be 0\n");
- }
_ep->maxburst = 1;
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] usb: gadget: composite: Show warning if function driver's descriptors are incomplete.
2021-11-09 10:19 [PATCH v3] usb: gadget: composite: Show warning if function driver's descriptors are incomplete Qihang Hu
@ 2021-11-09 12:31 ` Peter Chen
0 siblings, 0 replies; 2+ messages in thread
From: Peter Chen @ 2021-11-09 12:31 UTC (permalink / raw)
To: Qihang Hu; +Cc: balbi, gregkh, linux-usb, linux-kernel
On 21-11-09 18:19:36, Qihang Hu wrote:
> In the config_ep_by_speed_and_alt function, select the corresponding
> descriptor through g->speed. But some unsound function drivers may
But some 'legacy or not well designed' function drivers
> not support the corresponding speed. So, we can directly display
> warnings instead of panicking the kernel.
instead of 'causing kernel panic'
> At the same time, it
> indicates a problem with the function in the warning message.
It indicates the reasons in warning message.
>
> Signed-off-by: Qihang Hu <huqihang@oppo.com>
> ---
> Changes in v2:
> -Add warning message
>
> Changes in v3:
> -Change commit log
> -Delete extra blank lines
> -Modify 'incomplete_desc' to bool type
The latest changelog should be showed at the first.
> ---
> drivers/usb/gadget/composite.c | 39 ++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 72a9797dbbae..cb9c7edf9bbf 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -159,6 +159,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
> int want_comp_desc = 0;
>
> struct usb_descriptor_header **d_spd; /* cursor for speed desc */
> + struct usb_composite_dev *cdev;
> + bool incomplete_desc = false;
>
> if (!g || !f || !_ep)
> return -EIO;
> @@ -167,28 +169,43 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
> switch (g->speed) {
> case USB_SPEED_SUPER_PLUS:
> if (gadget_is_superspeed_plus(g)) {
> - speed_desc = f->ssp_descriptors;
> - want_comp_desc = 1;
> - break;
> + if (f->ssp_descriptors) {
> + speed_desc = f->ssp_descriptors;
> + want_comp_desc = 1;
> + break;
> + }
> + incomplete_desc = true;
> }
> fallthrough;
> case USB_SPEED_SUPER:
> if (gadget_is_superspeed(g)) {
> - speed_desc = f->ss_descriptors;
> - want_comp_desc = 1;
> - break;
> + if (f->ss_descriptors) {
> + speed_desc = f->ss_descriptors;
> + want_comp_desc = 1;
> + break;
> + }
> + incomplete_desc = true;
> }
> fallthrough;
> case USB_SPEED_HIGH:
> if (gadget_is_dualspeed(g)) {
> - speed_desc = f->hs_descriptors;
> - break;
> + if (f->hs_descriptors) {
> + speed_desc = f->hs_descriptors;
> + break;
> + }
> + incomplete_desc = true;
> }
> fallthrough;
> default:
> speed_desc = f->fs_descriptors;
> }
>
> + cdev = get_gadget_data(g);
> + if (incomplete_desc)
> + WARNING(cdev,
> + "%s doesn't hold the descriptors for current speed\n",
> + f->name);
> +
> /* find correct alternate setting descriptor */
> for_each_desc(speed_desc, d_spd, USB_DT_INTERFACE) {
> int_desc = (struct usb_interface_descriptor *)*d_spd;
> @@ -244,12 +261,8 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g,
> _ep->maxburst = comp_desc->bMaxBurst + 1;
> break;
> default:
> - if (comp_desc->bMaxBurst != 0) {
> - struct usb_composite_dev *cdev;
> -
> - cdev = get_gadget_data(g);
> + if (comp_desc->bMaxBurst != 0)
> ERROR(cdev, "ep0 bMaxBurst must be 0\n");
> - }
> _ep->maxburst = 1;
> break;
> }
> --
> 2.25.1
>
Except the typo issues, other things are ok for me. It shows an warning
message for not well designed function driver, instead of panic
the kernel. It depends on Greg that whether it should be panic directly
or show warning message to indicate the issue.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-09 12:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 10:19 [PATCH v3] usb: gadget: composite: Show warning if function driver's descriptors are incomplete Qihang Hu
2021-11-09 12:31 ` Peter Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.