From: Felipe Balbi <[email protected]>
When going into bus suspend/resume we _must_
call gadget driver's ->suspend/->resume callbacks
accordingly. This patch implements that very feature
which has been missing forever.
Cc: <[email protected]> # 3.14
Signed-off-by: Felipe Balbi <[email protected]>
Signed-off-by: David Cohen <[email protected]>
---
Hi,
This patch was introduced on v3.15.
But the issue it fixes already existed on v3.14 and v3.14 is a long term
support version.
I propose to backport it over there as well.
BR, David
---
drivers/usb/dwc3/gadget.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8f6738d46b14..1bb752736c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2012,6 +2012,24 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
}
}
+static void dwc3_suspend_gadget(struct dwc3 *dwc)
+{
+ if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
+ spin_unlock(&dwc->lock);
+ dwc->gadget_driver->suspend(&dwc->gadget);
+ spin_lock(&dwc->lock);
+ }
+}
+
+static void dwc3_resume_gadget(struct dwc3 *dwc)
+{
+ if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
+ spin_unlock(&dwc->lock);
+ dwc->gadget_driver->resume(&dwc->gadget);
+ spin_lock(&dwc->lock);
+ }
+}
+
static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
{
struct dwc3_ep *dep;
@@ -2391,6 +2409,23 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
dwc->link_state = next;
+ switch (next) {
+ case DWC3_LINK_STATE_U1:
+ if (dwc->speed == USB_SPEED_SUPER)
+ dwc3_suspend_gadget(dwc);
+ break;
+ case DWC3_LINK_STATE_U2:
+ case DWC3_LINK_STATE_U3:
+ dwc3_suspend_gadget(dwc);
+ break;
+ case DWC3_LINK_STATE_RESUME:
+ dwc3_resume_gadget(dwc);
+ break;
+ default:
+ /* do nothing */
+ break;
+ }
+
dev_vdbg(dwc->dev, "%s link %d\n", __func__, dwc->link_state);
}
--
2.1.4
On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> From: Felipe Balbi <[email protected]>
>
> When going into bus suspend/resume we _must_
> call gadget driver's ->suspend/->resume callbacks
> accordingly. This patch implements that very feature
> which has been missing forever.
>
> Cc: <[email protected]> # 3.14
> Signed-off-by: Felipe Balbi <[email protected]>
> Signed-off-by: David Cohen <[email protected]>
> ---
>
> Hi,
>
> This patch was introduced on v3.15.
> But the issue it fixes already existed on v3.14 and v3.14 is a long term
> support version.
> I propose to backport it over there as well.
What is the git commit id of the patch in Linus's tree?
thanks,
greg k-h
On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> From: Felipe Balbi <[email protected]>
missing the required:
[ Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9 ]
>
> When going into bus suspend/resume we _must_
> call gadget driver's ->suspend/->resume callbacks
> accordingly. This patch implements that very feature
> which has been missing forever.
>
> Cc: <[email protected]> # 3.14
> Signed-off-by: Felipe Balbi <[email protected]>
> Signed-off-by: David Cohen <[email protected]>
> ---
>
> Hi,
>
> This patch was introduced on v3.15.
> But the issue it fixes already existed on v3.14 and v3.14 is a long term
> support version.
Can you show me a log of this breaking anywhere ? Why do you consider
this a bug fix ? What sort of drawbacks did you notice ?
> I propose to backport it over there as well.
>
> BR, David
> ---
>
> drivers/usb/dwc3/gadget.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 8f6738d46b14..1bb752736c32 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2012,6 +2012,24 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
> }
> }
>
> +static void dwc3_suspend_gadget(struct dwc3 *dwc)
> +{
> + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
you also need Dan Carperter's commit which fixes this cut & paste error.
That's commit 73a30bfc0d526db899033165db6f95c427e70505
> + spin_unlock(&dwc->lock);
> + dwc->gadget_driver->suspend(&dwc->gadget);
> + spin_lock(&dwc->lock);
> + }
> +}
> +
> +static void dwc3_resume_gadget(struct dwc3 *dwc)
> +{
> + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
> + spin_unlock(&dwc->lock);
> + dwc->gadget_driver->resume(&dwc->gadget);
> + spin_lock(&dwc->lock);
> + }
> +}
> +
> static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
> {
> struct dwc3_ep *dep;
> @@ -2391,6 +2409,23 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
>
> dwc->link_state = next;
>
> + switch (next) {
> + case DWC3_LINK_STATE_U1:
> + if (dwc->speed == USB_SPEED_SUPER)
> + dwc3_suspend_gadget(dwc);
> + break;
> + case DWC3_LINK_STATE_U2:
> + case DWC3_LINK_STATE_U3:
> + dwc3_suspend_gadget(dwc);
> + break;
> + case DWC3_LINK_STATE_RESUME:
> + dwc3_resume_gadget(dwc);
> + break;
> + default:
> + /* do nothing */
> + break;
> + }
> +
> dev_vdbg(dwc->dev, "%s link %d\n", __func__, dwc->link_state);
> }
--
balbi
On Fri, Apr 17, 2015 at 02:43:27PM -0500, Felipe Balbi wrote:
> On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> > From: Felipe Balbi <[email protected]>
>
> missing the required:
>
> [ Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9 ]
>
> >
> > When going into bus suspend/resume we _must_
> > call gadget driver's ->suspend/->resume callbacks
> > accordingly. This patch implements that very feature
> > which has been missing forever.
> >
> > Cc: <[email protected]> # 3.14
> > Signed-off-by: Felipe Balbi <[email protected]>
> > Signed-off-by: David Cohen <[email protected]>
> > ---
> >
> > Hi,
> >
> > This patch was introduced on v3.15.
> > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > support version.
>
> Can you show me a log of this breaking anywhere ? Why do you consider
> this a bug fix ? What sort of drawbacks did you notice ?
>
> > I propose to backport it over there as well.
> >
> > BR, David
> > ---
> >
> > drivers/usb/dwc3/gadget.c | 35 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 8f6738d46b14..1bb752736c32 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -2012,6 +2012,24 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
> > }
> > }
> >
> > +static void dwc3_suspend_gadget(struct dwc3 *dwc)
> > +{
> > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
>
> you also need Dan Carperter's commit which fixes this cut & paste error.
That's Carpenter, sorry.
--
balbi
Hi Felipe,
On Fri, Apr 17, 2015 at 02:43:27PM -0500, Felipe Balbi wrote:
> On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> > From: Felipe Balbi <[email protected]>
>
> missing the required:
>
> [ Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9 ]
>
> >
> > When going into bus suspend/resume we _must_
> > call gadget driver's ->suspend/->resume callbacks
> > accordingly. This patch implements that very feature
> > which has been missing forever.
> >
> > Cc: <[email protected]> # 3.14
> > Signed-off-by: Felipe Balbi <[email protected]>
> > Signed-off-by: David Cohen <[email protected]>
> > ---
> >
> > Hi,
> >
> > This patch was introduced on v3.15.
> > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > support version.
>
> Can you show me a log of this breaking anywhere ? Why do you consider
> this a bug fix ? What sort of drawbacks did you notice ?
We're seeing BC1.2 compliance test failure. I borrowed this info from
the bug report :)
1. BC1.2 compliance testing - SDP2.0
-----------------------------------------------
1. On Connect to active Host (Expected result: 100mA to 500mA):
Actual result 100mA to 500mA
2. On Host Suspend (ER: Fall back to 0mA):
not falling back to 0mA, remains at 500mA
3. On Connect to Suspended Host (ER: 100mA to 0mA):
cable-props shown as 100mA, which means drawing a current of 100mA from
Suspended Host
4. On making Host active (ER: 500mA):
500mA
>
> > I propose to backport it over there as well.
> >
> > BR, David
> > ---
> >
> > drivers/usb/dwc3/gadget.c | 35 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 8f6738d46b14..1bb752736c32 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -2012,6 +2012,24 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
> > }
> > }
> >
> > +static void dwc3_suspend_gadget(struct dwc3 *dwc)
> > +{
> > + if (dwc->gadget_driver && dwc->gadget_driver->disconnect) {
>
> you also need Dan Carperter's commit which fixes this cut & paste error.
> That's commit 73a30bfc0d526db899033165db6f95c427e70505
Thanks. I'll add that to my next try.
Br, David
Hi Greg,
On Fri, Apr 17, 2015 at 09:42:57PM +0200, Greg KH wrote:
> On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> > From: Felipe Balbi <[email protected]>
> >
> > When going into bus suspend/resume we _must_
> > call gadget driver's ->suspend/->resume callbacks
> > accordingly. This patch implements that very feature
> > which has been missing forever.
> >
> > Cc: <[email protected]> # 3.14
> > Signed-off-by: Felipe Balbi <[email protected]>
> > Signed-off-by: David Cohen <[email protected]>
> > ---
> >
> > Hi,
> >
> > This patch was introduced on v3.15.
> > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > support version.
> > I propose to backport it over there as well.
>
> What is the git commit id of the patch in Linus's tree?
Sorry for the missing info. As Felipe replied, this is the commit:
Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9
I'm resending it to fix the commit message and add a second patch as
well.
Br, David
>
> thanks,
>
> greg k-h
Hi,
On Thu, Apr 23, 2015 at 03:37:48PM -0700, David Cohen wrote:
> On Fri, Apr 17, 2015 at 02:43:27PM -0500, Felipe Balbi wrote:
> > On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> > > From: Felipe Balbi <[email protected]>
> >
> > missing the required:
> >
> > [ Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9 ]
> >
> > >
> > > When going into bus suspend/resume we _must_
> > > call gadget driver's ->suspend/->resume callbacks
> > > accordingly. This patch implements that very feature
> > > which has been missing forever.
> > >
> > > Cc: <[email protected]> # 3.14
> > > Signed-off-by: Felipe Balbi <[email protected]>
> > > Signed-off-by: David Cohen <[email protected]>
> > > ---
> > >
> > > Hi,
> > >
> > > This patch was introduced on v3.15.
> > > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > > support version.
> >
> > Can you show me a log of this breaking anywhere ? Why do you consider
> > this a bug fix ? What sort of drawbacks did you notice ?
>
> We're seeing BC1.2 compliance test failure. I borrowed this info from
> the bug report :)
>
> 1. BC1.2 compliance testing - SDP2.0
> -----------------------------------------------
> 1. On Connect to active Host (Expected result: 100mA to 500mA):
> Actual result 100mA to 500mA
>
> 2. On Host Suspend (ER: Fall back to 0mA):
> not falling back to 0mA, remains at 500mA
>
> 3. On Connect to Suspended Host (ER: 100mA to 0mA):
> cable-props shown as 100mA, which means drawing a current of 100mA from
> Suspended Host
>
> 4. On making Host active (ER: 500mA):
> 500mA
But we don't support Battery Charging with dwc3 as of now :-) In fact,
just note that none of the BC registers are even defined in the current
driver anywhere. Seems like you should cherry-pick these to your vendor
tree, but v3.14 vanilla, because it doesn't support BC1.2, can't be
claimed to be at fault, right ?
I'll leave the final decision to Greg and I don't really oppose having
both patches on v3.14-stable, but this is not a bug fix in my view.
--
balbi
On Fri, Apr 24, 2015 at 02:48:27PM -0500, Felipe Balbi wrote:
> Hi,
Hi Felipe,
>
> On Thu, Apr 23, 2015 at 03:37:48PM -0700, David Cohen wrote:
> > On Fri, Apr 17, 2015 at 02:43:27PM -0500, Felipe Balbi wrote:
> > > On Fri, Apr 17, 2015 at 11:41:56AM -0700, David Cohen wrote:
> > > > From: Felipe Balbi <[email protected]>
> > >
> > > missing the required:
> > >
> > > [ Upstream commit bc5ba2e0b829c9397f96df1191c7d2319ebc36d9 ]
> > >
> > > >
> > > > When going into bus suspend/resume we _must_
> > > > call gadget driver's ->suspend/->resume callbacks
> > > > accordingly. This patch implements that very feature
> > > > which has been missing forever.
> > > >
> > > > Cc: <[email protected]> # 3.14
> > > > Signed-off-by: Felipe Balbi <[email protected]>
> > > > Signed-off-by: David Cohen <[email protected]>
> > > > ---
> > > >
> > > > Hi,
> > > >
> > > > This patch was introduced on v3.15.
> > > > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > > > support version.
> > >
> > > Can you show me a log of this breaking anywhere ? Why do you consider
> > > this a bug fix ? What sort of drawbacks did you notice ?
> >
> > We're seeing BC1.2 compliance test failure. I borrowed this info from
> > the bug report :)
> >
> > 1. BC1.2 compliance testing - SDP2.0
> > -----------------------------------------------
> > 1. On Connect to active Host (Expected result: 100mA to 500mA):
> > Actual result 100mA to 500mA
> >
> > 2. On Host Suspend (ER: Fall back to 0mA):
> > not falling back to 0mA, remains at 500mA
> >
> > 3. On Connect to Suspended Host (ER: 100mA to 0mA):
> > cable-props shown as 100mA, which means drawing a current of 100mA from
> > Suspended Host
> >
> > 4. On making Host active (ER: 500mA):
> > 500mA
>
> But we don't support Battery Charging with dwc3 as of now :-) In fact,
> just note that none of the BC registers are even defined in the current
> driver anywhere. Seems like you should cherry-pick these to your vendor
> tree, but v3.14 vanilla, because it doesn't support BC1.2, can't be
> claimed to be at fault, right ?
We could call it a missing feature that could lead to a potential bug :)
By your own comment, he "must" was stressed out:
'''
When going into bus suspend/resume we _must_
call gadget driver's ->suspend/->resume callbacks
accordingly. This patch implements that very feature
which has been missing forever.
'''
Since v3.14 is a LTS kernel and the changes are safe, it's worth to
consider.
>
> I'll leave the final decision to Greg and I don't really oppose having
> both patches on v3.14-stable, but this is not a bug fix in my view.
Thanks. I appreciate your feedback.
BR, David
PS: FWIW implementing features or fixing bugs aren't much different tasks:
https://geekwhisperin.files.wordpress.com/2009/09/bug-vs-feature.jpg
Hi,
On Fri, Apr 24, 2015 at 01:56:25PM -0700, David Cohen wrote:
> > > > > When going into bus suspend/resume we _must_
> > > > > call gadget driver's ->suspend/->resume callbacks
> > > > > accordingly. This patch implements that very feature
> > > > > which has been missing forever.
> > > > >
> > > > > Cc: <[email protected]> # 3.14
> > > > > Signed-off-by: Felipe Balbi <[email protected]>
> > > > > Signed-off-by: David Cohen <[email protected]>
> > > > > ---
> > > > >
> > > > > Hi,
> > > > >
> > > > > This patch was introduced on v3.15.
> > > > > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > > > > support version.
> > > >
> > > > Can you show me a log of this breaking anywhere ? Why do you consider
> > > > this a bug fix ? What sort of drawbacks did you notice ?
> > >
> > > We're seeing BC1.2 compliance test failure. I borrowed this info from
> > > the bug report :)
> > >
> > > 1. BC1.2 compliance testing - SDP2.0
> > > -----------------------------------------------
> > > 1. On Connect to active Host (Expected result: 100mA to 500mA):
> > > Actual result 100mA to 500mA
> > >
> > > 2. On Host Suspend (ER: Fall back to 0mA):
> > > not falling back to 0mA, remains at 500mA
> > >
> > > 3. On Connect to Suspended Host (ER: 100mA to 0mA):
> > > cable-props shown as 100mA, which means drawing a current of 100mA from
> > > Suspended Host
> > >
> > > 4. On making Host active (ER: 500mA):
> > > 500mA
> >
> > But we don't support Battery Charging with dwc3 as of now :-) In fact,
> > just note that none of the BC registers are even defined in the current
> > driver anywhere. Seems like you should cherry-pick these to your vendor
> > tree, but v3.14 vanilla, because it doesn't support BC1.2, can't be
> > claimed to be at fault, right ?
>
> We could call it a missing feature that could lead to a potential bug :)
> By your own comment, he "must" was stressed out:
sure it was because they really must be called :-) However, the only
actual problem that arises from not calling them is with something
that's not supported :-)
> When going into bus suspend/resume we _must_
> call gadget driver's ->suspend/->resume callbacks
> accordingly. This patch implements that very feature
> which has been missing forever.
> '''
>
> Since v3.14 is a LTS kernel and the changes are safe, it's worth to
> consider.
definitely worth to consider, but not as something that fixes BC1.2
because that's, as said, not supported in any public tree :-)
> > I'll leave the final decision to Greg and I don't really oppose having
> > both patches on v3.14-stable, but this is not a bug fix in my view.
>
> Thanks. I appreciate your feedback.
>
> BR, David
>
> PS: FWIW implementing features or fixing bugs aren't much different tasks:
> https://geekwhisperin.files.wordpress.com/2009/09/bug-vs-feature.jpg
Yes, I used to have that on my office door ;-)
--
balbi
On Sat, Apr 25, 2015 at 10:47:42AM -0500, Felipe Balbi wrote:
> Hi,
Hi Felipe,
>
> On Fri, Apr 24, 2015 at 01:56:25PM -0700, David Cohen wrote:
> > > > > > When going into bus suspend/resume we _must_
> > > > > > call gadget driver's ->suspend/->resume callbacks
> > > > > > accordingly. This patch implements that very feature
> > > > > > which has been missing forever.
> > > > > >
> > > > > > Cc: <[email protected]> # 3.14
> > > > > > Signed-off-by: Felipe Balbi <[email protected]>
> > > > > > Signed-off-by: David Cohen <[email protected]>
> > > > > > ---
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This patch was introduced on v3.15.
> > > > > > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > > > > > support version.
> > > > >
> > > > > Can you show me a log of this breaking anywhere ? Why do you consider
> > > > > this a bug fix ? What sort of drawbacks did you notice ?
> > > >
> > > > We're seeing BC1.2 compliance test failure. I borrowed this info from
> > > > the bug report :)
> > > >
> > > > 1. BC1.2 compliance testing - SDP2.0
> > > > -----------------------------------------------
> > > > 1. On Connect to active Host (Expected result: 100mA to 500mA):
> > > > Actual result 100mA to 500mA
> > > >
> > > > 2. On Host Suspend (ER: Fall back to 0mA):
> > > > not falling back to 0mA, remains at 500mA
> > > >
> > > > 3. On Connect to Suspended Host (ER: 100mA to 0mA):
> > > > cable-props shown as 100mA, which means drawing a current of 100mA from
> > > > Suspended Host
> > > >
> > > > 4. On making Host active (ER: 500mA):
> > > > 500mA
> > >
> > > But we don't support Battery Charging with dwc3 as of now :-) In fact,
> > > just note that none of the BC registers are even defined in the current
> > > driver anywhere. Seems like you should cherry-pick these to your vendor
> > > tree, but v3.14 vanilla, because it doesn't support BC1.2, can't be
> > > claimed to be at fault, right ?
> >
> > We could call it a missing feature that could lead to a potential bug :)
> > By your own comment, he "must" was stressed out:
>
> sure it was because they really must be called :-) However, the only
> actual problem that arises from not calling them is with something
> that's not supported :-)
>
> > When going into bus suspend/resume we _must_
> > call gadget driver's ->suspend/->resume callbacks
> > accordingly. This patch implements that very feature
> > which has been missing forever.
> > '''
> >
> > Since v3.14 is a LTS kernel and the changes are safe, it's worth to
> > consider.
>
> definitely worth to consider, but not as something that fixes BC1.2
> because that's, as said, not supported in any public tree :-)
Thanks for the reply.
The gadget having this issue is really out-of-tree (android gadget).
I could do 2 next steps:
1) I could drop android gadget and try to reproduce this issue using
legacy g_ffs, which supports adbd as well.
2) I could propose this change directly on google's android 3.14 tree
instead.
Would you prefer one of those?
Thanks,
David
On Mon, Apr 27, 2015 at 07:55:58AM -0700, David Cohen wrote:
> On Sat, Apr 25, 2015 at 10:47:42AM -0500, Felipe Balbi wrote:
> > Hi,
>
> Hi Felipe,
>
> >
> > On Fri, Apr 24, 2015 at 01:56:25PM -0700, David Cohen wrote:
> > > > > > > When going into bus suspend/resume we _must_
> > > > > > > call gadget driver's ->suspend/->resume callbacks
> > > > > > > accordingly. This patch implements that very feature
> > > > > > > which has been missing forever.
> > > > > > >
> > > > > > > Cc: <[email protected]> # 3.14
> > > > > > > Signed-off-by: Felipe Balbi <[email protected]>
> > > > > > > Signed-off-by: David Cohen <[email protected]>
> > > > > > > ---
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > This patch was introduced on v3.15.
> > > > > > > But the issue it fixes already existed on v3.14 and v3.14 is a long term
> > > > > > > support version.
> > > > > >
> > > > > > Can you show me a log of this breaking anywhere ? Why do you consider
> > > > > > this a bug fix ? What sort of drawbacks did you notice ?
> > > > >
> > > > > We're seeing BC1.2 compliance test failure. I borrowed this info from
> > > > > the bug report :)
> > > > >
> > > > > 1. BC1.2 compliance testing - SDP2.0
> > > > > -----------------------------------------------
> > > > > 1. On Connect to active Host (Expected result: 100mA to 500mA):
> > > > > Actual result 100mA to 500mA
> > > > >
> > > > > 2. On Host Suspend (ER: Fall back to 0mA):
> > > > > not falling back to 0mA, remains at 500mA
> > > > >
> > > > > 3. On Connect to Suspended Host (ER: 100mA to 0mA):
> > > > > cable-props shown as 100mA, which means drawing a current of 100mA from
> > > > > Suspended Host
> > > > >
> > > > > 4. On making Host active (ER: 500mA):
> > > > > 500mA
> > > >
> > > > But we don't support Battery Charging with dwc3 as of now :-) In fact,
> > > > just note that none of the BC registers are even defined in the current
> > > > driver anywhere. Seems like you should cherry-pick these to your vendor
> > > > tree, but v3.14 vanilla, because it doesn't support BC1.2, can't be
> > > > claimed to be at fault, right ?
> > >
> > > We could call it a missing feature that could lead to a potential bug :)
> > > By your own comment, he "must" was stressed out:
> >
> > sure it was because they really must be called :-) However, the only
> > actual problem that arises from not calling them is with something
> > that's not supported :-)
> >
> > > When going into bus suspend/resume we _must_
> > > call gadget driver's ->suspend/->resume callbacks
> > > accordingly. This patch implements that very feature
> > > which has been missing forever.
> > > '''
> > >
> > > Since v3.14 is a LTS kernel and the changes are safe, it's worth to
> > > consider.
> >
> > definitely worth to consider, but not as something that fixes BC1.2
> > because that's, as said, not supported in any public tree :-)
>
> Thanks for the reply.
> The gadget having this issue is really out-of-tree (android gadget).
> I could do 2 next steps:
> 1) I could drop android gadget and try to reproduce this issue using
> legacy g_ffs, which supports adbd as well.
> 2) I could propose this change directly on google's android 3.14 tree
> instead.
>
> Would you prefer one of those?
hehe, I'll let Greg decide, I'm not against the patch, as I said before,
but I'm also not entirely sure it fits within stable rules ;-)
cheers
--
balbi
