Re: Oddity with EP configuration

[Benjamin Herrenschmidt]

On Wed, 2017-06-14 at 12:00 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2017-06-14 at 10:33 +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-06-13 at 15:08 +1000, Benjamin Herrenschmidt wrote:
> > > Now, what I observe is that when the mass storage gets bound to the
> > > UDC driver:
> > > 
> > >  - First the endpoints are properly allocated by my match callback,
> > > ie, epautoconf is called to allocate EPs to the function.
> > > 
> > >  - But right away, composite.c calls usb_ep_autoconfig_release()
> > > effectively releasing those 2 endpoints.
> > 
> > I confirmed this by reverting to my old "single UDC" driver (so a more
> > standard setup without all my hooks) and putting a WARN_ON(!ep-
> > > claimed) in usb_ep_enable().
> 
> Ok,  this is a global issue. Creating functions via configfs leads
> to the same problem. I've added traces to epautoconf and to EP release
> and reset and you can see what happens here (I also have the
> WARN_ON(!ep->claimed) in usb_ep_enable):

I'm not having the stinking feeling that this is somewhat done on
purpose. ..

Since only one config is active at a time an the EPs are only
"enabled" when the config is activated (well set_alt really),
the same sets of EPs can be re-used for different configs and
that's pretty much what the code tries to do...

So an EP being "claimed" only matter within a single invocation of
usb_add_config() to sort out between the interfaces in that config...

Now if that's the case then it will of course not work for me as
I need to allocate from my global pool of EPs accross several gadgets
and thus several configs.

That leaves me with few solutions, what do you recommend:

 - The grossest one ... have the DT specify statically how many EPs
from the pool are assigned to each of the 5 gadget slots.

 - A better one but with nasty failure mode ... create a whole lot 
(all 15 ?) EP structs for each gadgets and internally allocate the HW
EPs in the enable() callback. That means it can fail at annoying times
such as when the host does a set_configuration etc...

 - Continue allocating EPs in match_ep, but release all the EPs for a
given UDC in a new UDC hook udc->gadget->ops->unbind() (or an EP op
"unbind" called for all EPs in the ep_list by the core at unbind time).
This means that we'll cleanup on unbind properly. We still end up
over allocating if, before bind, we create configs and delete some.

 - A variant of the above where we add tracking of the gadget and
#configs owning the EPs. That allows the cleanup to be done (via a new
EP unbind/release callback) from composite.c when a config is deleted,
by properly refcounting how many configs are using an EP. That's harder
as it's a bit more invasive on the overall core.

Any recommendation ? I'm happy to provide patches. I'll experiement
with the above approaches while waiting for your replies.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Gadget driver ->disconnect callback during unbinding

[Peter Chen]

On Tue, Jun 13, 2017 at 10:22:26AM -0400, Alan Stern wrote:
> On Tue, 13 Jun 2017, Felipe Balbi wrote:
> 
> > 
> > Hi Alan,
> > 
> > Alan Stern  writes:
> > > Felipe:
> > >
> > > A UDC driver will invoke the gadget driver's ->disconnect callback
> > > whenever the D+ pullup is turned off.  During gadget driver unbinding,
> > > this happens automatically when usb_gadget_remove_driver() calls
> > > usb_gadget_disconnect().
> > 
> > usb_gadget_disconnect() only calls UDC's ->pullup(), not gadget driver's
> > ->disconnect()
> > 
> > int usb_gadget_disconnect(struct usb_gadget *gadget)
> > {
> > int ret = 0;
> > 
> > if (!gadget->ops->pullup) {
> > ret = -EOPNOTSUPP;
> > goto out;
> > }
> > 
> > if (gadget->deactivated) {
> > /*
> >  * If gadget is deactivated we only save new state.
> >  * Gadget will stay disconnected after activation.
> >  */
> > gadget->connected = false;
> > goto out;
> > }
> > 
> > ret = gadget->ops->pullup(gadget, 0);
> > if (!ret)
> > gadget->connected = 0;
> > 
> > out:
> > trace_usb_gadget_disconnect(gadget, ret);
> > 
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(usb_gadget_disconnect);
> 
> Yes, but as I mentioned above, the pullup routine calls the gadget
> driver's disconnect method.
> 
> > > But immediately thereafter, usb_gadget_remove_driver() calls the gadget 
> > > driver's ->disconnect callback directly!  Do we have any reason for 
> > > doing this?  I don't see point to it.  Should that call be removed?
> > 
> > Unless I'm missing something, that call is necessary :-) Have you faced
> > any issues with it? Are there any UDCs calling gadget driver's
> > ->disconnect() from ->pullup() ?
> 
> I haven't faced any issues because gadget drivers don't seem to mind 
> ->disconnect getting called twice in a row.  :-)  (And note that the 
> API doesn't have a corresponding ->connect callback...  Although to 
> tell the truth, it's not clear what a gadget driver needs to do in 
> either callback.  Can we simply remove ->disconnect altogether?)
> 
> Both dummy-hcd and net2280 call ->disconnect from ->pullup.  I haven't
> checked other UDC drivers, but it seems like something they should all
> do.  Except perhaps in the case where the UDC driver doesn't have a
> pullup method.
> 

No, ->pullup is expected to be called from UDC core, and the UDC core
makes sure the coming ->disconnect at kinds of situations.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Oddity with EP configuration

[Benjamin Herrenschmidt]

On Wed, 2017-06-14 at 10:33 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-06-13 at 15:08 +1000, Benjamin Herrenschmidt wrote:
> > Now, what I observe is that when the mass storage gets bound to the
> > UDC driver:
> > 
> >  - First the endpoints are properly allocated by my match callback,
> > ie, epautoconf is called to allocate EPs to the function.
> > 
> >  - But right away, composite.c calls usb_ep_autoconfig_release()
> > effectively releasing those 2 endpoints.
> 
> I confirmed this by reverting to my old "single UDC" driver (so a more
> standard setup without all my hooks) and putting a WARN_ON(!ep-
> > claimed) in usb_ep_enable().

Ok,  this is a global issue. Creating functions via configfs leads
to the same problem. I've added traces to epautoconf and to EP release
and reset and you can see what happens here (I also have the
WARN_ON(!ep->claimed) in usb_ep_enable):

# cd /sys/kernel/config/usb_gadget
# mkdir g1
# cd g1
# echo "0x1d6b" > idVendor
# echo "0x0104" > idProduct
# mkdir strings/0x409
# echo "0123456789" > strings/0x409/serialnumber
# echo "Foo Inc." > strings/0x409/manufacturer
# echo "Bar Gadget" > strings/0x409/product
# mkdir configs/c.1
# mkdir configs/c.1/strings/0x409
# echo "Main config" > configs/c.1/strings/0x409/configuration
# mkdir functions/mass_storage.usb0
[   69.24] Mass Storage Function, version: 2009/09/11
[   69.24] LUN: removable file: (no medium)
# ln -s functions/mass_storage.usb0 configs/c.1
# echo "1e6a.usb-vhub" >UDC
[   78.20] udc 1e6a.usb-vhub: registering UDC driver [g1]
[   78.20] EP@9e11b244 "ep1" reset
[   78.21] EP@9e11b2a0 "ep2" reset
[   78.21] EP@9e11b2fc "ep3" reset
[   78.21] EP@9e11b358 "ep4" reset
[   78.21] EP@9e11b3b4 "ep5" reset
[   78.21] EP@9e11b410 "ep6" reset
[   78.21] EP@9e11b46c "ep7" reset
[   78.23] EP@9e11b4c8 "ep8" reset
[   78.23] EP@9e11b524 "ep9" reset
[   78.23] EP@9e11b580 "ep10" reset
[   78.23] EP@9e11b5dc "ep11" reset
[   78.23] EP@9e11b638 "ep12" reset
[   78.23] EP@9e11b694 "ep13" reset
[   78.23] EP@9e11b6f0 "ep14" reset
[   78.23] EP@9e11b74c "ep15" reset
[   78.26] configfs-gadget gadget: adding 'Mass Storage Function'/9e0c8b60 
to config 'c'/9f4a06a8
[   78.26] configfs-gadget gadget: I/O thread pid: 502
[   78.26] EP@9e11b244 "ep1" claimed
[   78.26] EP@9e11b2a0 "ep2" claimed
[   78.26] EP@9e11b244 "ep1" reset
[   78.26] EP@9e11b2a0 "ep2" reset
[   78.26] EP@9e11b2fc "ep3" reset
[   78.30] EP@9e11b358 "ep4" reset
[   78.30] EP@9e11b3b4 "ep5" reset
[   78.30] EP@9e11b410 "ep6" reset
[   78.30] EP@9e11b46c "ep7" reset
[   78.30] EP@9e11b4c8 "ep8" reset
[   78.30] EP@9e11b524 "ep9" reset
[   78.30] EP@9e11b580 "ep10" reset
[   78.30] EP@9e11b5dc "ep11" reset
[   78.30] EP@9e11b638 "ep12" reset
[   78.30] EP@9e11b694 "ep13" reset
[   78.30] EP@9e11b6f0 "ep14" reset
[   78.30] EP@9e11b74c "ep15" reset
[   78.30] EP@9e11b244 "ep1" reset
[   78.30] EP@9e11b2a0 "ep2" reset
[   78.34] EP@9e11b2fc "ep3" reset
[   78.34] EP@9e11b358 "ep4" reset
[   78.34] EP@9e11b3b4 "ep5" reset
[   78.34] EP@9e11b410 "ep6" reset
[   78.34] EP@9e11b46c "ep7" reset
[   78.34] EP@9e11b4c8 "ep8" reset
[   78.34] EP@9e11b524 "ep9" reset
[   78.34] EP@9e11b580 "ep10" reset
[   78.34] EP@9e11b5dc "ep11" reset
[   78.34] EP@9e11b638 "ep12" reset
[   78.34] EP@9e11b694 "ep13" reset
[   78.34] EP@9e11b6f0 "ep14" reset
[   78.34] EP@9e11b74c "ep15" reset
[   78.34] aspeed_vhub 1e6a.usb-vhub: start
[   78.40] aspeed_vhub 1e6a.usb-vhub: pullup(1)
[   78.40] aspeed_vhub 1e6a.usb-vhub: USB bus suspend
[   78.40] configfs-gadget gadget: suspend
# [   78.57] aspeed_vhub 1e6a.usb-vhub: USB bus resume
[   78.57] configfs-gadget gadget: resume
[   78.57] aspeed_vhub 1e6a.usb-vhub: USB bus reset
[   78.57] aspeed_vhub 1e6a.usb-vhub: USB status=09ca0712
[   78.64] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0100//0040 [in] st=0
[   78.65] aspeed_vhub 1e6a.usb-vhub: USB bus reset
[   78.65] aspeed_vhub 1e6a.usb-vhub: USB status=62c26014
[   78.73] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
05/00/001e// [out] st=0
[   78.73] aspeed_vhub 1e6a.usb-vhub: SET_ADDRESS: Got address 1e
[   78.75] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0100//0012 [in] st=0
[   78.76] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0200//0009 [in] st=0
[   78.77] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0200//0020 [in] st=0
[   78.78] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0300//00ff [in] st=0
[   78.78] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0302/0409/00ff [in] st=0
[   78.80] aspeed_vhub 1e6a.usb-vhub: EP0: SETUP packet 
06/80/0301/0409/00ff 

Re: [PATCH v15 2/7] power: add power sequence library

[Peter Chen]

On Tue, Jun 13, 2017 at 12:24:42PM +0200, Ulf Hansson wrote:
> [...]
> 
> > +
> > +/**
> > + * of_pwrseq_on - Carry out power sequence on for device node
> > + *
> > + * @np: the device node would like to power on
> > + *
> > + * Carry out a single device power on.  If multiple devices
> > + * need to be handled, use of_pwrseq_on_list() instead.
> > + *
> > + * Return a pointer to the power sequence instance on success,
> > + * or an error code otherwise.
> > + */
> > +struct pwrseq *of_pwrseq_on(struct device_node *np)
> > +{
> > +   struct pwrseq *pwrseq;
> > +   int ret;
> > +
> > +   pwrseq = pwrseq_find_available_instance(np);
> > +   if (!pwrseq)
> > +   return ERR_PTR(-ENOENT);
> 
> In case the pwrseq instance hasn't been registered yet, then there is
> no way to deal with -EPROBE_DEFER properly here.
> 
> I haven't been following the discussions in-depth during all
> iterations, so perhaps you have already discussed why doing it like
> this.

Yes, it has been discussed. In order to compare with compatible string
at dts, we need to have one registered pwrseq instance for each
pwrseq library, this pre-registered one is allocated using
postcore_initcall, and the new (eg, second) instance is registered
after pwrseq_get has succeeded.

Peter

> 
> Anyway, that means all pwrseq instances needs to be registered an
> early boot level, to be safe. To me, that seems like poor design
> choice.
> 
> Otherwise I think this looks okay to me.
> 

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

[Alan Cox]

On Tue, 13 Jun 2017 09:52:07 +0300
Tal Shorer  wrote:

> If a tty driver wants to notify the user of some exceptional event,
> such as a usb cdc acm device set_line_coding event, it needs a way to
> modify the mask returned by poll() and possible also add wait queues.
> In order to do that, we allow the driver to supply a poll() callback
> of its own, which will be called in n_tty_poll().
> 
> Signed-off-by: Tal Shorer 

You might be in any line discipline so you need to support that for each
ldisc that supports poll(). Also what do I do when I get an exception
event - what does it mean, how do I understand that, are you proposing a
standardized meaning ? Have you checked whether that conflicts with SuS
v3 or POSIX ? What will it do with existing code.

At the very least it probably has to be something you turn on, and you
might then want to model it on the pty/tty interface logic and extend
TIOCPKT a shade.

Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Alan Cox]

On Mon, 12 Jun 2017 20:26:13 +0300
Tal Shorer  wrote:

> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).

No this is not how to do it. We don't want weirdass ioctls for each
different tty device type.

There are two ways this can work. The first is actually done by plenty of
real physical hardware and that is to simply update the termios of the
logical channel to reflect the settings negotiated by the link layer
below (in your course USB ACM).

If that isn't sufficient then implement an ioctl that could work for *ALL*
tty devices - for example return a termios structure indicating the
relevant values on top of the current tty termios settings not some USB
ACM magic object.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Oddity with EP configuration

[Benjamin Herrenschmidt]

On Tue, 2017-06-13 at 15:08 +1000, Benjamin Herrenschmidt wrote:
> Now, what I observe is that when the mass storage gets bound to the
> UDC driver:
> 
>  - First the endpoints are properly allocated by my match callback,
> ie, epautoconf is called to allocate EPs to the function.
> 
>  - But right away, composite.c calls usb_ep_autoconfig_release()
> effectively releasing those 2 endpoints.

I confirmed this by reverting to my old "single UDC" driver (so a more
standard setup without all my hooks) and putting a WARN_ON(!ep-
>claimed) in usb_ep_enable().

A kernel with g_mass_storage built-in will WARN twice at boot.

The endpoints are claimed and immediately released, then the driver
proceeds to use released endpoints.

I'll try with a few more gadgets (I haven't dabbled with the non-legacy 
stuff yet but will give a try), but it looks like endpoint "allocation"
is rather broken at the moment.

I'm happy to try to fix it since my virtual hub needs that to be solid,
but it would be good if you guys could give me a hint of what is the
expected policy.

My gut feeling is that this should be the responsibility of the
functions. They are the ones calling epautoconf to claim the EPs in the
first place (typically in their bind callback), so they should be the
ones releasing them in unbind.

Any reason not to do it that way ?

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[John Youn]

On 6/13/2017 3:31 PM, Paul Zimmerman wrote:
> On Tue, 13 Jun 2017 21:57:42 +, John Youn  wrote:
>
>> On 6/13/2017 2:39 PM, Paul Zimmerman wrote:
>>> On Tue, 13 Jun 2017 18:33:09 +, John Youn  
>>> wrote:
>>>
 On 6/13/2017 12:32 AM, Felipe Balbi wrote:
>
> Hi,
>
> Thinh Nguyen  writes:
 this could be, I don't remember if I checked this or not :-)

 Really, the best way here, IMHO, would be to re-verify what's 
 going on
 with macOS and revert my orignal patch since it's, rather 
 clearly,
 wrong.

>>>
>>> Sure. Are you going to make a revert patch or I am?
>>
>> Well, after we really know what's going on with macOS and have a 
>> better
>> fix, then who makes the revert is less important as long as 
>> problems get
>> sorted out :-) Either way is fine for me.
>>
>
> Do you have any update on this issue?
>

 The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt 
 when
 invalid") still causes a regression for us. As there hasn't any 
 update
 for the macOS issue, can I submit a revert patch for this?
>>>
>>> I just came back from vacations ;-) I'll get back to this. Reverting
>>> that commit won't do any good as we'd be exchanging one regression 
>>> for
>>> another. We really need to understand what's going on.
>>
>> Hi Felipe,
>>
>> I think we worked around this same issue in the Synopsys vendor 
>> driver
>> after a customer reported a problem with 
>> CLEAR_FEATURE(ENDPOINT_HALT).
>> I no longer have access to either the databook or the codebase, so I
>> can't be sure about what the workaround was, but if either John or 
>> Thinh
>> can have a look at the Clear Stall code in the vendor driver they 
>> should
>> be able to figure it out.

 Thanks a lot Paul :-) Good to see you still have a look here every once
 in a while :-)

 John, Thinh could either of you check what Paul mentions here?

 cheers

>>>
>>> Can you provide more detail on the issue you see on MAC OS? and how to
>>> reproduce the issue?
>>>
>>
>> This issue has been a regression for us for a few months now, do you
>> have any update on this?
>
> ordered a mac to test this again. It'll take a few days.
>

 Thanks Felipe.

 Let us know if we can help in any way. If you can give details on your
 setup with macOS we can look into running it as well.

 Also curious whether or not you are seeing the same regression? It
 should apply to anyone using dwc3 running USB mass-storage CV.
>>>
>>> Hi John,
>>>
>>> Did you have a look at the Synopsys driver like I mentioned above?
>>> I'm pretty sure I fixed something similar to this there.
>>>
>>
>> Hi Paul,
>>
>> I haven't checked, but Thinh looked and couldn't find anything
>> relating to the macOS issue. Which is why we want more information
>> about the problem.
>>
>> I can take a look at it too later today when I get a chance.
>>
>> When you say you fixed the issue in the vendor driver, do you mean the
>> macOS problem (multiple clear stalls) or the regression that it caused
>> when it was fixed (the MSC data sequence issue)?
>
> Hi John,
>
> The issue was with the host sending a CLEAR_FEATURE(ENDPOINT_HALT) when the
> EP was not actually halted. The host does this to reset the EP data toggle /
> sequence number to 0, and it is allowed by some USB spec. The problem was
> that for DWC3 if you issue a Clear Stall when the EP is not halted, nothing
> happens. I *think* the fix was to issue a Set EP Config command instead,
> which does reset the sequence number, but I'm not 100% sure about that.
>

Hi Paul,

Ok thanks, that will definitely help looking into it.

Though our issue seems the opposite. Since Felipe's macOS patch
prevents the clear stall when already halted, but *without* that, the
controller doesn't clear the sequence number and fails the MSC CV
test.

John

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[Paul Zimmerman]

On Tue, 13 Jun 2017 21:57:42 +, John Youn  wrote:

> On 6/13/2017 2:39 PM, Paul Zimmerman wrote:
> > On Tue, 13 Jun 2017 18:33:09 +, John Youn  
> > wrote:
> >
> >> On 6/13/2017 12:32 AM, Felipe Balbi wrote:
> >>>
> >>> Hi,
> >>>
> >>> Thinh Nguyen  writes:
> >> this could be, I don't remember if I checked this or not :-)
> >>
> >> Really, the best way here, IMHO, would be to re-verify what's 
> >> going on
> >> with macOS and revert my orignal patch since it's, rather 
> >> clearly,
> >> wrong.
> >>
> >
> > Sure. Are you going to make a revert patch or I am?
> 
>  Well, after we really know what's going on with macOS and have a 
>  better
>  fix, then who makes the revert is less important as long as 
>  problems get
>  sorted out :-) Either way is fine for me.
> 
> >>>
> >>> Do you have any update on this issue?
> >>>
> >>
> >> The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt 
> >> when
> >> invalid") still causes a regression for us. As there hasn't any 
> >> update
> >> for the macOS issue, can I submit a revert patch for this?
> >
> > I just came back from vacations ;-) I'll get back to this. Reverting
> > that commit won't do any good as we'd be exchanging one regression 
> > for
> > another. We really need to understand what's going on.
> 
>  Hi Felipe,
> 
>  I think we worked around this same issue in the Synopsys vendor 
>  driver
>  after a customer reported a problem with 
>  CLEAR_FEATURE(ENDPOINT_HALT).
>  I no longer have access to either the databook or the codebase, so I
>  can't be sure about what the workaround was, but if either John or 
>  Thinh
>  can have a look at the Clear Stall code in the vendor driver they 
>  should
>  be able to figure it out.
> >>
> >> Thanks a lot Paul :-) Good to see you still have a look here every once
> >> in a while :-)
> >>
> >> John, Thinh could either of you check what Paul mentions here?
> >>
> >> cheers
> >>
> >
> > Can you provide more detail on the issue you see on MAC OS? and how to
> > reproduce the issue?
> >
> 
>  This issue has been a regression for us for a few months now, do you
>  have any update on this?
> >>>
> >>> ordered a mac to test this again. It'll take a few days.
> >>>
> >>
> >> Thanks Felipe.
> >>
> >> Let us know if we can help in any way. If you can give details on your
> >> setup with macOS we can look into running it as well.
> >>
> >> Also curious whether or not you are seeing the same regression? It
> >> should apply to anyone using dwc3 running USB mass-storage CV.
> >
> > Hi John,
> >
> > Did you have a look at the Synopsys driver like I mentioned above?
> > I'm pretty sure I fixed something similar to this there.
> >
> 
> Hi Paul,
> 
> I haven't checked, but Thinh looked and couldn't find anything
> relating to the macOS issue. Which is why we want more information
> about the problem.
> 
> I can take a look at it too later today when I get a chance.
> 
> When you say you fixed the issue in the vendor driver, do you mean the
> macOS problem (multiple clear stalls) or the regression that it caused
> when it was fixed (the MSC data sequence issue)?

Hi John,

The issue was with the host sending a CLEAR_FEATURE(ENDPOINT_HALT) when the
EP was not actually halted. The host does this to reset the EP data toggle /
sequence number to 0, and it is allowed by some USB spec. The problem was
that for DWC3 if you issue a Clear Stall when the EP is not halted, nothing
happens. I *think* the fix was to issue a Set EP Config command instead,
which does reset the sequence number, but I'm not 100% sure about that.

-- 
Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[John Youn]

On 6/13/2017 2:39 PM, Paul Zimmerman wrote:
> On Tue, 13 Jun 2017 18:33:09 +, John Youn  wrote:
>
>> On 6/13/2017 12:32 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Thinh Nguyen  writes:
>> this could be, I don't remember if I checked this or not :-)
>>
>> Really, the best way here, IMHO, would be to re-verify what's 
>> going on
>> with macOS and revert my orignal patch since it's, rather 
>> clearly,
>> wrong.
>>
>
> Sure. Are you going to make a revert patch or I am?

 Well, after we really know what's going on with macOS and have a 
 better
 fix, then who makes the revert is less important as long as 
 problems get
 sorted out :-) Either way is fine for me.

>>>
>>> Do you have any update on this issue?
>>>
>>
>> The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
>> invalid") still causes a regression for us. As there hasn't any 
>> update
>> for the macOS issue, can I submit a revert patch for this?
>
> I just came back from vacations ;-) I'll get back to this. Reverting
> that commit won't do any good as we'd be exchanging one regression for
> another. We really need to understand what's going on.

 Hi Felipe,

 I think we worked around this same issue in the Synopsys vendor driver
 after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
 I no longer have access to either the databook or the codebase, so I
 can't be sure about what the workaround was, but if either John or 
 Thinh
 can have a look at the Clear Stall code in the vendor driver they 
 should
 be able to figure it out.
>>
>> Thanks a lot Paul :-) Good to see you still have a look here every once
>> in a while :-)
>>
>> John, Thinh could either of you check what Paul mentions here?
>>
>> cheers
>>
>
> Can you provide more detail on the issue you see on MAC OS? and how to
> reproduce the issue?
>

 This issue has been a regression for us for a few months now, do you
 have any update on this?
>>>
>>> ordered a mac to test this again. It'll take a few days.
>>>
>>
>> Thanks Felipe.
>>
>> Let us know if we can help in any way. If you can give details on your
>> setup with macOS we can look into running it as well.
>>
>> Also curious whether or not you are seeing the same regression? It
>> should apply to anyone using dwc3 running USB mass-storage CV.
>
> Hi John,
>
> Did you have a look at the Synopsys driver like I mentioned above?
> I'm pretty sure I fixed something similar to this there.
>

Hi Paul,

I haven't checked, but Thinh looked and couldn't find anything
relating to the macOS issue. Which is why we want more information
about the problem.

I can take a look at it too later today when I get a chance.

When you say you fixed the issue in the vendor driver, do you mean the
macOS problem (multiple clear stalls) or the regression that it caused
when it was fixed (the MSC data sequence issue)?

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[Paul Zimmerman]

On Tue, 13 Jun 2017 18:33:09 +, John Youn  wrote:

> On 6/13/2017 12:32 AM, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Thinh Nguyen  writes:
>  this could be, I don't remember if I checked this or not :-)
> 
>  Really, the best way here, IMHO, would be to re-verify what's 
>  going on
>  with macOS and revert my orignal patch since it's, rather 
>  clearly,
>  wrong.
> 
> >>>
> >>> Sure. Are you going to make a revert patch or I am?
> >>
> >> Well, after we really know what's going on with macOS and have a 
> >> better
> >> fix, then who makes the revert is less important as long as 
> >> problems get
> >> sorted out :-) Either way is fine for me.
> >>
> >
> > Do you have any update on this issue?
> >
> 
>  The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
>  invalid") still causes a regression for us. As there hasn't any 
>  update
>  for the macOS issue, can I submit a revert patch for this?
> >>>
> >>> I just came back from vacations ;-) I'll get back to this. Reverting
> >>> that commit won't do any good as we'd be exchanging one regression for
> >>> another. We really need to understand what's going on.
> >>
> >> Hi Felipe,
> >>
> >> I think we worked around this same issue in the Synopsys vendor driver
> >> after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
> >> I no longer have access to either the databook or the codebase, so I
> >> can't be sure about what the workaround was, but if either John or 
> >> Thinh
> >> can have a look at the Clear Stall code in the vendor driver they 
> >> should
> >> be able to figure it out.
> 
>  Thanks a lot Paul :-) Good to see you still have a look here every once
>  in a while :-)
> 
>  John, Thinh could either of you check what Paul mentions here?
> 
>  cheers
> 
> >>>
> >>> Can you provide more detail on the issue you see on MAC OS? and how to
> >>> reproduce the issue?
> >>>
> >>
> >> This issue has been a regression for us for a few months now, do you
> >> have any update on this?
> >
> > ordered a mac to test this again. It'll take a few days.
> >
> 
> Thanks Felipe.
> 
> Let us know if we can help in any way. If you can give details on your
> setup with macOS we can look into running it as well.
> 
> Also curious whether or not you are seeing the same regression? It
> should apply to anyone using dwc3 running USB mass-storage CV.

Hi John,

Did you have a look at the Synopsys driver like I mentioned above?
I'm pretty sure I fixed something similar to this there.

-- 
Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks

[Alan Stern]

Using the syzkaller kernel fuzzer, Andrey Konovalov generated the
following error in gadgetfs:

> BUG: KASAN: use-after-free in __lock_acquire+0x3069/0x3690
> kernel/locking/lockdep.c:3246
> Read of size 8 at addr 88003a2bdaf8 by task kworker/3:1/903
> 
> CPU: 3 PID: 903 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #35
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  __dump_stack lib/dump_stack.c:16 [inline]
>  dump_stack+0x292/0x395 lib/dump_stack.c:52
>  print_address_description+0x78/0x280 mm/kasan/report.c:252
>  kasan_report_error mm/kasan/report.c:351 [inline]
>  kasan_report+0x230/0x340 mm/kasan/report.c:408
>  __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:429
>  __lock_acquire+0x3069/0x3690 kernel/locking/lockdep.c:3246
>  lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
>  __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
>  _raw_spin_lock+0x2f/0x40 kernel/locking/spinlock.c:151
>  spin_lock include/linux/spinlock.h:299 [inline]
>  gadgetfs_suspend+0x89/0x130 drivers/usb/gadget/legacy/inode.c:1682
>  set_link_state+0x88e/0xae0 drivers/usb/gadget/udc/dummy_hcd.c:455
>  dummy_hub_control+0xd7e/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2074
>  rh_call_control drivers/usb/core/hcd.c:689 [inline]
>  rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
>  usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
>  usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
>  usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
>  usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
>  usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
>  usb_clear_port_feature+0x74/0xa0 drivers/usb/core/hub.c:412
>  hub_port_disable+0x123/0x510 drivers/usb/core/hub.c:4177
>  hub_port_init+0x1ed/0x2940 drivers/usb/core/hub.c:4648
>  hub_port_connect drivers/usb/core/hub.c:4826 [inline]
>  hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
>  port_event drivers/usb/core/hub.c:5105 [inline]
>  hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
>  process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
>  process_scheduled_works kernel/workqueue.c:2157 [inline]
>  worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
>  kthread+0x363/0x440 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
> 
> Allocated by task 9958:
>  save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>  set_track mm/kasan/kasan.c:525 [inline]
>  kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
>  kmem_cache_alloc_trace+0x87/0x280 mm/slub.c:2745
>  kmalloc include/linux/slab.h:492 [inline]
>  kzalloc include/linux/slab.h:665 [inline]
>  dev_new drivers/usb/gadget/legacy/inode.c:170 [inline]
>  gadgetfs_fill_super+0x24f/0x540 drivers/usb/gadget/legacy/inode.c:1993
>  mount_single+0xf6/0x160 fs/super.c:1192
>  gadgetfs_mount+0x31/0x40 drivers/usb/gadget/legacy/inode.c:2019
>  mount_fs+0x9c/0x2d0 fs/super.c:1223
>  vfs_kern_mount.part.25+0xcb/0x490 fs/namespace.c:976
>  vfs_kern_mount fs/namespace.c:2509 [inline]
>  do_new_mount fs/namespace.c:2512 [inline]
>  do_mount+0x41b/0x2d90 fs/namespace.c:2834
>  SYSC_mount fs/namespace.c:3050 [inline]
>  SyS_mount+0xb0/0x120 fs/namespace.c:3027
>  entry_SYSCALL_64_fastpath+0x1f/0xbe
> 
> Freed by task 9960:
>  save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>  set_track mm/kasan/kasan.c:525 [inline]
>  kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
>  slab_free_hook mm/slub.c:1357 [inline]
>  slab_free_freelist_hook mm/slub.c:1379 [inline]
>  slab_free mm/slub.c:2961 [inline]
>  kfree+0xed/0x2b0 mm/slub.c:3882
>  put_dev+0x124/0x160 drivers/usb/gadget/legacy/inode.c:163
>  gadgetfs_kill_sb+0x33/0x60 drivers/usb/gadget/legacy/inode.c:2027
>  deactivate_locked_super+0x8d/0xd0 fs/super.c:309
>  deactivate_super+0x21e/0x310 fs/super.c:340
>  cleanup_mnt+0xb7/0x150 fs/namespace.c:1112
>  __cleanup_mnt+0x1b/0x20 fs/namespace.c:1119
>  task_work_run+0x1a0/0x280 kernel/task_work.c:116
>  exit_task_work include/linux/task_work.h:21 [inline]
>  do_exit+0x18a8/0x2820 kernel/exit.c:878
>  do_group_exit+0x14e/0x420 kernel/exit.c:982
>  get_signal+0x784/0x1780 kernel/signal.c:2318
>  do_signal+0xd7/0x2130 arch/x86/kernel/signal.c:808
>  exit_to_usermode_loop+0x1ac/0x240 arch/x86/entry/common.c:157
>  prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
>  syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
>  entry_SYSCALL_64_fastpath+0xbc/0xbe
> 
> The buggy address belongs to the object at 88003a2bdae0
>  which belongs to the cache kmalloc-1024 of size 1024
> The buggy address is located 24 bytes inside of
>  1024-byte region [88003a2bdae0, 88003a2bdee0)
> The buggy address belongs to the page:
> page:eae8ae00 count:1 mapcount:0 mapping:  (null)
> index:0x0 compound_mapcount: 0
> flags: 

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[John Youn]

On 6/13/2017 12:32 AM, Felipe Balbi wrote:
>
> Hi,
>
> Thinh Nguyen  writes:
 this could be, I don't remember if I checked this or not :-)

 Really, the best way here, IMHO, would be to re-verify what's 
 going on
 with macOS and revert my orignal patch since it's, rather clearly,
 wrong.

>>>
>>> Sure. Are you going to make a revert patch or I am?
>>
>> Well, after we really know what's going on with macOS and have a 
>> better
>> fix, then who makes the revert is less important as long as problems 
>> get
>> sorted out :-) Either way is fine for me.
>>
>
> Do you have any update on this issue?
>

 The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
 invalid") still causes a regression for us. As there hasn't any update
 for the macOS issue, can I submit a revert patch for this?
>>>
>>> I just came back from vacations ;-) I'll get back to this. Reverting
>>> that commit won't do any good as we'd be exchanging one regression for
>>> another. We really need to understand what's going on.
>>
>> Hi Felipe,
>>
>> I think we worked around this same issue in the Synopsys vendor driver
>> after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
>> I no longer have access to either the databook or the codebase, so I
>> can't be sure about what the workaround was, but if either John or Thinh
>> can have a look at the Clear Stall code in the vendor driver they should
>> be able to figure it out.

 Thanks a lot Paul :-) Good to see you still have a look here every once
 in a while :-)

 John, Thinh could either of you check what Paul mentions here?

 cheers

>>>
>>> Can you provide more detail on the issue you see on MAC OS? and how to
>>> reproduce the issue?
>>>
>>
>> This issue has been a regression for us for a few months now, do you
>> have any update on this?
>
> ordered a mac to test this again. It'll take a few days.
>

Thanks Felipe.

Let us know if we can help in any way. If you can give details on your
setup with macOS we can look into running it as well.

Also curious whether or not you are seeing the same regression? It
should apply to anyone using dwc3 running USB mass-storage CV.

Regards,
John


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[Thinh Nguyen]

Hi,

On 6/13/2017 12:32 AM, Felipe Balbi wrote:
> Thinh Nguyen  writes:
 this could be, I don't remember if I checked this or not :-)

 Really, the best way here, IMHO, would be to re-verify what's 
 going on
 with macOS and revert my orignal patch since it's, rather clearly,
 wrong.

>>>
>>> Sure. Are you going to make a revert patch or I am?
>>
>> Well, after we really know what's going on with macOS and have a 
>> better
>> fix, then who makes the revert is less important as long as problems 
>> get
>> sorted out :-) Either way is fine for me.
>>
>
> Do you have any update on this issue?
>

 The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
 invalid") still causes a regression for us. As there hasn't any update
 for the macOS issue, can I submit a revert patch for this?
>>>
>>> I just came back from vacations ;-) I'll get back to this. Reverting
>>> that commit won't do any good as we'd be exchanging one regression for
>>> another. We really need to understand what's going on.
>>
>> Hi Felipe,
>>
>> I think we worked around this same issue in the Synopsys vendor driver
>> after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
>> I no longer have access to either the databook or the codebase, so I
>> can't be sure about what the workaround was, but if either John or Thinh
>> can have a look at the Clear Stall code in the vendor driver they should
>> be able to figure it out.

 Thanks a lot Paul :-) Good to see you still have a look here every once
 in a while :-)

 John, Thinh could either of you check what Paul mentions here?

 cheers

>>>
>>> Can you provide more detail on the issue you see on MAC OS? and how to
>>> reproduce the issue?
>>>
>>
>> This issue has been a regression for us for a few months now, do you
>> have any update on this?
> 
> ordered a mac to test this again. It'll take a few days.
> 

Thanks Felipe,
Thinh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gadgetfs: how to wait for USB device initialization?

[Andrey Konovalov]

On Tue, Jun 13, 2017 at 6:22 PM, Tal Shorer  wrote:
> On Tue, Jun 13, 2017 at 7:02 PM, Tal Shorer  wrote:
>> On Tue, Jun 13, 2017 at 3:21 PM, Andrey Konovalov  
>> wrote:
>>> Hi!
>>>
>>> I'm trying to use gadgetfs to fuzz USB device drivers by simply
>>> connecting random devices for now.
>>>
>>> What I want to achieve right now is the following:
>>>
>>> 1. mount gadgetfs
>>> 2. emulate connection of a new USB device
>>> 3. wait for the device to finish initializing
>>> 4. unmount gadgetfs
>>> 5. goto 1
>>>
>>> The question is how do I wait for the device to finish initializing
>>> (the corresponding USB driver to finish probing?) before unmounting
>>> gadgetfs? As I understand, when I write device description to
>>> "/dev/gadget/dummy_udc" the initialization happens asynchronously
>>> after writing is done.
>>>
>>
>> You can use inotify on the state file inside a udc's sysfs directory.
>> It's awesome like that.
>> Here's a sample code that doesn't do any cleanups because I'm lazy:
>> https://gist.github.com/talshorer/61be1bc3472cc2a7b4866b9bd5a239ca
> And I forgot usage >:
> ./wait_udc_configured /sys/devices/platform/dummy_udc.0/udc/dummy_udc.0/state

Hi Tal,

I will try this, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb/gadget: potential deadlock in gadgetfs_suspend

[Andrey Konovalov]

On Tue, Jun 13, 2017 at 7:44 PM, Alan Stern  wrote:
> On Tue, 13 Jun 2017, Andrey Konovalov wrote:
>
>> Hi Alan,
>>
>> Thanks for the patch!
>>
>> I've been testing with your patch applied and the "bad spinlock magic"
>> crashes seem to be gone. However I got another crash (happened only
>> once over the night), which happens during "spin_lock_irqsave
>> (>lock, flags)":
>>
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault:  [#1] SMP KASAN
>> Dumping ftrace buffer:
>>(ftrace buffer empty)
>> Modules linked in:
>> CPU: 3 PID: 900 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #36
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Workqueue: usb_hub_wq hub_event
>> task: 88006c5aadc0 task.stack: 88006c62
>> RIP: 0010:__lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246
>> RSP: 0018:88006c625a80 EFLAGS: 00010006
>> RAX: dc00 RBX: 88006c5aadc0 RCX: 
>> RDX: 0003 RSI:  RDI: 11000d8c4bab
>> RBP: 88006c625fc0 R08: 0001 R09: 0001
>> R10: 88006c5ab698 R11: 87dd2e80 R12: dc00
>> R13: 0001 R14: 0018 R15: 
>> FS:  () GS:88006dd0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 20fd CR3: 6797e000 CR4: 06e0
>> Call Trace:
>>  lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
>>  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>  _raw_spin_lock_irqsave+0xc9/0x110 kernel/locking/spinlock.c:159
>>  gadgetfs_disconnect+0xf1/0x230 drivers/usb/gadget/legacy/inode.c:1664
>>  usb_gadget_udc_reset+0x3b/0xb0 drivers/usb/gadget/udc/core.c:1020
>>  set_link_state+0x648/0x9f0 drivers/usb/gadget/udc/dummy_hcd.c:446
>>  dummy_hub_control+0x11bb/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2243
>>  rh_call_control drivers/usb/core/hcd.c:689 [inline]
>>  rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
>>  usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
>>  usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
>>  usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
>>  usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
>>  usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
>>  set_port_feature+0x73/0x90 drivers/usb/core/hub.c:422
>>  hub_port_reset+0x277/0x1550 drivers/usb/core/hub.c:2772
>>  hub_port_init+0x7dc/0x2940 drivers/usb/core/hub.c:4510
>>  hub_port_connect drivers/usb/core/hub.c:4826 [inline]
>>  hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
>>  port_event drivers/usb/core/hub.c:5105 [inline]
>>  hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
>>  process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
>>  process_scheduled_works kernel/workqueue.c:2157 [inline]
>>  worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
>>  kthread+0x363/0x440 kernel/kthread.c:231
>>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
>> Code: e9 03 f3 48 ab 48 81 c4 18 05 00 00 44 89 e0 5b 41 5c 41 5d 41
>> 5e 41 5f 5d c3 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
>> 3c 02 00 0f 85 51 24 00 00 49 81 3e 40 ea 13 87 41 bd 00 00
>> RIP: __lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246 RSP:
>> 88006c625a80
>> ---[ end trace f5a7b971fc1b0546 ]---
>> Kernel panic - not syncing: Fatal exception
>> Shutting down cpus with NMI
>> Dumping ftrace buffer:
>>(ftrace buffer empty)
>> Kernel Offset: disabled
>> Rebooting in 86400 seconds..
>
> I guess the structure containing the spinlock was already deallocated
> when gadgetfs_disconnect was called.  But I don't see how that could
> have happened, with the patched locking.
>
> If you can't replicate this then there's not much hope of tracking it
> down.
>
> If you can, then perhaps it would help to trace each time any function
> calls either put_dev or get_dev in inode.c.

OK, I'll let you know if I manage to reproduce this. I'll report it in
a separate thread then.

Anyway, your patch fixes the crashes I reported initially, thanks!

>
> Alan Stern
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb/gadget: potential deadlock in gadgetfs_suspend

[Alan Stern]

On Tue, 13 Jun 2017, Andrey Konovalov wrote:

> Hi Alan,
> 
> Thanks for the patch!
> 
> I've been testing with your patch applied and the "bad spinlock magic"
> crashes seem to be gone. However I got another crash (happened only
> once over the night), which happens during "spin_lock_irqsave
> (>lock, flags)":
> 
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 3 PID: 900 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #36
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> task: 88006c5aadc0 task.stack: 88006c62
> RIP: 0010:__lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246
> RSP: 0018:88006c625a80 EFLAGS: 00010006
> RAX: dc00 RBX: 88006c5aadc0 RCX: 
> RDX: 0003 RSI:  RDI: 11000d8c4bab
> RBP: 88006c625fc0 R08: 0001 R09: 0001
> R10: 88006c5ab698 R11: 87dd2e80 R12: dc00
> R13: 0001 R14: 0018 R15: 
> FS:  () GS:88006dd0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 20fd CR3: 6797e000 CR4: 06e0
> Call Trace:
>  lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
>  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>  _raw_spin_lock_irqsave+0xc9/0x110 kernel/locking/spinlock.c:159
>  gadgetfs_disconnect+0xf1/0x230 drivers/usb/gadget/legacy/inode.c:1664
>  usb_gadget_udc_reset+0x3b/0xb0 drivers/usb/gadget/udc/core.c:1020
>  set_link_state+0x648/0x9f0 drivers/usb/gadget/udc/dummy_hcd.c:446
>  dummy_hub_control+0x11bb/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2243
>  rh_call_control drivers/usb/core/hcd.c:689 [inline]
>  rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
>  usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
>  usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
>  usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
>  usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
>  usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
>  set_port_feature+0x73/0x90 drivers/usb/core/hub.c:422
>  hub_port_reset+0x277/0x1550 drivers/usb/core/hub.c:2772
>  hub_port_init+0x7dc/0x2940 drivers/usb/core/hub.c:4510
>  hub_port_connect drivers/usb/core/hub.c:4826 [inline]
>  hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
>  port_event drivers/usb/core/hub.c:5105 [inline]
>  hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
>  process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
>  process_scheduled_works kernel/workqueue.c:2157 [inline]
>  worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
>  kthread+0x363/0x440 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
> Code: e9 03 f3 48 ab 48 81 c4 18 05 00 00 44 89 e0 5b 41 5c 41 5d 41
> 5e 41 5f 5d c3 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
> 3c 02 00 0f 85 51 24 00 00 49 81 3e 40 ea 13 87 41 bd 00 00
> RIP: __lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246 RSP:
> 88006c625a80
> ---[ end trace f5a7b971fc1b0546 ]---
> Kernel panic - not syncing: Fatal exception
> Shutting down cpus with NMI
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..

I guess the structure containing the spinlock was already deallocated 
when gadgetfs_disconnect was called.  But I don't see how that could 
have happened, with the patched locking.

If you can't replicate this then there's not much hope of tracking it 
down.

If you can, then perhaps it would help to trace each time any function 
calls either put_dev or get_dev in inode.c.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: host: ehci: workaround PME bug on AMD EHCI controller

[Bjorn Helgaas]

[+cc Rafael, linux-pm]

On Tue, Jun 13, 2017 at 12:21:15PM +0800, Kai-Heng Feng wrote:
> On Mon, Jun 12, 2017 at 10:18 PM, Alan Stern  
> wrote:
> > Let's get some help from people who understand PCI well.
> >
> > Here's the general problem: Kai-Heng has a PCI-based USB host
> > controller that advertises wakeup capability from D3, but it doesn't
> > assert PME# from D3 when it should.  For "lspci -vv" output, see
> >
> > http://marc.info/?l=linux-usb=149570231732519=2
> >
> > On Mon, 12 Jun 2017, Kai-Heng Feng wrote:
> >
> >> On Mon, Jun 12, 2017 at 3:04 PM, Kai-Heng Feng
> >>  wrote:
> >> > On Fri, Jun 9, 2017 at 10:43 PM, Alan Stern  
> >> > wrote:
> >> >> On Fri, 9 Jun 2017, Kai-Heng Feng wrote:
> >> >>
> >> >> Is this really the right solution?  Maybe it would be better to allow
> >> >> the controller to go into D3 provided no wakeup signal is needed.  You
> >> >> could do:
> >> >>
> >> >> device_set_wakeup_capable(>dev, 0);
> >> >
> >> > This doesn't work.
> >> > After applying this function, still nothing happens when devices get 
> >> > plugged in.
> >> > IIUC this function disable the wakeup function, but what I want to do
> >> > here is to have PME signal works even when runtime PM is enabled.
> >
> > This may indicate a bug in either the PCI or USB stacks (or both!).  If
> > a driver requires wakeup capability from runtime suspend but the device
> > does not provide it, the PCI core should not allow the device to go
> > into runtime suspend.  Or is that the driver's responsibility?
> >
> >> > I also saw some legacy PCI PM stuff, so I also tried:
> >> > device_set_wakeup_capable(>dev, 1);
> >> > ...doesn't work either.
> >> >
> >> >>
> >> >> Another alternative is to put the controller into D2 instead of D3, but
> >> >> (1) I don't know how to do that, and (2) we don't know if wakeup
> >> >> signalling works any better in D2 than it does in D3.
> >> >
> >> > I'll try if D2 works.
> >>
> >> Put the device into D2 instead of D3 can make the wakeup signaling
> >> work, i.e. USB devices can be correctly detected after plugged into
> >> EHCI port.
> >>
> >> Do you think this alternative an acceptable workaround?
> >
> > Yes, it is.  The difficulty is that I don't know how to tell the PCI
> > core that the device should go in D2 during runtime suspend instead of
> > D3.  Some sort of quirk may be needed -- perhaps Bjorn can help.
> >
> 
> FWIW, this is the diff I used to make the controller transits to D2
> instead of D3:
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 563901cd9c06..36663688404a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -922,6 +922,9 @@ int pci_set_power_state(struct pci_dev *dev,
> pci_power_t state)
> if (dev->current_state == state)
> return 0;
> 
> +   if (state > PCI_D2 && (dev->dev_flags & PCI_DEV_FLAGS_MAX_D2))
> +   state = PCI_D2;
> +
> __pci_start_power_transition(dev, state);
> 
> /* This device is quirked not to be put into D3, so
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 93326974ff4b..a2c1fe62974a 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -181,6 +181,8 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
> if (pdev->device == 0x7808) {
> ehci->use_dummy_qh = 1;
> ehci_info(ehci, "applying AMD
> SB700/SB800/Hudson-2/3 EHCI dummy qh workaround\n");
> +
> +   pdev->dev_flags |= PCI_DEV_FLAGS_MAX_D2;
> }
> break;
> case PCI_VENDOR_ID_VIA:
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8039f9f0ca05..6f86f2aa8548 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -188,6 +188,7 @@ enum pci_dev_flags {
>  * the direct_complete optimization.
>  */
> PCI_DEV_FLAGS_NEEDS_RESUME = (__force pci_dev_flags_t) (1 << 11),
> +   PCI_DEV_FLAGS_MAX_D2 = (__force pci_dev_flags_t) (1 << 12),
>  };
> 
>  enum pci_irq_reroute_variant {

The lspci output [1] shows:

  00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI 
Controller (rev 39) (prog-if 20 [EHCI])
Capabilities: [c0] Power Management version 2
  Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
  Status: D3 NoSoftRst- PME-Enable+ DSel=0 DScale=0 PME-
  Bridge: PM- B3+

The device claims it can assert PME# from D3hot.  If it can't, that
sounds like a hardware defect that should be addressed with a quirk.
Ideally we would also have a pointer to the AMD hardware erratum.

Is the following path involved here?

  pci_finish_runtime_suspend
target_state = pci_target_state()
  if (device_may_wakup())
if (dev->pme_support)
  ...
pci_set_power_state(..., target_state)

If so, I would naively expect 

Re: [PATCH net,stable] qmi_wwan: new Telewell and Sierra device IDs

[David Miller]

From: Bjørn Mork 
Date: Tue, 13 Jun 2017 19:10:18 +0200

> A new Sierra Wireless EM7305 device ID used in a Toshiba laptop,
> and two Longcheer device IDs entries used by Telewell TW-3G HSPA+
> branded modems.
> 
> Reported-by: Petr Kloc 
> Reported-by: Teemu Likonen 
> Signed-off-by: Bjørn Mork 
> ---
> The following patch should be backported to v4.9 and later stable kernels to
> make this one apply cleanly:
> 
>  14cf4a771b30 ("drivers: net: usb: qmi_wwan: add QMI_QUIRK_SET_DTR for Telit 
> PID 0x1201")

Ok, applied and queued up for -stable, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: qcserial: new Sierra Wireless EM7305 device ID

[Bjørn Mork]

A new Sierra Wireless EM7305 device ID used in a Toshiba laptop.

Reported-by: Petr Kloc 
Cc: 
Signed-off-by: Bjørn Mork 
---
 drivers/usb/serial/qcserial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
index fd509ed6cf70..652b4334b26d 100644
--- a/drivers/usb/serial/qcserial.c
+++ b/drivers/usb/serial/qcserial.c
@@ -158,6 +158,7 @@ static const struct usb_device_id id_table[] = {
{DEVICE_SWI(0x1199, 0x9056)},   /* Sierra Wireless Modem */
{DEVICE_SWI(0x1199, 0x9060)},   /* Sierra Wireless Modem */
{DEVICE_SWI(0x1199, 0x9061)},   /* Sierra Wireless Modem */
+   {DEVICE_SWI(0x1199, 0x9063)},   /* Sierra Wireless EM7305 */
{DEVICE_SWI(0x1199, 0x9070)},   /* Sierra Wireless MC74xx */
{DEVICE_SWI(0x1199, 0x9071)},   /* Sierra Wireless MC74xx */
{DEVICE_SWI(0x1199, 0x9078)},   /* Sierra Wireless EM74xx */
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net,stable] qmi_wwan: new Telewell and Sierra device IDs

[Bjørn Mork]

A new Sierra Wireless EM7305 device ID used in a Toshiba laptop,
and two Longcheer device IDs entries used by Telewell TW-3G HSPA+
branded modems.

Reported-by: Petr Kloc 
Reported-by: Teemu Likonen 
Signed-off-by: Bjørn Mork 
---
The following patch should be backported to v4.9 and later stable kernels to
make this one apply cleanly:

 14cf4a771b30 ("drivers: net: usb: qmi_wwan: add QMI_QUIRK_SET_DTR for Telit 
PID 0x1201")


 drivers/net/usb/qmi_wwan.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 949671ce4039..32a22f4e8356 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1192,6 +1192,8 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1199, 0x9056, 8)},/* Sierra Wireless Modem */
{QMI_FIXED_INTF(0x1199, 0x9057, 8)},
{QMI_FIXED_INTF(0x1199, 0x9061, 8)},/* Sierra Wireless Modem */
+   {QMI_FIXED_INTF(0x1199, 0x9063, 8)},/* Sierra Wireless EM7305 */
+   {QMI_FIXED_INTF(0x1199, 0x9063, 10)},   /* Sierra Wireless EM7305 */
{QMI_FIXED_INTF(0x1199, 0x9071, 8)},/* Sierra Wireless MC74xx */
{QMI_FIXED_INTF(0x1199, 0x9071, 10)},   /* Sierra Wireless MC74xx */
{QMI_FIXED_INTF(0x1199, 0x9079, 8)},/* Sierra Wireless EM74xx */
@@ -1206,6 +1208,8 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x1bc7, 0x1100, 3)},/* Telit ME910 */
{QMI_FIXED_INTF(0x1bc7, 0x1200, 5)},/* Telit LE920 */
{QMI_QUIRK_SET_DTR(0x1bc7, 0x1201, 2)}, /* Telit LE920, LE920A4 */
+   {QMI_FIXED_INTF(0x1c9e, 0x9801, 3)},/* Telewell TW-3G HSPA+ */
+   {QMI_FIXED_INTF(0x1c9e, 0x9803, 4)},/* Telewell TW-3G HSPA+ */
{QMI_FIXED_INTF(0x1c9e, 0x9b01, 3)},/* XS Stick W100-2 from 4G 
Systems */
{QMI_FIXED_INTF(0x0b3c, 0xc000, 4)},/* Olivetti Olicard 100 */
{QMI_FIXED_INTF(0x0b3c, 0xc001, 4)},/* Olivetti Olicard 120 */
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next v2 0/2] r8152: adjust runtime suspend/resume

[David Miller]

From: Hayes Wang 
Date: Tue, 13 Jun 2017 15:14:38 +0800

> v2:
> For #1, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().
> 
> v1:
> Improve the flow about runtime suspend/resume and make the code
> easy to read.

Series applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dt-bindings: usb: exynos-usb: Add missing required VDD properties

[Krzysztof Kozlowski]

Since commit bd8ce544ec35 ("usb: dwc3: exynos: Make provision for vdd
regulators") vdd33-supply and vdd10-supply are required so document them
in bindings.

Signed-off-by: Krzysztof Kozlowski 
---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt 
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 9b4dbe3b2acc..78ebebb66dad 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -92,6 +92,8 @@ Required properties:
   parent's address space
  - clocks: Clock IDs array as required by the controller.
  - clock-names: names of clocks correseponding to IDs in the clock property
+ - vdd10-supply: 1.0V powr supply
+ - vdd33-supply: 3.0V/3.3V power supply
 
 Sub-nodes:
 The dwc3 core should be added as subnode to Exynos dwc3 glue.
@@ -107,6 +109,8 @@ Example:
#address-cells = <1>;
#size-cells = <1>;
ranges;
+   vdd10-supply = <_reg>;
+   vdd33-supply = <_reg>;
 
dwc3 {
compatible = "synopsys,dwc3";
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gadgetfs: how to wait for USB device initialization?

[Tal Shorer]

On Tue, Jun 13, 2017 at 7:02 PM, Tal Shorer  wrote:
> On Tue, Jun 13, 2017 at 3:21 PM, Andrey Konovalov  
> wrote:
>> Hi!
>>
>> I'm trying to use gadgetfs to fuzz USB device drivers by simply
>> connecting random devices for now.
>>
>> What I want to achieve right now is the following:
>>
>> 1. mount gadgetfs
>> 2. emulate connection of a new USB device
>> 3. wait for the device to finish initializing
>> 4. unmount gadgetfs
>> 5. goto 1
>>
>> The question is how do I wait for the device to finish initializing
>> (the corresponding USB driver to finish probing?) before unmounting
>> gadgetfs? As I understand, when I write device description to
>> "/dev/gadget/dummy_udc" the initialization happens asynchronously
>> after writing is done.
>>
>
> You can use inotify on the state file inside a udc's sysfs directory.
> It's awesome like that.
> Here's a sample code that doesn't do any cleanups because I'm lazy:
> https://gist.github.com/talshorer/61be1bc3472cc2a7b4866b9bd5a239ca
And I forgot usage >:
./wait_udc_configured /sys/devices/platform/dummy_udc.0/udc/dummy_udc.0/state
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gadgetfs: how to wait for USB device initialization?

[Tal Shorer]

On Tue, Jun 13, 2017 at 3:21 PM, Andrey Konovalov  wrote:
> Hi!
>
> I'm trying to use gadgetfs to fuzz USB device drivers by simply
> connecting random devices for now.
>
> What I want to achieve right now is the following:
>
> 1. mount gadgetfs
> 2. emulate connection of a new USB device
> 3. wait for the device to finish initializing
> 4. unmount gadgetfs
> 5. goto 1
>
> The question is how do I wait for the device to finish initializing
> (the corresponding USB driver to finish probing?) before unmounting
> gadgetfs? As I understand, when I write device description to
> "/dev/gadget/dummy_udc" the initialization happens asynchronously
> after writing is done.
>

You can use inotify on the state file inside a udc's sysfs directory.
It's awesome like that.
Here's a sample code that doesn't do any cleanups because I'm lazy:
https://gist.github.com/talshorer/61be1bc3472cc2a7b4866b9bd5a239ca
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: let generic driver yield control iff specific driver has been enabled (was Re: Two regressions on BYT/T ASUS T100TA 4.12-rc: #2 Keyboard no longer works)

[Jiri Kosina]

On Tue, 13 Jun 2017, Benjamin Tissoires wrote:

> > I tried to cook some script that formats the list in the same way than
> > yours.
> > 
> > I just noticed that you have in CONFIG_HID_CHICONY:
> > { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_ASUS_AK1D) },
> > while in v4.12-rc5 it's:
> > { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
> 
> False alert on this one. I applied Jiri's patch on top of his for-next
> branch, and in this branch, this has been renamed into
> USB_DEVICE_ID_ASUS_AK1D.

Yeah, this is going to generate conflict in -next, but easy to resolve 
once it appears. I'll fix that up as soon as this patch hits Linus' tree.

> > Rest is almost OK with respect to device/driver allocation:
> > 
> > Also, comparing the raw number of devices, there are 356 devices in
> > 4.12-rc5, and you have 360 in your patch:
> > - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 
> > USB_DEVICE_ID_LOGITECH_G920_WHEEL) }
> >   is added twice (the one in CONFIG_HID_LOGITECH should be dropped IMO).
> 
> Only this one is would require a change now.

This single change has now been done to the branch.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gadgetfs: how to wait for USB device initialization?

[Alan Stern]

On Tue, 13 Jun 2017, Andrey Konovalov wrote:

> Hi!
> 
> I'm trying to use gadgetfs to fuzz USB device drivers by simply
> connecting random devices for now.
> 
> What I want to achieve right now is the following:
> 
> 1. mount gadgetfs
> 2. emulate connection of a new USB device
> 3. wait for the device to finish initializing
> 4. unmount gadgetfs
> 5. goto 1
> 
> The question is how do I wait for the device to finish initializing
> (the corresponding USB driver to finish probing?) before unmounting
> gadgetfs? As I understand, when I write device description to
> "/dev/gadget/dummy_udc" the initialization happens asynchronously
> after writing is done.

The most generic approach is to monitor the system log and wait for the 
appropriate messages to show up.  If you know a little more about the 
device (such as which driver it will bind to), you might be able to use 
a udev library to do the monitoring.

Or since the gadget driver in this case is part of your program, you 
could wait until your program sees all the USB requests that get sent 
during the initialization and probing procedures.

For a really simple approach, just wait a fixed amount of time, like 10 
seconds.  Unless the system is highly loaded or probing takes a lot 
longer than usual, that should be enough.

Alan Stern

> To mount and unmount gadgetfs right now I do:
> mount("none", "/dev/gadget", "gadgetfs", 0, NULL)
> umount2("/dev/gadget", MNT_FORCE | MNT_DETACH)
> 
> Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Gadget driver ->disconnect callback during unbinding

[Alan Stern]

On Tue, 13 Jun 2017, Felipe Balbi wrote:

> 
> Hi Alan,
> 
> Alan Stern  writes:
> > Felipe:
> >
> > A UDC driver will invoke the gadget driver's ->disconnect callback
> > whenever the D+ pullup is turned off.  During gadget driver unbinding,
> > this happens automatically when usb_gadget_remove_driver() calls
> > usb_gadget_disconnect().
> 
> usb_gadget_disconnect() only calls UDC's ->pullup(), not gadget driver's
> ->disconnect()
> 
> int usb_gadget_disconnect(struct usb_gadget *gadget)
> {
>   int ret = 0;
> 
>   if (!gadget->ops->pullup) {
>   ret = -EOPNOTSUPP;
>   goto out;
>   }
> 
>   if (gadget->deactivated) {
>   /*
>* If gadget is deactivated we only save new state.
>* Gadget will stay disconnected after activation.
>*/
>   gadget->connected = false;
>   goto out;
>   }
> 
>   ret = gadget->ops->pullup(gadget, 0);
>   if (!ret)
>   gadget->connected = 0;
> 
> out:
>   trace_usb_gadget_disconnect(gadget, ret);
> 
>   return ret;
> }
> EXPORT_SYMBOL_GPL(usb_gadget_disconnect);

Yes, but as I mentioned above, the pullup routine calls the gadget
driver's disconnect method.

> > But immediately thereafter, usb_gadget_remove_driver() calls the gadget 
> > driver's ->disconnect callback directly!  Do we have any reason for 
> > doing this?  I don't see point to it.  Should that call be removed?
> 
> Unless I'm missing something, that call is necessary :-) Have you faced
> any issues with it? Are there any UDCs calling gadget driver's
> ->disconnect() from ->pullup() ?

I haven't faced any issues because gadget drivers don't seem to mind 
->disconnect getting called twice in a row.  :-)  (And note that the 
API doesn't have a corresponding ->connect callback...  Although to 
tell the truth, it's not clear what a gadget driver needs to do in 
either callback.  Can we simply remove ->disconnect altogether?)

Both dummy-hcd and net2280 call ->disconnect from ->pullup.  I haven't
checked other UDC drivers, but it seems like something they should all
do.  Except perhaps in the case where the UDC driver doesn't have a
pullup method.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: let generic driver yield control iff specific driver has been enabled (was Re: Two regressions on BYT/T ASUS T100TA 4.12-rc: #2 Keyboard no longer works)

[Benjamin Tissoires]

On Jun 13 2017 or thereabouts, Benjamin Tissoires wrote:
> On Jun 13 2017 or thereabouts, Jiri Kosina wrote:
> > So I've now pushed the latest version to 'for-4.12/driver-matching-fix' of 
> > hid.git, and if no more issues are discovered, I'll push that to Linus 
> > this week so that we finally get rid of this long-lasting PITA (while 
> > still heading towards 'automatic' proper matching -- Benjamin already had 
> > some proposals how to tackle this).
> 
> Hi Jiri,
> 
> I tried to cook some script that formats the list in the same way than
> yours.
> 
> I just noticed that you have in CONFIG_HID_CHICONY:
>   { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_ASUS_AK1D) },
> while in v4.12-rc5 it's:
>   { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
>   ^^^

False alert on this one. I applied Jiri's patch on top of his for-next
branch, and in this branch, this has been renamed into
USB_DEVICE_ID_ASUS_AK1D.

> 
> Rest is almost OK with respect to device/driver allocation:
> 
> Also, comparing the raw number of devices, there are 356 devices in
> 4.12-rc5, and you have 360 in your patch:
> - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL) 
> }
>   is added twice (the one in CONFIG_HID_LOGITECH should be dropped IMO).

Only this one is would require a change now.

Cheers,
Benjamin

> - { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
>   appeared, which is fine according to hid-uclogic.c
> - The two bluetooth devices for hid-gfrm.c are legitimate too.
> 
> So fixing the extra USB_DEVICE_ID_LOGITECH_G920_WHEEL and the messed up
> USB_DEVICE_ID_CHICONY_AK1D should be enough to have it in Linus' tree.
> 
> BTW, the merge with your for-next branch is going to be tricky :(
> 
> Cheers,
> Benjamin
> 
> > 
> > Thanks,
> > 
> > -- 
> > Jiri Kosina
> > SUSE Labs
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: let generic driver yield control iff specific driver has been enabled (was Re: Two regressions on BYT/T ASUS T100TA 4.12-rc: #2 Keyboard no longer works)

[Benjamin Tissoires]

On Jun 13 2017 or thereabouts, Jiri Kosina wrote:
> So I've now pushed the latest version to 'for-4.12/driver-matching-fix' of 
> hid.git, and if no more issues are discovered, I'll push that to Linus 
> this week so that we finally get rid of this long-lasting PITA (while 
> still heading towards 'automatic' proper matching -- Benjamin already had 
> some proposals how to tackle this).

Hi Jiri,

I tried to cook some script that formats the list in the same way than
yours.

I just noticed that you have in CONFIG_HID_CHICONY:
{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_ASUS_AK1D) },
while in v4.12-rc5 it's:
{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
  ^^^

Rest is almost OK with respect to device/driver allocation:

Also, comparing the raw number of devices, there are 356 devices in
4.12-rc5, and you have 360 in your patch:
- { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL) }
  is added twice (the one in CONFIG_HID_LOGITECH should be dropped IMO).
- { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
  appeared, which is fine according to hid-uclogic.c
- The two bluetooth devices for hid-gfrm.c are legitimate too.

So fixing the extra USB_DEVICE_ID_LOGITECH_G920_WHEEL and the messed up
USB_DEVICE_ID_CHICONY_AK1D should be enough to have it in Linus' tree.

BTW, the merge with your for-next branch is going to be tricky :(

Cheers,
Benjamin

> 
> Thanks,
> 
> -- 
> Jiri Kosina
> SUSE Labs
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM

[Mathias Nyman]

On 06.06.2017 09:33, Thang Q. Nguyen wrote:

On Mon, Jun 5, 2017 at 9:33 PM, Mathias Nyman  wrote:

On 05.06.2017 15:57, Thang Q. Nguyen wrote:


On Mon, Jun 5, 2017 at 6:14 PM, Mathias Nyman
 wrote:


On 20.05.2017 10:24, Thang Q. Nguyen wrote:



XHCI specification 1.1 does not require xHCI 1.0 compliant controllers
to always enable hardware USB2 LPM.
However, the current xHCI driver always enable it by setting HLE=1 when
seeing HLC=1. This makes certain xHCI controllers that have broken USB2
HW LPM fail to work as there is no way to disable this feature.
This patch adds support to control disabling USB2 Hardware LPM via
DT/ACPI attribute.



   Wouldn't it be better to just keep  xhci->hw_lpm_support = 0 if the
host
doesn't support Hardware LPM Capability, (HLC)?

This should prevent all those extra steps getting here just to do
nothing.


No, HLC = 0 means the host doesn't support Hardware LPM.
The problem here is the host support Hardware LPM but there is a bug
in host controller that make the LPM fail to work.



So the host support hw LPM, and has its HLC capability bit set,
but in the end it just doesn't work at all, and should be prevented.


When debugging the host controller, we detect there are some holes in
the current usb specifications which can can result in inter-operating
problems between USB Host Controller and USB PHY. To be more specific,
the specs have not clarified the resume recovery timing after the port
has just waken up from L1. This can lead to different interpretations
of the specs by Host Controller and PHY. What happened in our case is
that a Host controller cannot work with a PHY right after resuming
from L1 because these two Vendors have different views of the specs
regarding LPM timing after L1. These views are contradictory and
cannot work together.

If Host Controller and PHY are from the same vendor, they might have
some "internal handshake mechanisms" to avoid these holes of the USB
specs. However, these mechanisms are not standardized in USB specs;
and not all vendors follow these mechanisms. In fact, we have observed
this kind of "internal handshake mechanism" in HOST Controller and PHY
from SYNOPSYS DWC. So, we can say that if users use Host Controller
and PHY from different Vendors, the inteopering problems after waking
up from L1 are more likely to occur.



Can you explain the reason why you prefer preventing hw lpm inside the
xhci_set_usb2_hardware_lpm() function instead of preventing hw lpm usage
all together for this platform -i.e. by not setting xhci->hw_lpm_support

The reason I don't change in the xhci_add_in_port() function inside
xhci-mem.c is because of the description for xhci->hw_lpm_support in
the drivers/usb/host/xhci.h header file: support xHCI 1.0 spec USB2
hardware LPM. Per my understanding, this attribute is used to indicate
if the host supports HW LPM and this can be checked via HLC.
My intension is to support an option for user to disable the HW LPM
because of some reasons (in my case because HW LPM is broken).


I think we should keep supporting new options separate from workarounds
for broken HW.




Again, something like:
if (temp & XHCI_HLC && !(xhci->quirks & XHCI_HW_LPM_BROKEN))
 xhci->hw_lpm_support = 1;

This should work too. But the DT/ACPI attribute should change to
"usb2-lpm-broken".


This would be more clear for future developers and prevent them from
enabling hw lpm for this host to gain some powersaving.

A new feature allowing optional host hw lpm disabling can then be written 
separately,
preferable without using the quirk bits.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] xhci: AMD Promontory USB disable port support

[Mathias Nyman]

On 13.06.2017 14:26, Jiahau Chang wrote:

2017-06-07 16:02 GMT+08:00 Mathias Nyman :

On 06.06.2017 13:13, Jiahau Chang wrote:


v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK

For AMD Promontory xHCI host, although you can disable USB 2.0 ports in
BIOS
settings, those ports will be enabled anyway after you remove a device on
that port and re-plug it in again. It's a known limitation of the chip.
As a workaround we can clear the PORT_WAKE_BITS.

Signed-off-by: Jiahau Chang 
---
   drivers/usb/host/xhci-hub.c |  2 ++
   drivers/usb/host/xhci-pci.c | 13 +
   drivers/usb/host/xhci.h |  2 ++
   3 files changed, 17 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0dde49c..8994676 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1461,6 +1461,8 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 t2 |= PORT_WKOC_E | PORT_WKCONN_E;
 t2 &= ~PORT_WKDISC_E;
 }
+   if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) &&
(hcd->speed < HCD_USB3))
+   t2 &= ~PORT_WAKE_BITS;



This will disable connect/disconnect/overcurrent wake for all usb2 ports on
AMD Promontory host.
I don't think that is what you want.

Is there a way for the driver to see which ports are disabled in bios?
If yes, then it would be better to just disable wake for those ports


Sorry for reply late. Our customers always request remote wake-up
feature only, they don’t want to support
connect/disconnect/overcurrent wake according to experience on other
OS. Due to support disable port feature, WOE and WCE bit should be
disabled.



I'm still a bit uncomfortable with this.

This change will make a hardcoded policy decision to never support Wake on
Connect/Disconnect/Overcurrent on USB2 ports for any AMD Promontory based 
product
now or in the future.

Is there really no way the driver can know if a port is disabled in BIOS?
Something in ACPI DSDT under xhci or port devices, or some bits in the PORTSC 
register?

-Mathias


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

gadgetfs: how to wait for USB device initialization?

[Andrey Konovalov]

Hi!

I'm trying to use gadgetfs to fuzz USB device drivers by simply
connecting random devices for now.

What I want to achieve right now is the following:

1. mount gadgetfs
2. emulate connection of a new USB device
3. wait for the device to finish initializing
4. unmount gadgetfs
5. goto 1

The question is how do I wait for the device to finish initializing
(the corresponding USB driver to finish probing?) before unmounting
gadgetfs? As I understand, when I write device description to
"/dev/gadget/dummy_udc" the initialization happens asynchronously
after writing is done.

To mount and unmount gadgetfs right now I do:
mount("none", "/dev/gadget", "gadgetfs", 0, NULL)
umount2("/dev/gadget", MNT_FORCE | MNT_DETACH)

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] xhci: AMD Promontory USB disable port support

[Jiahau Chang]

2017-06-07 16:02 GMT+08:00 Mathias Nyman :
> On 06.06.2017 13:13, Jiahau Chang wrote:
>>
>> v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
>>
>> For AMD Promontory xHCI host, although you can disable USB 2.0 ports in
>> BIOS
>> settings, those ports will be enabled anyway after you remove a device on
>> that port and re-plug it in again. It's a known limitation of the chip.
>> As a workaround we can clear the PORT_WAKE_BITS.
>>
>> Signed-off-by: Jiahau Chang 
>> ---
>>   drivers/usb/host/xhci-hub.c |  2 ++
>>   drivers/usb/host/xhci-pci.c | 13 +
>>   drivers/usb/host/xhci.h |  2 ++
>>   3 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index 0dde49c..8994676 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -1461,6 +1461,8 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
>> t2 |= PORT_WKOC_E | PORT_WKCONN_E;
>> t2 &= ~PORT_WKDISC_E;
>> }
>> +   if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) &&
>> (hcd->speed < HCD_USB3))
>> +   t2 &= ~PORT_WAKE_BITS;
>
>
> This will disable connect/disconnect/overcurrent wake for all usb2 ports on
> AMD Promontory host.
> I don't think that is what you want.
>
> Is there a way for the driver to see which ports are disabled in bios?
> If yes, then it would be better to just disable wake for those ports
>
Sorry for reply late. Our customers always request remote wake-up
feature only, they don’t want to support
connect/disconnect/overcurrent wake according to experience on other
OS. Due to support disable port feature, WOE and WCE bit should be
disabled.


> -Mathias
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Rafael J. Wysocki]

On Tuesday, June 13, 2017 07:54:22 AM Dominik Brodowski wrote:
> Rafael,
> 
> On Mon, Jun 12, 2017 at 10:46:47PM +0200, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > > spurious SCI wakeups from suspend-to-idle) which is still there in 
> > > 4.12-rc4 but
> > > will be reverted shortly, because it triggered issues on quite a few 
> > > systems.
> > > 
> > > The last patch in the series is a counterpart of the above commit with a 
> > > few
> > > modifications, mostly to avoid affecting suspend-to-RAM and to reorder 
> > > messages
> > > printed to kernel logs to make them somewhat less confusing.
> > 
> > Here's a v2 which is very similar to the previous one, except for 3 things:
> > - There are few build fixes in the last patch.
> > - The patch from Hans mentioned previously is included now (as [7/8]).
> > - There is an additional PCI change related to config space saving/restoring
> >   and PME.
> > 
> > If anyone has any objections or concerns regarding this, please speak up 
> > now,
> > as I'm going to put it into linux-next shortly to allow it to receive some 
> > more
> > testing than commit eed4d47efe95 had had before it went it.
> 
> suspend-to-mem works as expected, also with this v2 applied. Thanks!

Thanks for the confirmation!

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Rafael J. Wysocki]

On Tuesday, June 13, 2017 10:52:52 AM Greg KH wrote:
> On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki 
> > 
> > hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> > PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> > and may become problematic.
> > 
> > It is unnecessary, because the PCI bus type carries out post-suspend
> > cleanup of all PCI devices during resume and that covers all things
> > done by the pci_back_from_sleep().  There is no reason why USB cannot
> > follow all of the other PCI devices in that respect.
> > 
> > It will become problematic after subsequent changes that make it
> > possible to go back to sleep again after executing dpm_resume_noirq()
> > if no valid system wakeup events have been detected at that point.
> > Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> > will cause the wakeup status of the devices in question to be cleared
> > and if any of them has triggered system wakeup, that event may be
> > missed then.
> > 
> > For the above reasons, drop the pci_back_from_sleep() invocation
> > from hcd_pci_resume_noirq().
> > 
> > Signed-off-by: Rafael J. Wysocki 
> > Acked-by: Alan Stern 
> 
> Acked-by: Greg Kroah-Hartman 

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: let generic driver yield control iff specific driver has been enabled (was Re: Two regressions on BYT/T ASUS T100TA 4.12-rc: #2 Keyboard no longer works)

[Jiri Kosina]

On Fri, 9 Jun 2017, Hans de Goede wrote:

> So I've decided to just go ahead and give this patch a second pair
> of eyes, it is mostly good, but I did find some issues, comments
> inline.

Thanks for all the comments, highly appreciated.

[ ... snipped the ones that have been put into v2 ... ]

> Last (wrt logitech) there is an issue with the logitech ff drivers, hid-lg.h /
> hid-lg4ff.h has the following structure:
> 
> #ifdef CONFIG_LOGITECH_FF
> int lgff_init(struct hid_device *hdev);
> #else
> static inline int lgff_init(struct hid_device *hdev) { return -1; }
> #endif
> 
> Notice the return -1 in the inline stub, this causes lg_probe() to fail,
> I believe this should be changed to return 0 for all 4 ff variants, so
> that things will continue to work without ff.

Hm, I see your point, but this is completely unrelated to this change and 
should eventually be fixed separately.

> Last you seem to be missing the following block (already missing before this
> commit):
> 
> #if IS_ENABLED(CONFIG_HID_GFRM
>   { HID_USB_DEVICE(0x58, 0x2000) },
>   { HID_USB_DEVICE(0x471, 0x2210) },

I think you meant HID_BLUETOOTH_DEVICE() here.

> #endif
> 
> The following entries get removed without showing up somewhere else:
> 
> - { HID_USB_DEVICE(USB_VENDOR_ID_LG, USB_DEVICE_ID_LG_MELFAS_MT) },
> 
> It is the only HID_USB_DEVICE entry (instead of MT_USB_DEVICE) in
> hid-multitouch.c,
> as such it probably belongs in a:

Yeah, my script didn't count with this, thanks a lot for catching it.

> #if IS_ENABLED(CONFIG_HID_MULTITOUCH
>   { HID_USB_DEVICE(USB_VENDOR_ID_LG, USB_DEVICE_ID_LG_MELFAS_MT) },
> #endif

So I've now pushed the latest version to 'for-4.12/driver-matching-fix' of 
hid.git, and if no more issues are discovered, I'll push that to Linus 
this week so that we finally get rid of this long-lasting PITA (while 
still heading towards 'automatic' proper matching -- Benjamin already had 
some proposals how to tackle this).

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v15 2/7] power: add power sequence library

[Ulf Hansson]

[...]

> +
> +/**
> + * of_pwrseq_on - Carry out power sequence on for device node
> + *
> + * @np: the device node would like to power on
> + *
> + * Carry out a single device power on.  If multiple devices
> + * need to be handled, use of_pwrseq_on_list() instead.
> + *
> + * Return a pointer to the power sequence instance on success,
> + * or an error code otherwise.
> + */
> +struct pwrseq *of_pwrseq_on(struct device_node *np)
> +{
> +   struct pwrseq *pwrseq;
> +   int ret;
> +
> +   pwrseq = pwrseq_find_available_instance(np);
> +   if (!pwrseq)
> +   return ERR_PTR(-ENOENT);

In case the pwrseq instance hasn't been registered yet, then there is
no way to deal with -EPROBE_DEFER properly here.

I haven't been following the discussions in-depth during all
iterations, so perhaps you have already discussed why doing it like
this.

Anyway, that means all pwrseq instances needs to be registered an
early boot level, to be safe. To me, that seems like poor design
choice.

[...]

Otherwise I think this looks okay to me.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb/gadget: potential deadlock in gadgetfs_suspend

[Andrey Konovalov]

On Mon, Jun 12, 2017 at 10:25 PM, Alan Stern  wrote:
>
> As you surmised, this was caused by a race.  The race was between
> dummy_udc_stop() and set_link_state(), both in dummy_hcd.c.  A symptom
> of this race is that the first routine clears dum->driver while the
> second dereferences it, and neither access is protected by a lock.
>
> It turns out this problem affects at least one other UDC driver
> (net2280.c).  Below is a patch that adds the missing lock (and removes
> some unneeded unlocks) from these drivers.  It turns out that the
> disconnect, reset, suspend, and resume callbacks should all be invoked
> while the UDC's lock is held, because they can race with gadget driver
> unbinding.
>
> Adding locked regions can lead to the possibility of deadlock or
> lockdep violations.  Please let me know if the patch below raises any
> problems.

Hi Alan,

Thanks for the patch!

I've been testing with your patch applied and the "bad spinlock magic"
crashes seem to be gone. However I got another crash (happened only
once over the night), which happens during "spin_lock_irqsave
(>lock, flags)":

kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault:  [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 3 PID: 900 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #36
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
task: 88006c5aadc0 task.stack: 88006c62
RIP: 0010:__lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246
RSP: 0018:88006c625a80 EFLAGS: 00010006
RAX: dc00 RBX: 88006c5aadc0 RCX: 
RDX: 0003 RSI:  RDI: 11000d8c4bab
RBP: 88006c625fc0 R08: 0001 R09: 0001
R10: 88006c5ab698 R11: 87dd2e80 R12: dc00
R13: 0001 R14: 0018 R15: 
FS:  () GS:88006dd0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 20fd CR3: 6797e000 CR4: 06e0
Call Trace:
 lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
 _raw_spin_lock_irqsave+0xc9/0x110 kernel/locking/spinlock.c:159
 gadgetfs_disconnect+0xf1/0x230 drivers/usb/gadget/legacy/inode.c:1664
 usb_gadget_udc_reset+0x3b/0xb0 drivers/usb/gadget/udc/core.c:1020
 set_link_state+0x648/0x9f0 drivers/usb/gadget/udc/dummy_hcd.c:446
 dummy_hub_control+0x11bb/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2243
 rh_call_control drivers/usb/core/hcd.c:689 [inline]
 rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
 usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
 usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
 usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
 usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
 usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
 set_port_feature+0x73/0x90 drivers/usb/core/hub.c:422
 hub_port_reset+0x277/0x1550 drivers/usb/core/hub.c:2772
 hub_port_init+0x7dc/0x2940 drivers/usb/core/hub.c:4510
 hub_port_connect drivers/usb/core/hub.c:4826 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
 port_event drivers/usb/core/hub.c:5105 [inline]
 hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
 process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
 process_scheduled_works kernel/workqueue.c:2157 [inline]
 worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
 kthread+0x363/0x440 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
Code: e9 03 f3 48 ab 48 81 c4 18 05 00 00 44 89 e0 5b 41 5c 41 5d 41
5e 41 5f 5d c3 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
3c 02 00 0f 85 51 24 00 00 49 81 3e 40 ea 13 87 41 bd 00 00
RIP: __lock_acquire+0xbe5/0x3690 kernel/locking/lockdep.c:3246 RSP:
88006c625a80
---[ end trace f5a7b971fc1b0546 ]---
Kernel panic - not syncing: Fatal exception
Shutting down cpus with NMI
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..

>
> Alan Stern
>
>
>
> Index: usb-4.x/drivers/usb/gadget/legacy/inode.c
> ===
> --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c
> +++ usb-4.x/drivers/usb/gadget/legacy/inode.c
> @@ -1679,9 +1679,10 @@ static void
>  gadgetfs_suspend (struct usb_gadget *gadget)
>  {
> struct dev_data *dev = get_gadget_data (gadget);
> +   unsigned long   flags;
>
> INFO (dev, "suspended from state %d\n", dev->state);
> -   spin_lock (>lock);
> +   spin_lock_irqsave(>lock, flags);
> switch (dev->state) {
> case STATE_DEV_SETUP:   // VERY odd... host died??
> case STATE_DEV_CONNECTED:
> @@ -1692,7 +1693,7 @@ gadgetfs_suspend (struct usb_gadget *gad
>   

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

[Greg Kroah-Hartman]

On Tue, Jun 13, 2017 at 11:57:14AM +0200, Johan Hovold wrote:
> On Tue, Jun 13, 2017 at 11:48:41AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 06, 2017 at 12:27:04PM +0100, Alan Cox wrote:
> > > On Tue,  6 Jun 2017 12:54:40 +0200
> > > Johan Hovold  wrote:
> > > 
> > > > Remove three ifdefed and broken implementations of TIOCSSERIAL and
> > > > TIOCGICOUNT, and parity handling in set_termios which had suffered
> > > > severe bit rot.
> > > 
> > > I would be amazed if the IRDA code still works. It's not been tested
> > > properly for years and it never followed the tty rules properly in the
> > > first place - so this looks good, although moving IRDA into staging
> > > and /dev/null would IMHO be far better.
> > > 
> > > IRDA is dead, and IR remotes are handled via completely different code.
> > 
> > I agree, and we should move it into staging and out of the tree, I'll
> > send netdev some patches for that...
> 
> Sounds good, but please consider applying this patch before moving to
> staging as people have been modifying this dead code as part of
> kernel-wide updates even though hasn't even compiled for quite some
> time.

I already applied it :)

I'll wait for 4.13-rc1 before doing it (if I remember...)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

[Johan Hovold]

On Tue, Jun 13, 2017 at 11:57:14AM +0200, Johan Hovold wrote:
> On Tue, Jun 13, 2017 at 11:48:41AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 06, 2017 at 12:27:04PM +0100, Alan Cox wrote:
> > > On Tue,  6 Jun 2017 12:54:40 +0200
> > > Johan Hovold  wrote:
> > > 
> > > > Remove three ifdefed and broken implementations of TIOCSSERIAL and
> > > > TIOCGICOUNT, and parity handling in set_termios which had suffered
> > > > severe bit rot.
> > > 
> > > I would be amazed if the IRDA code still works. It's not been tested
> > > properly for years and it never followed the tty rules properly in the
> > > first place - so this looks good, although moving IRDA into staging
> > > and /dev/null would IMHO be far better.
> > > 
> > > IRDA is dead, and IR remotes are handled via completely different code.
> > 
> > I agree, and we should move it into staging and out of the tree, I'll
> > send netdev some patches for that...
> 
> Sounds good, but please consider applying this patch before moving to
> staging as people have been modifying this dead code as part of
> kernel-wide updates even though hasn't even compiled for quite some
> time.

That is, just that which you ended up doing. :)

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

[Johan Hovold]

On Tue, Jun 13, 2017 at 11:48:41AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 06, 2017 at 12:27:04PM +0100, Alan Cox wrote:
> > On Tue,  6 Jun 2017 12:54:40 +0200
> > Johan Hovold  wrote:
> > 
> > > Remove three ifdefed and broken implementations of TIOCSSERIAL and
> > > TIOCGICOUNT, and parity handling in set_termios which had suffered
> > > severe bit rot.
> > 
> > I would be amazed if the IRDA code still works. It's not been tested
> > properly for years and it never followed the tty rules properly in the
> > first place - so this looks good, although moving IRDA into staging
> > and /dev/null would IMHO be far better.
> > 
> > IRDA is dead, and IR remotes are handled via completely different code.
> 
> I agree, and we should move it into staging and out of the tree, I'll
> send netdev some patches for that...

Sounds good, but please consider applying this patch before moving to
staging as people have been modifying this dead code as part of
kernel-wide updates even though hasn't even compiled for quite some
time.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/9] tty: drop broken alt-speed handling

[Greg Kroah-Hartman]

On Tue, Jun 06, 2017 at 12:54:32PM +0200, Johan Hovold wrote:
> Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated
> since v2.1.69 and has been broken for all tty drivers but serial-core
> since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting
> speed via SPD_*") without anyone noticing (for four years).
> 
> Instead of reverting the offending commit, lets get rid of this legacy
> code while adding back a warning about the SPD flags being deprecated to
> the drivers that once implemented it. Note that drivers implementing
> SPD_CUST will continue to support using a custom divisor.
> 
> Also note that serial-core did not rely on the TTY layer for SPD
> handling and continues to support it while warning about deprecation
> (since 2003 at least).
> 
> Greg, I suggest you take it all trough the TTY tree even if merging
> through multiple trees and applying the final patch once everything is
> upstream would be an option. Also the irda clean up does not depend on
> the rest of series as the code implementing SPD handling was ifdefed
> out.

I've taken these all through my tty tree, nice work, thanks for the
cleanups.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

[Greg Kroah-Hartman]

On Tue, Jun 06, 2017 at 12:27:04PM +0100, Alan Cox wrote:
> On Tue,  6 Jun 2017 12:54:40 +0200
> Johan Hovold  wrote:
> 
> > Remove three ifdefed and broken implementations of TIOCSSERIAL and
> > TIOCGICOUNT, and parity handling in set_termios which had suffered
> > severe bit rot.
> 
> I would be amazed if the IRDA code still works. It's not been tested
> properly for years and it never followed the tty rules properly in the
> first place - so this looks good, although moving IRDA into staging
> and /dev/null would IMHO be far better.
> 
> IRDA is dead, and IR remotes are handled via completely different code.

I agree, and we should move it into staging and out of the tree, I'll
send netdev some patches for that...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

On Tue, Jun 13, 2017 at 12:19 PM, Greg KH  wrote:
> On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer wrote:
>> The user can issue USB_F_GET_LINE_CODING to get the current line coding
>> as set by the host (or the default if unset yet).
>>
>> Signed-off-by: Tal Shorer 
>> ---
>>  Documentation/ioctl/ioctl-number.txt |  1 +
>>  drivers/usb/gadget/function/f_acm.c  | 19 +++
>>  include/uapi/linux/usb/f_acm.h   | 12 
>
> Where is this ioctl being called?  On the tty device?  If so, which one?
> The gadget driver's tty device node?  Or somewhere else?
On an acm ttyGS* fd, yes.
>
> confused at the different levels here, sorry.
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Greg KH]

On Tue, Jun 13, 2017 at 09:52:12AM +0300, Tal Shorer wrote:
> The user can issue USB_F_GET_LINE_CODING to get the current line coding
> as set by the host (or the default if unset yet).
> 
> Signed-off-by: Tal Shorer 
> ---
>  Documentation/ioctl/ioctl-number.txt |  1 +
>  drivers/usb/gadget/function/f_acm.c  | 19 +++
>  include/uapi/linux/usb/f_acm.h   | 12 

Where is this ioctl being called?  On the tty device?  If so, which one?
The gadget driver's tty device node?  Or somewhere else?

confused at the different levels here, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Greg KH]

On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> and may become problematic.
> 
> It is unnecessary, because the PCI bus type carries out post-suspend
> cleanup of all PCI devices during resume and that covers all things
> done by the pci_back_from_sleep().  There is no reason why USB cannot
> follow all of the other PCI devices in that respect.
> 
> It will become problematic after subsequent changes that make it
> possible to go back to sleep again after executing dpm_resume_noirq()
> if no valid system wakeup events have been detected at that point.
> Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> will cause the wakeup status of the devices in question to be cleared
> and if any of them has triggered system wakeup, that event may be
> missed then.
> 
> For the above reasons, drop the pci_back_from_sleep() invocation
> from hcd_pci_resume_noirq().
> 
> Signed-off-by: Rafael J. Wysocki 
> Acked-by: Alan Stern 

Acked-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/11] Creation of "usb_device_auth" LSM hook

[Salvatore Mesoraca]

2017-06-12 23:31 GMT+02:00 Casey Schaufler :
> Return the error reported by the hook rather than -EPERM.

Agreed, anyway this part will be, probably, dropped in
the next version (read Greg and Krzysztof answers).
I'm sorry :(

Thank you very much for the time you spent on this.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/11] Creation of "usb_device_auth" LSM hook

[Salvatore Mesoraca]

2017-06-12 21:38 GMT+02:00 Greg Kroah-Hartman :
> No, like Krzysztof said, you can already do this today, just fine, from
> userspace.  I think that support has been there for over a decade now,
> why are you not taking advantage of this already?
> No need to add extra stuff to the kernel at all to do this

Honesty, now that you mention it, I agree with you.
This feature can be implemented in userspace and I'm the first one to
think that what can be implemented in userspace should stay in
userspace.

> sorry you
> implemented all of this for no reason :(

Don't worry, usb filtering is just a small part of this patchset.
Let's hope the rest will end up being more useful. :)

Thank you very much for taking the time to answer me.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/11] Creation of "usb_device_auth" LSM hook

[Salvatore Mesoraca]

2017-06-12 19:35 GMT+02:00 Krzysztof Opasiak :
> Could you please explain me why we need LSM for this?
>
> There are tools like usbguard[1] and as far as I can tell it looks like they
> can do everything for you...

I have to admit that this is the first time I read about usbguard and it made
me realize that this feature can be implemented in userspace, so no need
to modify the kernel. :)
You are right.

Thank you very much for taking the time to answer me.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt

[Felipe Balbi]

Hi,

Thinh Nguyen  writes:
>>> this could be, I don't remember if I checked this or not :-)
>>>
>>> Really, the best way here, IMHO, would be to re-verify what's going 
>>> on
>>> with macOS and revert my orignal patch since it's, rather clearly,
>>> wrong.
>>>
>>
>> Sure. Are you going to make a revert patch or I am?
>
> Well, after we really know what's going on with macOS and have a 
> better
> fix, then who makes the revert is less important as long as problems 
> get
> sorted out :-) Either way is fine for me.
>

 Do you have any update on this issue?

>>>
>>> The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
>>> invalid") still causes a regression for us. As there hasn't any update
>>> for the macOS issue, can I submit a revert patch for this?
>>
>> I just came back from vacations ;-) I'll get back to this. Reverting
>> that commit won't do any good as we'd be exchanging one regression for
>> another. We really need to understand what's going on.
>
> Hi Felipe,
>
> I think we worked around this same issue in the Synopsys vendor driver
> after a customer reported a problem with CLEAR_FEATURE(ENDPOINT_HALT).
> I no longer have access to either the databook or the codebase, so I
> can't be sure about what the workaround was, but if either John or Thinh
> can have a look at the Clear Stall code in the vendor driver they should
> be able to figure it out.
>>>
>>> Thanks a lot Paul :-) Good to see you still have a look here every once
>>> in a while :-)
>>>
>>> John, Thinh could either of you check what Paul mentions here?
>>>
>>> cheers
>>>
>> 
>> Can you provide more detail on the issue you see on MAC OS? and how to
>> reproduce the issue?
>> 
>
> This issue has been a regression for us for a few months now, do you 
> have any update on this?

ordered a mac to test this again. It'll take a few days.

-- 
balbi


signature.asc
Description: PGP signature

Re: Gadget driver ->disconnect callback during unbinding

[Felipe Balbi]

Hi Alan,

Alan Stern  writes:
> Felipe:
>
> A UDC driver will invoke the gadget driver's ->disconnect callback
> whenever the D+ pullup is turned off.  During gadget driver unbinding,
> this happens automatically when usb_gadget_remove_driver() calls
> usb_gadget_disconnect().

usb_gadget_disconnect() only calls UDC's ->pullup(), not gadget driver's
->disconnect()

int usb_gadget_disconnect(struct usb_gadget *gadget)
{
int ret = 0;

if (!gadget->ops->pullup) {
ret = -EOPNOTSUPP;
goto out;
}

if (gadget->deactivated) {
/*
 * If gadget is deactivated we only save new state.
 * Gadget will stay disconnected after activation.
 */
gadget->connected = false;
goto out;
}

ret = gadget->ops->pullup(gadget, 0);
if (!ret)
gadget->connected = 0;

out:
trace_usb_gadget_disconnect(gadget, ret);

return ret;
}
EXPORT_SYMBOL_GPL(usb_gadget_disconnect);

> But immediately thereafter, usb_gadget_remove_driver() calls the gadget 
> driver's ->disconnect callback directly!  Do we have any reason for 
> doing this?  I don't see point to it.  Should that call be removed?

Unless I'm missing something, that call is necessary :-) Have you faced
any issues with it? Are there any UDCs calling gadget driver's
->disconnect() from ->pullup() ?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH net-next v2 0/2] r8152: adjust runtime suspend/resume

[Hayes Wang]

v2:
For #1, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().

v1:
Improve the flow about runtime suspend/resume and make the code
easy to read.

Hayes Wang (2):
  r8152: split rtl8152_resume function
  r8152: move calling delay_autosuspend function

 drivers/net/usb/r8152.c | 107 
 1 file changed, 62 insertions(+), 45 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next v2 1/2] r8152: split rtl8152_resume function

[Hayes Wang]

Split rtl8152_resume() into rtl8152_runtime_resume() and
rtl8152_system_resume().

Besides, replace GFP_KERNEL with GFP_NOIO for usb_submit_urb().

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 99 ++---
 1 file changed, 61 insertions(+), 38 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 5a02053..2d238b5 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3686,6 +3686,61 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
+static int rtl8152_runtime_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   struct napi_struct *napi = >napi;
+
+   tp->rtl_ops.autosuspend_en(tp, false);
+   napi_disable(napi);
+   set_bit(WORK_ENABLE, >flags);
+
+   if (netif_carrier_ok(netdev)) {
+   if (rtl8152_get_speed(tp) & LINK_STATUS) {
+   rtl_start_rx(tp);
+   } else {
+   netif_carrier_off(netdev);
+   tp->rtl_ops.disable(tp);
+   netif_info(tp, link, netdev, "linking down\n");
+   }
+   }
+
+   napi_enable(napi);
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   smp_mb__after_atomic();
+
+   if (!list_empty(>rx_done))
+   napi_schedule(>napi);
+
+   usb_submit_urb(tp->intr_urb, GFP_NOIO);
+   } else {
+   if (netdev->flags & IFF_UP)
+   tp->rtl_ops.autosuspend_en(tp, false);
+
+   clear_bit(SELECTIVE_SUSPEND, >flags);
+   }
+
+   return 0;
+}
+
+static int rtl8152_system_resume(struct r8152 *tp)
+{
+   struct net_device *netdev = tp->netdev;
+
+   netif_device_attach(netdev);
+
+   if (netif_running(netdev) && netdev->flags & IFF_UP) {
+   tp->rtl_ops.up(tp);
+   netif_carrier_off(netdev);
+   set_bit(WORK_ENABLE, >flags);
+   usb_submit_urb(tp->intr_urb, GFP_NOIO);
+   }
+
+   return 0;
+}
+
 static int rtl8152_runtime_suspend(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
@@ -3784,50 +3839,18 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
 static int rtl8152_resume(struct usb_interface *intf)
 {
struct r8152 *tp = usb_get_intfdata(intf);
-   struct net_device *netdev = tp->netdev;
+   int ret;
 
mutex_lock(>control);
 
-   if (!test_bit(SELECTIVE_SUSPEND, >flags))
-   netif_device_attach(netdev);
-
-   if (netif_running(netdev) && netdev->flags & IFF_UP) {
-   if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   struct napi_struct *napi = >napi;
-
-   tp->rtl_ops.autosuspend_en(tp, false);
-   napi_disable(napi);
-   set_bit(WORK_ENABLE, >flags);
-   if (netif_carrier_ok(netdev)) {
-   if (rtl8152_get_speed(tp) & LINK_STATUS) {
-   rtl_start_rx(tp);
-   } else {
-   netif_carrier_off(netdev);
-   tp->rtl_ops.disable(tp);
-   netif_info(tp, link, netdev,
-  "linking down\n");
-   }
-   }
-   napi_enable(napi);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   if (!list_empty(>rx_done))
-   napi_schedule(>napi);
-   } else {
-   tp->rtl_ops.up(tp);
-   netif_carrier_off(netdev);
-   set_bit(WORK_ENABLE, >flags);
-   }
-   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
-   } else if (test_bit(SELECTIVE_SUSPEND, >flags)) {
-   if (netdev->flags & IFF_UP)
-   tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   }
+   if (test_bit(SELECTIVE_SUSPEND, >flags))
+   ret = rtl8152_runtime_resume(tp);
+   else
+   ret = rtl8152_system_resume(tp);
 
mutex_unlock(>control);
 
-   return 0;
+   return ret;
 }
 
 static int rtl8152_reset_resume(struct usb_interface *intf)
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next v2 2/2] r8152: move calling delay_autosuspend function

[Hayes Wang]

Move calling delay_autosuspend() in rtl8152_runtime_suspend(). Calling
delay_autosuspend() as late as possible.

The original flows are
   1. check if the driver/device is busy now.
   2. set wake events.
   3. enter runtime suspend.

If the wake event occurs between (1) and (2), the device may miss it. Besides,
to avoid the runtime resume occurs after runtime suspend immediately, move the
checking to the end of rtl8152_runtime_suspend().

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2d238b5..b916418 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3752,13 +3752,6 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
if (netif_running(netdev) && test_bit(WORK_ENABLE, >flags)) {
u32 rcr = 0;
 
-   if (delay_autosuspend(tp)) {
-   clear_bit(SELECTIVE_SUSPEND, >flags);
-   smp_mb__after_atomic();
-   ret = -EBUSY;
-   goto out1;
-   }
-
if (netif_carrier_ok(netdev)) {
u32 ocp_data;
 
@@ -3792,6 +3785,11 @@ static int rtl8152_runtime_suspend(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
napi_enable(napi);
}
+
+   if (delay_autosuspend(tp)) {
+   rtl8152_runtime_resume(tp);
+   ret = -EBUSY;
+   }
}
 
 out1:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] net: phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
Acked-by: Sergei Shtylyov 
---
v0 ->v1:
* These files were missing in v0
* drivers/net/ethernet/renesas/ravb_main.c
* drivers/net/ethernet/renesas/sh_eth.c
* drivers/net/ethernet/ti/netcp_ethss.c
* Add Acked-by: Sergei Shtylyov

v1 -> v2:
* Adjust to net-next tree

v2 -> v3:
* These files were missing in v1
* drivers/net/ethernet/apm/xgene-v2/ethtool.c
* drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
* drivers/net/ethernet/broadcom/b44.c
* drivers/net/ethernet/broadcom/bcm63xx_enet.c
* drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
* drivers/net/ethernet/mediatek/mtk_eth_soc.c
* drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
* drivers/net/ethernet/ti/cpsw.c
* drivers/staging/netlogic/xlr_net.c
---
 drivers/net/ethernet/apm/xgene-v2/ethtool.c  |  4 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c  |  8 ++--
 drivers/net/ethernet/broadcom/b44.c  |  4 +++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c |  5 -
 drivers/net/ethernet/broadcom/genet/bcmgenet.c   |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c  |  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c   |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c|  4 +++-
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c |  2 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c   |  5 ++---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c  |  4 +++-
 drivers/net/ethernet/renesas/ravb_main.c | 14 +++---
 drivers/net/ethernet/renesas/sh_eth.c|  5 ++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c |  5 ++---
 drivers/net/ethernet/ti/cpsw.c   |  8 
 drivers/net/ethernet/ti/netcp_ethss.c|  8 +++-
 drivers/net/phy/phy.c| 10 +-
 drivers/net/usb/lan78xx.c|  2 +-
 drivers/staging/netlogic/xlr_net.c   |  5 -
 include/linux/phy.h  |  4 ++--
 net/dsa/slave.c  |  9 +
 21 files changed, 68 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene-v2/ethtool.c 
b/drivers/net/ethernet/apm/xgene-v2/ethtool.c
index be4..d31ad82 100644
--- a/drivers/net/ethernet/apm/xgene-v2/ethtool.c
+++ b/drivers/net/ethernet/apm/xgene-v2/ethtool.c
@@ -157,7 +157,9 @@ static int xge_get_link_ksettings(struct net_device *ndev,
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
 }
 
 static int xge_set_link_ksettings(struct net_device *ndev,
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index 559963b..4f50f11 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -131,13 +131,17 @@ static int xgene_get_link_ksettings(struct net_device 
*ndev,
if (phydev == NULL)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
} else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) {
if (pdata->mdio_driver) {
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg |
diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..f411936 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,9 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..ea3c906 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,10 @@ static int 

Re: [PATCH v3] USB: qcserial: expose methods for modem control

[Johan Hovold]

On Mon, Jun 12, 2017 at 11:52:41AM -0700, Magnus Lynch wrote:
> The qcserial driver fails to expose the .tiocmget and .tiocmset methods
> available from usb_wwan. These methods are required by ioctl commands
> dealing with the modem control signals DTR, RTS, etc.
> 
> With these methods not set ioctl calls intended to control the DTR state
> will fail. For example, pppd drops and raises DTR in preparation to
> dialing the modem, which handles the case of the modem already being
> connected by making it hang up and return to command mode. DTR control
> being unavailable will lead to a protracted failure to connect as the
> modem will be stuck in a state not responsive to command.
> 
> I have tested that with this patch the described case is handled
> successfully. There is an analogous method for .ioctl available from
> usb_wwan (as used in option.c) but I conservatively omitted that for
> lack of familiarity.
> 
> Signed-off-by: Magnus Lynch 
> 
> ---
> Apologies for bad formatting. Further attempting to resolve that.

Thanks for fixing it up, all looks good now. 

Applied for -next.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/6] reset: APIs to manage a list of resets

[Vivek Gautam]

On 06/01/2017 10:21 PM, Philipp Zabel wrote:

A set of patches to allow consumers to get and de/assert or trigger
a number of resets at the same time. A patch on top of Vivek's original
API extension is added to hide the reset_control_array behind a struct
reset_control so that the consumer doesn't have care about the difference
between a singular reset control and reset control controlling an array
of resets, except when requesting the control.

This series also contains reset controls patches for dwc3-of-simple
and tegra pmc drivers.
A small patch is added in this series to correctly re-order the
resource handling in dwc3_of_simple_remove().

The series is tested on torvald's master branch the device tree
patches to enable usb on db820c.

Changes since v4:
  - Added a patch to hide reset control arrays behind struct reset_control
and adapted the consumer patches. This could be merged with the reset
array API patch if we think this is a good idea.


Hi Philipp,

I tested this series again on my db820c setup. Things work fine.
We can merge this series after fixing the 3/6 patch in this series.

Thanks

Hi Felipe,
If you are okay with changes in dwc3, can you kindly consider Ack'ing this
series. May be Philipp can pull the entire series after fixing.

Thanks
Vivek



Changes since v3:
  - Squashed of_reset_control_get_count() patch in the second patch that
adds the reset control array APIs.
  - The error path after getting count through of_reset_control_get_count()
now returns NULL pointer in case when 'optional' flag is true.
  - Added code in reset_control_array_assert() to deassert the
already asserted resets in the error case.
  - Using of_reset_control_array_get_optional_exclusive() in dwc3 patch
to request the reset control array.
  - Added a patch to fix the order in which resources are handled in
dwc3_of_simple_remove() path.
  - Added tegra_powergate->reset to take care of single reset control
passed from the client drivers.

Changes since v2:
  - Addressed comments to make APIs inline with gpiod API.
  - Moved number of reset controls in 'struct reset_control_array'
so that the footprint is reduced.
  - of_reset_control_array_get() and devm_reset_control_array_get()
now return pointer to the newly created reset control array.
  - Added comments to mention that the reset control array APIs don't
guarantee any particular order when handling the reset controls.
  - Dropped 'name' from reset_control_array' since the interface is meant
for a bunch of anonymous resets that can all be asserted or deasserted
in arbitrary order.
  - Fixed returns for APIs reported by kbuild.
  - Fixed 'for' clause guards reported by kbuild.

Changes since v1:
  - Addressed comment for error handling in of_reset_control_get_count()
  - Added patch to manage reset controller array.
  - Rebased dwc3-of-simple changes based on the new set of APIs
for reset control array.
  - Added a patch for soc/tegra/pmc driver to use the new set of
reset control array APIs.

Philipp Zabel (2):
   reset: use kref for reference counting
   reset: hide reset control arrays behind struct reset_control

Vivek Gautam (4):
   reset: Add APIs to manage array of resets
   usb: dwc3: of-simple: Re-order resource handling in remove
   usb: dwc3: of-simple: Add support to get resets for the device
   soc/tegra: pmc: Use the new reset APIs to manage reset controllers

  drivers/reset/core.c  | 234 --
  drivers/soc/tegra/pmc.c   |  82 -
  drivers/usb/dwc3/dwc3-of-simple.c |  29 -
  include/linux/reset.h |  73 
  4 files changed, 344 insertions(+), 74 deletions(-)



--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/8] usb: gadget: u_serial: propagate poll() to the next layer

[Tal Shorer]

In order for a serial function to add flags to the poll() mask of their
tty files, propagate the poll() callback to the next layer so it can
return a mask if it sees fit to do so.

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/u_serial.c | 16 
 drivers/usb/gadget/function/u_serial.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 9b0805f..d466f58 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1025,6 +1025,21 @@ static int gs_break_ctl(struct tty_struct *tty, int 
duration)
return status;
 }
 
+static unsigned int gs_poll(struct tty_struct *tty, struct file *file,
+   poll_table *wait)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   unsigned int mask = 0;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->poll)
+   mask |= gser->poll(gser, file, wait);
+   spin_unlock_irq(>port_lock);
+   return mask;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1035,6 +1050,7 @@ static const struct tty_operations gs_tty_ops = {
.chars_in_buffer =  gs_chars_in_buffer,
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
+   .poll = gs_poll,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index c20210c..ce00840 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -12,6 +12,7 @@
 #ifndef __U_SERIAL_H
 #define __U_SERIAL_H
 
+#include 
 #include 
 #include 
 
@@ -50,6 +51,8 @@ struct gserial {
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
int (*send_break)(struct gserial *p, int duration);
+   unsigned int (*poll)(struct gserial *p, struct file *file,
+   poll_table *wait);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/8] usb: gadget: f_acm: validate set_line_coding requests

[Tal Shorer]

We shouldn't accept malformed set_line_coding requests.
All values were taken from table 17 (section 6.3.11) of the cdc1.2 spec
available at http://www.usb.org/developers/docs/devclass_docs/
The table is in the file PTSN120.pdf.

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/f_acm.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5e3828d..e023313 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -326,13 +326,22 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
struct usb_cdc_line_coding  *value = req->buf;
 
/* REVISIT:  we currently just remember this data.
-* If we change that, (a) validate it first, then
-* (b) update whatever hardware needs updating,
-* (c) worry about locking.  This is information on
-* the order of 9600-8-N-1 ... most of which means
-* nothing unless we control a real RS232 line.
-*/
-   acm->port_line_coding = *value;
+   * If we change that,
+   * (a) update whatever hardware needs updating,
+   * (b) worry about locking.  This is information on
+   * the order of 9600-8-N-1 ... most of which means
+   * nothing unless we control a real RS232 line.
+   */
+   dev_dbg(>gadget->dev,
+   "acm ttyGS%d set_line_coding: %d %d %d %d\n",
+   acm->port_num, le32_to_cpu(value->dwDTERate),
+   value->bCharFormat, value->bParityType,
+   value->bDataBits);
+   if (value->bCharFormat > 2 || value->bParityType > 4 ||
+   value->bDataBits < 5 || value->bDataBits > 8)
+   usb_ep_set_halt(ep);
+   else
+   acm->port_line_coding = *value;
}
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

[Tal Shorer]

If a tty driver wants to notify the user of some exceptional event,
such as a usb cdc acm device set_line_coding event, it needs a way to
modify the mask returned by poll() and possible also add wait queues.
In order to do that, we allow the driver to supply a poll() callback
of its own, which will be called in n_tty_poll().

Signed-off-by: Tal Shorer 
---
 drivers/tty/n_tty.c| 2 ++
 include/linux/tty_driver.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..7af8c29 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2394,6 +2394,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, 
struct file *file,
tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
tty_write_room(tty) > 0)
mask |= POLLOUT | POLLWRNORM;
+   if (tty->ops->poll)
+   mask |= tty->ops->poll(tty, file, wait);
return mask;
 }
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index b742b5e..630ef03 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -243,6 +243,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct tty_struct;
 struct tty_driver;
@@ -285,6 +286,8 @@ struct tty_operations {
int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
int (*get_icount)(struct tty_struct *tty,
struct serial_icounter_struct *icount);
+   unsigned int (*poll)(struct tty_struct *tty, struct file *file,
+   poll_table *wait);
 #ifdef CONFIG_CONSOLE_POLL
int (*poll_init)(struct tty_driver *driver, int line, char *options);
int (*poll_get_char)(struct tty_driver *driver, int line);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/8] usb: gadget: f_acm: add an ioctl to get the current line coding

[Tal Shorer]

The user can issue USB_F_GET_LINE_CODING to get the current line coding
as set by the host (or the default if unset yet).

Signed-off-by: Tal Shorer 
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 drivers/usb/gadget/function/f_acm.c  | 19 +++
 include/uapi/linux/usb/f_acm.h   | 12 
 3 files changed, 32 insertions(+)
 create mode 100644 include/uapi/linux/usb/f_acm.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 1e9fcb4..3d70680 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -329,6 +329,7 @@ Code  Seq#(hex) Include FileComments
 0xCA   80-8F   uapi/scsi/cxlflash_ioctl.h
 0xCB   00-1F   CBM serial IEC bus  in development:


+0xCD   10-1F   linux/usb/f_acm.h
 0xCD   01  linux/reiserfs_fs.h
 0xCF   02  fs/cifs/ioctl.c
 0xDB   00-0F   drivers/char/mwave/mwavepub.h
diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 188d314..5feea7c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "u_serial.h"
 
@@ -611,6 +612,23 @@ static int acm_send_break(struct gserial *port, int 
duration)
return acm_notify_serial_state(acm);
 }
 
+static int acm_ioctl(struct gserial *port, unsigned int cmd, unsigned long arg)
+{
+   struct f_acm*acm = port_to_acm(port);
+   int ret = -ENOIOCTLCMD;
+
+   switch (cmd) {
+   case USB_F_ACM_GET_LINE_CODING:
+   if (copy_to_user((__user void *)arg, >port_line_coding,
+   sizeof(acm->port_line_coding)))
+   ret = -EFAULT;
+   else
+   ret = 0;
+   break;
+   }
+   return ret;
+}
+
 /*-*/
 
 /* ACM function driver setup/binding */
@@ -749,6 +767,7 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
+   acm->port.ioctl = acm_ioctl;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
diff --git a/include/uapi/linux/usb/f_acm.h b/include/uapi/linux/usb/f_acm.h
new file mode 100644
index 000..51f96f0
--- /dev/null
+++ b/include/uapi/linux/usb/f_acm.h
@@ -0,0 +1,12 @@
+/* f_acm.h -- Header file for USB CDC-ACM gadget function */
+
+#ifndef __UAPI_LINUX_USB_F_ACM_H
+#define __UAPI_LINUX_USB_F_ACM_H
+
+#include 
+#include 
+
+/* The 0xCD code is also used by reiserfs. we use 0x10-0x1F range */
+#define USB_F_ACM_GET_LINE_CODING _IOR(0xCD, 0x10, struct usb_cdc_line_coding)
+
+#endif /* __UAPI_LINUX_USB_F_ACM_H */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 8/8] usb: gadget: u_serial: remove port_line_config from struct gserial

[Tal Shorer]

GetLineCoding and SetLineCoding are a cdc-acm thing. It doesn't make
sense to have that in the generic u_serial layer. Moreso, f_acm has its
own port_line_coding in its own struct and it uses that, while the one
in struct gserial is set once upon initialization and then never used.
Also, the initialized never-used values were invalid, with bDataBits
and bCharFormat having each other's value.

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/u_serial.c | 22 ++
 drivers/usb/gadget/function/u_serial.h |  3 ---
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 8d9abf1..654d4a6 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -129,9 +129,6 @@ struct gs_port {
wait_queue_head_t   drain_wait; /* wait while writes drain */
boolwrite_busy;
wait_queue_head_t   close_wait;
-
-   /* REVISIT this state ... */
-   struct usb_cdc_line_coding port_line_coding;/* 8-N-1 etc */
 };
 
 static struct portmaster {
@@ -1314,7 +1311,7 @@ static void gserial_console_exit(void)
 #endif
 
 static int
-gs_port_alloc(unsigned port_num, struct usb_cdc_line_coding *coding)
+gs_port_alloc(unsigned port_num)
 {
struct gs_port  *port;
int ret = 0;
@@ -1343,7 +1340,6 @@ gs_port_alloc(unsigned port_num, struct 
usb_cdc_line_coding *coding)
INIT_LIST_HEAD(>write_pool);
 
port->port_num = port_num;
-   port->port_line_coding = *coding;
 
ports[port_num].port = port;
 out:
@@ -1392,18 +1388,12 @@ EXPORT_SYMBOL_GPL(gserial_free_line);
 
 int gserial_alloc_line(unsigned char *line_num)
 {
-   struct usb_cdc_line_coding  coding;
struct device   *tty_dev;
int ret;
int port_num;
 
-   coding.dwDTERate = cpu_to_le32(9600);
-   coding.bCharFormat = 8;
-   coding.bParityType = USB_CDC_NO_PARITY;
-   coding.bDataBits = USB_CDC_1_STOP_BITS;
-
for (port_num = 0; port_num < MAX_U_SERIAL_PORTS; port_num++) {
-   ret = gs_port_alloc(port_num, );
+   ret = gs_port_alloc(port_num);
if (ret == -EBUSY)
continue;
if (ret)
@@ -1491,11 +1481,6 @@ int gserial_connect(struct gserial *gser, u8 port_num)
gser->ioport = port;
port->port_usb = gser;
 
-   /* REVISIT unclear how best to handle this state...
-* we don't really couple it with the Linux TTY.
-*/
-   gser->port_line_coding = port->port_line_coding;
-
/* REVISIT if waiting on "carrier detect", signal. */
 
/* if it's already open, start I/O ... and notify the serial
@@ -1543,9 +1528,6 @@ void gserial_disconnect(struct gserial *gser)
/* tell the TTY glue not to do I/O here any more */
spin_lock_irqsave(>port_lock, flags);
 
-   /* REVISIT as above: how best to track this? */
-   port->port_line_coding = gser->port_line_coding;
-
port->port_usb = NULL;
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index 8d0901e..0549efe 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -44,9 +44,6 @@ struct gserial {
struct usb_ep   *in;
struct usb_ep   *out;
 
-   /* REVISIT avoid this CDC-ACM support harder ... */
-   struct usb_cdc_line_coding port_line_coding;/* 9600-8-N-1 etc */
-
/* notification callbacks */
void (*connect)(struct gserial *p);
void (*disconnect)(struct gserial *p);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/8] usb: gadget: f_acm: initialize port_line_coding when creating an instance

[Tal Shorer]

Initialize acm->port_line_coding with something that makes sense so
that we can return a valid line coding if the host requests
GetLineCoding before requesting SetLineCoding

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/f_acm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index e023313..188d314 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -763,6 +763,12 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
acm->port.func.unbind = acm_unbind;
acm->port.func.free_func = acm_free_func;
 
+   /* initialize port_line_coding with something that makes sense */
+   acm->port_line_coding.dwDTERate = cpu_to_le32(9600);
+   acm->port_line_coding.bCharFormat = USB_CDC_1_STOP_BITS;
+   acm->port_line_coding.bParityType = USB_CDC_NO_PARITY;
+   acm->port_line_coding.bDataBits = 8;
+
return >port.func;
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/8] usb: gadget: u_serial: propagate ioctl() to the next layer

[Tal Shorer]

In order for a serial function to implement its own ioctl() calls,
propagate the ioctl() callback to the next layer so it can handle it if
it sees fit to do so.

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/u_serial.c | 15 +++
 drivers/usb/gadget/function/u_serial.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index d466f58..8d9abf1 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1040,6 +1040,20 @@ static unsigned int gs_poll(struct tty_struct *tty, 
struct file *file,
return mask;
 }
 
+static int gs_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long 
arg)
+{
+   struct gs_port *port = tty->driver_data;
+   struct gserial *gser;
+   int ret = -ENOIOCTLCMD;
+
+   spin_lock_irq(>port_lock);
+   gser = port->port_usb;
+   if (gser && gser->ioctl)
+   ret = gser->ioctl(gser, cmd, arg);
+   spin_unlock_irq(>port_lock);
+   return ret;
+}
+
 static const struct tty_operations gs_tty_ops = {
.open = gs_open,
.close =gs_close,
@@ -1051,6 +1065,7 @@ static const struct tty_operations gs_tty_ops = {
.unthrottle =   gs_unthrottle,
.break_ctl =gs_break_ctl,
.poll = gs_poll,
+   .ioctl =gs_ioctl,
 };
 
 /*-*/
diff --git a/drivers/usb/gadget/function/u_serial.h 
b/drivers/usb/gadget/function/u_serial.h
index ce00840..8d0901e 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -53,6 +53,7 @@ struct gserial {
int (*send_break)(struct gserial *p, int duration);
unsigned int (*poll)(struct gserial *p, struct file *file,
poll_table *wait);
+   int (*ioctl)(struct gserial *p, unsigned int cmd, unsigned long arg);
 };
 
 /* utilities to allocate/free request and buffer */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 7/8] usb: gadget: f_acm: notify the user on SetLineCoding

[Tal Shorer]

Notify the user with a POLLPRI event when the host issues a
SetLineCoding request so that they can act upon it, for example by
configuring the line coding on a real serial port.

The event is cleared when the user reads the line coding using
USB_F_ACM_GET_LINE_CODING ioctl()

Signed-off-by: Tal Shorer 
---
 drivers/usb/gadget/function/f_acm.c | 38 ++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 5feea7c..0983999 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -58,6 +58,9 @@ struct f_acm {
struct usb_request  *notify_req;
 
struct usb_cdc_line_coding  port_line_coding;   /* 8-N-1 etc */
+   /* we have a SetLineCoding request that the user haven't read yet */
+   bool set_line_coding_pending;
+   wait_queue_head_t set_line_coding_waitq;
 
/* SetControlLineState request -- CDC 1.1 section 6.2.14 (INPUT) */
u16 port_handshake_bits;
@@ -326,23 +329,19 @@ static void acm_complete_set_line_coding(struct usb_ep 
*ep,
} else {
struct usb_cdc_line_coding  *value = req->buf;
 
-   /* REVISIT:  we currently just remember this data.
-   * If we change that,
-   * (a) update whatever hardware needs updating,
-   * (b) worry about locking.  This is information on
-   * the order of 9600-8-N-1 ... most of which means
-   * nothing unless we control a real RS232 line.
-   */
dev_dbg(>gadget->dev,
"acm ttyGS%d set_line_coding: %d %d %d %d\n",
acm->port_num, le32_to_cpu(value->dwDTERate),
value->bCharFormat, value->bParityType,
value->bDataBits);
if (value->bCharFormat > 2 || value->bParityType > 4 ||
-   value->bDataBits < 5 || value->bDataBits > 8)
+   value->bDataBits < 5 || value->bDataBits > 8) {
usb_ep_set_halt(ep);
-   else
+   } else {
acm->port_line_coding = *value;
+   acm->set_line_coding_pending = true;
+   wake_up_interruptible(>set_line_coding_waitq);
+   }
}
 }
 
@@ -598,6 +597,19 @@ static void acm_disconnect(struct gserial *port)
acm_notify_serial_state(acm);
 }
 
+static unsigned int acm_poll(struct gserial *port, struct file *file,
+   poll_table *wait)
+{
+   unsigned int mask = 0;
+   struct f_acm *acm = port_to_acm(port);
+
+   poll_wait(file, >set_line_coding_waitq, wait);
+   if (acm->set_line_coding_pending)
+   mask |= POLLPRI;
+   return mask;
+}
+
+
 static int acm_send_break(struct gserial *port, int duration)
 {
struct f_acm*acm = port_to_acm(port);
@@ -620,10 +632,12 @@ static int acm_ioctl(struct gserial *port, unsigned int 
cmd, unsigned long arg)
switch (cmd) {
case USB_F_ACM_GET_LINE_CODING:
if (copy_to_user((__user void *)arg, >port_line_coding,
-   sizeof(acm->port_line_coding)))
+   sizeof(acm->port_line_coding))) {
ret = -EFAULT;
-   else
+   } else {
ret = 0;
+   acm->set_line_coding_pending = false;
+   }
break;
}
return ret;
@@ -763,11 +777,13 @@ static struct usb_function *acm_alloc_func(struct 
usb_function_instance *fi)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(>lock);
+   init_waitqueue_head(>set_line_coding_waitq);
 
acm->port.connect = acm_connect;
acm->port.disconnect = acm_disconnect;
acm->port.send_break = acm_send_break;
acm->port.ioctl = acm_ioctl;
+   acm->port.poll = acm_poll;
 
acm->port.func.name = "acm";
acm->port.func.strings = acm_strings;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/8] Allow f_acm gadgets to notify the user about SetLineCoding requests

[Tal Shorer]

I'm currently working on a project where I'd like to have an omap board
running linux be a usb-to-uart converter (using f_acm), and I've ran
into an issue: there's no way for the application to know if the host
has issued a SetLineCoding requests (after which parity/baudrate should
be changed to match the host's request).

This series adds the support necessary to achieve that:
- Allowing tty drivers to supply a poll() function to notify the user of
driver-specific events.
- Propagating poll() and ioctl() from u_serial to the next layer (f_acm)
in this case.
- Let the user read the current line coding set by the host (via an
ioctl() call).
- Notify the user when there's a pending SetLineCoding request they
haven't read yet

The last patch also removes up the port_line_config field from
struct gserial. It made no sense to have there (and had a REVISIT
comment at every turn), it was never used and it was initialized with
invalid values.

Changes from v1:
- patch 5 was messed up, which made patch 6 also messed up. fixed both
  of these.

Tal Shorer (8):
  tty: add a poll() callback in struct tty_operations
  usb: gadget: u_serial: propagate poll() to the next layer
  usb: gadget: f_acm: validate set_line_coding requests
  usb: gadget: u_serial: propagate ioctl() to the next layer
  usb: gadget: f_acm: initialize port_line_coding when creating an
instance
  usb: gadget: f_acm: add an ioctl to get the current line coding
  usb: gadget: f_acm: notify the user on SetLineCoding
  usb: gadget: u_serial: remove port_line_config from struct gserial

 Documentation/ioctl/ioctl-number.txt   |  1 +
 drivers/tty/n_tty.c|  2 ++
 drivers/usb/gadget/function/f_acm.c| 66 +-
 drivers/usb/gadget/function/u_serial.c | 53 ---
 drivers/usb/gadget/function/u_serial.h |  7 ++--
 include/linux/tty_driver.h |  3 ++
 include/uapi/linux/usb/f_acm.h | 12 +++
 7 files changed, 113 insertions(+), 31 deletions(-)
 create mode 100644 include/uapi/linux/usb/f_acm.h

--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/6] reset: hide reset control arrays behind struct reset_control

[Vivek Gautam]

Hi Philipp,


On 06/01/2017 10:22 PM, Philipp Zabel wrote:

Reset controls already may control multiple reset lines with a single
hardware bit. So from the user perspective, reset control arrays are not
at all different from single reset controls.
Therefore, hide reset control arrays behind struct reset_control to
avoid having to introduce new API functions for array (de)assert/reset.

Cc: Vivek Gautam 
Cc: Jon Hunter 
Signed-off-by: Philipp Zabel 
---
  drivers/reset/core.c  | 225 ++
  include/linux/reset.h |  44 +++---
  2 files changed, 128 insertions(+), 141 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 1747000757211..c8fb4426b218a 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -43,11 +43,24 @@ struct reset_control {
unsigned int id;
struct kref refcnt;
bool shared;
+   bool array;
atomic_t deassert_count;
atomic_t triggered_count;
  };
  
  /**

+ * struct reset_control_array - an array of reset controls
+ * @base: reset control for compatibility with reset control API functions
+ * @num_rstcs: number of reset controls
+ * @rstc: array of reset controls
+ */
+struct reset_control_array {
+   struct reset_control base;
+   unsigned int num_rstcs;
+   struct reset_control *rstc[];
+};
+
+/**
   * of_reset_simple_xlate - translate reset_spec to the reset line number
   * @rcdev: a pointer to the reset controller device
   * @reset_spec: reset line specifier as found in the device tree
@@ -135,6 +148,65 @@ int devm_reset_controller_register(struct device *dev,
  }
  EXPORT_SYMBOL_GPL(devm_reset_controller_register);
  
+static inline struct reset_control_array *

+rstc_to_array(struct reset_control *rstc) {
+   return container_of(rstc, struct reset_control_array, base);
+}
+
+static int reset_control_array_reset(struct reset_control_array *resets)
+{
+   int ret, i;
+
+   for (i = 0; i < resets->num_rstcs; i++) {
+   ret = reset_control_reset(resets->rstc[i]);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+
+static int reset_control_array_assert(struct reset_control_array *resets)
+{
+   int ret, i;
+
+   for (i = 0; i < resets->num_rstcs; i++) {
+   ret = reset_control_assert(resets->rstc[i]);
+   if (ret)
+   goto err;
+   }
+
+   return 0;
+
+err:
+   while (i--)
+   reset_control_deassert(resets->rstc[i]);
+   return ret;
+}
+
+static int reset_control_array_deassert(struct reset_control_array *resets)
+{
+   int ret, i;
+
+   for (i = 0; i < resets->num_rstcs; i++) {
+   ret = reset_control_deassert(resets->rstc[i]);
+   if (ret)
+   goto err;
+   }
+
+   return 0;
+
+err:
+   while (i--)
+   reset_control_assert(resets->rstc[i]);
+   return ret;
+}
+
+static inline bool reset_control_is_array(struct reset_control *rstc)
+{
+   return rstc->array;
+}
+
  /**
   * reset_control_reset - reset the controlled device
   * @rstc: reset controller
@@ -158,6 +230,9 @@ int reset_control_reset(struct reset_control *rstc)
if (WARN_ON(IS_ERR(rstc)))
return -EINVAL;
  
+	if (reset_control_is_array(rstc))

+   return reset_control_array_reset(rstc_to_array(rstc));
+
if (!rstc->rcdev->ops->reset)
return -ENOTSUPP;
  
@@ -202,6 +277,9 @@ int reset_control_assert(struct reset_control *rstc)

if (WARN_ON(IS_ERR(rstc)))
return -EINVAL;
  
+	if (reset_control_is_array(rstc))

+   return reset_control_array_assert(rstc_to_array(rstc));
+
if (!rstc->rcdev->ops->assert)
return -ENOTSUPP;
  
@@ -240,6 +318,9 @@ int reset_control_deassert(struct reset_control *rstc)

if (WARN_ON(IS_ERR(rstc)))
return -EINVAL;
  
+	if (reset_control_is_array(rstc))

+   return reset_control_array_deassert(rstc_to_array(rstc));
+
if (!rstc->rcdev->ops->deassert)
return -ENOTSUPP;
  
@@ -266,7 +347,7 @@ int reset_control_status(struct reset_control *rstc)

if (!rstc)
return 0;
  
-	if (WARN_ON(IS_ERR(rstc)))

+   if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
return -EINVAL;
  
  	if (rstc->rcdev->ops->status)

@@ -404,6 +485,16 @@ struct reset_control *__reset_control_get(struct device 
*dev, const char *id,
  }
  EXPORT_SYMBOL_GPL(__reset_control_get);
  
+static void reset_control_array_put(struct reset_control_array *resets)

+{
+   int i;
+
+   mutex_lock(_list_mutex);
+   for (i = 0; i < resets->num_rstcs; i++)
+   __reset_control_put_internal(resets->rstc[i]);
+   mutex_unlock(_list_mutex);
+}
+
  /**
   * 

[PATCH] usb: Fix typo in the definition of Endpoint[out]Request

[Benjamin Herrenschmidt]

The current definition is wrong. This breaks my upcoming
Aspeed virtual hub driver.

Signed-off-by: Benjamin Herrenschmidt 
---
 include/linux/usb/hcd.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 40edf6a..8a1552e 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -561,9 +561,9 @@ extern void usb_ep0_reinit(struct usb_device *);
((USB_DIR_IN|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
 
 #define EndpointRequest \
-   ((USB_DIR_IN|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
+   ((USB_DIR_IN|USB_TYPE_STANDARD|USB_RECIP_ENDPOINT)<<8)
 #define EndpointOutRequest \
-   ((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_INTERFACE)<<8)
+   ((USB_DIR_OUT|USB_TYPE_STANDARD|USB_RECIP_ENDPOINT)<<8)
 
 /* class requests from the USB 2.0 hub spec, table 11-15 */
 #define HUB_CLASS_REQ(dir, type, request) dir) | (type)) << 8) | (request))


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

patch "USB: usbip: convert to use DRIVER_ATTR_RW" added to driver-core-next

[gregkh]

This is a note to let you know that I've just added the patch titled

USB: usbip: convert to use DRIVER_ATTR_RW

to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-next branch.

The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)

The patch will also be merged in the next major kernel release
during the merge window.

If you have any questions about this process, please let me know.


>From cc3d53def83a99636e16ceb70a79eedc61fddc23 Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman 
Date: Fri, 9 Jun 2017 11:03:14 +0200
Subject: USB: usbip: convert to use DRIVER_ATTR_RW

We are trying to get rid of DRIVER_ATTR(), and the usbip driver
attribute can be trivially changed to use DRIVER_ATTR_RW().

Cc: Valentina Manea 
Cc: 
Acked-by: Shuah Khan 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/usb/usbip/stub_main.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
index 44ab43fc4fcc..e74fbb7f4a32 100644
--- a/drivers/usb/usbip/stub_main.c
+++ b/drivers/usb/usbip/stub_main.c
@@ -134,7 +134,7 @@ int del_match_busid(char *busid)
return ret;
 }
 
-static ssize_t show_match_busid(struct device_driver *drv, char *buf)
+static ssize_t match_busid_show(struct device_driver *drv, char *buf)
 {
int i;
char *out = buf;
@@ -149,7 +149,7 @@ static ssize_t show_match_busid(struct device_driver *drv, 
char *buf)
return out - buf;
 }
 
-static ssize_t store_match_busid(struct device_driver *dev, const char *buf,
+static ssize_t match_busid_store(struct device_driver *dev, const char *buf,
 size_t count)
 {
int len;
@@ -181,8 +181,7 @@ static ssize_t store_match_busid(struct device_driver *dev, 
const char *buf,
 
return -EINVAL;
 }
-static DRIVER_ATTR(match_busid, S_IRUSR | S_IWUSR, show_match_busid,
-  store_match_busid);
+static DRIVER_ATTR_RW(match_busid);
 
 static ssize_t rebind_store(struct device_driver *dev, const char *buf,
 size_t count)
-- 
2.13.1


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html