Re: [PATCH 2/2] phy: mapphone-mdm6600: Improve phy related runtime PM calls

2018-11-22 Thread Kishon Vijay Abraham I
Hi Tony,

On 17/11/18 7:07 PM, Tony Lindgren wrote:
> I noticed that phy_pm_runtime_get_sync() and phy_pm_runtime_put() are not
> currently doing anything for phy-mapphone-mdm6600, only the sysfs interface
> for works for "auto" and "on".
> 
> This is because of the shared GPIO pins between mdm6600 USB port and n_gsm
> port. We have not enabled runtime PM for the phy driver until after we've
> booted up mdm6600 properly to the USB mode. Otherwise phy_create() would
> have called pm_runtime_enable() and pm_runtime_no_callbacks() automatically
> on init.
> 
> Let's fix this by registering the phy a bit later after we've powered up the
> mdm6600 USB port.
> 
> And as the PM runtime support is only needed for the n_gsm mode and not for
> USB, we can allow the device to idle between phy_mdm6600_power_on() and
> phy_mdm6600_power_off(). Note that for suspend, runtime_pm is already
> disabled for the phy so we need to check for phy_pm_runtime_enabled().
> 
> Cc: Pavel Machek 
> Cc: Sebastian Reichel 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/phy/motorola/phy-mapphone-mdm6600.c | 71 +++--
>  1 file changed, 51 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c 
> b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define PHY_MDM6600_PHY_DELAY_MS 4000/* PHY enable 2.2s to 3.5s */
>  #define PHY_MDM6600_ENABLED_DELAY_MS 8000/* 8s more total for MDM6600 */
> @@ -124,12 +125,22 @@ static int phy_mdm6600_power_on(struct phy *x)
>  {
>   struct phy_mdm6600 *ddata = phy_get_drvdata(x);
>   struct gpio_desc *enable_gpio = ddata->ctrl_gpios[PHY_MDM6600_ENABLE];
> + int error;
>  
>   if (!ddata->enabled)
>   return -ENODEV;
>  
> + error = pinctrl_pm_select_default_state(ddata->dev);
> + if (error)
> + dev_warn(ddata->dev, "%s: error with default_state: %i\n",
> +  __func__, error);
> +
>   gpiod_set_value_cansleep(enable_gpio, 1);
>  
> + /* Allow aggressive PM for USB, it's only needed for n_gsm port */
> + if (phy_pm_runtime_enabled(ddata->generic_phy))
> + phy_pm_runtime_put(ddata->generic_phy);

phy_*() API's are generally added to be used by the consumer driver. I guess in
this case we can directly use pm_runtime_enabled(x).

Thanks
Kishon


RE: RESEND: About VBUS glitch happen on DWC3 host mode enabling process.

2018-11-22 Thread Ran Wang
Hello Felipe,

Felipe Balbi  wrote:
> Hi,
> 
> Ran Wang  writes:
> > Hello Felipe,
> >
> > We’ve found on some Layerscape platforms which integrating DWC3
> 
> which platform is that? Is it supported upstream? Which kernel version are
> you using?

We reproduced this issue on board LS1043ARDB with latest upstream code 
(git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, HEAD: 
651022382c7("Linux 4.20-rc1")).

> > USB3.0 DRD controller, VBUS glitch happened and caused some USB drives
> 
> what do you mean by glitch?

Please see below simple diagram:
   Here DWC3 enable host mode, VBUS on
+5V   /\   /---
0V  /  \/
 Here do xhci reset, 
VBUS drop for 15us, then back to +5V again

> > enumeration fail, like to discuss the details as below.
> >
> > Story is that, we found once function dwc3_core_init_mode() got called
> > and enabled host mode by calling dwc3_set_prtcap(dwc,
> > DWC3_GCTL_PRTCAP_HOST) which would write register [DWC3_GCTL] bit
> > 12~13, so the pin (such as USB_DRVVBUS) which control VBUS driving
> > chip’s EN pin would be pulled high immediately, so did the VBUS (up to
> > +5V).
> 
> this seems specific to your platform. Please clarify a little more? Do you 
> have
> a discrete charge pump tied to some output signal from dwc3?

Actually we have observed other Layerscape platforms (such as LS1012AFRWY,
LS1088ARDB-PB) have the same issue. They all integrate DWC3 IP as a USB3.0
controller (please see arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi as 
example,
search 'snps,dwc3'). About the discrete charge pump, we do have some for VBUS 
side,
but seems no help on this.

As to the more information of LS1043ARDB bard, please download it from 
https://www.nxp.com/webapp/Download?colCode=LS1043ARDBRM&Parent_nodeId=1441121579998719223109&Parent_pageType=product&Parent_nodeId=1441121579998719223109&Parent_pageType=product&Parent_nodeId=1441121579998719223109&Parent_pageType=product
  . 

In a simple word, the USB VBUS related part look like below(datasheet of VBUS
switch chip NX5P3090 can be downloaded at 
https://www.nxp.com/docs/en/data-sheet/NX5P3090.pdf):
SoC LS1043A pin USB_DRVVBUS ---> discrete VBUS switch chip NX5P3090 pin EN, 
chip NX5P3090 pin VBUS ---> USB physical connector pin VBUS (with some discrete 
charge pump connected)

> > Then, DWC3 core driver continued to call function
> > dwc3_host_init()->platform_device_add(xhci)…
> > xhci_plat_probe()->usb_add_hcd()->xhci_plat_setup()->xhci_gen_setup->
> > xhci_reset(), which would reset xHCI controller. At this point, the
> > VBUS EN pin (USB_DRVVBUS) was pulled low for about 15us, causing the
> 
> why is that pin pulled low? XHCI reset shouldn't be a global reset. Did your
> HW engineer tie all reset lines together? If so, there's nothing I can do to
> help.

That's the point I also want to make clear: do you mean that the VBUS control
signal come from DWC3 IP should not be pulled down when xHCI controller
conduct reset? 
And sorry that I am not quite sure about the 'global reset' you mentioned. Do
you mean to a DWC3 global reset or SoC reset? 

My understanding is that since VBUS control signal only be meaningful in USB
host mode (xHCI), so it might be in the scope/control of xHCI controller, 
meaning
that xHCI reset trigger VBUS/USB_DRVVBUS(EN) pulled low might make sense,
am I right? And the information come from DWC3 IP design has confirmed that
PORTSC[PP] will be de-asserted during HCRST, it seems this is native behavior on
DWC3 IP.

> > VBUS did the same drop too, then back to normal voltage when HW reset
> > complete. We have confirmed this whole process according to scope
> > waveform with test code on DWC3 driver. Impact is that VBUS glitch has
> > let some USB drives (such as Transcend 4GB USB2.0 (jetflash) and
> > Kingston 16GB USB2.0 DTGE9) malfunction during enumeration
> > (particularly happen when drive is connected to root-hub port prior to
> > Linux boot).
> 
> okay
> 
> > Per my understanding, VBUS need to keep +5V once enabled without any
> > drop/unstable. And above glitch looks like caused by the gap between
> > DWC3 design and driver init procedure.
> 
> why are you blaming the driver here? We don't know of any such platform
> that has problems with this. Do you mean to say that because your HW
> engineer made a choice of tying host reset to global reset, you end up having
> an issue? That's something else entirely that SW can't help you with.

I didn't mean to blame driver alone, just found the time interval between host
mode enabling and host reset causing a observable VBUS control signal glitch
happen we didn't expected. And experiments proved that VBUS on between host
mode enabling and host reset might not be necessary and can avoid this 
potential risk.

> I have no idea about anything nxp has done, no access to documentation,
> no

Re: [PATCH 0/5 v7] Keep rtsx_usb suspended when there's no card

2018-11-22 Thread Kai Heng Feng
Hi Ulf,

> On Nov 8, 2018, at 8:17 PM, Oleksandr Natalenko  
> wrote:
> 
>> This is based on Ulf's work [1] [2].
>> This patch series can keep rtsx_usb suspended, to save ~0.5W on Intel
>> platforms and ~1.5W on AMD platforms.
>> [1] https://patchwork.kernel.org/patch/10440583/
>> [2] https://patchwork.kernel.org/patch/10445725/
> 
> Tested-by: Oleksandr Natalenko 

Do you think this series is ready to be merged to v4.20?

Kai-Heng

> 
> -- 
>  Oleksandr Natalenko (post-factum)



Re: [PATCH] USB: Disable USB2 LPM at shutdown

2018-11-22 Thread Kai Heng Feng
Hi,

> On Nov 7, 2018, at 1:34 AM, Kai-Heng Feng  wrote:
> 
> The QCA Rome USB Bluetooth controller has several issues once LPM gets
> enabled:
> - Fails to get enumerated in coldboot. [1]
> - Drains more power (~ 0.2W) when the system is in S5. [2]
> - Disappears after a warmboot. [2]
> 
> The issue happens because the device lingers at LPM L1 in S5, so device
> can't get enumerated even after a reboot.
> 
> Disalbe LPM at shutdown can solve the issue.
> 
> [1] https://bugs.launchpad.net/bugs/1757218
> [2] https://patchwork.kernel.org/patch/10607097/
> 
> Signed-off-by: Kai-Heng Feng 

Would it be possible to merge this patch?

Kai-Heng

> ---
> drivers/usb/core/port.c | 10 ++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 1a06a4b5fbb1..15f6924a4d84 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -285,6 +285,15 @@ static int usb_port_runtime_suspend(struct device *dev)
> }
> #endif
> 
> +static void usb_port_shutdown(struct device *dev)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> +
> + if (port_dev->child)
> + usb_set_usb2_hardware_lpm(port_dev->child, 0);
> +
> +}
> +
> static const struct dev_pm_ops usb_port_pm_ops = {
> #ifdef CONFIG_PM
>   .runtime_suspend =  usb_port_runtime_suspend,
> @@ -301,6 +310,7 @@ struct device_type usb_port_device_type = {
> static struct device_driver usb_port_driver = {
>   .name = "usb",
>   .owner = THIS_MODULE,
> + .shutdown = usb_port_shutdown,
> };
> 
> static int link_peers(struct usb_port *left, struct usb_port *right)
> -- 
> 2.19.1
> 



Re: [PATCH v2] USB: Don't enable LPM if it's already enabled

2018-11-22 Thread Kai Heng Feng
Hi Greg,

> On Oct 31, 2018, at 1:52 PM, Kai-Heng Feng  
> wrote:
> 
> USB Bluetooth controller QCA ROME (0cf3:e007) sometimes stops working
> after S3:
> [ 165.110742] Bluetooth: hci0: using NVM file: qca/nvm_usb_0302.bin
> [ 168.432065] Bluetooth: hci0: Failed to send body at 4 of 1953 (-110)
> 
> After some experiments, I found that disabling LPM can workaround the
> issue.
> 
> On some platforms, the USB power is cut during S3, so the driver uses
> reset-resume to resume the device. During port resume, LPM gets enabled
> twice, by usb_reset_and_verify_device() and usb_port_resume().
> 
> So let's enable LPM for just once, as this solves the issue for the
> device in question.
> 
> Signed-off-by: Kai-Heng Feng 

Would it be possible to merge this patch?

Kai-Heng

> ---
> v2:
> -  Check udev->usb2_hw_lpm_enabled before calling usb_port_resume().
> 
> drivers/usb/core/hub.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c6077d582d29..02fa574e98ce 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -3523,7 +3523,8 @@ int usb_port_resume(struct usb_device *udev, 
> pm_message_t msg)
>   hub_port_logical_disconnect(hub, port1);
>   } else  {
>   /* Try to enable USB2 hardware LPM */
> - if (udev->usb2_hw_lpm_capable == 1)
> + if (udev->usb2_hw_lpm_capable == 1 &&
> + !udev->usb2_hw_lpm_enabled)
>   usb_set_usb2_hardware_lpm(udev, 1);
> 
>   /* Try to enable USB3 LTM */
> -- 
> 2.19.1
> 



Re: pl2303 regression after v4.17. Bisected to 7041d9c3f01b

2018-11-22 Thread Florian Zumbiehl
Hi,

> T:  Bus=01 Lev=02 Prnt=03 Port=01 Cnt=01 Dev#=  4 Spd=12  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=0557 ProdID=2008 Rev=03.00
> S:  Manufacturer=Prolific Technology Inc.
> S:  Product=USB-Serial Controller D
> C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303
> 
> and
> 
> T:  Bus=01 Lev=02 Prnt=02 Port=03 Cnt=03 Dev#=  6 Spd=12  MxCh= 0
> D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=067b ProdID=2303 Rev=04.00
> S:  Manufacturer=Prolific Technology Inc.
> S:  Product=USB-Serial Controller D
> C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

The ones that I have at hand all look like this:

T:  Bus=05 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  3 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=067b ProdID=2303 Rev=03.00
S:  Manufacturer=Prolific Technology Inc.
S:  Product=USB-Serial Controller
C:  #Ifs= 1 Cfg#= 1 Atr=80 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

So, yours and mine are all type 'HX' according to the driver's
categorization ...

The only common differences would be the product description string having a
"D" suffix in your version and bmAttributes being 0xa0 vs. 0x80, so yours
claim to support remote wakeup. The former seems like a bad idea to base
hardware detection on, but the latter might indicate a different chip with
different features?!

Can you tell what the physical chip is labeled as, if anything?

@Johan: What does the USB device info of your adapter(s) say?

Regards, Florian


[PATCH v2] USB: serial: ftdi_sio: Use rounding instead of truncating when calculating baud rate divisors.

2018-11-22 Thread Nikolaj Fogh

Improve baud-rate generation by using rounding-to-closest instead of
truncation in divisor calculation.

Results have been verified by logic analyzer on an FT232RT (232BM) chip.
The following table shows the wanted baud rate, the baud rate obtained
with the old method (truncation), with the new method (rounding) and the
baud rate generated by the windows 10 driver. The numbers in parentheses
is the error.

+- Wanted --+-- Old ---+-- New ---+-- Win ---+
|   9600    |   9600  (0.00%)  |   9604  (0.05%)  |   9605 (0.05%)  |
|   19200   |   19200 (0.00%)  |   19199 (0.01%)  |   19198 (0.01%)  |
|   38400   |   38395 (0.01%)  |   38431 (0.08%)  |   38394 (0.02%)  |
|   57600   |   57725 (0.22%)  |   57540 (0.10%)  |   57673 (0.13%)  |
|  115200   |  115307 (0.09%)  |  115330 (0.11%)  |  115320 (0.10%)  |
|  921600   |  919963 (0.18%)  |  920386 (0.13%)  |  920810 (0.09%)  |
|  961200   |  996512 (3.67%)  |  956480 (0.49%)  |  956937 (0.44%)  |
+---+--+--+--+

The error due to noise in the measurements is in the order of a few
tenths of a %. As can be seen, the baud rate for 961200 is significantly
improved for some rates, and corresponds to the output given by the
windows driver.

The theoretical baud rate has been calculated for all baud rates from 1
to 3M, and as expected, the error is centered around 0, with a triangle
shape instead of a sawtooth, so the maximum error is decreased to half.

Signed-off-by: Nikolaj Fogh 

---
 drivers/usb/serial/ftdi_sio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 609198d9594c..0edbd3427548 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1130,7 +1130,7 @@ static unsigned short int 
ftdi_232am_baud_base_to_divisor(int baud, int base)

 {
 unsigned short int divisor;
 /* divisor shifted 3 bits to the left */
-    int divisor3 = base / 2 / baud;
+    int divisor3 = DIV_ROUND_CLOSEST(base, 2 * baud);
 if ((divisor3 & 0x7) == 7)
     divisor3++; /* round x.7/8 up to x+1 */
 divisor = divisor3 >> 3;
@@ -1156,7 +1156,7 @@ static u32 ftdi_232bm_baud_base_to_divisor(int 
baud, int base)

 static const unsigned char divfrac[8] = { 0, 3, 2, 4, 1, 5, 6, 7 };
 u32 divisor;
 /* divisor shifted 3 bits to the left */
-    int divisor3 = base / 2 / baud;
+    int divisor3 = DIV_ROUND_CLOSEST(base, 2 * baud);
 divisor = divisor3 >> 3;
 divisor |= (u32)divfrac[divisor3 & 0x7] << 14;
 /* Deal with special cases for highest baud rates. */
@@ -1179,7 +1179,7 @@ static u32 ftdi_2232h_baud_base_to_divisor(int 
baud, int base)

 int divisor3;

 /* hi-speed baud rate is 10-bit sampling instead of 16-bit */
-    divisor3 = base * 8 / (baud * 10);
+    divisor3 = DIV_ROUND_CLOSEST(base * 8, baud * 10);

 divisor = divisor3 >> 3;
 divisor |= (u32)divfrac[divisor3 & 0x7] << 14;
--
2.19.1


Re: [PATCH v3 net-next 13/21] net: usb: aqc111: Add support for TSO

2018-11-22 Thread David Miller
From: Igor Russkikh 
Date: Wed, 21 Nov 2018 10:13:48 +

> @@ -832,6 +832,10 @@ static struct sk_buff *aqc111_tx_fixup(struct usbnet 
> *dev, struct sk_buff *skb,
>   /*Length of actual data*/
>   tx_desc |= skb->len & AQ_TX_DESC_LEN_MASK;
>  
> + /* TSO MSS */
> + tx_desc |= ((u64)(skb_shinfo(skb)->gso_size & AQ_TX_DESC_MSS_MASK)) <<
> +AQ_TX_DESC_MSS_SHIFT;
 ...
> +#define AQ_TX_DESC_MSS_MASK  0x7FFF

This implies a GSO size limit, which you need to advertise properly in
netdevice->gso_max_size.

Also, where is the TSO feature flag being set to actually enable the
stack sending your GSO frames?


Re: [PATCH v3 net-next 10/21] net: usb: aqc111: Add checksum offload support

2018-11-22 Thread David Miller
From: Igor Russkikh 
Date: Wed, 21 Nov 2018 10:13:42 +

> +static void aqc111_rx_checksum(struct sk_buff *skb, u64 *pkt_desc)
> +{
 ...
> + /* checksum error bit is set */
> + if (*pkt_desc & AQ_RX_PD_L4_ERR || *pkt_desc & AQ_RX_PD_L3_ERR)
> + return;
 ...
> @@ -661,6 +698,7 @@ static int aqc111_rx_fixup(struct usbnet *dev, struct 
> sk_buff *skb)
>   skb_set_tail_pointer(new_skb, new_skb->len);
>  
>   new_skb->truesize = new_skb->len + sizeof(struct sk_buff);
> + aqc111_rx_checksum(new_skb, pkt_desc);

This is another reason to use a 'cpu_desc' local variable to hold the
endian translated descriptor value so you don't have to dereference
this thing over and over again.


Re: [PATCH v3 net-next 09/21] net: usb: aqc111: Implement RX data path

2018-11-22 Thread David Miller
From: Igor Russkikh 
Date: Wed, 21 Nov 2018 10:13:39 +

> + desc_hdr = *(u64 *)skb_tail_pointer(skb);
> + le64_to_cpus(&desc_hdr);

This is:

desc_hdr = le64_to_cpup(skb_tail_pointer(skb));

> + /* Get the first RX packet descriptor */
> + pkt_desc = (u64 *)(skb->data + desc_offset);
> +
> + while (pkt_count--) {
> + u32 pkt_len = 0;
> + u32 pkt_len_with_padd = 0;
> +
> + le64_to_cpus(pkt_desc);

Probably better to load the translated value into a local variable
once:

u64 cpu_desc = le64_to_cpup(pkt_desc);

and then use 'cpu_desc' instead of dereferencing "*pkt_desc" over and
over again.

> + /* Clone SKB */
> + new_skb = skb_clone(skb, GFP_ATOMIC);
> +
> + if (!new_skb)
> + goto err;
> +
> + new_skb->len = pkt_len;
> + skb_pull(new_skb, AQ_RX_HW_PAD);
> + skb_set_tail_pointer(new_skb, new_skb->len);
> +
> + new_skb->truesize = new_skb->len + sizeof(struct sk_buff);

I see lots of USB drivers doing this, but it's not correct.

We have a SKB_TRUESIZE() macro, please use it.


Re: [PATCH v3 net-next 08/21] net: usb: aqc111: Implement TX data path

2018-11-22 Thread David Miller
From: Igor Russkikh 
Date: Wed, 21 Nov 2018 10:13:37 +

> + if (padding_size != 0)
> + skb_put(skb, padding_size);

I think you want to use skb_put_zero() here rather than leaving it
uninitialized.  I know it's padding, but if for some reason this leaks
onto the wire or elsewhere we'll regret not initializing this buffer.

> + /* Copy TX header */
> + skb_push(skb, sizeof(tx_desc));
> + cpu_to_le64s(&tx_desc);
> + skb_copy_to_linear_data(skb, &tx_desc, sizeof(tx_desc));

I seriously wonder if, with all of the invariants wrt. length modulus
and padding, that you can safely just go:

__le64 *p;

p = skb_push(skb, sizeof(tx_desc));
*p = cpu_to_le64(tx_desc);

or something like that.



Re: Support Mac address pass through on Dell DS1000 dock

2018-11-22 Thread Oliver Neukum
On Do, 2018-11-22 at 12:06 +0100, Frédéric Parrenin wrote:
> Le 22/11/2018 à 11:22, Oliver Neukum a écrit :
> > On Mi, 2018-11-21 at 16:50 +0100, Frédéric Parrenin  wrote:
> > > which over rides the Mac on the dock. So, the answer is, "it's not an
> > > issue if the dock supports it, as the laptop BIOS is what is determining
> > > if it is supported".
> > > 
> > > So what is the truth?
> > 
> > For pass thru you must meet multiple conditions:
> > 
> > /* if this is not an RTL8153-AD, no eFuse mac passthru set,
> >   * or system doesn't provide valid _SB.AMAC this will be
> >   * be expected to non-zero
> >   */>
> > 
> > They can be manually verified. Do you need a debugging patch?
> 
> OK, let us try a debugging patch.

Here you go.
PLease enable dynamic debugging for the driver and ramp up
your logging level.

Regards
Oliver
From 3661ff35782f7b26df3204f4f7a18929e0c74ff7 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 22 Nov 2018 18:08:43 +0100
Subject: [PATCH] rtl8152: debugging for MAC pass-through

This adds debugging statements for failure cases in MAC
pass-through

Signed-off-by: Oliver Neukum 
---
 drivers/net/usb/r8152.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 7345a2258ee4..3ed52806164c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1168,19 +1168,28 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
 
 	/* test for -AD variant of RTL8153 */
 	ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
-	if ((ocp_data & AD_MASK) != 0x1000)
+	if ((ocp_data & AD_MASK) != 0x1000) {
+		netif_dbg(tp, probe, tp->netdev,
+"Not an AD type.\n");
 		return -ENODEV;
+	}
 
 	/* test for MAC address pass-through bit */
 	ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, EFUSE);
-	if ((ocp_data & PASS_THRU_MASK) != 1)
+	if ((ocp_data & PASS_THRU_MASK) != 1) {
+		netif_dbg(tp, probe, tp->netdev,
+"pass-through bit not set.\n");
 		return -ENODEV;
+	}
 
 	/* returns _AUXMAC_#AABBCCDDEEFF# */
 	status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
 	obj = (union acpi_object *)buffer.pointer;
-	if (!ACPI_SUCCESS(status))
+	if (!ACPI_SUCCESS(status)) {
+		netif_warn(tp, probe, tp->netdev,
+"pass-through firmware failure.\n");
 		return -ENODEV;
+	}
 	if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
 		netif_warn(tp, probe, tp->netdev,
 			   "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
-- 
2.16.4



Re: [GIT PULL] USB driver fixes for 4.20-rc4

2018-11-22 Thread pr-tracker-bot
The pull request you sent on Thu, 22 Nov 2018 09:45:39 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git tags/usb-4.20-rc4

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4cd731953d620b7e4e999a90d13db58b88c5e95b

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker


Re: Support Mac address pass through on Dell DS1000 dock

2018-11-22 Thread Bjørn Mork
 writes:
> On Nov 22, 2018 03:41, Frédéric Parrenin 
>  wrote:
>>
>> A guy answered on the amazon page that he has Mac pass-through working
>> on windows 10 with this exact same dock station.
>> So it really seems to be a linux-only problem.
>
> Well thanks for checking further. This is quite surprising to me.

Maybe it's by accident because the Windows driver is matching too many
devices or something?  Do you happen know how this feature was
implemented in Windows?

> You can see in official documentation it's not mentioned:
>
> https://www.dell.com/support/article/us/en/04/sln301147/what-is-mac-address-pass-through-?lang=en
>
> I'll look forward to seeing what the debugging patch shows and need to
> confir with an extended team about results after holidays.

I don't remember if I mentioned this when you created the r8152
implementation, but this feature could have been easily implemented in
userspace given a minimalistic ACPI driver to just read the system MAC.
This way it would depend only on the Dell BIOS support, and not on some
specific dock.  It would even work with third party dongles etc.  Having
it in userspace would also have made it easier to implement an end user
specific policy scheme. There is no need to restrict ourselves to the
limited creativeness of Windows developers ;-)



Bjørn


Re: pl2303 regression after v4.17. Bisected to 7041d9c3f01b

2018-11-22 Thread Jarkko Nikula

On 11/21/18 5:12 PM, Florian Zumbiehl wrote:

I have two pl2303 based adapters which behave the same:

ID 0557:2008 ATEN International Co., Ltd UC-232A Serial Port [pl2303]
ID 067b:2303 Prolific Technology, Inc. PL2303 Serial Port


What is the output of usb-devices for those?


T:  Bus=01 Lev=02 Prnt=03 Port=01 Cnt=01 Dev#=  4 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=0557 ProdID=2008 Rev=03.00
S:  Manufacturer=Prolific Technology Inc.
S:  Product=USB-Serial Controller D
C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

and

T:  Bus=01 Lev=02 Prnt=02 Port=03 Cnt=03 Dev#=  6 Spd=12  MxCh= 0
D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=067b ProdID=2303 Rev=04.00
S:  Manufacturer=Prolific Technology Inc.
S:  Product=USB-Serial Controller D
C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

--
Jarkko


Re: Support Mac address pass through on Dell DS1000 dock

2018-11-22 Thread Frédéric Parrenin



Le 22/11/2018 à 11:22, Oliver Neukum a écrit :

On Mi, 2018-11-21 at 16:50 +0100, Frédéric Parrenin  wrote:

which over rides the Mac on the dock. So, the answer is, "it's not an
issue if the dock supports it, as the laptop BIOS is what is determining
if it is supported".

So what is the truth?

For pass thru you must meet multiple conditions:

/* if this is not an RTL8153-AD, no eFuse mac passthru set,
  * or system doesn't provide valid _SB.AMAC this will be
  * be expected to non-zero
  */>

They can be manually verified. Do you need a debugging patch?

OK, let us try a debugging patch.

Thanks.

FP



Re: RESEND: About VBUS glitch happen on DWC3 host mode enabling process.

2018-11-22 Thread Felipe Balbi

Hi,

Ran Wang  writes:
> Hello Felipe,
>
> We’ve found on some Layerscape platforms which integrating DWC3

which platform is that? Is it supported upstream? Which kernel version
are you using?

> USB3.0 DRD controller, VBUS glitch happened and caused some USB drives

what do you mean by glitch?

> enumeration fail, like to discuss the details as below.
>  
> Story is that, we found once function dwc3_core_init_mode() got called
> and enabled host mode by calling dwc3_set_prtcap(dwc,
> DWC3_GCTL_PRTCAP_HOST) which would write register [DWC3_GCTL] bit
> 12~13, so the pin (such as USB_DRVVBUS) which control VBUS driving
> chip’s EN pin would be pulled high immediately, so did the VBUS (up to
> +5V).

this seems specific to your platform. Please clarify a little more? Do
you have a discrete charge pump tied to some output signal from dwc3?

> Then, DWC3 core driver continued to call function
> dwc3_host_init()->platform_device_add(xhci)…
> xhci_plat_probe()->usb_add_hcd()->xhci_plat_setup()->xhci_gen_setup->
> xhci_reset(), which would reset xHCI controller. At this point, the
> VBUS EN pin (USB_DRVVBUS) was pulled low for about 15us, causing the

why is that pin pulled low? XHCI reset shouldn't be a global reset. Did
your HW engineer tie all reset lines together? If so, there's nothing I
can do to help.

> VBUS did the same drop too, then back to normal voltage when HW reset
> complete. We have confirmed this whole process according to scope
> waveform with test code on DWC3 driver. Impact is that VBUS glitch has
> let some USB drives (such as Transcend 4GB USB2.0 (jetflash) and
> Kingston 16GB USB2.0 DTGE9) malfunction during enumeration
> (particularly happen when drive is connected to root-hub port prior to
> Linux boot).

okay

> Per my understanding, VBUS need to keep +5V once enabled without any
> drop/unstable. And above glitch looks like caused by the gap between
> DWC3 design and driver init procedure.

why are you blaming the driver here? We don't know of any such platform
that has problems with this. Do you mean to say that because your HW
engineer made a choice of tying host reset to global reset, you end up
having an issue? That's something else entirely that SW can't help you
with.

I have no idea about anything nxp has done, no access to documentation,
nothing at all. I need you to do a better job at explaining the
situation starting with kernel version you're using, if platform is
supported upstream, etc.



> One of probably workaround come to my mind is to program all root-hub
> ports’ PORTSC[PP] to 0b immediately after enabling host mode (calling
> dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST)), so VBUS will keep 0V
> till xhci is reset by xhci driver like above. I have test this and it
> works.

dwc3 will _not_ touch xHCI registers, sorry. If you need something like
that, you need to do it as a quirk in xhci-plat.c

> I know it is not an good fix because DWC3 core driver would touch xHCI
> controller which ought to be handled by xHCI driver instead, so like
> to send out this mail for discussion first, see if anyone has better
> solution on this issue.

please answer my other questions first, after we fully understand the
scenario and the problem, then we can discuss implementation details.

cheers

-- 
balbi


signature.asc
Description: PGP signature


Re: Support Mac address pass through on Dell DS1000 dock

2018-11-22 Thread Oliver Neukum
On Mi, 2018-11-21 at 16:50 +0100, Frédéric Parrenin  wrote:
> 
> which over rides the Mac on the dock. So, the answer is, "it's not an 
> issue if the dock supports it, as the laptop BIOS is what is determining 
> if it is supported".
> 
> So what is the truth?

For pass thru you must meet multiple conditions:

/* if this is not an RTL8153-AD, no eFuse mac passthru set,
 * or system doesn't provide valid _SB.AMAC this will be
 * be expected to non-zero
 */>

They can be manually verified. Do you need a debugging patch?

Regards
Oliver



Re: Support Mac address pass through on Dell DS1000 dock

2018-11-22 Thread Frédéric Parrenin



Le 21/11/2018 à 17:10, Frédéric Parrenin a écrit :


Le 21/11/2018 à 17:03, mario.limoncie...@dell.com a écrit :



Sent from Workspace ONE Boxer
On Nov 21, 2018 09:51, Frédéric Parrenin 
 wrote:

>
>
> [EXTERNAL EMAIL]
>
>
> Le 21/11/2018 à 15:19, Frédéric Parrenin a écrit :
> >
> > Le 21/11/2018 à 15:01, mario.limoncie...@dell.com a écrit :
> >> On Nov 21, 2018 02:09, Greg KH  wrote:
> >> >
> >> >
> >> > [EXTERNAL EMAIL]
> >> >
> >> > On Tue, Nov 20, 2018 at 09:10:22PM +0100, Bjørn Mork wrote:
> >> > > Greg KH  writes:
> >> > >
> >> > > > I do not see any USB networking device here at all.
> >> > >
> >> > > No, It wasn't easy to see. But it's there both with and 
without the

> >> > > feature enabled:
> >> > >
> >> > >  /: Bus 02.Port 1: Dev 1, Class=root_hub, 
Driver=xhci_hcd/6p, 5000M

> >> > > | Port 4: Dev 10, If 0, Class=Hub, Driver=hub/7p, 5000M
> >> > >   |__ Port 2: Dev 11, If 0, Class=Vendor Specific Class,
> >> Driver=r8152, 5000M
> >> >
> >> > Ah, sorry, I missed that.
> >> >
> >> > > This feature is only for convenience and should never 
prevent the
> >> > > network from working. Unless the other end is confused by 
seeing

> >> the
> >> > > same mac address on two ports...  Better take down or 
disconnect the

> >> > > undocked interface after docking.
> >> >
> >> > Ok, so it seems that this driver does not support the 
pass-through

> >> > option for some reason.
> >> >
> >> > Mario, any ideas what Frédéric can do here to help debug the
> >> issue?  He
> >> > has some Dell hardware that does not seem to be working 
properly with

> >> > the pass-through option.
> >> >
> >> > I place odds that the driver needs some kind of update for newer
> >> device
> >> > signatures, which is what some of us worried about a long time 
ago

> >> when
> >> > this patch first went in...
> >> >
> >>
> >> I don't have the whole context for this thread but I think I'm
> >> following that the question is why MAC address pass thru isn't
> >> activating on DS1000?
> >>
> >> The criteria that we previously put in for this patch is still the
> >> correct criteria; R8153-AD with a particular eFuse blown.
> >>
> >> I never heard DS1000 is supposed to meet that criteria. I thought
> >> official documentation referred to TB16 and WD15 only for Mac
> >> passthrough.
> >>
> >> FYI I'm on holiday right now so it might take me some time to get
> >> back after this response.
> >>
> >>
> > Some online shops selling the DS1000 mention it supports Mac Address
> > pass-through, for example:
> >
> > https://shop.bechtle.de/medias/VXav7KboWIcl8FlvCBYV58-30.pdf
> > 
https://www.amazon.com/Dell-USB-Type-Monitor-Release-VT96R/dp/B01G6VL3I4/ref=sr_1_1?ie=UTF8&qid=1542809915&sr=8-1&keywords=DS1000

> >
> >
> > But actually, this might be a wrong information.
> > Sorry for the noise if this is the case, and too bad the DS1000 does
> > not support pass-through.
> >
> I asked the question on amazon.com, and the guy said:
>
> > Dell  Mac address pass-through is enabled in the BIOS of the Dell 
laptop

> which over rides the Mac on the dock. So, the answer is, "it's not an
> issue if the dock supports it, as the laptop BIOS is what is 
determining

> if it is supported".
>
> So what is the truth?
>
> Unfortunately, I don't have Windows on that machine to test.
>

They work in tandem. A Mac address for the system needs to be 
reserved at manufacturing time. It's stored in NVRAM for the system 
BIOS to access and populate into an ACPI table.


If your system is too old or no Mac address is reserved for that 
model (it's currently typically Latitude, Precision, XPS only 
feature) then no ACPI entry is created.


The driver will parse that information when using appropriate 
components like I described in previous email.


Yes, I have a Latitude 5285 which does support Mac Address pass through.
But when I activate that option in the bios and connect to the DS1000, 
the network does not work and "ip a" does not display the laptop's Mac 
address but the dock Mac address.
The question I have if whether this is the DS1000's fault (it does not 
support that feature) or linux's fault.


A guy answered on the amazon page that he has Mac pass-through working 
on windows 10 with this exact same dock station.

So it really seems to be a linux-only problem.


[GIT PULL] USB driver fixes for 4.20-rc4

2018-11-22 Thread Greg KH
The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:

  Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git tags/usb-4.20-rc4

for you to fetch changes up to 63529eaa6164ef7ab4b907b25ac3648177e5e78f:

  usb: cdc-acm: add entry for Hiro (Conexant) modem (2018-11-20 12:12:06 +0100)


USB fixes for 4.20-rc4

Here are a number of small USB fixes for 4.20-rc4.

There's the usual xhci and dwc2/3 fixes as well as a few minor other
issues resolved for problems that have been reported.  Full details are
in the shortlog.

All have been in linux-next for a while with no reported issues.

Signed-off-by: Greg Kroah-Hartman 


Aaron Ma (2):
  usb: xhci: fix uninitialized completion when USB3 port got wrong status
  usb: xhci: fix timeout for transition from RExit to U0

Andy Shevchenko (1):
  usb: dwc3: core: Clean up ULPI device

Cherian, George (1):
  xhci: Add quirk to workaround the errata seen on Cavium Thunder-X2 Soc

Dan Carpenter (1):
  usb: dwc2: pci: Fix an error code in probe

Dennis Wassenberg (1):
  usb: core: Fix hub port connection events lost

Emmanuel Pescosta (1):
  usb: quirks: Add delay-init quirk for Corsair K70 LUX RGB

Felipe Balbi (1):
  usb: dwc3: gadget: fix ISOC TRB type on unaligned transfers

Greg Kroah-Hartman (1):
  Merge tag 'fixes-for-v4.20-rc2' of git://git.kernel.org/.../balbi/usb 
into usb-linus

Kai-Heng Feng (2):
  USB: Wait for extra delay time after USB_PORT_FEAT_RESET for quirky hub
  USB: quirks: Add no-lpm quirk for Raydium touchscreens

Kuppuswamy Sathyanarayanan (1):
  usb: dwc3: Fix NULL pointer exception in dwc3_pci_remove()

Maarten Jacobs (1):
  usb: cdc-acm: add entry for Hiro (Conexant) modem

Mathias Nyman (3):
  xhci: Fix leaking USB3 shared_hcd at xhci removal
  xhci: handle port status events for removed USB3 hcd
  usb: xhci: Prevent bus suspend if a port connect change or polling state 
is detected

Mattias Jacobsson (1):
  USB: misc: appledisplay: add 20" Apple Cinema Display

Sandeep Singh (1):
  xhci: Add check for invalid byte size error when UAS devices are 
connected.

Shen Jing (1):
  Revert "usb: gadget: ffs: Fix BUG when userland exits with submitted AIO 
transfers"

Thinh Nguyen (1):
  usb: dwc3: gadget: Properly check last unaligned/zero chain TRB

 Documentation/admin-guide/kernel-parameters.txt |  2 +
 drivers/usb/class/cdc-acm.c |  3 ++
 drivers/usb/core/hub.c  | 18 +--
 drivers/usb/core/quirks.c   | 14 ++
 drivers/usb/dwc2/pci.c  |  1 +
 drivers/usb/dwc3/core.c |  1 +
 drivers/usb/dwc3/dwc3-pci.c |  4 +-
 drivers/usb/dwc3/gadget.c   |  8 +--
 drivers/usb/gadget/function/f_fs.c  | 26 +++---
 drivers/usb/host/xhci-histb.c   |  6 ++-
 drivers/usb/host/xhci-hub.c | 66 ++---
 drivers/usb/host/xhci-mtk.c |  6 ++-
 drivers/usb/host/xhci-pci.c |  6 +++
 drivers/usb/host/xhci-plat.c|  6 ++-
 drivers/usb/host/xhci-ring.c| 45 -
 drivers/usb/host/xhci-tegra.c   |  1 +
 drivers/usb/host/xhci.c |  2 -
 drivers/usb/host/xhci.h |  3 +-
 drivers/usb/misc/appledisplay.c |  1 +
 include/linux/usb/quirks.h  |  3 ++
 20 files changed, 167 insertions(+), 55 deletions(-)


[PATCHv3] USB: serial: mos7840: Add a product ID for the new product

2018-11-22 Thread JackyChou


From: JackyChou 

For now, pause to add PID 0x7843 in the driver.

Simplify the processes of some functions.
Such modifications will not affect the old devices and will make
the addition of new product (0x7843) more flexible in the future.

Signed-off-by: JackyChou 
---
 drivers/usb/serial/mos7840.c | 48 
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index b42bad85097a..0ca945dce377 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -298,15 +298,10 @@ static int mos7840_set_uart_reg(struct usb_serial_port
*port, __u16 reg,
val = val & 0x00ff;
/* For the UART control registers, the application number need
   to be Or'ed */
-   if (port->serial->num_ports == 4) {
+   if (port->serial->num_ports == 2 && port->port_number != 0)
+   val |= ((__u16)port->port_number + 2) << 8;
+   else
val |= ((__u16)port->port_number + 1) << 8;
-   } else {
-   if (port->port_number == 0) {
-   val |= ((__u16)port->port_number + 1) << 8;
-   } else {
-   val |= ((__u16)port->port_number + 2) << 8;
-   }
-   }
dev_dbg(&port->dev, "%s application number is %x\n", __func__, val);
return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ,
   MCS_WR_RTYPE, val, reg, NULL, 0,
@@ -332,15 +327,10 @@ static int mos7840_get_uart_reg(struct usb_serial_port
*port, __u16 reg,
return -ENOMEM;
 
/* Wval  is same as application number */
-   if (port->serial->num_ports == 4) {
+   if (port->serial->num_ports == 2 && port->port_number != 0)
+   Wval = ((__u16)port->port_number + 2) << 8;
+   else
Wval = ((__u16)port->port_number + 1) << 8;
-   } else {
-   if (port->port_number == 0) {
-   Wval = ((__u16)port->port_number + 1) << 8;
-   } else {
-   Wval = ((__u16)port->port_number + 2) << 8;
-   }
-   }
dev_dbg(&port->dev, "%s application number is %x\n", __func__,
Wval);
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
  MCS_RD_RTYPE, Wval, reg, buf,
VENDOR_READ_LENGTH,
@@ -2146,22 +2136,16 @@ static int mos7840_port_probe(struct usb_serial_port
*port)
mos7840_port->SpRegOffset = 0x0;
mos7840_port->ControlRegOffset = 0x1;
mos7840_port->DcrRegOffset = 0x4;
-   } else if ((mos7840_port->port_num == 2) && (serial->num_ports ==
4)) {
-   mos7840_port->SpRegOffset = 0x8;
-   mos7840_port->ControlRegOffset = 0x9;
-   mos7840_port->DcrRegOffset = 0x16;
-   } else if ((mos7840_port->port_num == 2) && (serial->num_ports ==
2)) {
-   mos7840_port->SpRegOffset = 0xa;
-   mos7840_port->ControlRegOffset = 0xb;
-   mos7840_port->DcrRegOffset = 0x19;
-   } else if ((mos7840_port->port_num == 3) && (serial->num_ports ==
4)) {
-   mos7840_port->SpRegOffset = 0xa;
-   mos7840_port->ControlRegOffset = 0xb;
-   mos7840_port->DcrRegOffset = 0x19;
-   } else if ((mos7840_port->port_num == 4) && (serial->num_ports ==
4)) {
-   mos7840_port->SpRegOffset = 0xc;
-   mos7840_port->ControlRegOffset = 0xd;
-   mos7840_port->DcrRegOffset = 0x1c;
+   } else {
+   u8 port_offset;
+
+   if ((mos7840_port->port_num == 2) && (serial->num_ports ==
2))
+   port_offset = 1;
+   else
+   port_offset = mos7840_port->port_num - 2;
+   mos7840_port->SpRegOffset = 0x8 + (2 * port_offset);
+   mos7840_port->ControlRegOffset = 0x9 + (2 * port_offset);
+   mos7840_port->DcrRegOffset = 0x16 + (3 * port_offset);
}
mos7840_dump_serial_port(port, mos7840_port);
mos7840_set_port_private(port, mos7840_port);
-- 
2.17.1