RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
> > From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com] Hi, > > > > What I mean is that in my opinion it should be done in a separate > > patch, because the newly introduced USB_VBUS_DRAW_SUSPEND is not > used > > anywhere else in your patch. The meaning of this change is "use a > > symbolic name rather than an explicit number" and it is unrelated to > > making composite gadget meet usb compliance for vbus draw. In other > > words, if you don't do this change at all the compliance is still > > maintained, because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, > so > > what the compiler eventually sees is the same whether the change is > > made or not. > > > > My idea: > > > > [PATCHv2 0/2] usb gadget vbus draw compilance > >[PATCHv2 1/2] usb: gadget: composite: avoid using a magic number > >>> substituting an explicit "2" with USB_VBUS_DRAW_SUSPEND goes > > here << > >[PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus > draw > >>> the rest of your changes go here << > > > > > Hi, Andrzej. When I try to split into two patches, I realized that it is not applicable. Because the original code doesn't distinguish different usb types, but new symbols does. So I cannot make a patch just avoid using a magic number without modify the control logic. Hence, I'd prefer doing them in a signal patch. I will send new modified patch. Thanks! Changbin. > > > > Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com] Hi, What I mean is that in my opinion it should be done in a separate patch, because the newly introduced USB_VBUS_DRAW_SUSPEND is not used anywhere else in your patch. The meaning of this change is use a symbolic name rather than an explicit number and it is unrelated to making composite gadget meet usb compliance for vbus draw. In other words, if you don't do this change at all the compliance is still maintained, because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the compiler eventually sees is the same whether the change is made or not. My idea: [PATCHv2 0/2] usb gadget vbus draw compilance [PATCHv2 1/2] usb: gadget: composite: avoid using a magic number substituting an explicit 2 with USB_VBUS_DRAW_SUSPEND goes here [PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw the rest of your changes go here Hi, Andrzej. When I try to split into two patches, I realized that it is not applicable. Because the original code doesn't distinguish different usb types, but new symbols does. So I cannot make a patch just avoid using a magic number without modify the control logic. Hence, I'd prefer doing them in a signal patch. I will send new modified patch. Thanks! Changbin. Andrzej -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Thanks for explanation. I am agree with you that separate changes into two
patches.
Will send out new patch soon.
Regards
Du, Changbin
> From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
> Hi,
>
> W dniu 24.07.2015 o 06:11, Du, Changbin pisze:
> > Thanks, Pietrasiewicz.
> >> From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
> >> W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
> >>> >From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17
> 00:00:00
> >>>void composite_disconnect(struct usb_gadget *gadget)
> >>>{
> >>> struct usb_composite_dev*cdev = get_gadget_data(gadget);
> >>> @@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
> >> *gadget)
> >>>
> >>> cdev->suspended = 1;
> >>>
> >>> - usb_gadget_vbus_draw(gadget, 2);
> >>> + usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
> >>>}
> >>
> >> This looks like an unrelated change. I think it should go first
> >> in a separate patch which eliminates usage of "magic" numbers.
> >>
> > This change does make sense. As you know, when device is reset, it is in a
> 'unconfigured'
> > state. Compliance Test equipment may also measure vbus current at this
> moment.
>
> I am not questioning the change itself.
>
> What I mean is that in my opinion it should be done in a separate patch,
> because the newly introduced USB_VBUS_DRAW_SUSPEND is not used
> anywhere else in your patch. The meaning of this change is "use a symbolic
> name rather than an explicit number" and it is unrelated to
> making composite gadget meet usb compliance for vbus draw. In other
> words,
> if you don't do this change at all the compliance is still maintained,
> because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the
> compiler eventually sees is the same whether the change is made or not.
>
> My idea:
>
> [PATCHv2 0/2] usb gadget vbus draw compilance
>[PATCHv2 1/2] usb: gadget: composite: avoid using a magic number
>>> substituting an explicit "2" with USB_VBUS_DRAW_SUSPEND goes
> here <<
>[PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw
>>> the rest of your changes go here <<
>
> >
> >>>}
> >>> @@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
> >> composite_driver_template = {
> >>> .unbind = composite_unbind,
> >>>
> >>> .setup = composite_setup,
> >>> - .reset = composite_disconnect,
> >>> + .reset = composite_reset,
> >>> .disconnect = composite_disconnect,
> >>>
> >>
> >> A similar "template" is in drivers/usb/gadget/configfs.c. Shouldn't the
> "reset"
> >> method be changed there as well?
> >>
> > Yes, it also need to change. I will change it as well.
> >
> >>
> >>>
> >>> +/* USB2 compliance requires that un-configured current draw <=
> 100mA,
> >>> + * USB3 requires it <= 150mA, OTG requires it <= 2.5mA.
> >>> + */
> >>> +#define USB2_VBUS_DRAW_UNCONF100
> >>> +#define USB3_VBUS_DRAW_UNCONF150
> >>> +#define USB_OTG_VBUS_DRAW_UNCONF 2
> >>
> >>
> >>> +#define USB_VBUS_DRAW_SUSPEND2
> >>
> >> separate patch
> >>
> > Sorry, I didn't get your idea. Why separate these macros definition?
>
> Please see above.
>
> Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Hi,
W dniu 24.07.2015 o 06:11, Du, Changbin pisze:
Thanks, Pietrasiewicz.
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
>From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
*gadget)
cdev->suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of "magic" numbers.
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this moment.
I am not questioning the change itself.
What I mean is that in my opinion it should be done in a separate patch,
because the newly introduced USB_VBUS_DRAW_SUSPEND is not used
anywhere else in your patch. The meaning of this change is "use a symbolic
name rather than an explicit number" and it is unrelated to
making composite gadget meet usb compliance for vbus draw. In other words,
if you don't do this change at all the compliance is still maintained,
because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the
compiler eventually sees is the same whether the change is made or not.
My idea:
[PATCHv2 0/2] usb gadget vbus draw compilance
[PATCHv2 1/2] usb: gadget: composite: avoid using a magic number
>> substituting an explicit "2" with USB_VBUS_DRAW_SUSPEND goes here <<
[PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw
>> the rest of your changes go here <<
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar "template" is in drivers/usb/gadget/configfs.c. Shouldn't the "reset"
method be changed there as well?
Yes, it also need to change. I will change it as well.
+/* USB2 compliance requires that un-configured current draw <= 100mA,
+ * USB3 requires it <= 150mA, OTG requires it <= 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Sorry, I didn't get your idea. Why separate these macros definition?
Please see above.
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Hi,
W dniu 24.07.2015 o 06:11, Du, Changbin pisze:
Thanks, Pietrasiewicz.
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
*gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this moment.
I am not questioning the change itself.
What I mean is that in my opinion it should be done in a separate patch,
because the newly introduced USB_VBUS_DRAW_SUSPEND is not used
anywhere else in your patch. The meaning of this change is use a symbolic
name rather than an explicit number and it is unrelated to
making composite gadget meet usb compliance for vbus draw. In other words,
if you don't do this change at all the compliance is still maintained,
because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the
compiler eventually sees is the same whether the change is made or not.
My idea:
[PATCHv2 0/2] usb gadget vbus draw compilance
[PATCHv2 1/2] usb: gadget: composite: avoid using a magic number
substituting an explicit 2 with USB_VBUS_DRAW_SUSPEND goes here
[PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw
the rest of your changes go here
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the reset
method be changed there as well?
Yes, it also need to change. I will change it as well.
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Sorry, I didn't get your idea. Why separate these macros definition?
Please see above.
Andrzej
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Thanks for explanation. I am agree with you that separate changes into two
patches.
Will send out new patch soon.
Regards
Du, Changbin
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
Hi,
W dniu 24.07.2015 o 06:11, Du, Changbin pisze:
Thanks, Pietrasiewicz.
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17
00:00:00
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
*gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this
moment.
I am not questioning the change itself.
What I mean is that in my opinion it should be done in a separate patch,
because the newly introduced USB_VBUS_DRAW_SUSPEND is not used
anywhere else in your patch. The meaning of this change is use a symbolic
name rather than an explicit number and it is unrelated to
making composite gadget meet usb compliance for vbus draw. In other
words,
if you don't do this change at all the compliance is still maintained,
because the value of USB_VBUS_DRAW_SUSPEND is 2 anyway, so what the
compiler eventually sees is the same whether the change is made or not.
My idea:
[PATCHv2 0/2] usb gadget vbus draw compilance
[PATCHv2 1/2] usb: gadget: composite: avoid using a magic number
substituting an explicit 2 with USB_VBUS_DRAW_SUSPEND goes
here
[PATCHv2 2/2] usb: gadget: composite: meet usb compliance for vbus draw
the rest of your changes go here
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the
reset
method be changed there as well?
Yes, it also need to change. I will change it as well.
+/* USB2 compliance requires that un-configured current draw =
100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF100
+#define USB3_VBUS_DRAW_UNCONF150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND2
separate patch
Sorry, I didn't get your idea. Why separate these macros definition?
Please see above.
Andrzej
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Thanks, Pietrasiewicz.
> From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
> W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
> >>From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00
> > void composite_disconnect(struct usb_gadget *gadget)
> > {
> > struct usb_composite_dev*cdev = get_gadget_data(gadget);
> > @@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
> *gadget)
> >
> > cdev->suspended = 1;
> >
> > - usb_gadget_vbus_draw(gadget, 2);
> > + usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
> > }
>
> This looks like an unrelated change. I think it should go first
> in a separate patch which eliminates usage of "magic" numbers.
>
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this moment.
> > }
> > @@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
> composite_driver_template = {
> > .unbind = composite_unbind,
> >
> > .setup = composite_setup,
> > - .reset = composite_disconnect,
> > + .reset = composite_reset,
> > .disconnect = composite_disconnect,
> >
>
> A similar "template" is in drivers/usb/gadget/configfs.c. Shouldn't the
> "reset"
> method be changed there as well?
>
Yes, it also need to change. I will change it as well.
>
> >
> > +/* USB2 compliance requires that un-configured current draw <= 100mA,
> > + * USB3 requires it <= 150mA, OTG requires it <= 2.5mA.
> > + */
> > +#define USB2_VBUS_DRAW_UNCONF 100
> > +#define USB3_VBUS_DRAW_UNCONF 150
> > +#define USB_OTG_VBUS_DRAW_UNCONF 2
>
>
> > +#define USB_VBUS_DRAW_SUSPEND 2
>
> separate patch
>
Sorry, I didn't get your idea. Why separate these macros definition?
>
> Thanks,
>
> AP
Regards
Changbin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Hi Changbin,
(I assume I address your name properly, if not please excuse)
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev->suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of "magic" numbers.
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev->config->MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev->suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar "template" is in drivers/usb/gadget/configfs.c. Shouldn't the "reset"
method be changed there as well?
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw <= 100mA,
+ * USB3 requires it <= 150mA, OTG requires it <= 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Thanks,
AP
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
>From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
From: "Du, Changbin"
Date: Thu, 23 Jul 2015 20:08:04 +0800
Subject: [PATCH] usb/gadget: make composite gadget meet usb compliance for
vbus draw
USB-IF compliance requirement limits the vbus current according to
current state. USB2 Spec requires that un-configured current must
be <= 100mA, for USB3 is 150mA, OTG is 2.5mA. So set corresponding
vbus draw current according to device mode.
Signed-off-by: Du, Changbin
---
drivers/usb/gadget/composite.c | 39 ---
include/linux/usb/composite.h | 8
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 4e3447b..fdfd625 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -610,6 +610,21 @@ static void device_qual(struct usb_composite_dev *cdev)
qual->bRESERVED = 0;
}
+static unsigned unconfigured_vbus_draw(struct usb_composite_dev *cdev)
+{
+ struct usb_gadget *g = cdev->gadget;
+ unsigned power;
+
+ if (gadget_is_otg(g))
+ power = USB_OTG_VBUS_DRAW_UNCONF;
+ else if (g->speed == USB_SPEED_SUPER)
+ power = USB3_VBUS_DRAW_UNCONF;
+ else
+ power = USB2_VBUS_DRAW_UNCONF;
+
+ return power;
+}
+
/*-*/
static void reset_config(struct usb_composite_dev *cdev)
@@ -634,7 +649,7 @@ static int set_config(struct usb_composite_dev *cdev,
struct usb_gadget *gadget = cdev->gadget;
struct usb_configuration *c = NULL;
int result = -EINVAL;
- unsignedpower = gadget_is_otg(gadget) ? 8 : 100;
+ unsignedpower = unconfigured_vbus_draw(cdev);
int tmp;
if (number) {
@@ -1829,6 +1844,15 @@ done:
return value;
}
+static void composite_reset(struct usb_gadget *gadget)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+
+ DBG(cdev, "reset\n");
+ usb_gadget_vbus_draw(gadget, unconfigured_vbus_draw(cdev));
+ composite_disconnect(gadget);
+}
+
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev->suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev->config->MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev->suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw <= 100mA,
+ * USB3 requires it <= 150mA, OTG requires it <= 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
+
/**
* struct usb_composite_driver - groups configurations into a gadget
* @name: For diagnostics, identifies the driver.
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
From: Du, Changbin changbin...@intel.com
Date: Thu, 23 Jul 2015 20:08:04 +0800
Subject: [PATCH] usb/gadget: make composite gadget meet usb compliance for
vbus draw
USB-IF compliance requirement limits the vbus current according to
current state. USB2 Spec requires that un-configured current must
be = 100mA, for USB3 is 150mA, OTG is 2.5mA. So set corresponding
vbus draw current according to device mode.
Signed-off-by: Du, Changbin changbin...@intel.com
---
drivers/usb/gadget/composite.c | 39 ---
include/linux/usb/composite.h | 8
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 4e3447b..fdfd625 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -610,6 +610,21 @@ static void device_qual(struct usb_composite_dev *cdev)
qual-bRESERVED = 0;
}
+static unsigned unconfigured_vbus_draw(struct usb_composite_dev *cdev)
+{
+ struct usb_gadget *g = cdev-gadget;
+ unsigned power;
+
+ if (gadget_is_otg(g))
+ power = USB_OTG_VBUS_DRAW_UNCONF;
+ else if (g-speed == USB_SPEED_SUPER)
+ power = USB3_VBUS_DRAW_UNCONF;
+ else
+ power = USB2_VBUS_DRAW_UNCONF;
+
+ return power;
+}
+
/*-*/
static void reset_config(struct usb_composite_dev *cdev)
@@ -634,7 +649,7 @@ static int set_config(struct usb_composite_dev *cdev,
struct usb_gadget *gadget = cdev-gadget;
struct usb_configuration *c = NULL;
int result = -EINVAL;
- unsignedpower = gadget_is_otg(gadget) ? 8 : 100;
+ unsignedpower = unconfigured_vbus_draw(cdev);
int tmp;
if (number) {
@@ -1829,6 +1844,15 @@ done:
return value;
}
+static void composite_reset(struct usb_gadget *gadget)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+
+ DBG(cdev, reset\n);
+ usb_gadget_vbus_draw(gadget, unconfigured_vbus_draw(cdev));
+ composite_disconnect(gadget);
+}
+
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev-config-MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev-suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
+
/**
* struct usb_composite_driver - groups configurations into a gadget
* @name: For diagnostics, identifies the driver.
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Thanks, Pietrasiewicz.
From: Andrzej Pietrasiewicz [mailto:andrze...@samsung.com]
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget
*gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
This change does make sense. As you know, when device is reset, it is in a
'unconfigured'
state. Compliance Test equipment may also measure vbus current at this moment.
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the
reset
method be changed there as well?
Yes, it also need to change. I will change it as well.
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Sorry, I didn't get your idea. Why separate these macros definition?
Thanks,
AP
Regards
Changbin
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb/gadget: make composite gadget meet usb compliance for vbus draw
Hi Changbin,
(I assume I address your name properly, if not please excuse)
W dniu 23.07.2015 o 14:34, Du, Changbin pisze:
From 0a8e0d63a9887735c6782d7b0c15c2c1fdf1952a Mon Sep 17 00:00:00 2001
snip
void composite_disconnect(struct usb_gadget *gadget)
{
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2095,7 +2119,7 @@ void composite_suspend(struct usb_gadget *gadget)
cdev-suspended = 1;
- usb_gadget_vbus_draw(gadget, 2);
+ usb_gadget_vbus_draw(gadget, USB_VBUS_DRAW_SUSPEND);
}
This looks like an unrelated change. I think it should go first
in a separate patch which eliminates usage of magic numbers.
void composite_resume(struct usb_gadget *gadget)
@@ -2117,10 +2141,11 @@ void composite_resume(struct usb_gadget *gadget)
}
maxpower = cdev-config-MaxPower;
-
- usb_gadget_vbus_draw(gadget, maxpower ?
- maxpower : CONFIG_USB_GADGET_VBUS_DRAW);
- }
+ if (!maxpower)
+ maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+ } else
+ maxpower = unconfigured_vbus_draw(cdev);
+ usb_gadget_vbus_draw(gadget, maxpower);
cdev-suspended = 0;
}
@@ -2132,7 +2157,7 @@ static const struct usb_gadget_driver
composite_driver_template = {
.unbind = composite_unbind,
.setup = composite_setup,
- .reset = composite_disconnect,
+ .reset = composite_reset,
.disconnect = composite_disconnect,
A similar template is in drivers/usb/gadget/configfs.c. Shouldn't the reset
method be changed there as well?
.suspend= composite_suspend,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 2511469..90b434d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -333,6 +333,14 @@ enum {
USB_GADGET_FIRST_AVAIL_IDX,
};
+/* USB2 compliance requires that un-configured current draw = 100mA,
+ * USB3 requires it = 150mA, OTG requires it = 2.5mA.
+ */
+#define USB2_VBUS_DRAW_UNCONF 100
+#define USB3_VBUS_DRAW_UNCONF 150
+#define USB_OTG_VBUS_DRAW_UNCONF 2
+#define USB_VBUS_DRAW_SUSPEND 2
separate patch
Thanks,
AP
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
