Re: [PATCH v2 2/3] usb: chipidea: Hook into mux framework to toggle usb switch

[Peter Chen]

On Fri, Jul 14, 2017 at 02:40:04PM -0700, Stephen Boyd wrote:
>  
> @@ -175,6 +176,10 @@ static int host_start(struct ci_hdrc *ci)
>   if (ci_otg_is_fsm_mode(ci)) {
>   otg->host = >self;
>   hcd->self.otg_port = 1;
> + } else {
> + ret = mux_control_select(ci->platdata->usb_switch, 1);

It is better to use MACRO for 1 and 0.

> + if (ret)
> + goto disable_reg;
>   }
>   }
>  
> @@ -195,6 +200,8 @@ static void host_stop(struct ci_hdrc *ci)
>   struct usb_hcd *hcd = ci->hcd;
>  
>   if (hcd) {
> + if (!ci_otg_is_fsm_mode(ci))
> + mux_control_deselect(ci->platdata->usb_switch);
>   if (ci->platdata->notify_event)
>   ci->platdata->notify_event(ci,
>   CI_HDRC_CONTROLLER_STOPPED_EVENT);
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index d68b125796f9..deb18099e168 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "ci.h"
>  #include "udc.h"
> @@ -1964,16 +1965,26 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
>  
>  static int udc_id_switch_for_device(struct ci_hdrc *ci)
>  {
> + int ret = 0;
> +
>   if (ci->is_otg)
>   /* Clear and enable BSV irq */
>   hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
>   OTGSC_BSVIS | OTGSC_BSVIE);
>  
> - return 0;
> + if (!ci_otg_is_fsm_mode(ci))
> + ret = mux_control_select(ci->platdata->usb_switch, 0);
> +
> + if (ci->is_otg && ret)
> + hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);

Should use !ret?

Peter

> +
> + return ret;
>  }
>  
>  static void udc_id_switch_for_host(struct ci_hdrc *ci)
>  {
> + mux_control_deselect(ci->platdata->usb_switch);
> +
>   /*
>* host doesn't care B_SESSION_VALID event
>* so clear and disbale BSV irq
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index c5fdfcf99828..3b27e333de1d 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -9,6 +9,7 @@
>  #include 
>  
>  struct ci_hdrc;
> +struct mux_control;
>  
>  /**
>   * struct ci_hdrc_cable - structure for external connector cable state 
> tracking
> @@ -74,6 +75,7 @@ struct ci_hdrc_platform_data {
>   /* VBUS and ID signal state tracking, using extcon framework */
>   struct ci_hdrc_cablevbus_extcon;
>   struct ci_hdrc_cableid_extcon;
> + struct mux_control  *usb_switch;
>   u32 phy_clkgate_delay_us;
>  };
>  
> -- 
> 2.10.0.297.gf6727b0
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 

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 v16 2/7] power: add power sequence library

[Peter Chen]

On Mon, Jul 17, 2017 at 03:39:07PM +0200, Rafael J. Wysocki wrote:
> > Sorry, I should describe more.
> > 
> > Let's take USB bus as an example, when the new USB device is at the
> > host port, the device structure at device model is not created until
> > it is discoverable by the USB bus. If this new USB device needs to be
> > powered on before can be discoverable by the bus, the device structure
> > will be not created without powering on operation. The code usb_alloc_dev
> > (drivers/usb/core/usb.c) is only called for discoverable device.
> > 
> > Unlike the other bus, eg, platform bus, it creates device structure
> > according to DT node. The USB bus was designed for hot plug model, the
> > device structure is for discoverable device. In recent years, we begin
> > to have some hard-wired USB device, Eg, onboard USB-hub, onboard USB 4G
> > Modem, etc at the market. It needs some board level power operation before
> > it can be found by the USB bus. This patch set is designed primarily for
> > fix this kind of problem. You will see at at pwrseq_generic.c, we use DT
> > version clock API of_clk_get and DT version gpio API of_get_named_gpio_flags
> > instead of device structure version, like devm_clk_get and
> > devm_gpiod_get_optional.
> > 
> > MMC system has similar use case, it creates power sequence platform
> > device for this issue, but all those power stuffs (clock, gpio, etc)
> > may not be suitable as a dedicated virtual device at DT, they are belonged
> > to one physical device, so this patch set is created to see if this issue
> > can be fixed better.
> 
> OK, thanks for the explanation.
> 
> The above needs to be part of your problem statement.

Ok, I will add it to cover letter.

> > 
> > The bus will power up all device nodes in this bus according to DT
> > information, the device structure has not created at this time.
> 
> OK
> 
> I still think that the information on power resources depended on by devices
> should be used for power management as well as for the initial power-up.
> 
> The most straightforward way to arrange for that would be to make it possible
> to find the DT node matching the device after the device has been discovered
> and struct device created for it, say by USB.  That would require adding some
> more information on the device to the DT node, probably.

After the device is created, the device node structure is under struct
device, say dev->of_node. The most difficulty for this issue is the
device creation is dynamic and is after the physical device is
discovered by the bus, the initial power-up is needed before the device
can be discovered by the bus.

> 
> Then, the DT device nodes would be used for the initial power-up and next, 
> after
> discovering a device, you'd do a lookup in the DT, find the node matching it
> and read the power resuources information from there to populate the device's
> power state structure.  From that point on you can simply use the interface I
> suggested.
> 

Just like I said above, without initial power-up, the device can't be
discovered by the bus.

-- 

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] usb: dwc2: gadget: On USB RESET reset device address to zero

[John Youn]

On 07/11/2017 03:25 AM, Minas Harutyunyan wrote:
> Reseted DEVADDR field in DCFG to zero on USB RESET.
>
> Device address in DCFG register does not reset to zero,
> which required to pass enumeration, after disconnect and
> reconnect.
>
> Signed-off-by: Minas Harutyunyan 
> ---
>  drivers/usb/dwc2/gadget.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 98a4a79e7f6e..c5c0a26a4d66 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3561,6 +3561,9 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
>   /* Report disconnection if it is not already done. */
>   dwc2_hsotg_disconnect(hsotg);
>
> + /* Reset device address to zero */
> + __bic32(hsotg->regs + DCFG, DCFG_DEVADDR_MASK);
> +
>   if (usb_status & GOTGCTL_BSESVLD && connected)
>   dwc2_hsotg_core_init_disconnected(hsotg, true);
>   }
>

Acked-by: John Youn 


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: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

Hi,
On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote:
> Hi,
> 
> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun  
> wrote:
> > Hi,
> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
> >> Hi,
> >>
> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
> >>  wrote:
> >> > Hi Martin,
> >> >
> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote:
> >> >> This series is the outcome of a discussion with Felipe Balbi,
> >> >> see [0] and [1].
> >> >> The quick-summary of this is:
> >> >> - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> >> >>   correct
> >> >> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> >> >>   ohci-platform.c) do not have a limitation on the number of PHYs - they
> >> >>   support one PHY per actual host port
> >> >> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> >> >>   or three USB2 ports enabled on the internal root-hub. The SoCs also
> >> >>   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> >> >>   internally "connected" to the dwc3 roothub) need to be powered on,
> >> >>   otherwise USB devices cannot be enumerated (even if just one PHY is
> >> >>   disabled and if the device is plugged into another, enabled port)
> >> >>
> >> >> In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> >> >> to work-around the problem that I could not pass multiple PHYs to the
> >> >> dwc3 controller.
> >> >> This was rejected by Rob Herring (which was definitely the thing to do 
> >> >> in
> >> >> my opinion), see [2]
> >> >>
> >> >> This series adds a new "platform-roothub". This can be configured 
> >> >> through
> >> >> devicetree by passing a child-node with "reg = <0>" to the USB
> >> >> controller. Additionally there has to be a child-node for each port on
> >> >> the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> >> >> property. This allows modeling the root-hub in devicetree similar to the
> >> >> USB device binding (documented in 
> >> >> devicetree/bindings/usb/usb-device.txt)
> >> >> This avoids and backwards-compatibility problems (which was a concern
> >> >> regardless of the solution, see [3]) since the binding for the root-hub
> >> >> was previously not specified (and we're not using the "phys" property of
> >> >> the controller, which might have served different purposes before,
> >> >> depending on the drivers).
> >> >>
> >> >> Additionally this integrates the new platform-roothub into xhci-plat.c
> >> >> which automatically enables it for the dwc3 driver (in host-mode).
> >> >>
> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support
> >> > dual-role mode? leave them to peripheral side as felipe suggested
> >> > before? If so, no port1 node for roothub, is there any problem when
> >> > change the port1 to host-only mode?
> >> on Amlogic Meson GXL we have the following IP blocks:
> >> - 2x USB2 PHYs, some external component has to tell them which mode
> >> (host/device) they should operate in
> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
> >>
> >> on Amlogic Meson GXL it could work like this:
> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
> >> tell the USB2 PHY in which mode it should operate.
> >>
> >> please note that device mode and OTG support on Amlogic Meson GXL is
> >> more complicated, as it uses dwc2 and dwc3 controllers in combination:
> >> - dwc3 is reponsible for host-only mode
> >> - dwc2 is responsible for device-only mode
> >> - OTG detection is done by the USB3 PHY
> >>
> >> would you mind sharing a short overview of host/device/OTG support
> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
> >> implementation is quite special, so comparing this with another
> >> implementation (for example the Mediatek one) may help spotting
> >> potential issues.
> >>
> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
> > as following:
> thank you for sharing this!
> 
> > 1. device mode works as HS only:
> >
> > u2phy0 --- dual-role/OTG
> >
> > u2phy1 ---|
> >   +  U3 host-only
> > u3phy0 ---|
> >
> > ...
> > u2phy4 ---|
> >   +  U3 host-only
> > u3phy3 ---|
> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
> > dual-role mode, u2phy1 & u3phy0 are host-only)
> >
> > 2. device mode works as HS & SS, or host only:
> >
> > u2phy0 ---|
> >   + dual-role or host-only
> > u3phy0 ---|
> >
> > ...
> > u2phy3 ---|
> >   + U3 host-only
> > u3phy3 ---|
> >
> > u2phy4 --- U2 host-only
> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
> OK, so in both cases only one port (with one u2phy and one u3phy) is
> dual-role capable
Yes
> 
> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
> > pin), and will take all 

Re: CPU lock-ups with 4.12.0+ kernels related to usb_storage

[Arthur Marsh]

Arthur Marsh wrote on 14/07/17 04:18:



Alan Stern wrote on 14/07/17 02:30:


All right.  In the meantime, changing usb-storage won't hurt.
Arthur, can you test the patch below?

Alan Stern



Index: usb-4.x/drivers/usb/storage/usb.c
===
--- usb-4.x.orig/drivers/usb/storage/usb.c
+++ usb-4.x/drivers/usb/storage/usb.c
@@ -315,6 +315,7 @@ static int usb_stor_control_thread(void
  {
  struct us_data *us = (struct us_data *)__us;
  struct Scsi_Host *host = us_to_host(us);
+struct scsi_cmnd *srb;
  for (;;) {
  usb_stor_dbg(us, "*** thread sleeping\n");
@@ -330,6 +331,7 @@ static int usb_stor_control_thread(void
  scsi_lock(host);
  /* When we are called with no command pending, we're done */
+srb = us->srb;
  if (us->srb == NULL) {
  scsi_unlock(host);
  mutex_unlock(>dev_mutex);
@@ -398,14 +400,11 @@ static int usb_stor_control_thread(void
  /* lock access to the state */
  scsi_lock(host);
-/* indicate that the command is done */
-if (us->srb->result != DID_ABORT << 16) {
-usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
- us->srb->result);
-us->srb->scsi_done(us->srb);
-} else {
+/* was the command aborted? */
+if (us->srb->result == DID_ABORT << 16) {
  SkipForAbort:
  usb_stor_dbg(us, "scsi command aborted\n");
+srb = NULL;/* Don't call srb->scsi_done() */
  }
  /*
@@ -429,6 +428,13 @@ SkipForAbort:
  /* unlock the device pointers */
  mutex_unlock(>dev_mutex);
+
+/* now that the locks are released, notify the SCSI core */
+if (srb) {
+usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
+srb->result);
+srb->scsi_done(srb);
+}
  } /* for (;;) */
  /* Wait until we are told to stop */


Hi, just to confirm no further lock-ups occurred in the last 4 days with 
this patch applied.


Arthur.
--
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: Dell DA200 adapter errors

[Greg KH]

On Mon, Jul 17, 2017 at 08:57:53PM +0200, Jose Antonio de la Torre wrote:
> Hi, I am trying to use the Dell DA200 USB-C adapter
> (http://accessories.ap.dell.com/sna/productdetail.aspx?c=sg=en=bsd=sgbsd1=470-abnl)
> In Debian 9 wiith Kernel 4.11.0-1.

I have that thing, it's really really flaky.  Make sure you have the
latest BIOS update, and please update to a newer kernel, 4.11.0 is old
for xhci fixes.

> The first time it seems to work and I am able to use the adapter without
> problems. The error comes when I try to disconnect and reconnect the
> adapter. When I disconnect the adapter I get this error:
> 
> [  +5.027933] xhci_hcd :0a:00.0: Stop command ring failed, maybe the
> host is dead
> [  +0.11] xhci_hcd :0a:00.0: Host halt failed, -19
> [  +0.02] xhci_hcd :0a:00.0: Abort command ring failed
> [  +0.04] xhci_hcd :0a:00.0: HC died; cleaning up
> [  +0.12] xhci_hcd :0a:00.0: Timeout while waiting for configure
> endpoint
> 
> I don't know what is the problem. Also, when I try to suspend the computer
> after disconnect the adapter the computer wakes up inmediatly.

What do you mean by this?

What computer are you using this on?

And again, it's a really odd device, this is really a PCI hotplug device
for a xhci controller.  Even on the latest kernel releases I sometimes
have problems with mine.

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

Dell DA200 adapter errors

[Jose Antonio de la Torre]

Hi, I am trying to use the Dell DA200 USB-C adapter 
(http://accessories.ap.dell.com/sna/productdetail.aspx?c=sg=en=bsd=sgbsd1=470-abnl) 
In Debian 9 wiith Kernel 4.11.0-1.


The first time it seems to work and I am able to use the adapter without 
problems. The error comes when I try to disconnect and reconnect the 
adapter. When I disconnect the adapter I get this error:


[  +5.027933] xhci_hcd :0a:00.0: Stop command ring failed, maybe the 
host is dead

[  +0.11] xhci_hcd :0a:00.0: Host halt failed, -19
[  +0.02] xhci_hcd :0a:00.0: Abort command ring failed
[  +0.04] xhci_hcd :0a:00.0: HC died; cleaning up
[  +0.12] xhci_hcd :0a:00.0: Timeout while waiting for configure 
endpoint


I don't know what is the problem. Also, when I try to suspend the 
computer after disconnect the adapter the computer wakes up inmediatly.


Thanks in advance.



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [RFCv2 usb-next 3/3] usb: host: xhci: plat: integrate the platform-roothub

[Rob Herring]

On Thu, Jul 13, 2017 at 12:59:39PM +0200, Martin Blumenstingl wrote:
> This enables the platform-roothub for the xhci-plat driver. This allows
> specifying a PHY for each port via devicetree. All PHYs will then be
> enabled/disabled by the platform-roothub driver.
> 
> One example where this is required is the Amlogic GXL and GXM SoCs:
> They are using a dwc3 USB controller with up to three ports enabled on
> the internal roothub. Using only the top-level "phy" properties does not
> work here since one can only specify one "usb2-phy" and one "usb3-phy",
> while actually at least two "usb2-phy" have to be specified.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++

Acked-by: Rob Herring 

>  drivers/usb/host/Kconfig   |  1 +
>  drivers/usb/host/xhci-plat.c   | 27 
> +-
>  drivers/usb/host/xhci.h|  2 ++
>  4 files changed, 36 insertions(+), 1 deletion(-)
--
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: [RFCv2 usb-next 1/3] dt-bindings: usb: add the documentation for USB root-hub

[Rob Herring]

On Thu, Jul 13, 2017 at 12:59:37PM +0200, Martin Blumenstingl wrote:
> A USB root-hub may have several PHYs which need to be configured before
> the root-hub starts working.
> This adds the documentation for such a USB root-hub.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  .../devicetree/bindings/usb/usb-roothub.txt| 46 
> ++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt

Acked-by: Rob Herring 
--
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: chipidea: Hook into mux framework to toggle usb switch

[Rob Herring]

On Thu, Jul 13, 2017 at 03:29:43PM -0700, Stephen Boyd wrote:
> Quoting Peter Rosin (2017-07-11 23:45:24)
> > On 2017-07-12 03:02, Stephen Boyd wrote:
> > > @@ -102,4 +107,7 @@ Example:
> > >   rx-burst-size-dword = <0x10>;
> > >   extcon = <0>, <_id>;
> > >   phy-clkgate-delay-us = <400>;
> > > + mux-controls = <_switch>;
> > > + mux-control-names = "usb_switch";

Pointless to have a name when there is only 1.

> > > + usb-switch-states = <0>, <1>;
> > 
> > I don't see the need for usb-switch-states? Just assume states 0/1 and
> > if someone later needs some other states, make them add a property that
> > overrides the defaults. Just document that 0 is host and 1 is device.
> > 
> 
> Fine by me. Rob H do you agree?

Yes.

Rob
--
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/3] USB Mux support for Chipidea

[Rob Herring]

On Thu, Jul 13, 2017 at 03:35:02PM -0700, Stephen Boyd wrote:
> Quoting Peter Rosin (2017-07-11 22:04:46)
> > On 2017-07-12 03:02, Stephen Boyd wrote:
> > > This patchset adds support for the TC7USB40MU usb mux found on 
> > > db410c 96boards platforms via the new multiplexer framework and
> > > hooks that into the chipidea driver. This allows us to properly
> > > control host or device mode on this board via the sysfs knob.
> > > 
> > > So far I've only tested this on db410c, and there are some rough
> > > edges to finish off before it can merge. Also I'm experiencing
> > > odd behavior with switching the role while gadget is enabled and
> > > the micro-usb cable is kept connected. Not sure what's wrong but
> > > it seems like the gadget never gets disconnected? I'll investigate
> > > more.
> > > 
> > > TODO:
> > > 
> > >  1. The mux framework has to be selected for consumers to use it. We'll
> > > need some stubs in the consumer header file to allow compilation to
> > > continue without mux always enabled by consumers.
> > 
> > Instead of "depends on MULTIPLEXER", just add "select MULTIPLEXER"
> > to the Kconfig. Otherwise, you'll have to convince Linus that we
> > really do need a Kconfig question for the subsystem :-)
> > 
> > https://lkml.org/lkml/2017/7/4/118
> 
> Ok. I'll add a select to the chipidea driver.
> 
> > 
> > >  2. We probably need some sort of mux_control_get_optional() API so that
> > > we know if there was an error getting the mux control, instead of just
> > > ignoring errors. For now I can pass up EPROBE_DEFER errors and ignore
> > > other errors and consider it "missing from DT".
> > 
> > Yes, mux_control_get_optional should be easy to add.
> > 
> > >  3. Maybe we can get rid of the mux driver and just use mux-gpio.c with
> > > a compatible string update? I split it off because we may want to
> > > support the "S" pin on the TC7USB40MU one day that shuts off both
> 
> Oh this is a typo. I mean "OE" pin.
> 
> > > mux outputs.
> > 
> > Maybe no need for a compatible update either, if it works to do something
> > like this in the DT?

Please keep the compatible. Use "gpio-mux" as a fallback if you wish.

Rob
--
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 2/2] usb: phy-generic: Remove unneeded gpiod NULL check

[Fabio Estevam]

From: Fabio Estevam 

The gpiod API checks for NULL descriptors, so there is no need to
duplicate the check in the driver.

Signed-off-by: Fabio Estevam 
---
 drivers/usb/phy/phy-generic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 89d6e7a..675a716 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -263,8 +263,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
usb_phy_generic *nop,
dev_err(dev, "Error requesting RESET or VBUS GPIO\n");
return err;
}
-   if (nop->gpiod_reset)
-   gpiod_direction_output(nop->gpiod_reset, 1);
+   gpiod_direction_output(nop->gpiod_reset, 1);
 
nop->phy.otg = devm_kzalloc(dev, sizeof(*nop->phy.otg),
GFP_KERNEL);
-- 
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 1/2] usb: pxa27x_udc: Remove unneeded gpiod NULL check

[Fabio Estevam]

From: Fabio Estevam 

The gpiod API checks for NULL descriptors, so there is no need to
duplicate the check in the driver.

Signed-off-by: Fabio Estevam 
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index d48e239..404ae6d 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2475,8 +2475,7 @@ static int pxa_udc_probe(struct platform_device *pdev)
PTR_ERR(udc->gpiod));
return PTR_ERR(udc->gpiod);
}
-   if (udc->gpiod)
-   gpiod_direction_output(udc->gpiod, 0);
+   gpiod_direction_output(udc->gpiod, 0);
 
udc->clk = devm_clk_get(>dev, NULL);
if (IS_ERR(udc->clk))
-- 
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] Adding TD620 Device ID in PL2303

[Greg KH]

On Mon, Jul 17, 2017 at 04:34:57PM +, Schoon, Michael wrote:
> Sorry about that.  How's this:
> 
> Please add support for HP TD620 Linear Display device to the pl2303 module.  
> The USB Device ID is 03f0:0956 for this 2x20 display.
> 
> --- drivers/usb/serial/pl2303.c.orig2017-07-14 13:43:59.029860481 -0500
> +++ drivers/usb/serial/pl2303.c 2017-07-14 13:16:27.928149225 -0500
> @@ -92,6 +92,7 @@ static const struct usb_device_id id_tab
> { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
> +   { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },
> { USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
> { USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
> { USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },

Why not try sending it to yourself to see if it will apply (hint, it
didn't work...)

Also, always use scripts/checkpatch.pl, otherwise you will have grumpy
maintainers telling you to use scripts/checkpatch.pl to fix the obvious
issues in your patch...

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] Adding TD620 Device ID in PL2303

[Schoon, Michael]

Sorry about that.  How's this:

Please add support for HP TD620 Linear Display device to the pl2303 module.  
The USB Device ID is 03f0:0956 for this 2x20 display.

--- drivers/usb/serial/pl2303.c.orig2017-07-14 13:43:59.029860481 -0500
+++ drivers/usb/serial/pl2303.c 2017-07-14 13:16:27.928149225 -0500
@@ -92,6 +92,7 @@ static const struct usb_device_id id_tab
{ USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
+   { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },
{ USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
{ USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
{ USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
--- drivers/usb/serial/pl2303.h.orig2017-07-14 13:44:08.893761456 -0500
+++ drivers/usb/serial/pl2303.h 2017-07-14 13:15:07.016508374 -0500
@@ -125,6 +125,7 @@
 #define HP_LCM220_PRODUCT_ID   0x3139
 #define HP_LCM960_PRODUCT_ID   0x3239
 #define HP_LD220_PRODUCT_ID0x3524
+#define HP_TD620_PRODUCT_ID 0x0956
 
 /* Cressi Edy (diving computer) PC interface */
 #define CRESSI_VENDOR_ID   0x04b8



-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org] 
Sent: Monday, July 17, 2017 10:28 AM
To: Schoon, Michael 
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH] Adding TD620 Device ID in PL2303

On Mon, Jul 17, 2017 at 03:19:10PM +, Schoon, Michael wrote:
>     Please add support for HP TD620 Linear Display device to the pl2303 
> module.  The USB Device ID is 03f0:0956 for this 2x20 display.
>     
> Changes to be committed:
>     modified:   drivers/usb/serial/pl2303.c
>     modified:   drivers/usb/serial/pl2303.h
>     

Intersting indentation :(

Also, what is this "Changes to be..." line for?

And finally, did you run this through scripts/checkpatch.pl?

> --- drivers/usb/serial/pl2303.c 2017-07-14 13:16:27.928149225 
> -0500
>     +++ drivers/usb/serial/pl2303.c.orig    2017-07-14 
> 13:43:59.029860481 -0500
>     @@ -92,7 +92,6 @@ static const struct usb_device_id id_tab
>     { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
>     { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
>     { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
>     -   { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },

Well, now finally, your patch is backwards and whitespace damaged, making it 
impossible to apply :(

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] Adding TD620 Device ID in PL2303

[Greg KH]

On Mon, Jul 17, 2017 at 03:19:10PM +, Schoon, Michael wrote:
>     Please add support for HP TD620 Linear Display device to the pl2303 
> module.  The USB Device ID is 03f0:0956 for this 2x20 display.
>     
> Changes to be committed:
>     modified:   drivers/usb/serial/pl2303.c
>     modified:   drivers/usb/serial/pl2303.h
>     

Intersting indentation :(

Also, what is this "Changes to be..." line for?

And finally, did you run this through scripts/checkpatch.pl?

> --- drivers/usb/serial/pl2303.c 2017-07-14 13:16:27.928149225 -0500
>     +++ drivers/usb/serial/pl2303.c.orig    2017-07-14 13:43:59.029860481 
> -0500
>     @@ -92,7 +92,6 @@ static const struct usb_device_id id_tab
>     { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
>     { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
>     { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
>     -   { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },

Well, now finally, your patch is backwards and whitespace damaged,
making it impossible to apply :(

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

[PATCH] Adding TD620 Device ID in PL2303

[Schoon, Michael]

    Please add support for HP TD620 Linear Display device to the pl2303 module. 
 The USB Device ID is 03f0:0956 for this 2x20 display.
    
Changes to be committed:
    modified:   drivers/usb/serial/pl2303.c
    modified:   drivers/usb/serial/pl2303.h
    
--- drivers/usb/serial/pl2303.c 2017-07-14 13:16:27.928149225 -0500
    +++ drivers/usb/serial/pl2303.c.orig    2017-07-14 13:43:59.029860481 
-0500
    @@ -92,7 +92,6 @@ static const struct usb_device_id id_tab
    { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
    { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
    { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
    -   { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },
    { USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
    { USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
    { USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
    
--- drivers/usb/serial/pl2303.h 2017-07-14 13:15:07.016508374 -0500
    +++ drivers/usb/serial/pl2303.h.orig    2017-07-14 13:44:08.893761456 
-0500
    @@ -125,7 +125,6 @@
 #define HP_LCM220_PRODUCT_ID   0x3139
 #define HP_LCM960_PRODUCT_ID   0x3239
 #define HP_LD220_PRODUCT_ID    0x3524
    -#define HP_TD620_PRODUCT_ID 0x0956
    
 /* Cressi Edy (diving computer) PC interface */
 #define CRESSI_VENDOR_ID   0x04b8

Signed-off-by: Michael Schoon 
--
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] f16443a034: EIP:arch_local_irq_restore

[Alan Stern]

On Mon, 17 Jul 2017, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern  writes:
> >> >> > On Thu, 29 Jun 2017, kernel test robot wrote:
> >> >> >
> >> >> >> FYI, we noticed the following commit:
> >> >> >> 
> >> >> >> commit: f16443a034c7aa359ddf6f0f9bc40d01ca31faea ("USB: gadgetfs, 
> >> >> >> dummy-hcd, net2280: fix locking for callbacks")
> >> >> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 
> >> >> >> master
> >> >> >> 
> >> >> >> in testcase: trinity
> >> >> >> with following parameters:
> >> >> >> 
> >> >> >>  runtime: 300s
> >> >> >> 
> >> >> >> test-description: Trinity is a linux system call fuzz tester.
> >> >> >> test-url: http://codemonkey.org.uk/projects/trinity/
> >> >> >> 
> >> >> >> 
> >> >> >> on test machine: qemu-system-x86_64 -enable-kvm -m 420M
> >> >> >> 
> >> >> >> caused below changes (please refer to attached dmesg/kmsg for entire 
> >> >> >> log/backtrace):
> >> >> > ...
> >> >> >
> >> >> > I won't include the entire report.  The gist is that we have a 
> >> >> > problem 
> >> >> > with lock ordering.  The report is about dummy-hcd, but this could 
> >> >> > affect any UDC driver.
> >> >> >
> >> >> >  1. When a SETUP request arrives, composite_setup() acquires
> >> >> >   cdev->lock before calling the function driver's callback.
> >> >> >   When that callback submits a reply, it causes the UDC driver
> >> >> >   to acquire its private lock.
> >> >> 
> >> >> this only seems to happen before calling set_config():
> >> >> 
> >> >> case USB_REQ_SET_CONFIGURATION:
> >> >> if (ctrl->bRequestType != 0)
> >> >> goto unknown;
> >> >> if (gadget_is_otg(gadget)) {
> >> >> if (gadget->a_hnp_support)
> >> >> DBG(cdev, "HNP available\n");
> >> >> else if (gadget->a_alt_hnp_support)
> >> >> DBG(cdev, "HNP on another port\n");
> >> >> else
> >> >> VDBG(cdev, "HNP inactive\n");
> >> >> }
> >> >> spin_lock(>lock);
> >> >> value = set_config(cdev, ctrl, w_value);
> >> >> spin_unlock(>lock);
> >> >> break;
> >> >
> >> > That's true.  Why is the lock held for set_config() but not for any of 
> >> > the other callbacks?
> >> 
> >> this is really old code from Dave. Your guess is as good as mine :-(
> >> 
> >> > Doesn't that mean the other callbacks can race with function
> >> > unregistration?
> >> 
> >> Probably not as UDCs are required to cancel transfers and kill all
> >> endpoints before unregistering. We would probably just giveback a few
> >> requests with -ESHUTDOWN and prevent new ones from being queued to HW,
> >> no?
> >
> > But SETUP callbacks aren't associated with pending requests.  They get
> 
> they are if ->setup() returns DELAYED_STATUS.

No.  In the DELAYED_STATUS case, the gadget driver submits a request 
_after_ the SETUP packet is received.

In no case is there a pending request that an incoming SETUP packet is
associated with.

> > generated whenever a SETUP packet is received, even if the gadget
> > driver has no requests queued.  Cancelling transfers won't prevent
> > them.
> >
> > Killing all endpoints might or might not do the trick.  Does killing
> > ep0 prevent the UDC driver from receiving SETUP packets?  This may vary 
> > between UDC drivers.
> 
> no, it does not. If ->setup() fails, we are required to Stall and
> restart ep0. Restarting ep0 involves preparing it to receive a new
> SETUP request.

I wasn't talking about stalling or restarting ep0; I was talking about
killing it.

> > There are also the other callbacks (reset, disconnect, bus suspend, and 
> > bus resume), which aren't associated with endpoints or requests at all.
> >
> > Probably drivers really should have something like synchronize_irq() in
> > the udc_stop routine.  That would solve a lot of problems (although it 
> > wouldn't help dummy_hcd, which doesn't rely on interrupts).
> 
> Hmm, free_irq() calls synchronize_irq(). UDCs should just request_irq()
> on udc_start() and free_irq() on udc_stop(). That's what dwc3 is doing.

In general, I agree.  But free_irq() isn't enough; you also have to 
tell the UDC hardware to stop generating interrupt requests.  Otherwise 
you'll end up with a "nobody cared!" error.

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: JMS567 USB3.0 scsi scan error

[Oliver Neukum]

Am Montag, den 10.07.2017, 15:06 +0200 schrieb Oliver Neukum:
> 
> The old storage driver unconditionally limits inquiries to 36 bytes. UAS does 
> not
> have that limit. That seems to be a bit optimistic. Could you test the 
> attached patch?
> 

Hi,

is there a problem testing. Should I make a test kernel?

Regards
Oliver

--
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 v16 2/7] power: add power sequence library

[Rafael J. Wysocki]

On Monday, July 10, 2017 10:28:15 AM Peter Chen wrote:
> On Sat, Jul 08, 2017 at 02:14:56PM +0200, Rafael J. Wysocki wrote:
> > On Saturday, July 08, 2017 01:51:15 PM Peter Chen wrote:
> > > On Fri, Jul 07, 2017 at 03:03:06PM +0200, Rafael J. Wysocki wrote:
> > > > On Friday, July 07, 2017 04:01:07 PM Peter Chen wrote:
> > > > > On Fri, Jul 07, 2017 at 03:13:48AM +0200, Rafael J. Wysocki wrote:
> > > > > > > 
> > > > > > > - Can I write new code for it or I need to depend on something?
> > > > > > 
> > > > > > There is nothing this code needs to depend on AFAICS, but there are 
> > > > > > existing
> > > > > > solutions in this problem space (ACPI power management, genpd), so 
> > > > > > it needs to
> > > > > > be careful enough about possible overlaps etc.
> > > > > > 
> > > > > > > I find there is already "power state" concept at documentation.
> > > > > > > Documentation/ABI/testing/sysfs-devices-power_state
> > > > > > 
> > > > > > This is ACPI-specific and only in sysfs directories representing 
> > > > > > ACPI device
> > > > > > objects (which aren't physical devices).
> > > > > > 
> > > > > > Anyway, since ACPI covers the problem space you are working in 
> > > > > > already,
> > > > > > your code has to be mutually exclusive with it.
> > > > > > 
> > > > > > > - If I can write the new code for it, except the problems I want
> > > > > > > to fix, are there any other use cases I need to consider?
> > > > > > 
> > > > > > I would start simple and focus on the particular problem at hand, 
> > > > > > that is
> > > > > > devices with two power states ("on" and "off") where the "on" state
> > > > > > depends on a number of clocks and/or GPIOs.  Still, I'd also avoid 
> > > > > > making
> > > > > > design choices that might prevent it from being extended in the 
> > > > > > future
> > > > > > if need be.
> > > > > > 
> > > > > > One major problem I can see is how to "attach" the power states 
> > > > > > framework
> > > > > > to a particular device (once we have discovered that it should be 
> > > > > > used with
> > > > > > that device).
> > > > > > 
> > > > > > For bus types that don't do power management of their own you could 
> > > > > > follow
> > > > > > ACPI (and genpd) and provide a PM domain for this purpose, but bus 
> > > > > > types
> > > > > > doing their own PM (like USB) will probably need to be treated 
> > > > > > differently.
> > > > > > In those cases the bus type code will have to know that it should 
> > > > > > call some
> > > > > > helpers to switch power states of devices.
> > > > > > 
> > > > > 
> > > > > After thinking more, using a power state framework is seems too heavy
> > > > > for this use case. This use case is just do some clock and gpio
> > > > > operations before device is created, and do some put operations
> > > > > after device is deleted. We just need some helpers in one structure
> > > > > (called "power sequence" or "power state") for this purpose.
> > > > > 
> > > > > For the use case, the clock and gpio operation can be done after 
> > > > > device
> > > > > is created, the power domain is more suitable.
> > > > 
> > > > There is a problem with PM domains that they only provide hooks for 
> > > > runtime PM
> > > > and system suspend/resume (including hibernation) and not for generic
> > > > "power up" and "power down" operations that may need to be carried out 
> > > > at
> > > > probe time before the runtime PM framework can be used (and analogously
> > > > at remove time).
> > > > 
> > > > I would consider starting with the patch below or similar.
> > > > 
> > > > Then you can define something like POWER_STATE_SEQUENCE type for your
> > > > case and basically use almost what you have already with it, except that
> > > > struct pwrsec_generic will now become struct power_state_sequence and
> > > > struct power_state_info will be embedded in it instead of struct pwrsec.
> > > > 
> > > > The major comceptual difference is that ->power_up and ->power_down are
> > > > now available at the level of the device that needs the power sequence 
> > > > and
> > > > pm_device_power_up/down() can be used wherever necessary (in the code,
> > > > in a bus type, in a controller driver or even in the driver for this 
> > > > particular
> > > > device).
> > > 
> > > Rafeal, thanks for your patch.
> > > 
> > > The biggest problem for my use case is the device is still not created.
> > > How can I call pm_device_power_up(dev)?
> > 
> > Can you please elaborate on that a bit?
> > 
> 
> Sorry, I should describe more.
> 
> Let's take USB bus as an example, when the new USB device is at the
> host port, the device structure at device model is not created until
> it is discoverable by the USB bus. If this new USB device needs to be
> powered on before can be discoverable by the bus, the device structure
> will be not created without powering on operation. The code usb_alloc_dev
> (drivers/usb/core/usb.c) is only called for discoverable device.
> 
> Unlike the other bus, eg, 

Re: [PATCH v5] xhci: Bad Ethernet performance plugged in ASM1042A host

[Mathias Nyman]

On 11.07.2017 00:44, mario.limoncie...@dell.com wrote:

-Original Message-
From: Andy Shevchenko [mailto:andy.shevche...@gmail.com]
Sent: Monday, July 10, 2017 3:57 PM
To: Limonciello, Mario 
Cc: Mathias Nyman ; USB ; mathias.ny...@intel.com; david.lai...@aculab.com;
Greg Kroah-Hartman ; Felipe Balbi
; brain_...@asmedia.com.tw;
justin_cy_c...@asmedia.com.tw; Wang, Keith ;
yd_ts...@asmedia.com.tw; lars_ch...@asmedia.com.tw;
arequip...@gmail.com; jia...@gmail.com
Subject: Re: [PATCH v5] xhci: Bad Ethernet performance plugged in ASM1042A host

On Mon, Jul 10, 2017 at 11:38 PM,   wrote:

-Original Message-
From: Jiahau Chang [mailto:jia...@gmail.com]
Sent: Thursday, July 6, 2017 8:58 PM
To: Mathias Nyman 
Cc: linux-usb@vger.kernel.org; mathias.ny...@intel.com;
david.lai...@aculab.com; gre...@linuxfoundation.org;
felipe.ba...@linux.intel.com; brain_...@asmedia.com.tw; Limonciello, Mario
; justin_cy_c...@asmedia.com.tw; Wang,

Keith

; yd_ts...@asmedia.com.tw; Jiahau Chang
; Ian Pilcher 
Subject: Re: [PATCH v5] xhci: Bad Ethernet performance plugged in ASM1042A

host


2017-06-28 21:42 GMT+08:00 Mathias Nyman

:

On 22.06.2017 07:49, Jiahau Chang wrote:


When USB Ethernet is plugged in ASMEDIA ASM1042A xHCI host, bad
performance was manifesting in Web browser use (like download
large file such as ISO image). It is known limitation of
ASM1042A that is not compatible with driver scheduling,
As a workaround we can modify flow control handling of ASM1042A.
The register we modify is changes the behavior

Signed-off-by: Jiahau Chang 
Signed-off-by: Ian Pilcher 
---



Thanks, looks good, but checkpatch complains about:


+   usleep_range(50, 50);



having same min and max value.
Does usleep_range(40,60) work for you? or some other range?


It works to use usleep_range(40,60);
Thanks for help us to upstream the patch.


I can change that myself, no need to resend.



Matthias,

Can you still get this in for 4.13?


He is on vacation for few weeks.

Perhaps in rcX.




Thanks for the heads up, I wasn't aware since I didn't get back any OOO or 
anything like that.
I hope this can make it into one of the rc's indeed then when he returns.



Pushed to my for-usb-linus branch, will send forward to Greg (4.13-rcx) if 
branch passes smoke test.

I'm at the office this week, I'll be on vacation again after that for two more 
weeks

-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 v3] usb: xhci: Issue stop EP command only when the EP state is running

[Mathias Nyman]

On 12.07.2017 08:30, Shyam Sundar S K wrote:



On 6/22/2017 4:34 PM, Mathias Nyman wrote:

On 22.06.2017 13:44, Shyam Sundar S K wrote:


On 6/19/2017 9:42 AM, Shyam Sundar S K wrote:

on AMD platforms with SNPS 3.1 USB controller if stop endpoint command is
issued the controller does not respond, when the EP is not in running
state. HW completes the command execution and reports
"Context State Error" completion code. This is as per the spec. However
HW on receiving the second command additionally marks EP to Flow control
state in HW which is RTL bug. This bug causes the HW not to respond
to any further doorbells that are rung by the driver. This makes the EP
to not functional anymore and causes gross functional failures.

As a workaround, not to hit this problem, its better we check the EP state
and issue the stop EP command only when the EP is in running state.

Also, with the inclusion of this patch, there maybe a possible race
condition that may get triggered as mentioned in the xHCI spec 4.6.9

Signed-off-by: Shyam Sundar S K 
Signed-off-by: Nehal Shah 
---
   drivers/usb/host/xhci-hub.c | 30 +-
   1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0dde49c..d39b1e0 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -398,17 +398,29 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
   spin_lock_irqsave(>lock, flags);
   for (i = LAST_EP_INDEX; i > 0; i--) {
   if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
-struct xhci_command *command;
-command = xhci_alloc_command(xhci, false, false,
- GFP_NOWAIT);
-if (!command) {
-spin_unlock_irqrestore(>lock, flags);
-xhci_free_command(xhci, cmd);
-return -ENOMEM;

+/* Check endpoint is running before stopping it,
+ * AMD SNPS 3.1 xHC may misbehave otherwise
+ */
+
+struct xhci_ep_ctx *ep_ctx;
+
+ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, i);
+
+if (GET_EP_CTX_STATE(ep_ctx) ==  EP_STATE_RUNNING) {
+struct xhci_command *command;
+
+command = xhci_alloc_command(xhci, false, false,
+ GFP_NOWAIT);
+if (!command) {
+spin_unlock_irqrestore(>lock,
+   flags);
+xhci_free_command(xhci, cmd);
+return -ENOMEM;
+}
+xhci_queue_stop_endpoint(xhci, command,
+ slot_id, i, suspend);
   }
-xhci_queue_stop_endpoint(xhci, command, slot_id, i,
- suspend);
   }
   }
   xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);

Hi Mathias,

I think, all your recommendations were taken care in the above patch. Any 
feedback on this ?



Looks good so far,
I'll start looking/applying/testing it more after the midsummer festivities.
I'll let you know if any new issues come up, otherwise I'll just apply it and 
send
it forward


Hi Mathias,

Any updates on this ?



Added a simplified version of this to my for-usb-linus branch:

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=for-usb-linus=af58c6348ce66860b45ae14391df8511bea5904d

I'll run some quick smoketests on that branch, then I'll send it forwared to 
4.13-rcx

-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 2/2] usb: host: pci_quirks: Force hard reset of Renesas uPD72020x USB controller

[Mathias Nyman]

On 10.07.2017 18:52, Marc Zyngier wrote:

The Renesas uPD72020x XHCI controller seems to suffer from a
really annoying bug, where it may retain some of its DMA programming
across a XHCI reset, and despite the driver correctly programming new
DMA addresses. This is visible if the device has been using 64bit DMA
addresses, and is then switched to using 32bit DMA addresses. The top
32bits of the address (now zero) are ignored are replaced by the 32bit
from the *previous* programming. Sticking with 64bit DMA always works,
but doesn't seem very appropriate.

A PCI reset of the device restores the normal functionnality, which
is done at probe time. Unfortunately, this has to be done before
any quirk has been discovered, hence the intrusive nature of the fix.

Signed-off-by: Marc Zyngier 
---
  drivers/usb/host/pci-quirks.c | 20 
  drivers/usb/host/pci-quirks.h |  1 +
  drivers/usb/host/xhci-pci.c   |  7 +++
  3 files changed, 28 insertions(+)



Acked-by: Mathias Nyman 


--
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] usb: serial: Add support for Qivicon USB ZigBee dongle

[Johan Hovold]

On Mon, Jul 03, 2017 at 11:30:49AM -0500, Dan Williams wrote:
> On Mon, 2017-07-03 at 10:16 +0200, Frans Klaver wrote:
> > On Mon, Jul 3, 2017 at 10:13 AM, Johan Hovold 
> > wrote:
> > > On Fri, Jun 30, 2017 at 02:49:55PM +0200, Frans Klaver wrote:
> > > > On Fri, Jun 30, 2017 at 2:44 PM, Stefan Triller  > > > ller.de> wrote:
> > > > > The German Telekom offers a ZigBee USB Stick under the brand
> > > > > name Qivicon
> > > > > for their SmartHome Home Base in its 1. Generation. The
> > > > > productId is not
> > > > > known by the according kernel module, this patch adds support
> > > > > for it.
> > > > > 
> > > > > Signed-off-by: Stefan Triller 
> > > > 
> > > > Reviewed-by: Frans Klaver 
> > > 
> > > Thanks for the patch and review. I've queued this one up for -rc2.
> > 
> > I guess this could go into stable as well?
> 
> Random question, but what protocol does the serial interface speak?

Good question. Seems to be something SiLabs have come up with called
EmberZNet Serial Protocol (EZSP):

https://www.silabs.com/documents/public/application-notes/AN706.pdf
https://www.silabs.com/documents/public/user-guides/UG101.pdf

Patch now applied for -rc2.

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: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi,

On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun  wrote:
> Hi,
> On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
>> Hi,
>>
>> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
>>  wrote:
>> > Hi Martin,
>> >
>> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote:
>> >> This series is the outcome of a discussion with Felipe Balbi,
>> >> see [0] and [1].
>> >> The quick-summary of this is:
>> >> - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>> >>   correct
>> >> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>> >>   ohci-platform.c) do not have a limitation on the number of PHYs - they
>> >>   support one PHY per actual host port
>> >> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>> >>   or three USB2 ports enabled on the internal root-hub. The SoCs also
>> >>   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>> >>   internally "connected" to the dwc3 roothub) need to be powered on,
>> >>   otherwise USB devices cannot be enumerated (even if just one PHY is
>> >>   disabled and if the device is plugged into another, enabled port)
>> >>
>> >> In my first attempt to get USB supported on the GXL and GXM SoCs I tried
>> >> to work-around the problem that I could not pass multiple PHYs to the
>> >> dwc3 controller.
>> >> This was rejected by Rob Herring (which was definitely the thing to do in
>> >> my opinion), see [2]
>> >>
>> >> This series adds a new "platform-roothub". This can be configured through
>> >> devicetree by passing a child-node with "reg = <0>" to the USB
>> >> controller. Additionally there has to be a child-node for each port on
>> >> the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
>> >> property. This allows modeling the root-hub in devicetree similar to the
>> >> USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
>> >> This avoids and backwards-compatibility problems (which was a concern
>> >> regardless of the solution, see [3]) since the binding for the root-hub
>> >> was previously not specified (and we're not using the "phys" property of
>> >> the controller, which might have served different purposes before,
>> >> depending on the drivers).
>> >>
>> >> Additionally this integrates the new platform-roothub into xhci-plat.c
>> >> which automatically enables it for the dwc3 driver (in host-mode).
>> >>
>> > How to handle the phy0(one u2phy and one u3phy) when port1 support
>> > dual-role mode? leave them to peripheral side as felipe suggested
>> > before? If so, no port1 node for roothub, is there any problem when
>> > change the port1 to host-only mode?
>> on Amlogic Meson GXL we have the following IP blocks:
>> - 2x USB2 PHYs, some external component has to tell them which mode
>> (host/device) they should operate in
>> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
>>
>> on Amlogic Meson GXL it could work like this:
>> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
>> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
>> tell the USB2 PHY in which mode it should operate.
>>
>> please note that device mode and OTG support on Amlogic Meson GXL is
>> more complicated, as it uses dwc2 and dwc3 controllers in combination:
>> - dwc3 is reponsible for host-only mode
>> - dwc2 is responsible for device-only mode
>> - OTG detection is done by the USB3 PHY
>>
>> would you mind sharing a short overview of host/device/OTG support
>> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
>> implementation is quite special, so comparing this with another
>> implementation (for example the Mediatek one) may help spotting
>> potential issues.
>>
> MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
> as following:
thank you for sharing this!

> 1. device mode works as HS only:
>
> u2phy0 --- dual-role/OTG
>
> u2phy1 ---|
>   +  U3 host-only
> u3phy0 ---|
>
> ...
> u2phy4 ---|
>   +  U3 host-only
> u3phy3 ---|
> (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
> dual-role mode, u2phy1 & u3phy0 are host-only)
>
> 2. device mode works as HS & SS, or host only:
>
> u2phy0 ---|
>   + dual-role or host-only
> u3phy0 ---|
>
> ...
> u2phy3 ---|
>   + U3 host-only
> u3phy3 ---|
>
> u2phy4 --- U2 host-only
> (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
OK, so in both cases only one port (with one u2phy and one u3phy) is
dual-role capable

> mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
> pin), and will take all phys it needed, include host-only phys;
> But if just host-only mode is supported, we can skip mtu3 driver and
> make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
I see, in your example it's the mtu3
(Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
mode switching. I assume 

Re: [PATCH v2 1/3] mux: Add mux_control_get_optional() API

[Peter Rosin]

Generally looks like I imagined, but there are a few nits and some
things that I'd like to do differently. Comments inline. Thanks!

On 2017-07-14 23:40, Stephen Boyd wrote:
> Sometimes drivers only use muxes under certain scenarios. For
> example, the chipidea usb controller may be connected to a usb
> switch on some platforms, and connected directly to a usb port on
> others. The driver won't know one way or the other though, so add
> a mux_control_get_optional() API that allows the driver to
> differentiate errors getting the mux from there not being a mux
> for the driver to use at all.
> 
> Cc: Jonathan Cameron 
> Cc: Philipp Zabel 
> Signed-off-by: Stephen Boyd 
> ---
>  Documentation/driver-model/devres.txt |  1 +
>  drivers/mux/mux-core.c| 98 
> ---
>  include/linux/mux/consumer.h  |  4 ++
>  3 files changed, 83 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/driver-model/devres.txt 
> b/Documentation/driver-model/devres.txt
> index 30e04f7a690d..4fdd3e63ff8b 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -342,6 +342,7 @@ MUX
>devm_mux_chip_alloc()
>devm_mux_chip_register()
>devm_mux_control_get()
> +  devm_mux_control_get_optional()
>  
>  PER-CPU MEM
>devm_alloc_percpu()
> diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
> index 90b8995f07cb..a0e5bf16f02f 100644
> --- a/drivers/mux/mux-core.c
> +++ b/drivers/mux/mux-core.c
> @@ -289,6 +289,9 @@ EXPORT_SYMBOL_GPL(devm_mux_chip_register);
>   */
>  unsigned int mux_control_states(struct mux_control *mux)
>  {
> + if (!mux)
> + return 0;
> +

I don't think this is appropriate. For this function, it might be ok,
but...

>   return mux->states;
>  }
>  EXPORT_SYMBOL_GPL(mux_control_states);
> @@ -338,6 +341,9 @@ int mux_control_select(struct mux_control *mux, unsigned 
> int state)
>  {
>   int ret;
>  
> + if (!mux)
> + return 0;
> +

...here and for other cases below it's very odd to return "ok", when
-EINVAL or something seems much more appropriate. And if -EINVAL is
returned here, the benefit of returning fake values for anything
pretty much falls apart.

I simply don't like it, and prefer if the consumer code is arranged
to not call the mux functions when the optional get() does not find
the mux.

>   ret = down_killable(>lock);
>   if (ret < 0)
>   return ret;
> @@ -370,6 +376,9 @@ int mux_control_try_select(struct mux_control *mux, 
> unsigned int state)
>  {
>   int ret;
>  
> + if (!mux)
> + return 0;
> +
>   if (down_trylock(>lock))
>   return -EBUSY;
>  
> @@ -398,6 +407,9 @@ int mux_control_deselect(struct mux_control *mux)
>  {
>   int ret = 0;
>  
> + if (!mux)
> + return 0;
> +
>   if (mux->idle_state != MUX_IDLE_AS_IS &&
>   mux->idle_state != mux->cached_state)
>   ret = mux_control_set(mux, mux->idle_state);
> @@ -422,14 +434,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct 
> device_node *np)
>   return dev ? to_mux_chip(dev) : NULL;
>  }
>  
> -/**
> - * mux_control_get() - Get the mux-control for a device.
> - * @dev: The device that needs a mux-control.
> - * @mux_name: The name identifying the mux-control.
> - *
> - * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> - */
> -struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +struct mux_control *
> +__mux_control_get(struct device *dev, const char *mux_name, bool optional)
>  {
>   struct device_node *np = dev->of_node;
>   struct of_phandle_args args;
> @@ -441,6 +447,8 @@ struct mux_control *mux_control_get(struct device *dev, 
> const char *mux_name)
>   if (mux_name) {
>   index = of_property_match_string(np, "mux-control-names",
>mux_name);
> + if (index == -EINVAL && optional)
> + return NULL;

What about -ENODATA? And if an optional mux is found here, but lookup
fails later in e.g. the of_parse_phandle_with_args call, then I think
an error should be returned. Because that seems like an indication that
DT specifies that there *should* be a mux, but then there isn't one.

>   if (index < 0) {
>   dev_err(dev, "mux controller '%s' not found\n",
>   mux_name);
> @@ -451,6 +459,8 @@ struct mux_control *mux_control_get(struct device *dev, 
> const char *mux_name)
>   ret = of_parse_phandle_with_args(np,
>"mux-controls", "#mux-control-cells",
>index, );
> + if (ret == -ENOENT && optional)
> + return NULL;
>   if (ret) {
>   dev_err(dev, "%s: failed to get mux-control 

Re: PL2303 Linux driver (adding new chip version PID and GPIO)

[Johan Hovold]

On Fri, Jul 14, 2017 at 10:32:49AM +, Chu.Mike [朱堅宜] wrote:

> 保密警語: 
> 本電子郵件內容及其附加檔案均視為機密資料，受保密合約保護或依法不得洩漏。其內容僅供指定收件人按限定範圍或特殊目的合法使用，未經授權者收到此信息均無權閱讀、使用、複製、洩漏或散佈。若您並非本郵件之指定收件人，請即刻回覆郵件並永久刪除此郵件及其附件和銷毀所有複印文件。電子郵件的傳輸可能遭攔截、損毀、遺失、破壞、遲到或不完整、或包含病毒，無法保證其安全或無誤。寄件人不承擔因本電子郵件的錯誤或遺漏所產生的任何損害賠償責任。
>  Confidentiality Notice: This e-mail message together with any attachments 
> thereto (if any) is confidential, protected under an enforceable 
> non-disclosure agreement, intended only for the use of the named recipient(s) 
> above and may contain information that is privileged, belonging to 
> professional work products or exempt from disclosure under applicable laws. 
> Any unauthorized review, use, copying, disclosure, or distribution of any 
> information contained in or attached to this transmission is strictly 
> prohibited and may be against the laws. If you have received this message in 
> error, or are not the intended recipient(s), please immediately notify the 
> sender by e-mail and delete this e-mail message, all copies, and any attached 
> documentation from your computer. E-mail transmission cannot be guaranteed to 
> be secure or error-free as information could be intercepted, corrupted, lost, 
> destroyed, arrive late or incomplete, or contain viruses. The sender 
> therefore does not accept liability for any damage caused by any errors or 
> omissions in the contents of this email.

Oh, and make sure to not include any mail footers like this one, or we
really won't be able to help out at all.

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: [USB] f16443a034: EIP:arch_local_irq_restore

[Felipe Balbi]

Hi,

Alan Stern  writes:
>> >> > On Thu, 29 Jun 2017, kernel test robot wrote:
>> >> >
>> >> >> FYI, we noticed the following commit:
>> >> >> 
>> >> >> commit: f16443a034c7aa359ddf6f0f9bc40d01ca31faea ("USB: gadgetfs, 
>> >> >> dummy-hcd, net2280: fix locking for callbacks")
>> >> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>> >> >> 
>> >> >> in testcase: trinity
>> >> >> with following parameters:
>> >> >> 
>> >> >>runtime: 300s
>> >> >> 
>> >> >> test-description: Trinity is a linux system call fuzz tester.
>> >> >> test-url: http://codemonkey.org.uk/projects/trinity/
>> >> >> 
>> >> >> 
>> >> >> on test machine: qemu-system-x86_64 -enable-kvm -m 420M
>> >> >> 
>> >> >> caused below changes (please refer to attached dmesg/kmsg for entire 
>> >> >> log/backtrace):
>> >> > ...
>> >> >
>> >> > I won't include the entire report.  The gist is that we have a problem 
>> >> > with lock ordering.  The report is about dummy-hcd, but this could 
>> >> > affect any UDC driver.
>> >> >
>> >> >  1. When a SETUP request arrives, composite_setup() acquires
>> >> > cdev->lock before calling the function driver's callback.
>> >> > When that callback submits a reply, it causes the UDC driver
>> >> > to acquire its private lock.
>> >> 
>> >> this only seems to happen before calling set_config():
>> >> 
>> >>   case USB_REQ_SET_CONFIGURATION:
>> >>   if (ctrl->bRequestType != 0)
>> >>   goto unknown;
>> >>   if (gadget_is_otg(gadget)) {
>> >>   if (gadget->a_hnp_support)
>> >>   DBG(cdev, "HNP available\n");
>> >>   else if (gadget->a_alt_hnp_support)
>> >>   DBG(cdev, "HNP on another port\n");
>> >>   else
>> >>   VDBG(cdev, "HNP inactive\n");
>> >>   }
>> >>   spin_lock(>lock);
>> >>   value = set_config(cdev, ctrl, w_value);
>> >>   spin_unlock(>lock);
>> >>   break;
>> >
>> > That's true.  Why is the lock held for set_config() but not for any of 
>> > the other callbacks?
>> 
>> this is really old code from Dave. Your guess is as good as mine :-(
>> 
>> > Doesn't that mean the other callbacks can race with function
>> > unregistration?
>> 
>> Probably not as UDCs are required to cancel transfers and kill all
>> endpoints before unregistering. We would probably just giveback a few
>> requests with -ESHUTDOWN and prevent new ones from being queued to HW,
>> no?
>
> But SETUP callbacks aren't associated with pending requests.  They get

they are if ->setup() returns DELAYED_STATUS.

> generated whenever a SETUP packet is received, even if the gadget
> driver has no requests queued.  Cancelling transfers won't prevent
> them.
>
> Killing all endpoints might or might not do the trick.  Does killing
> ep0 prevent the UDC driver from receiving SETUP packets?  This may vary 
> between UDC drivers.

no, it does not. If ->setup() fails, we are required to Stall and
restart ep0. Restarting ep0 involves preparing it to receive a new
SETUP request.

> There are also the other callbacks (reset, disconnect, bus suspend, and 
> bus resume), which aren't associated with endpoints or requests at all.
>
> Probably drivers really should have something like synchronize_irq() in
> the udc_stop routine.  That would solve a lot of problems (although it 
> wouldn't help dummy_hcd, which doesn't rely on interrupts).

Hmm, free_irq() calls synchronize_irq(). UDCs should just request_irq()
on udc_start() and free_irq() on udc_stop(). That's what dwc3 is doing.

-- 
balbi
--
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: PL2303 Linux driver (adding new chip version PID and GPIO)

[Johan Hovold]

On Fri, Jul 14, 2017 at 12:47:29PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 14, 2017 at 10:32:49AM +, Chu.Mike [朱堅宜] wrote:
> > Hi Greg,
> > 
> > This patch is for adding our new chip PL2303TB (UART, 12 GPIO, PWN)
> > PID is 2304 and GPIO function codes. Attached is datasheet. Can you
> > help us?
> 
> Adding a new device is is simple, but adding custom ioctls for gpio
> control is not ok.  That needs to tie into the gpio subsystem instead.
> I think work is ongoing with some other drivers to do this, so you
> should have some examples on the mailing list to look at.
> 
> Johan would have more details about this, Johan?

Yes, you can take a look at the cp210x driver which recently gained gpio
support for an example:

 cf5276ce7867 ("USB: serial: cp210x: Adding GPIO support for
CP2105")

Basically you should register one (or more) gpio chips when probing the
device while taking the current pin settings into account (e.g. don't
registers gpios for pins that are currently being used for uart
functions).

Also make sure to split this up in logical changes; for example, add
support for your new device first (e.g. vid/pid entry + whatever changes
are required to talk to the new chip), and then add gpio support on top
of that.

And just let us know if you have any further questions regarding any of
this.

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: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

Hi,
On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
> Hi,
> 
> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
>  wrote:
> > Hi Martin,
> >
> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote:
> >> This series is the outcome of a discussion with Felipe Balbi,
> >> see [0] and [1].
> >> The quick-summary of this is:
> >> - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> >>   correct
> >> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> >>   ohci-platform.c) do not have a limitation on the number of PHYs - they
> >>   support one PHY per actual host port
> >> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> >>   or three USB2 ports enabled on the internal root-hub. The SoCs also
> >>   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> >>   internally "connected" to the dwc3 roothub) need to be powered on,
> >>   otherwise USB devices cannot be enumerated (even if just one PHY is
> >>   disabled and if the device is plugged into another, enabled port)
> >>
> >> In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> >> to work-around the problem that I could not pass multiple PHYs to the
> >> dwc3 controller.
> >> This was rejected by Rob Herring (which was definitely the thing to do in
> >> my opinion), see [2]
> >>
> >> This series adds a new "platform-roothub". This can be configured through
> >> devicetree by passing a child-node with "reg = <0>" to the USB
> >> controller. Additionally there has to be a child-node for each port on
> >> the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> >> property. This allows modeling the root-hub in devicetree similar to the
> >> USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
> >> This avoids and backwards-compatibility problems (which was a concern
> >> regardless of the solution, see [3]) since the binding for the root-hub
> >> was previously not specified (and we're not using the "phys" property of
> >> the controller, which might have served different purposes before,
> >> depending on the drivers).
> >>
> >> Additionally this integrates the new platform-roothub into xhci-plat.c
> >> which automatically enables it for the dwc3 driver (in host-mode).
> >>
> > How to handle the phy0(one u2phy and one u3phy) when port1 support
> > dual-role mode? leave them to peripheral side as felipe suggested
> > before? If so, no port1 node for roothub, is there any problem when
> > change the port1 to host-only mode?
> on Amlogic Meson GXL we have the following IP blocks:
> - 2x USB2 PHYs, some external component has to tell them which mode
> (host/device) they should operate in
> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
> 
> on Amlogic Meson GXL it could work like this:
> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
> tell the USB2 PHY in which mode it should operate.
> 
> please note that device mode and OTG support on Amlogic Meson GXL is
> more complicated, as it uses dwc2 and dwc3 controllers in combination:
> - dwc3 is reponsible for host-only mode
> - dwc2 is responsible for device-only mode
> - OTG detection is done by the USB3 PHY
> 
> would you mind sharing a short overview of host/device/OTG support
> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
> implementation is quite special, so comparing this with another
> implementation (for example the Mediatek one) may help spotting
> potential issues.
> 
MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
as following:

1. device mode works as HS only:

u2phy0 --- dual-role/OTG

u2phy1 ---|
  +  U3 host-only
u3phy0 ---|

...
u2phy4 ---|
  +  U3 host-only
u3phy3 ---|
(e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
dual-role mode, u2phy1 & u3phy0 are host-only)

2. device mode works as HS & SS, or host only:

u2phy0 ---|
  + dual-role or host-only
u3phy0 ---|

...
u2phy3 ---|
  + U3 host-only
u3phy3 ---|

u2phy4 --- U2 host-only
(e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)

mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
pin), and will take all phys it needed, include host-only phys; 
But if just host-only mode is supported, we can skip mtu3 driver and
make use of xhci-mtk driver directly, then xhci-mtk will take all phys.

> >>
> >> Changes since RFCv1 at [4]:
> >> - split the usb-xhci dt-binding documentation into a separate patch
> >> - fixed a typo ("usb-phy" -> "phys" in the dt-binding example)
> >> - rebased to apply against latest usb-next
> >>
> >>
> >> [0] 
> >> http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
> >> [1] 
> >> http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
> >> [2] 
> >>