Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Greg KH]

On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
> r8153 on Dell TB dock corrupts rx packets.
> 
> The root cause is not found yet, but disabling rx checksumming can
> workaround the issue. We can use this connection to decide if it's
> a Dell TB dock:
> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> 
> BugLink: https://bugs.launchpad.net/bugs/1729674
> Cc: Mario Limonciello 
> Signed-off-by: Kai-Heng Feng 
> ---
>  drivers/net/usb/r8152.c | 33 -
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index d51d9abf7986..58b80b5e7803 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -27,6 +27,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  /* Information for net-next */
>  #define NETNEXT_VERSION  "09"
> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
>   return version;
>  }
>  
> +/* Ethernet on Dell TB 15/16 dock is connected this way:
> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> + * We use this connection to make sure r8153 is on the Dell TB dock.
> + */
> +static bool check_dell_tb_dock(struct usb_device *udev)
> +{
> + struct usb_device *hub = udev->parent;
> + struct usb_device *root_hub;
> + struct pci_dev *controller;
> +
> + if (!hub)
> + return false;
> +
> + if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
> +   le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
> + return false;
> +
> + root_hub = hub->parent;
> + if (!root_hub || root_hub->parent)
> + return false;
> +
> + controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);

That's a very scary, and dangerous, cast.  You can not ever be sure that
the hub really is a "root hub" like this.

> + if (controller->vendor == 0x1b21 && controller->device == 0x1142)
> + return true;

Why can't you just look at the USB device itself and go off of a quirk
in it?  Something like a version or string or something else?

This sounds like a USB host controller issue, not a USB device issue,
can't we fix the "real" problem here instead of this crazy work-around?
Odds are any device plugged into the hub should have the same issue,
right?

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: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Thu, 23 Nov 2017, Greg Kroah-Hartman wrote:

> On Wed, Nov 22, 2017 at 10:20:49PM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 22 Nov 2017, Joe Perches wrote:
> >
> > > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > > There is no need to #define the license of the driver, just put it in
> > > > the MODULE_LICENSE() line directly as a text string.
> > > >
> > > > This allows tools that check that the module license matches the source
> > > > code license to work properly, as there is no need to unwind the
> > > > unneeded dereference.
> > > []
> > > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> > > []
> > > > @@ -76,7 +76,6 @@
> > > >
> > > >  #define MOD_AUTHOR "Option Wireless"
> > > >  #define MOD_DESCRIPTION"USB High Speed Option 
> > > > driver"
> > > > -#define MOD_LICENSE"GPL"
> > > >
> > > >  #define HSO_MAX_NET_DEVICES10
> > > >  #define HSO__MAX_MTU   2048
> > > > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> > > >
> > > >  MODULE_AUTHOR(MOD_AUTHOR);
> > > >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > > > -MODULE_LICENSE(MOD_LICENSE);
> > > > +MODULE_LICENSE("GPL");
> > >
> > > Probably all of these MODULE_(MOD_) uses could be
> > > simplified as well.
> > >
> > > Perhaps there's utility in a (cocci?) script that looks for
> > > used-once
> > > macro #defines in various types of macros.
> >
> > What about module_version, eg:
> >
> > diff -u -p a/drivers/ata/pata_pdc202xx_old.c
> > b/drivers/ata/pata_pdc202xx_old.c
> > --- a/drivers/ata/pata_pdc202xx_old.c
> > +++ b/drivers/ata/pata_pdc202xx_old.c
> > @@ -21,7 +21,6 @@
> >  #include 
> >
> >  #define DRV_NAME "pata_pdc202xx_old"
> > -#define DRV_VERSION "0.4.3"
> >
> >  static int pdc2026x_cable_detect(struct ata_port *ap)
> >  {
> > @@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
> >  MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
> >  MODULE_LICENSE("GPL");
> >  MODULE_DEVICE_TABLE(pci, pdc202xx);
> > -MODULE_VERSION(DRV_VERSION);
> > +MODULE_VERSION("0.4.3");
>
> I've just deleted MODULE_VERSION() entirely from some subsystems, as
> once the driver is in the kernel source tree, the "version" makes almost
> no sense at all.
>
> But I know some companies love incrementing it (some network and scsi
> drivers specifically), so those might want to keep it around for some
> odd reason.

OK, that seems like a simple soluton.  Thanks.

julia
--
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: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Greg Kroah-Hartman]

On Wed, Nov 22, 2017 at 10:20:49PM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 22 Nov 2017, Joe Perches wrote:
> 
> > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > There is no need to #define the license of the driver, just put it in
> > > the MODULE_LICENSE() line directly as a text string.
> > >
> > > This allows tools that check that the module license matches the source
> > > code license to work properly, as there is no need to unwind the
> > > unneeded dereference.
> > []
> > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> > []
> > > @@ -76,7 +76,6 @@
> > >
> > >  #define MOD_AUTHOR   "Option Wireless"
> > >  #define MOD_DESCRIPTION  "USB High Speed Option driver"
> > > -#define MOD_LICENSE  "GPL"
> > >
> > >  #define HSO_MAX_NET_DEVICES  10
> > >  #define HSO__MAX_MTU 2048
> > > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> > >
> > >  MODULE_AUTHOR(MOD_AUTHOR);
> > >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > > -MODULE_LICENSE(MOD_LICENSE);
> > > +MODULE_LICENSE("GPL");
> >
> > Probably all of these MODULE_(MOD_) uses could be
> > simplified as well.
> >
> > Perhaps there's utility in a (cocci?) script that looks for
> > used-once
> > macro #defines in various types of macros.
> 
> What about module_version, eg:
> 
> diff -u -p a/drivers/ata/pata_pdc202xx_old.c
> b/drivers/ata/pata_pdc202xx_old.c
> --- a/drivers/ata/pata_pdc202xx_old.c
> +++ b/drivers/ata/pata_pdc202xx_old.c
> @@ -21,7 +21,6 @@
>  #include 
> 
>  #define DRV_NAME "pata_pdc202xx_old"
> -#define DRV_VERSION "0.4.3"
> 
>  static int pdc2026x_cable_detect(struct ata_port *ap)
>  {
> @@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
>  MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
>  MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(pci, pdc202xx);
> -MODULE_VERSION(DRV_VERSION);
> +MODULE_VERSION("0.4.3");

I've just deleted MODULE_VERSION() entirely from some subsystems, as
once the driver is in the kernel source tree, the "version" makes almost
no sense at all.

But I know some companies love incrementing it (some network and scsi
drivers specifically), so those might want to keep it around for some
odd reason.

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] r8152: disable rx checksum offload on Dell TB dock

[Kai-Heng Feng]

r8153 on Dell TB dock corrupts rx packets.

The root cause is not found yet, but disabling rx checksumming can
workaround the issue. We can use this connection to decide if it's
a Dell TB dock:
Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller

BugLink: https://bugs.launchpad.net/bugs/1729674
Cc: Mario Limonciello 
Signed-off-by: Kai-Heng Feng 
---
 drivers/net/usb/r8152.c | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index d51d9abf7986..58b80b5e7803 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -27,6 +27,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 /* Information for net-next */
 #define NETNEXT_VERSION"09"
@@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
return version;
 }
 
+/* Ethernet on Dell TB 15/16 dock is connected this way:
+ * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
+ * We use this connection to make sure r8153 is on the Dell TB dock.
+ */
+static bool check_dell_tb_dock(struct usb_device *udev)
+{
+   struct usb_device *hub = udev->parent;
+   struct usb_device *root_hub;
+   struct pci_dev *controller;
+
+   if (!hub)
+   return false;
+
+   if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
+ le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
+   return false;
+
+   root_hub = hub->parent;
+   if (!root_hub || root_hub->parent)
+   return false;
+
+   controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);
+
+   if (controller->vendor == 0x1b21 && controller->device == 0x1142)
+   return true;
+
+   return false;
+}
+
 static int rtl8152_probe(struct usb_interface *intf,
 const struct usb_device_id *id)
 {
@@ -5202,7 +5233,7 @@ static int rtl8152_probe(struct usb_interface *intf,
NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
NETIF_F_IPV6_CSUM | NETIF_F_TSO6;
 
-   if (tp->version == RTL_VER_01) {
+   if (tp->version == RTL_VER_01 || check_dell_tb_dock(udev)) {
netdev->features &= ~NETIF_F_RXCSUM;
netdev->hw_features &= ~NETIF_F_RXCSUM;
}
-- 
2.14.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

Re: [PATCH v3 10/16] phy: qcom-qmp: Move register offsets to header file

[Manu Gautam]

Hi,


On 11/22/2017 10:56 PM, Stephen Boyd wrote:
> On 11/21/2017 01:23 AM, Manu Gautam wrote:
>> New revision (v3) of QMP PHY uses different offsets
>> for almost all of the registers. Hence, move these
>> definitions to header file so that updated offsets
>> can be added for QMP v3.
>>
>> Signed-off-by: Manu Gautam 
>> ---
> Why? It would only be included into one C file so it's really not necessary.

Yes, it is not necessary but it keep QMP PHY driver clean. V3 PHY duplicates 
all the
#defines and this could continue for future revisions as well.
Please refer to this patch:
 - [PATCH v3 11/16] phy: qcom-qmp: Add register offsets for QMP V3 PHY


-- 
The Qualcomm Innovation Center, Inc. is a member of the 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

Re: [PATCH v3 03/16] phy: qcom-qmp: Power-on PHY before initialization

[Manu Gautam]

Hi,

On 11/22/2017 11:33 PM, Stephen Boyd wrote:
> On 11/21/2017 01:23 AM, Manu Gautam wrote:
>> PHY must be powered on before turning ON clocks and
>> attempting to initialize it. Driver is exposing
>> separate init and power_on routines for this.
>> Apparently USB dwc3 core driver performs power-on after
>> init. Also, poweron and init for QMP PHY need to be
> Why does dwc3 driver power on after init? Seems backwards.

There are not many PHY drivers implementing power_on, rather they
rely on init to take care of complete initialization. However though
the name indicates power_on, but PHY drivers are not using it to
turn on power supplies but rather PHY register operations to enable/
start PHY - somewhat like init only.

>
>> executed together always, hence remove poweron callback
>> from phy_ops and explicitly perform this from com_init,
> Why do they need to be executed together?

Hardware programming guide requires PHY supplies to be ON before
it is initialized. And if PHY supplies were turned-off, then it must
be reset after turning them ON.


>
>> similar changes needed for poweroff. On similar lines move
>> clk_enable from init to com_init which can be called once
>> for multi lane PHYs.
> Please add parenthesis, clk_enable() for example, to functions so we
> know they're functions.

Ok.

>
>> Signed-off-by: Manu Gautam 
>> ---
>>  drivers/phy/qualcomm/phy-qcom-qmp.c | 61 
>> +
>>  1 file changed, 21 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
>> b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> index 90794dd..2f427e3 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> @@ -720,33 +720,6 @@ static void qcom_qmp_phy_configure(void __iomem *base,
>>  }
>>  }
>>  
>> -static int qcom_qmp_phy_poweron(struct phy *phy)
>> -{
>> -struct qmp_phy *qphy = phy_get_drvdata(phy);
>> -struct qcom_qmp *qmp = qphy->qmp;
>> -int num = qmp->cfg->num_vregs;
>> -int ret;
>> -
>> -dev_vdbg(>dev, "Powering on QMP phy\n");
>> -
>> -/* turn on regulator supplies */
>> -ret = regulator_bulk_enable(num, qmp->vregs);
>> -if (ret)
>> -dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
>> -
>> -return ret;
>> -}
>> -
>> -static int qcom_qmp_phy_poweroff(struct phy *phy)
>> -{
>> -struct qmp_phy *qphy = phy_get_drvdata(phy);
>> -struct qcom_qmp *qmp = qphy->qmp;
>> -
>> -regulator_bulk_disable(qmp->cfg->num_vregs, qmp->vregs);
>> -
>> -return 0;
>> -}
>> -
>>  static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>  {
>>  const struct qmp_phy_cfg *cfg = qmp->cfg;
>> @@ -759,6 +732,19 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>  return 0;
>>  }
>>  
>> +/* turn on regulator supplies */
>> +ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
>> +if (ret) {
>> +mutex_unlock(>phy_mutex);
>> +return ret;
> This could also be a goto.

Yes, I can replace with goto here.

>
>> +}
>> +
>> +ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
>> +if (ret) {
>> +dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
>> +goto err_clk_enable;
>> +}
>> +
>>  for (i = 0; i < cfg->num_resets; i++) {
>>  ret = reset_control_deassert(qmp->resets[i]);
>>  if (ret) {

-- 
The Qualcomm Innovation Center, Inc. is a member of the 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

Re: [PATCH 11/11] usb/gadget: Make it again possible to enable the legacy drivers without enabling USB_ETH

[Bart Van Assche]

On Tue, 2017-10-31 at 11:03 -0700, Bart Van Assche wrote:
> Considerable time ago the legacy gadget menu was added inside the
> USB_ETH choice. I think this was a mistake and that the legacy
> gadget menu should have been added after "endchoice" instead of
> before. Hence this patch.
> 
> Fixes: commit 8443f2d2b778 ("usb: gadget: Gadget directory cleanup - group 
> legacy gadgets")
> Signed-off-by: Bart Van Assche 
> Cc: Nicholas Bellinger 
> Cc: Andrzej Pietrasiewicz 
> Cc: Felipe Balbi 
> Cc: linux-usb@vger.kernel.org

Hello Andrzej and Felipe,

Can one or both of you have a look at this patch?

Thanks,

Bart.N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH 1/2] usb: xhci: add relaxed timing quirk bit

[Adam Wallis]

On 11/22/2017 10:24 AM, Mathias Nyman wrote:
[..]
> 
> We know have at least two hosts/platforms that need custom interrupt 
> moderation
> values
> 
> How about adding a u32 device property for xhci with the interrupt moderation
> interval in
> nanoseconds?  And also add a u32 imod_interval variable to struct xhci_hcd?
>  
> imod_interval can be set to the current default 4ns (160*250ns) and
> overwritten if
> device_property_read_u32() returns something else.
> 

Isn't the 160 value quite aggressive anyway? Section 5.5.2.2 of the xHCI spec
says that maximum observable interrupt rate should never exceed 8000
interrupts/second. I believe the IMOD value in the most aggressive case would
then be 500 by this statement [ 1 / (250e-9 * 500) = 8000 irqs/second ]

Perhaps I am misreading the spec or just doing the math wrong? With the default
value of 160, we are interrupting 25,000 irq/second...which is over 3 times the
maximum stated value (again, assuming I did the math right)

Anyway, my preference would be to set the IMOD default val to 4000 (~1ms) per
the recommended value in Table 49 of the spec and allow platforms to adjust as
necessary from that point.

Thoughts on this?

> XHCI_MTK_HOST could then use whatever preferred device propery interval value,
> and we can get rid of using XHCI_MTK_HOST quirk flag when setting up the IMODI
>  
> -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

Adam

-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the 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 AUTOSEL for 4.14 19/51] usb: dwc2: Fix UDC state tracking

[alexander . levin]

From: John Stultz 

[ Upstream commit ce2b21a4e5ce042c0a42c9db8fa9e0f849427d5e ]

It has been noticed that the dwc2 udc state reporting doesn't
seem to work (at least on HiKey boards). Where after the initial
setup, the sysfs /sys/class/udc/f72c.usb/state file would
report "configured" no matter the state of the OTG port.

This patch adds a call so that we report to the UDC layer when
the gadget device is disconnected.

This patch does depend on the previous patch ("usb: dwc2:
Improve gadget state disconnection handling") in this patch set
in order to properly work.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: Amit Pundir 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0d8e09ccb59c..f4a8ffad3e16 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3202,6 +3202,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
 
call_gadget(hsotg, disconnect);
hsotg->lx_state = DWC2_L3;
+
+   usb_gadget_set_state(>gadget, USB_STATE_NOTATTACHED);
 }
 
 /**
-- 
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 AUTOSEL for 4.14 20/51] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

[alexander . levin]

From: John Stultz 

[ Upstream commit 9b481092c2a31a6b630aff9c28f0145bf6683787 ]

We've found that while in host mode, using Android, if one runs
the command:
  stop adbd

The existing usb devices being utilized in host mode are disconnected.
This is most visible with usb networking devices.

This seems to be due to adbd closing the file:
  /dev/usb-ffs/adb/ep0
Which calls ffs_ep0_release() and the following backtrace:

[] dwc2_hsotg_ep_disable+0x148/0x150
[] dwc2_hsotg_udc_stop+0x60/0x110
[] usb_gadget_remove_driver+0x58/0x78
[] usb_gadget_unregister_driver+0x74/0xe8
[] unregister_gadget+0x28/0x58
[] unregister_gadget_item+0x2c/0x40
[] ffs_data_clear+0xe8/0xf8
[] ffs_data_reset+0x20/0x58
[] ffs_data_closed+0x98/0xe8
[] ffs_ep0_release+0x20/0x30

Then when dwc2_hsotg_ep_disable() is called, we call
kill_all_requests() which causes a bunch of the following
messages:

dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
init: Service 'adbd' (pid 1915) killed by signal 9
init: Sending signal 9 to service 'adbd' (pid 1915) process group...
init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_update_urb_state_abn(): trimming xfer length

And the usb devices connected are basically hung at this point.

It seems like if we're in host mode, we probably shouldn't run
the dwc2_hostg_ep_disable logic, so this patch returns an error
in that case.

With this patch (along with the previous patch in this set), we avoid
the mismatched interrupts and connected usb devices continue to function.

I'm not sure if some other solution would be better here, but this seems
to work, so I wanted to send it out for input on what the right approach
should be.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: YongQin Liu 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index f4a8ffad3e16..603c21612051 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4006,6 +4006,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
return -EINVAL;
}
 
+   if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
+   dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
+   return -EINVAL;
+   }
+
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
 
spin_lock_irqsave(>lock, flags);
-- 
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 AUTOSEL for 4.4 07/16] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

[alexander . levin]

From: John Stultz 

[ Upstream commit 9b481092c2a31a6b630aff9c28f0145bf6683787 ]

We've found that while in host mode, using Android, if one runs
the command:
  stop adbd

The existing usb devices being utilized in host mode are disconnected.
This is most visible with usb networking devices.

This seems to be due to adbd closing the file:
  /dev/usb-ffs/adb/ep0
Which calls ffs_ep0_release() and the following backtrace:

[] dwc2_hsotg_ep_disable+0x148/0x150
[] dwc2_hsotg_udc_stop+0x60/0x110
[] usb_gadget_remove_driver+0x58/0x78
[] usb_gadget_unregister_driver+0x74/0xe8
[] unregister_gadget+0x28/0x58
[] unregister_gadget_item+0x2c/0x40
[] ffs_data_clear+0xe8/0xf8
[] ffs_data_reset+0x20/0x58
[] ffs_data_closed+0x98/0xe8
[] ffs_ep0_release+0x20/0x30

Then when dwc2_hsotg_ep_disable() is called, we call
kill_all_requests() which causes a bunch of the following
messages:

dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
init: Service 'adbd' (pid 1915) killed by signal 9
init: Sending signal 9 to service 'adbd' (pid 1915) process group...
init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_update_urb_state_abn(): trimming xfer length

And the usb devices connected are basically hung at this point.

It seems like if we're in host mode, we probably shouldn't run
the dwc2_hostg_ep_disable logic, so this patch returns an error
in that case.

With this patch (along with the previous patch in this set), we avoid
the mismatched interrupts and connected usb devices continue to function.

I'm not sure if some other solution would be better here, but this seems
to work, so I wanted to send it out for input on what the right approach
should be.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: YongQin Liu 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 9d791701b0f4..c1935eafcbf6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2834,6 +2834,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
return -EINVAL;
}
 
+   if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
+   dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
+   return -EINVAL;
+   }
+
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
 
spin_lock_irqsave(>lock, flags);
-- 
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 AUTOSEL for 4.4 06/16] usb: dwc2: Fix UDC state tracking

[alexander . levin]

From: John Stultz 

[ Upstream commit ce2b21a4e5ce042c0a42c9db8fa9e0f849427d5e ]

It has been noticed that the dwc2 udc state reporting doesn't
seem to work (at least on HiKey boards). Where after the initial
setup, the sysfs /sys/class/udc/f72c.usb/state file would
report "configured" no matter the state of the OTG port.

This patch adds a call so that we report to the UDC layer when
the gadget device is disconnected.

This patch does depend on the previous patch ("usb: dwc2:
Improve gadget state disconnection handling") in this patch set
in order to properly work.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: Amit Pundir 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0abf73c91beb..9d791701b0f4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2206,6 +2206,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
 
call_gadget(hsotg, disconnect);
hsotg->lx_state = DWC2_L3;
+
+   usb_gadget_set_state(>gadget, USB_STATE_NOTATTACHED);
 }
 
 /**
-- 
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 AUTOSEL for 4.9 12/24] usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode

[alexander . levin]

From: John Stultz 

[ Upstream commit 9b481092c2a31a6b630aff9c28f0145bf6683787 ]

We've found that while in host mode, using Android, if one runs
the command:
  stop adbd

The existing usb devices being utilized in host mode are disconnected.
This is most visible with usb networking devices.

This seems to be due to adbd closing the file:
  /dev/usb-ffs/adb/ep0
Which calls ffs_ep0_release() and the following backtrace:

[] dwc2_hsotg_ep_disable+0x148/0x150
[] dwc2_hsotg_udc_stop+0x60/0x110
[] usb_gadget_remove_driver+0x58/0x78
[] usb_gadget_unregister_driver+0x74/0xe8
[] unregister_gadget+0x28/0x58
[] unregister_gadget_item+0x2c/0x40
[] ffs_data_clear+0xe8/0xf8
[] ffs_data_reset+0x20/0x58
[] ffs_data_closed+0x98/0xe8
[] ffs_ep0_release+0x20/0x30

Then when dwc2_hsotg_ep_disable() is called, we call
kill_all_requests() which causes a bunch of the following
messages:

dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
init: Service 'adbd' (pid 1915) killed by signal 9
init: Sending signal 9 to service 'adbd' (pid 1915) process group...
init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason 
is unknown
dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
dwc2 f72c.usb: dwc2_update_urb_state_abn(): trimming xfer length

And the usb devices connected are basically hung at this point.

It seems like if we're in host mode, we probably shouldn't run
the dwc2_hostg_ep_disable logic, so this patch returns an error
in that case.

With this patch (along with the previous patch in this set), we avoid
the mismatched interrupts and connected usb devices continue to function.

I'm not sure if some other solution would be better here, but this seems
to work, so I wanted to send it out for input on what the right approach
should be.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: YongQin Liu 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index dbbc4cdad707..4d01d77a12c6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3117,6 +3117,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
return -EINVAL;
}
 
+   if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
+   dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
+   return -EINVAL;
+   }
+
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
 
spin_lock_irqsave(>lock, flags);
-- 
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 AUTOSEL for 4.9 11/24] usb: dwc2: Fix UDC state tracking

[alexander . levin]

From: John Stultz 

[ Upstream commit ce2b21a4e5ce042c0a42c9db8fa9e0f849427d5e ]

It has been noticed that the dwc2 udc state reporting doesn't
seem to work (at least on HiKey boards). Where after the initial
setup, the sysfs /sys/class/udc/f72c.usb/state file would
report "configured" no matter the state of the OTG port.

This patch adds a call so that we report to the UDC layer when
the gadget device is disconnected.

This patch does depend on the previous patch ("usb: dwc2:
Improve gadget state disconnection handling") in this patch set
in order to properly work.

Cc: Wei Xu 
Cc: Guodong Xu 
Cc: Amit Pundir 
Cc: YongQin Liu 
Cc: John Youn 
Cc: Minas Harutyunyan 
Cc: Douglas Anderson 
Cc: Chen Yu 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Acked-by: Minas Harutyunyan 
Tested-by: Minas Harutyunyan 
Reported-by: Amit Pundir 
Signed-off-by: John Stultz 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
---
 drivers/usb/dwc2/gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index cfdd5c3da236..dbbc4cdad707 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2467,6 +2467,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
 
call_gadget(hsotg, disconnect);
hsotg->lx_state = DWC2_L3;
+
+   usb_gadget_set_state(>gadget, USB_STATE_NOTATTACHED);
 }
 
 /**
-- 
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: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > There is no need to #define the license of the driver, just put it in
> > the MODULE_LICENSE() line directly as a text string.
> >
> > This allows tools that check that the module license matches the source
> > code license to work properly, as there is no need to unwind the
> > unneeded dereference.
> []
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> []
> > @@ -76,7 +76,6 @@
> >
> >  #define MOD_AUTHOR "Option Wireless"
> >  #define MOD_DESCRIPTION"USB High Speed Option driver"
> > -#define MOD_LICENSE"GPL"
> >
> >  #define HSO_MAX_NET_DEVICES10
> >  #define HSO__MAX_MTU   2048
> > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> >
> >  MODULE_AUTHOR(MOD_AUTHOR);
> >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > -MODULE_LICENSE(MOD_LICENSE);
> > +MODULE_LICENSE("GPL");
>
> Probably all of these MODULE_(MOD_) uses could be
> simplified as well.
>
> Perhaps there's utility in a (cocci?) script that looks for
> used-once
> macro #defines in various types of macros.

What about module_version, eg:

diff -u -p a/drivers/ata/pata_pdc202xx_old.c
b/drivers/ata/pata_pdc202xx_old.c
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -21,7 +21,6 @@
 #include 

 #define DRV_NAME "pata_pdc202xx_old"
-#define DRV_VERSION "0.4.3"

 static int pdc2026x_cable_detect(struct ata_port *ap)
 {
@@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
 MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, pdc202xx);
-MODULE_VERSION(DRV_VERSION);
+MODULE_VERSION("0.4.3");

julia
--
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 1/2] usb: xhci: add relaxed timing quirk bit

[Adam Wallis]

On 11/22/2017 10:24 AM, Mathias Nyman wrote:
> On 22.11.2017 02:07, Adam Wallis wrote:
>> On 11/21/2017 3:06 PM, Rob Herring wrote:
>>
>> [..]
>>
 I like where you are going with this. Are you saying that I could read for 
 a
 device property read from firmware (for DTB or ACPI) like DWC3 does for
 "snps,hird-threshold"?
>>>
>>> Is that for the same thing? If so, drop the vendor prefix and use
>>> that. Otherwise, a separate property should really be something that
>>> is per board rather than per SoC.
>>
>> I don't think that's exactly the same property, but it's the same idea I 
>> would
>> prefer to go with. That way, an integer can be passed in via the firmware 
>> tables.
>>
>>>
 If you mean this, where do you recommend I store the
 desired IRQ_CONTROL value - in struct xhci_hcd ?
>>>
>>> No idea.
>>>
 Or by "compatible" strings, did you mean storing hard-coded values in the
 of_device_id usb_xhci_of_match[] array? This would still be hard-coding
 (which I
 would like to avoid) and also would not work for the ACPI case.
>>>
>>> ACPI has match tables too?
>>>
>>
>> Yes, you can use DSD in a way that is similar to OF properties
>>
>>> It would only be hardcoded per compatible which should be per SoC. Do
>>> you need per board/device tuning? If so, use a property.
>>>
>>
>> The reason why I think it should dynamic via firmware tables is that
>>
>> * It's much less invasive for vendors to update their DT tables if they need 
>> to
>> adjust on a per device/controller/family/etc basis then to adjust a 
>> properties
>> table in xhci-plat
>> * This would lead to less polluting in xhci-plat code
>>
>>> 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
>>>
>>
>> I will provide an updated proposed patch sometime this week. I also hope to 
>> get
>> some feedback from Mathias to see what he prefers.
> 
> We know have at least two hosts/platforms that need custom interrupt 
> moderation
> values
> 
> How about adding a u32 device property for xhci with the interrupt moderation
> interval in
> nanoseconds?  And also add a u32 imod_interval variable to struct xhci_hcd?
>  
> imod_interval can be set to the current default 4ns (160*250ns) and
> overwritten if
> device_property_read_u32() returns something else.
> 
> XHCI_MTK_HOST could then use whatever preferred device propery interval value,
> and we can get rid of using XHCI_MTK_HOST quirk flag when setting up the IMODI
>  
> -Mathias
> 

This sounds excellent Mathias. Let me put together a patch along these lines.
Thanks for the feedback!

Adam

> 
> -- 
> 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


-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the 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] USB: usbip: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/usb/usbip/vhci_rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c
index 90577e8b2282..a9813f1d507d 100644
--- a/drivers/usb/usbip/vhci_rx.c
+++ b/drivers/usb/usbip/vhci_rx.c
@@ -31,7 +31,7 @@ struct urb *pickup_urb_and_free_priv(struct vhci_device 
*vdev, __u32 seqnum)
/* fall through */
case -ECONNRESET:
dev_info(>dev->dev,
-"urb %p was unlinked %ssynchronuously.\n", urb,
+"urb %p was unlinked %ssynchronously.\n", urb,
 status == -ENOENT ? "" : "a");
break;
case -EINPROGRESS:
-- 
2.14.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

Re: [PATCH v3 03/16] phy: qcom-qmp: Power-on PHY before initialization

[Stephen Boyd]

On 11/21/2017 01:23 AM, Manu Gautam wrote:
> PHY must be powered on before turning ON clocks and
> attempting to initialize it. Driver is exposing
> separate init and power_on routines for this.
> Apparently USB dwc3 core driver performs power-on after
> init. Also, poweron and init for QMP PHY need to be

Why does dwc3 driver power on after init? Seems backwards.

> executed together always, hence remove poweron callback
> from phy_ops and explicitly perform this from com_init,

Why do they need to be executed together?

> similar changes needed for poweroff. On similar lines move
> clk_enable from init to com_init which can be called once
> for multi lane PHYs.

Please add parenthesis, clk_enable() for example, to functions so we
know they're functions.

>
> Signed-off-by: Manu Gautam 
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 61 
> +
>  1 file changed, 21 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c 
> b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 90794dd..2f427e3 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -720,33 +720,6 @@ static void qcom_qmp_phy_configure(void __iomem *base,
>   }
>  }
>  
> -static int qcom_qmp_phy_poweron(struct phy *phy)
> -{
> - struct qmp_phy *qphy = phy_get_drvdata(phy);
> - struct qcom_qmp *qmp = qphy->qmp;
> - int num = qmp->cfg->num_vregs;
> - int ret;
> -
> - dev_vdbg(>dev, "Powering on QMP phy\n");
> -
> - /* turn on regulator supplies */
> - ret = regulator_bulk_enable(num, qmp->vregs);
> - if (ret)
> - dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
> -
> - return ret;
> -}
> -
> -static int qcom_qmp_phy_poweroff(struct phy *phy)
> -{
> - struct qmp_phy *qphy = phy_get_drvdata(phy);
> - struct qcom_qmp *qmp = qphy->qmp;
> -
> - regulator_bulk_disable(qmp->cfg->num_vregs, qmp->vregs);
> -
> - return 0;
> -}
> -
>  static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>  {
>   const struct qmp_phy_cfg *cfg = qmp->cfg;
> @@ -759,6 +732,19 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>   return 0;
>   }
>  
> + /* turn on regulator supplies */
> + ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> + if (ret) {
> + mutex_unlock(>phy_mutex);
> + return ret;

This could also be a goto.

> + }
> +
> + ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
> + if (ret) {
> + dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
> + goto err_clk_enable;
> + }
> +
>   for (i = 0; i < cfg->num_resets; i++) {
>   ret = reset_control_deassert(qmp->resets[i]);
>   if (ret) {

-- 
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

Re: [PATCH v3 10/16] phy: qcom-qmp: Move register offsets to header file

[Stephen Boyd]

On 11/21/2017 01:23 AM, Manu Gautam wrote:
> New revision (v3) of QMP PHY uses different offsets
> for almost all of the registers. Hence, move these
> definitions to header file so that updated offsets
> can be added for QMP v3.
>
> Signed-off-by: Manu Gautam 
> ---

Why? It would only be included into one C file so it's really not necessary.

-- 
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

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > There is no need to #define the license of the driver, just put it in
> > the MODULE_LICENSE() line directly as a text string.
> >
> > This allows tools that check that the module license matches the source
> > code license to work properly, as there is no need to unwind the
> > unneeded dereference.
> []
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> []
> > @@ -76,7 +76,6 @@
> >
> >  #define MOD_AUTHOR "Option Wireless"
> >  #define MOD_DESCRIPTION"USB High Speed Option driver"
> > -#define MOD_LICENSE"GPL"
> >
> >  #define HSO_MAX_NET_DEVICES10
> >  #define HSO__MAX_MTU   2048
> > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> >
> >  MODULE_AUTHOR(MOD_AUTHOR);
> >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > -MODULE_LICENSE(MOD_LICENSE);
> > +MODULE_LICENSE("GPL");
>
> Probably all of these MODULE_(MOD_) uses could be
> simplified as well.
>
> Perhaps there's utility in a (cocci?) script that looks for
> used-once
> macro #defines in various types of macros.

It could be possible.  It's a bit tricky due to ifdefs that Coccinelle
doesn't see and header files, but perhaps in special cases like this there
is not much worry.

julia

>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1511370336.6989.100.camel%40perches.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
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: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Joe Perches]

On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> There is no need to #define the license of the driver, just put it in
> the MODULE_LICENSE() line directly as a text string.
> 
> This allows tools that check that the module license matches the source
> code license to work properly, as there is no need to unwind the
> unneeded dereference.
[]
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
[]
> @@ -76,7 +76,6 @@
>  
>  #define MOD_AUTHOR   "Option Wireless"
>  #define MOD_DESCRIPTION  "USB High Speed Option driver"
> -#define MOD_LICENSE  "GPL"
>  
>  #define HSO_MAX_NET_DEVICES  10
>  #define HSO__MAX_MTU 2048
> @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
>  
>  MODULE_AUTHOR(MOD_AUTHOR);
>  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> -MODULE_LICENSE(MOD_LICENSE);
> +MODULE_LICENSE("GPL");

Probably all of these MODULE_(MOD_) uses could be
simplified as well.

Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

--
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: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Greg Kroah-Hartman]

On Wed, Nov 22, 2017 at 09:05:36AM -0800, Joe Perches wrote:
> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > There is no need to #define the license of the driver, just put it in
> > the MODULE_LICENSE() line directly as a text string.
> > 
> > This allows tools that check that the module license matches the source
> > code license to work properly, as there is no need to unwind the
> > unneeded dereference.
> []
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> []
> > @@ -76,7 +76,6 @@
> >  
> >  #define MOD_AUTHOR "Option Wireless"
> >  #define MOD_DESCRIPTION"USB High Speed Option driver"
> > -#define MOD_LICENSE"GPL"
> >  
> >  #define HSO_MAX_NET_DEVICES10
> >  #define HSO__MAX_MTU   2048
> > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> >  
> >  MODULE_AUTHOR(MOD_AUTHOR);
> >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > -MODULE_LICENSE(MOD_LICENSE);
> > +MODULE_LICENSE("GPL");
> 
> Probably all of these MODULE_(MOD_) uses could be
> simplified as well.

Agreed, I did that for a bunch of USB drivers, need to do it for others
as well.

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] usb: bdc: fix platform_no_drv_owner.cocci warnings

[Vasyl Gomonovych]

Remove .owner field if calls are used which set it automatically

drivers/usb/gadget/udc/bdc/bdc_core.c:645:3-8: No need to set .owner here. The 
core will do it.
Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

Signed-off-by: Vasyl Gomonovych 
---
 drivers/usb/gadget/udc/bdc/bdc_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c 
b/drivers/usb/gadget/udc/bdc/bdc_core.c
index d39f070acbd7..01b44e159623 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -642,7 +642,6 @@ static SIMPLE_DEV_PM_OPS(bdc_pm_ops, bdc_suspend,
 static struct platform_driver bdc_driver = {
.driver = {
.name   = BRCM_BDC_NAME,
-   .owner  = THIS_MODULE,
.pm = _pm_ops,
.of_match_table = bdc_of_match,
},
-- 
1.9.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

Re: [PATCH 1/2] usb: xhci: add relaxed timing quirk bit

[Mathias Nyman]

On 22.11.2017 02:07, Adam Wallis wrote:

On 11/21/2017 3:06 PM, Rob Herring wrote:

[..]


I like where you are going with this. Are you saying that I could read for a
device property read from firmware (for DTB or ACPI) like DWC3 does for
"snps,hird-threshold"?


Is that for the same thing? If so, drop the vendor prefix and use
that. Otherwise, a separate property should really be something that
is per board rather than per SoC.


I don't think that's exactly the same property, but it's the same idea I would
prefer to go with. That way, an integer can be passed in via the firmware 
tables.




If you mean this, where do you recommend I store the
desired IRQ_CONTROL value - in struct xhci_hcd ?


No idea.


Or by "compatible" strings, did you mean storing hard-coded values in the
of_device_id usb_xhci_of_match[] array? This would still be hard-coding (which I
would like to avoid) and also would not work for the ACPI case.


ACPI has match tables too?



Yes, you can use DSD in a way that is similar to OF properties


It would only be hardcoded per compatible which should be per SoC. Do
you need per board/device tuning? If so, use a property.



The reason why I think it should dynamic via firmware tables is that

* It's much less invasive for vendors to update their DT tables if they need to
adjust on a per device/controller/family/etc basis then to adjust a properties
table in xhci-plat
* This would lead to less polluting in xhci-plat code


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



I will provide an updated proposed patch sometime this week. I also hope to get
some feedback from Mathias to see what he prefers.


We know have at least two hosts/platforms that need custom interrupt moderation 
values

How about adding a u32 device property for xhci with the interrupt moderation 
interval in
nanoseconds?  And also add a u32 imod_interval variable to struct xhci_hcd?
 
imod_interval can be set to the current default 4ns (160*250ns) and overwritten if

device_property_read_u32() returns something else.

XHCI_MTK_HOST could then use whatever preferred device propery interval value,
and we can get rid of using XHCI_MTK_HOST quirk flag when setting up the IMODI
 
-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

[PATCH] USB: gadget: legacy: remove redundant zero assignment to variable 'value'

[Colin King]

From: Colin Ian King 

The variable value is being assigned to zero but that value is never being
read.  Either value is being reassigned in the following if condition, or
it is never read and the function returns. In both cases the assignment is
redundant and can be removed. Cleans up clang warning:

drivers/usb/gadget/legacy/inode.c:1473:4: warning: Value stored to 'value'
is never read

Signed-off-by: Colin Ian King 
---
 drivers/usb/gadget/legacy/inode.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 9343ec436485..cb8e1761d405 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1470,7 +1470,6 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
dev->setup_wLength = w_length;
dev->setup_out_ready = 0;
dev->setup_out_error = 0;
-   value = 0;
 
/* read DATA stage for OUT right away */
if (unlikely (!dev->setup_in && w_length)) {
-- 
2.14.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

RE: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201

[Chris Welch]

> -Original Message-
> From: Vignesh R [mailto:vigne...@ti.com]
> Sent: Wednesday, November 22, 2017 5:41 AM
> To: Chris Welch ; Quadros, Roger
> 
> Cc: linux-usb@vger.kernel.org; linux-...@vger.kernel.org; Joao Pinto
> ; KISHON VIJAY ABRAHAM 
> Subject: Re: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201
> 
> 
> 
> On Tuesday 21 November 2017 08:20 PM, Chris Welch wrote:
> >
> >
> >> -Original Message-
> >> From: Vignesh R [mailto:vigne...@ti.com]
> >> Sent: Tuesday, November 21, 2017 12:48 AM
> >> To: Roger Quadros 
> >> Cc: Chris Welch ;
> >> linux-usb@vger.kernel.org; linux-...@vger.kernel.org; Joao Pinto
> >> ; KISHON VIJAY ABRAHAM 
> >> Subject: Re: xhci_hcd HC died; cleaning up with TUSB7340 and
> >> µPD720201
> >>
> >>
> >>
> >> On Monday 20 November 2017 07:01 PM, Roger Quadros wrote:
> >>> On 20/11/17 15:19, Vignesh R wrote:
> 
> 
>  On Monday 20 November 2017 01:31 PM, Roger Quadros wrote:
>  [...]
> >>
> >> So, could you try reverting commit 8c934095fa2f3 and also apply
> >> below patch and let me know if that fixes the issue?
> >>
> >> ---
> >>
> >> diff --git a/drivers/pci/dwc/pci-dra7xx.c
> >> b/drivers/pci/dwc/pci-dra7xx.c index e77a4ceed74c..8280abc56f30
> >> 100644
> >> --- a/drivers/pci/dwc/pci-dra7xx.c
> >> +++ b/drivers/pci/dwc/pci-dra7xx.c
> >> @@ -259,10 +259,17 @@ static irqreturn_t
> >> dra7xx_pcie_msi_irq_handler(int irq, void *arg)
> >> u32 reg;
> >>
> >> reg = dra7xx_pcie_readl(dra7xx,
> >> PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
> >> +   dra7xx_pcie_writel(dra7xx,
> >> + PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
> >>
> >> switch (reg) {
> >> case MSI:
> >> -   dw_handle_msi_irq(pp);
> >> +   /*
> >> +* Need to make sure no MSI IRQs are pending before
> >> +* exiting handler, else the wrapper will not catch new
> >> +* IRQs. So loop around till dw_handle_msi_irq() 
> >> returns
> >> +* IRQ_NONE
> >> +*/
> >> +   while (dw_handle_msi_irq(pp) != IRQ_NONE);
> >
> > The patch looks good, I haven't had a failure in a few days of testing.
> >
> > You should also look at incorporating the following that I needed to change 
> > to
> get our product working.  The first change fixes a miss by one error with the
> interrupt lines.
> >
> > The second change extends a patch you developed for errata i870 but we
> found is applicable to RC operation as well as EPs.  Thanks very much for your
> help!
> 
> BTW, do you have a test case which fails w/o errata i870 workaround?

Yes, we have a PEX8606 PCI switch attached to the RC which fails to probe if 
the i870 workaround is not used.

The code picks up the wrong class for the switch if the work around is not in 
place.

> 
> 
> --
> Regards
> Vignesh
N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7

[Stefan Wahren]

Hi Minas,

> Stefan Wahren  hat am 22. November 2017 um 12:21 
> geschrieben:
> 
> 
> Hi Minas,
> 
> > Minas Harutyunyan  hat am 21. November 2017 
> > um 13:02 geschrieben:
> >
> > Hi Stefan,
> > 
> > We have prepared patch for this issue in July-August'17.
> > Find attached 2 patch files. Please apply patches and test. If issue 
> > gone, we will send these patches to LKML by regular flow.
> 
> thanks, but the first patch doesn't apply. My version see below, i hope i 
> didn't break anything.
> 
> Unfortunately after applying both patches the issue still persists (EP 1-7 
> fifo size to 512) and the EP 1-7 TX total size increases from 3776 to 3792. I 
> will follow my u-boot theory  ...

okay there is no influence of u-boot. If i boot directly from the RPi 
Bootloader it shows the same behavior.

I did the following test cases with enabled debug:
1. boot RPI Zero (OTG configured per DT) without anything connected to the OTG 
port
2. boot RPI Zero (OTG configured per DT) only a OTG cable connected to the OTG 
port

test case 1 (nothing connected, no issue):
dwc2 2098.usb: mapped PA 2098 to VA dc85
dwc2 2098.usb: registering common handler for irq33
dwc2 2098.usb: Core Release: 2.80a (snpsid=4f54280a)
dwc2 2098.usb: Forcing mode to host
dwc2 2098.usb: NonPeriodic TXFIFO size: 32
dwc2 2098.usb: RXFIFO size: 256
dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
dwc2 2098.usb: DCFG=0x0020, DCTL=0x, DIEPMSK=
dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x00201000
dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x1020
dwc2 2098.usb: DPTx[2] FSize=512, StAddr=0x1220
dwc2 2098.usb: DPTx[3] FSize=512, StAddr=0x1420
dwc2 2098.usb: DPTx[4] FSize=512, StAddr=0x1620
dwc2 2098.usb: DPTx[5] FSize=512, StAddr=0x1820
dwc2 2098.usb: DPTx[6] FSize=768, StAddr=0x1a20
dwc2 2098.usb: DPTx[7] FSize=768, StAddr=0x1d20
...
dwc2 2098.usb: Core Global Registers
dwc2 2098.usb: GOTGCTL@0xDC85 : 0x000D
dwc2 2098.usb: GOTGINT@0xDC850004 : 0x
dwc2 2098.usb: GAHBCFG@0xDC850008 : 0x0030
dwc2 2098.usb: GUSBCFG@0xDC85000C : 0x1700
dwc2 2098.usb: GRSTCTL@0xDC850010 : 0x8000
dwc2 2098.usb: GINTSTS@0xDC850014 : 0x04008C22
dwc2 2098.usb: GINTMSK@0xDC850018 : 0xD806
dwc2 2098.usb: GRXSTSR@0xDC85001C : 0xFADEA4E0
dwc2 2098.usb: GRXFSIZ@0xDC850024 : 0x1000
dwc2 2098.usb: GNPTXFSIZ  @0xDC850028 : 0x00201000
dwc2 2098.usb: GNPTXSTS   @0xDC85002C : 0x00080020
dwc2 2098.usb: GI2CCTL@0xDC850030 : 0x
dwc2 2098.usb: GPVNDCTL   @0xDC850034 : 0x
dwc2 2098.usb: GGPIO  @0xDC850038 : 0x
dwc2 2098.usb: GUID   @0xDC85003C : 0x2708A000
dwc2 2098.usb: GSNPSID@0xDC850040 : 0x4F54280A
dwc2 2098.usb: GHWCFG1@0xDC850044 : 0x
dwc2 2098.usb: GHWCFG2@0xDC850048 : 0x228DDD50
dwc2 2098.usb: GHWCFG3@0xDC85004C : 0x0FF000E8
dwc2 2098.usb: GHWCFG4@0xDC850050 : 0x1FF00020
dwc2 2098.usb: GLPMCFG@0xDC850054 : 0x75736230
dwc2 2098.usb: GPWRDN @0xDC850058 : 0x
dwc2 2098.usb: GDFIFOCFG  @0xDC85005C : 0x
dwc2 2098.usb: HPTXFSIZ   @0xDC850100 : 0x
dwc2 2098.usb: PCGCTL @0xDC850E00 : 0x
...
dwc2 2098.usb: gintsts=04008c22  gintmsk=d806
dwc2 2098.usb: Mode Mismatch Interrupt: currently in Device mode
dwc2 2098.usb: USB SUSPEND
dwc2 2098.usb: DSTS=0x3
dwc2 2098.usb: DSTS.Suspend Status=1 HWCFG4.Power Optimize=0
dwc2 2098.usb: dwc2_hsotg_irq: 04008420  (d806) retry 8

test case 2 (OTG cable connected, issue):
dwc2 2098.usb: mapped PA 2098 to VA dc85
dwc2 2098.usb: registering common handler for irq33
dwc2 2098.usb: Core Release: 2.80a (snpsid=4f54280a)
dwc2 2098.usb: Forcing mode to device
dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter 
g_tx_fifo_size[6]=768
dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter 
g_tx_fifo_size[7]=768
dwc2 2098.usb: NonPeriodic TXFIFO size: 32
dwc2 2098.usb: RXFIFO size: 256
dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
dwc2 2098.usb: DCFG=0x, DCTL=0x, DIEPMSK=
dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x01001000
dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x2000
dwc2 2098.usb: DPTx[2] FSize=512, StAddr=0x2000
dwc2 2098.usb: DPTx[3] FSize=512, StAddr=0x2000
dwc2 2098.usb: DPTx[4] FSize=512, StAddr=0x2000
dwc2 2098.usb: DPTx[5] FSize=512, StAddr=0x2000
dwc2 2098.usb: DPTx[6] FSize=512, 

Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7

[Stefan Wahren]

Hi Minas,

> Minas Harutyunyan  hat am 21. November 2017 
> um 13:02 geschrieben:
>
> Hi Stefan,
> 
> We have prepared patch for this issue in July-August'17.
> Find attached 2 patch files. Please apply patches and test. If issue 
> gone, we will send these patches to LKML by regular flow.

thanks, but the first patch doesn't apply. My version see below, i hope i 
didn't break anything.

Unfortunately after applying both patches the issue still persists (EP 1-7 fifo 
size to 512) and the EP 1-7 TX total size increases from 3776 to 3792. I will 
follow my u-boot theory  ...

>From 12fcc090bc7588275c1d942009676cb3fa5129f2 Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan 
Date: Wed, 22 Nov 2017 11:15:16 +0100
Subject: [PATCH] usb: dwc2: Fix TxFIFO setup issue

In host mode reading from DPTXSIZn returning invalid value(0) in
dwc2_check_param_tx_fifo_sizes function.

Added g_tx_fifo_size array in dwc2_hw_params structure in which stored
power on reset valus of DPTXSIZn registers in device mode (forced to
device).

Updated dwc2_get_hwparams function to write DPTXFSIZn to array.

Modyfied dwc2_check_param_tx_fifo_sizes function accordingly.

Change-Id: I61d3db753b1bc06f0f2caf40df350a09655f18fd
Signed-off-by: Gevorg Sahakyan 
Signed-off-by: Stefan Wahren 
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/params.c | 11 +--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 8367d4f..47e9092 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -587,6 +587,7 @@ struct dwc2_hw_params {
unsigned utmi_phy_data_width:2;
u32 snpsid;
u32 dev_ep_dirs;
+   u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 };
 
 /* Size of control and EP0 buffers */
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a3ffe97..04f1868 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -469,8 +469,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct 
dwc2_hsotg *hsotg)
}
 
for (fifo = 1; fifo <= fifo_count; fifo++) {
-   dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) &
-   FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+   dptxfszn = hsotg->hw_params.g_tx_fifo_size[fifo];
 
if (hsotg->params.g_tx_fifo_size[fifo] < min ||
hsotg->params.g_tx_fifo_size[fifo] >  dptxfszn) {
@@ -617,6 +616,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 {
struct dwc2_hw_params *hw = >hw_params;
unsigned int width;
+   int fifo, fifo_count;
u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
u32 grxfsiz;
 
@@ -705,6 +705,13 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
GRXFSIZ_DEPTH_SHIFT;
 
+   fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
+
+   for (fifo = 1; fifo <= fifo_count; fifo++) {
+   u32 val = dwc2_readl(hsotg->regs + DPTXFSIZN(fifo));
+   hw->g_tx_fifo_size[fifo] = (val & FIFOSIZE_DEPTH_MASK) >>
+  FIFOSIZE_DEPTH_SHIFT;
+   }
return 0;
 }
 
-- 
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: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201

[Vignesh R]

On Tuesday 21 November 2017 08:20 PM, Chris Welch wrote:
> 
> 
>> -Original Message-
>> From: Vignesh R [mailto:vigne...@ti.com]
>> Sent: Tuesday, November 21, 2017 12:48 AM
>> To: Roger Quadros 
>> Cc: Chris Welch ; linux-usb@vger.kernel.org;
>> linux-...@vger.kernel.org; Joao Pinto ; KISHON VIJAY
>> ABRAHAM 
>> Subject: Re: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201
>>
>>
>>
>> On Monday 20 November 2017 07:01 PM, Roger Quadros wrote:
>>> On 20/11/17 15:19, Vignesh R wrote:


 On Monday 20 November 2017 01:31 PM, Roger Quadros wrote:
 [...]
>>
>> So, could you try reverting commit 8c934095fa2f3 and also apply
>> below patch and let me know if that fixes the issue?
>>
>> ---
>>
>> diff --git a/drivers/pci/dwc/pci-dra7xx.c
>> b/drivers/pci/dwc/pci-dra7xx.c index e77a4ceed74c..8280abc56f30
>> 100644
>> --- a/drivers/pci/dwc/pci-dra7xx.c
>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>> @@ -259,10 +259,17 @@ static irqreturn_t
>> dra7xx_pcie_msi_irq_handler(int irq, void *arg)
>> u32 reg;
>>
>> reg = dra7xx_pcie_readl(dra7xx,
>> PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
>> +   dra7xx_pcie_writel(dra7xx,
>> + PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
>>
>> switch (reg) {
>> case MSI:
>> -   dw_handle_msi_irq(pp);
>> +   /*
>> +* Need to make sure no MSI IRQs are pending before
>> +* exiting handler, else the wrapper will not catch new
>> +* IRQs. So loop around till dw_handle_msi_irq() returns
>> +* IRQ_NONE
>> +*/
>> +   while (dw_handle_msi_irq(pp) != IRQ_NONE);
> 
> The patch looks good, I haven't had a failure in a few days of testing.  
> 
> You should also look at incorporating the following that I needed to change 
> to get our product working.  The first change fixes a miss by one error with 
> the interrupt lines.  
> 
> The second change extends a patch you developed for errata i870 but we found 
> is applicable to RC operation as well as EPs.  Thanks very much for your help!

BTW, do you have a test case which fails w/o errata i870 workaround?


-- 
Regards
Vignesh
--
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 v7] xhci : AMD Promontory USB disable port support

[Greg KH]

On Wed, Nov 22, 2017 at 05:45:14PM +0800, Joe Lee wrote:
> For AMD Promontory xHCI host,
> although you can disable USB 2.0 ports in BIOSsettings,
> 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: Joe Lee 

Minor coding style nits below.  Did you run the patch through
scripts/checkpatch.pl?

> ---
> v7: add a empty function for the case 
> when CONFIG_USB_PCI is not defined in pci-quirks.h
> v6: Fix coding format.
> v5: Add check disable port setting before set PORT_WAKE_BITS
> v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
> v3: Fix some checkpatch.pl
> ---
>  drivers/usb/host/pci-quirks.c | 118 
> ++
>  drivers/usb/host/pci-quirks.h |   2 +
>  drivers/usb/host/xhci-hub.c   |   6 +++
>  drivers/usb/host/xhci-pci.c   |  12 +
>  drivers/usb/host/xhci.h   |   2 +-
>  5 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 6dda362..6909d05 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -65,6 +65,23 @@
>  #define  AX_INDXC0x30
>  #define  AX_DATAC0x34
>  
> +#define PT_ADDR_INDEX0xE8
> +#define PT_READ_INDEX0xE4
> +#define PT_SIG_1_ADDR0xA520
> +#define PT_SIG_2_ADDR0xA521
> +#define PT_SIG_3_ADDR0xA522
> +#define PT_SIG_4_ADDR0xA523
> +#define PT_SIG_1_DATA0x78
> +#define PT_SIG_2_DATA0x56
> +#define PT_SIG_3_DATA0x34
> +#define PT_SIG_4_DATA0x12
> +#define PT_4_PORT_1_REG  0xB521
> +#define PT_4_PORT_2_REG  0xB522
> +#define PT_2_PORT_1_REG  0xD520
> +#define PT_2_PORT_2_REG  0xD521
> +#define PT_1_PORT_1_REG  0xD522
> +#define PT_1_PORT_2_REG  0xD523
> +
>  #define  NB_PCIE_INDX_ADDR   0xe0
>  #define  NB_PCIE_INDX_DATA   0xe4
>  #define  PCIE_P_CNTL 0x10040
> @@ -511,6 +528,107 @@ void usb_amd_dev_put(void)
>  }
>  EXPORT_SYMBOL_GPL(usb_amd_dev_put);
>  
> +int usb_amd_pt_check_port(struct device *device, int port)

Should this function return a bool?

> +{
> + unsigned char value;
> + struct pci_dev *pdev;
> +
> + pdev = to_pci_dev(device);
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value != PT_SIG_1_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value != PT_SIG_2_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value != PT_SIG_3_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value != PT_SIG_4_DATA)
> + return 0;
> + if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
> + /* device is AMD_PROMONTORYA_4(0x43b9) or
> +  * PROMONTORYA_3(0x43ba)
> +  * disable port setting PT_4_PORT_1_REG[7..1] is
> +  * USB2.0 port6 to 0
> +  * PT_4_PORT_2_REG[6..0] is USB 13 to port 7
> +  * 0 : disable ;1 : enable
> +  */

Odd comment style, please match the other multi-line comment style that
is in this file.

> + if (port > 6) {
> + pci_write_config_word(pdev, PT_ADDR_INDEX,
> + PT_4_PORT_2_REG);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value & (1<<(port - 7)))
> + return 0;
> + else
> + return 1;
> + } else {
> + pci_write_config_word(pdev, PT_ADDR_INDEX,
> + PT_4_PORT_1_REG);
> + pci_read_config_byte(pdev, PT_READ_INDEX, );
> + if (value & (1<<(port + 1)))
> + return 0;
> + else
> + return 1;
> + }
> + } else if (pdev->device == 0x43bb) {
> + /* device is AMD_PROMONTORYA_2(0x43bb)
> +  * disable port setting PT_2_PORT_1_REG[7..5] is
> +  * USB2.0 port2 to
> +  * PT_2_PORT_2_REG[5..0] is USB9 to port3
> +  * 0 : disable ;1 : enable
> +  */
> + if (port > 2) {
> + 

[PATCH v7] xhci : AMD Promontory USB disable port support

[Joe Lee]

For AMD Promontory xHCI host,
although you can disable USB 2.0 ports in BIOSsettings,
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: Joe Lee 
---
v7: add a empty function for the case 
when CONFIG_USB_PCI is not defined in pci-quirks.h
v6: Fix coding format.
v5: Add check disable port setting before set PORT_WAKE_BITS
v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
v3: Fix some checkpatch.pl
---
 drivers/usb/host/pci-quirks.c | 118 ++
 drivers/usb/host/pci-quirks.h |   2 +
 drivers/usb/host/xhci-hub.c   |   6 +++
 drivers/usb/host/xhci-pci.c   |  12 +
 drivers/usb/host/xhci.h   |   2 +-
 5 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 6dda362..6909d05 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -65,6 +65,23 @@
 #defineAX_INDXC0x30
 #defineAX_DATAC0x34
 
+#define PT_ADDR_INDEX  0xE8
+#define PT_READ_INDEX  0xE4
+#define PT_SIG_1_ADDR  0xA520
+#define PT_SIG_2_ADDR  0xA521
+#define PT_SIG_3_ADDR  0xA522
+#define PT_SIG_4_ADDR  0xA523
+#define PT_SIG_1_DATA  0x78
+#define PT_SIG_2_DATA  0x56
+#define PT_SIG_3_DATA  0x34
+#define PT_SIG_4_DATA  0x12
+#define PT_4_PORT_1_REG0xB521
+#define PT_4_PORT_2_REG0xB522
+#define PT_2_PORT_1_REG0xD520
+#define PT_2_PORT_2_REG0xD521
+#define PT_1_PORT_1_REG0xD522
+#define PT_1_PORT_2_REG0xD523
+
 #defineNB_PCIE_INDX_ADDR   0xe0
 #defineNB_PCIE_INDX_DATA   0xe4
 #definePCIE_P_CNTL 0x10040
@@ -511,6 +528,107 @@ void usb_amd_dev_put(void)
 }
 EXPORT_SYMBOL_GPL(usb_amd_dev_put);
 
+int usb_amd_pt_check_port(struct device *device, int port)
+{
+   unsigned char value;
+   struct pci_dev *pdev;
+
+   pdev = to_pci_dev(device);
+   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value != PT_SIG_1_DATA)
+   return 0;
+   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value != PT_SIG_2_DATA)
+   return 0;
+   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value != PT_SIG_3_DATA)
+   return 0;
+   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value != PT_SIG_4_DATA)
+   return 0;
+   if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
+   /* device is AMD_PROMONTORYA_4(0x43b9) or
+* PROMONTORYA_3(0x43ba)
+* disable port setting PT_4_PORT_1_REG[7..1] is
+* USB2.0 port6 to 0
+* PT_4_PORT_2_REG[6..0] is USB 13 to port 7
+* 0 : disable ;1 : enable
+*/
+   if (port > 6) {
+   pci_write_config_word(pdev, PT_ADDR_INDEX,
+   PT_4_PORT_2_REG);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value & (1<<(port - 7)))
+   return 0;
+   else
+   return 1;
+   } else {
+   pci_write_config_word(pdev, PT_ADDR_INDEX,
+   PT_4_PORT_1_REG);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value & (1<<(port + 1)))
+   return 0;
+   else
+   return 1;
+   }
+   } else if (pdev->device == 0x43bb) {
+   /* device is AMD_PROMONTORYA_2(0x43bb)
+* disable port setting PT_2_PORT_1_REG[7..5] is
+* USB2.0 port2 to
+* PT_2_PORT_2_REG[5..0] is USB9 to port3
+* 0 : disable ;1 : enable
+*/
+   if (port > 2) {
+   pci_write_config_word(pdev, PT_ADDR_INDEX,
+   PT_2_PORT_2_REG);
+   pci_read_config_byte(pdev, PT_READ_INDEX, );
+   if (value & (1<<(port - 3)))
+   return 0;
+   else
+   return 1;
+   } else {
+   pci_write_config_word(pdev, PT_ADDR_INDEX,
+

Re: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201

[Vignesh R]

On Tuesday 21 November 2017 08:20 PM, Chris Welch wrote:
> 
> 
>> -Original Message-
>> From: Vignesh R [mailto:vigne...@ti.com]
>> Sent: Tuesday, November 21, 2017 12:48 AM
>> To: Roger Quadros 
>> Cc: Chris Welch ; linux-usb@vger.kernel.org;
>> linux-...@vger.kernel.org; Joao Pinto ; KISHON VIJAY
>> ABRAHAM 
>> Subject: Re: xhci_hcd HC died; cleaning up with TUSB7340 and µPD720201
>>
>>
>>
>> On Monday 20 November 2017 07:01 PM, Roger Quadros wrote:
>>> On 20/11/17 15:19, Vignesh R wrote:


 On Monday 20 November 2017 01:31 PM, Roger Quadros wrote:
 [...]
>>
>> So, could you try reverting commit 8c934095fa2f3 and also apply
>> below patch and let me know if that fixes the issue?
>>
>> ---
>>
>> diff --git a/drivers/pci/dwc/pci-dra7xx.c
>> b/drivers/pci/dwc/pci-dra7xx.c index e77a4ceed74c..8280abc56f30
>> 100644
>> --- a/drivers/pci/dwc/pci-dra7xx.c
>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>> @@ -259,10 +259,17 @@ static irqreturn_t
>> dra7xx_pcie_msi_irq_handler(int irq, void *arg)
>> u32 reg;
>>
>> reg = dra7xx_pcie_readl(dra7xx,
>> PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
>> +   dra7xx_pcie_writel(dra7xx,
>> + PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
>>
>> switch (reg) {
>> case MSI:
>> -   dw_handle_msi_irq(pp);
>> +   /*
>> +* Need to make sure no MSI IRQs are pending before
>> +* exiting handler, else the wrapper will not catch new
>> +* IRQs. So loop around till dw_handle_msi_irq() returns
>> +* IRQ_NONE
>> +*/
>> +   while (dw_handle_msi_irq(pp) != IRQ_NONE);
> 
> The patch looks good, I haven't had a failure in a few days of testing.  

Thanks for testing! I will submit patches formally with some minor tweaks.

> 
> You should also look at incorporating the following that I needed to change 
> to get our product working.  The first change fixes a miss by one error with 
> the interrupt lines.  

Yeah, this is a known issue and has been discussed[1] before. We are
exploring better solution

> 
> The second change extends a patch you developed for errata i870 but we found 
> is applicable to RC operation as well as EPs.  Thanks very much for your help!

Thanks for pointing that out, will submit a patch for errata workaround.

[1] https://lkml.org/lkml/2016/9/14/241

Regards
Vignesh
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> old mode 100644
> new mode 100755
> index defa272..6245d89
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -238,8 +238,8 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port 
> *pp)
> dev_err(dev, "No PCIe Intc node found\n");
> return -ENODEV;
> }
> -
> -   dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
> +// PCI interrupt lines start at 1 not zero so need to add 1
> +   dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, 4 + 1,
>_domain_ops, pp);
> if (!dra7xx->irq_domain) {
> dev_err(dev, "Failed to get a INTx IRQ domain\n");
> @@ -706,10 +706,16 @@ static int __init dra7xx_pcie_probe(struct 
> platform_device *pdev)
> dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE,
>DEVICE_TYPE_RC);
>  
> +   // Errata i870 applies to RC as well as EP
> +   ret = dra7xx_pcie_ep_legacy_mode(dev);
> +   if (ret)
> +   goto err_gpio;
> +
> ret = dra7xx_add_pcie_port(dra7xx, pdev);
> if (ret < 0)
> goto err_gpio;
> break;
> 
> 
>
> To avoid this kind of looping, shouldn't we be disabling all IRQ
> events while the interrupt handler is running and enable them just
> before we return from the hardirq handler?

 IIUC, you are saying to disable all MSIs at PCIe designware core
 level, then call dw_handle_msi_irq() and then enable MSIs after
 hardirq returns. But, the problem is if PCIe EP raises another MSI
 after the call to EP's handler but before re-enabling MSIs, then it
 will be ignored as IRQs are not yet enabled.
 Ideally, EP's support Per Vector Masking(PVM) which allow RC to
 prevent EP from sending MSI messages for sometime. But,
 unfortunately, the cards mentioned here don't support this feature.
>>>
>>> I'm not aware of MSIs.
>>>
>>> But for any typical hardware, there should be an interrupt event
>>> enable register and an interrupt mask register.
>>>
>>> In the IRQ handler, we mask the interrupt but still keep the interrupt
>>> events enabled so that 

Re: usbip port number limits

[Juan Zea]

Hi Yuyang,

> 
> - Mensaje original -
> De: "Yuyang Du" 
> Para: "Juan Zea" 
> CC: "Shuah Khan" , sh...@kernel.org, "Bjørn Mork" 
> , linux-usb@vger.kernel.org, "Valentina Manea" 
> 
> Enviados: Miércoles, 22 de Noviembre 2017 8:54:03
> Asunto: Re: usbip port number limits
> 
> Hi Juan,
> 
> On Tue, Nov 21, 2017 at 12:24:01PM +0100, Juan Zea wrote:
> >   Please excuse me... I have reported two different problems in this post, 
> > and I think things are mixing up a bit. My bad. I've found two different 
> > problems:
> > 
> > 1.- When compiling vhci driver with multiple hubs (in latest kernel), usbip 
> > tool doesn't seem able to "jump" to the second controller. As far as I 
> > understand, it seems to be a problem with high speed devices trying to get 
> > connected to super speed ports. I made a patch for avoiding this situation 
> > which is almost the same as the one you just sent. Both patches work. I can 
> > compile with several hubs of one port, and connect two devices via usbip 
> > and the second gets connected to the second controller (skipping the super 
> > speed port of first controller).
> > 
> > 2.- The problem is some of these devices don't work, whatsoever controller 
> > or hub compiling configuration I do. A fingerprint reader and also recently 
> > discovered flash usb sticks don't work either. You can connect them with 
> > usbip tool, and they list with lsusb with no problem at all. But when you 
> > try to use them, you get this "kernel BUG at 
> > drivers/usb/usbip/vhci_hcd.c:669!" (by the way, the line number has changed 
> > in the latest pull from master I've just tested, but it's the same line of 
> > code). This second problem is the one I bisected in my last message. And I 
> > got to the commit 03cd00d538a6feb0492cd153edf256ef7d7bd95e, in which both 
> > devices stop working. 
>  
> It'd be good if you can give greater detail of the exact (as much
> info as possible) setting of the fingerprint reader case.
> 

The bug launched in my repository reads:
 669 BUG_ON(!urb->transfer_buffer && urb->transfer_buffer_length);


For my tests, I'm using a kvm machine, with the kernel I want to test. For 
yesterday's test, I used latest commit: 
e1d1ea549b57790a3d8cf6300e6ef86118d692a3. I build the kernel with this line:
make x86_64_defconfig; make kvmconfig; cat ../extra.txt >> .config; make -j8
These are the contents of extra.txt:
CONFIG_USBIP_CORE=m
CONFIG_USBIP_VHCI_HCD=m
CONFIG_USBIP_VHCI_HC_PORTS=1
CONFIG_USBIP_VHCI_NR_HCS=4

With this, I get a kvm machine running, in which I load the vhci driver on 
startup. This driver creates this devices:
root@kernel-tester:~# lsusb
Bus 008 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 007 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 006 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 005 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

I compile the usbip tools from within this machine on startup too (from the 
same repository as kernel).


The server is my own laptop, running this kernel: 4.11.0-14-generic, also host 
to the kvm machine. Network for kvm is a bridge.

In my laptop, the fingerprint reader lists like this:
Bus 002 Device 004: ID 05ba:000a DigitalPersona, Inc. Fingerprint Reader

And this is dmesg when connecting it to my laptop:
[ 3941.459772] usb 2-1.3: new full-speed USB device number 4 using ehci-pci
[ 3941.575718] usb 2-1.3: New USB device found, idVendor=05ba, idProduct=000a
[ 3941.575722] usb 2-1.3: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[ 3941.575725] usb 2-1.3: Product: Biokey 200 Fingerprint Scanner
[ 3941.575728] usb 2-1.3: Manufacturer: ZKSoftware Inc.
[ 3941.575730] usb 2-1.3: SerialNumber: {----}

Reproduce steps:

1.- Having my laptop with usbip_host module loaded, and 'usbipd -d' running in 
terminal, bind the reader:
# usbip -d bind -b 2-1.3
usbip: debug: usbip.c:141:[run_command] running command: `bind'
usbip: info: bind device on busid 2-1.3: complete

dmesg reads: 
[ 4092.811059] usbip-host 2-1.3: usbip-host: register new device (bus 2 dev 4)

2.- From kvm machine, with vhci_hcd module loaded, and usbip tools compiled and 
installed:
# usbip -d attach -r 172.26.8.66 -b 2-1.3
usbip: debug: usbip.c:141:[run_command] running command: `attach'
libusbip: debug: vhci_driver.c:264:[usbip_vhci_driver_open] available ports: 8
libusbip: debug: vhci_driver.c:275:[usbip_vhci_driver_open] available 
controllers: 4
libusbip: debug: vhci_driver.c:128:[refresh_imported_device_list] controller 0
libusbip: debug: vhci_driver.c:68:[parse_status] hub hs 

Re: usbip port number limits

[Yuyang Du]

On Tue, Nov 21, 2017 at 07:33:41AM -0700, Shuah Khan wrote:
> I have been debugging on 4.12 and this bad commit. Low speed devices can be
> attached, but you see the following and then shutdown hangs.

Oh, just tested a high speed USB stick, I got the message.

I'm working on it.
--
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