Q: How does gadgetfs knows that host initiated IN transfer ?

2018-03-01 Thread Ran Shalit
Hello,

I am developing simple loopback using gadgetfs, but I am a bit
confused as to how gadgefs knows that host initiated IN transfer.

Gadgetfs use read/write on endpoints, so as to my understanding, it can only:

1. When using "read" on OUT endpoint - accept a new transfer from host
to device.

2. When using "write" on IN endpoint - start transfer from device to host .

(1) above seems to be simple to understand, but I have
misunderstanding about (2):

Isn't it that a write into IN endpoint should be accepted only when
host initiated a transaction (device should not initiate transaction
by itself) ?

If it must , than How does gadget knows that host initiated a
transaction in the IN endpoint ? (it probably need to be checked
before doing the write in the IN transaction)

Thank you,
ranran
--
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: Continuous USB resets on NXP i.MX 6ULL device

2018-03-01 Thread Peter Chen
 
> >>
> > For non-core registers, i.mx6ull can use i.mx6q as its base setting,
> > the bits you mentioned may also be reserved at imx6qdl RM.
> >
> just for my understanding: MX6_BM_NON_BURST_SETTING is a no-op on
> i.MX6ULL or not properly documented in reference manual?

Not properly documented in reference manual.

Peter


Re: [PATCH v5 12/12] extcon: axp288: Set USB role where necessary

2018-03-01 Thread Chanwoo Choi
Hi,

Basically, I have no objection. But I'll reply the my ack tag 
after finishing the review of 'devcon and usb_role_switch' from USB maintainer.

And I have a question.
Before this patch, extcon-axp288 is used to detect charger connector
and extcon-intel-int3496 is used to detect the USB_HOST connector
on one h/w device?

Best Regards,
Chanwoo Choi
Samsung Electronics

On 2018년 03월 01일 00:07, Hans de Goede wrote:
> The AXP288 BC1.2 charger detection / extcon code may seem like a strange
> place to add code to control the USB role-switch on devices with an AXP288,
> but there are 2 reasons to do this inside the axp288 extcon code:
> 
> 1) On many devices the USB role is controlled by ACPI AML code, but the AML
>code only switches between the host and none roles, because of Windows
>not really using device mode. To make device mode work we need to toggle
>between the none/device roles based on Vbus presence, and the axp288
>extcon gets interrupts on Vbus insertion / removal.
> 
> 2) In order for our BC1.2 charger detection to work properly the role
>mux must be properly set to device mode before we do the detection.
> 
> Also note the Kconfig help-text / obsolete depends on USB_PHY which are
> remnants from older never upstreamed code also controlling the mux from
> the axp288 extcon code.
> 
> This commit also adds code to get notifications from the INT3496 extcon
> device, which is used on some devices to notify the kernel about id-pin
> changes instead of them being handled through AML code.
> 
> This fixes:
> -Device mode not working on most CHT devices with an AXP288
> -Host mode not working on devices with an INT3496 ACPI device
> -Charger-type misdetection (always SDP) on devices with an INT3496 when the
>  USB role (always) gets initialized as host
> 
> Reviewed-by: Heikki Krogerus 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---
> Changes in v4:
> -Add Andy's Reviewed-by
> 
> Changes in v2:
> -Add depends on X86 to Kconfig (the AXP288 PMIC is only used on X86)
> -Use new acpi_dev_get_first_match_name() helper to get the INT3496 device-name
> -Add Heikki's Reviewed-by
> ---
>  drivers/extcon/Kconfig |   3 +-
>  drivers/extcon/extcon-axp288.c | 177 
> +++--
>  2 files changed, 171 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index a7bca4207f44..de15bf55895b 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -30,7 +30,8 @@ config EXTCON_ARIZONA
>  
>  config EXTCON_AXP288
>   tristate "X-Power AXP288 EXTCON support"
> - depends on MFD_AXP20X && USB_PHY
> + depends on MFD_AXP20X && USB_SUPPORT && X86
> + select USB_ROLE_SWITCH
>   help
> Say Y here to enable support for USB peripheral detection
> and USB MUX switching by X-Power AXP288 PMIC.
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 3ec4c715e240..51e77c7a32c2 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -1,6 +1,7 @@
>  /*
>   * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
>   *
> + * Copyright (c) 2017-2018 Hans de Goede 
>   * Copyright (C) 2015 Intel Corporation
>   * Author: Ramakrishna Pallala 
>   *
> @@ -14,6 +15,8 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -25,6 +28,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
>  
>  /* Power source status register */
>  #define PS_STAT_VBUS_TRIGGER BIT(0)
> @@ -97,9 +105,19 @@ struct axp288_extcon_info {
>   struct device *dev;
>   struct regmap *regmap;
>   struct regmap_irq_chip_data *regmap_irqc;
> + struct usb_role_switch *role_sw;
> + struct work_struct role_work;
>   int irq[EXTCON_IRQ_END];
>   struct extcon_dev *edev;
> + struct extcon_dev *id_extcon;
> + struct notifier_block id_nb;
>   unsigned int previous_cable;
> + bool vbus_attach;
> +};
> +
> +static const struct x86_cpu_id cherry_trail_cpu_ids[] = {
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT, X86_FEATURE_ANY },
> + {}
>  };
>  
>  /* Power up/down reason string array */
> @@ -137,20 +155,74 @@ static void axp288_extcon_log_rsi(struct 
> axp288_extcon_info *info)
>   regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
>  }
>  
> -static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> +/*
> + * The below code to control the USB role-switch on devices with an AXP288
> + * may seem out of place, but there are 2 reasons why this is the best place
> + * to control the USB role-switch on such devices:
> + * 1) On many devices the USB role is controlled by AML 

Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

2018-03-01 Thread Chanwoo Choi
On 2018년 02월 28일 22:44, Andrzej Hajda wrote:
> On 27.02.2018 23:26, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2018년 02월 27일 21:05, Andrzej Hajda wrote:
>>> On 27.02.2018 12:08, Chanwoo Choi wrote:
 Hi,

 On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
> From: Maciej Purski 
>
> Currently MHL chip must be turned on permanently to detect MHL cable. It
> duplicates micro-USB controller's (MUIC) functionality and consumes
> unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
> only if it detects MHL cable.
>
> Signed-off-by: Maciej Purski 
> Signed-off-by: Andrzej Hajda 
> ---
> v5: updated extcon API
>
> This is rework of the patch by Maciej with following changes:
> - use micro-USB port bindings to get extcon, instead of extcon property,
> - fixed remove sequence,
> - fixed extcon get state logic.
>
> Code finding extcon node is hacky IMO, I guess ultimately it should be 
> done
> via some framework (maybe even extcon), or at least via helper, I hope it
> can stay as is until the proper solution will be merged.
>
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/bridge/sil-sii8620.c | 97 
> ++--
>  1 file changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 9e785b8e0ea2..62b0adabcac2 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -17,6 +17,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -25,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -81,6 +83,10 @@ struct sii8620 {
>   struct edid *edid;
>   unsigned int gen2_write_burst:1;
>   enum sii8620_mt_state mt_state;
> + struct extcon_dev *extcon;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_wq;
> + int cable_state;
>   struct list_head mt_queue;
>   struct {
>   int r_size;
> @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct 
> sii8620 *ctx)
>   ctx->rc_dev = rc_dev;
>  }
>  
> +static void sii8620_cable_out(struct sii8620 *ctx)
> +{
> + disable_irq(to_i2c_client(ctx->dev)->irq);
> + sii8620_hw_off(ctx);
> +}
> +
> +static void sii8620_extcon_work(struct work_struct *work)
> +{
> + struct sii8620 *ctx =
> + container_of(work, struct sii8620, extcon_wq);
> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> +
> + if (state == ctx->cable_state)
> + return;
> +
> + ctx->cable_state = state;
> +
> + if (state > 0)
> + sii8620_cable_in(ctx);
> + else
> + sii8620_cable_out(ctx);
> +}
> +
> +static int sii8620_extcon_notifier(struct notifier_block *self,
> + unsigned long event, void *ptr)
> +{
> + struct sii8620 *ctx =
> + container_of(self, struct sii8620, extcon_nb);
> +
> + schedule_work(>extcon_wq);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int sii8620_extcon_init(struct sii8620 *ctx)
> +{
> + struct extcon_dev *edev;
> + struct device_node *musb, *muic;
> + int ret;
> +
> + /* get micro-USB connector node */
> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> + /* next get micro-USB Interface Controller node */
> + muic = of_get_next_parent(musb);
> +
> + if (!muic) {
> + dev_info(ctx->dev, "no extcon found, switching to 'always on' 
> mode\n");
> + return 0;
> + }
> +
> + edev = extcon_find_edev_by_node(muic);
> + of_node_put(muic);
> + if (IS_ERR(edev)) {
> + if (PTR_ERR(edev) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_err(ctx->dev, "Invalid or missing extcon\n");
> + return PTR_ERR(edev);
> + }
> +
> + ctx->extcon = edev;
> + ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
> + INIT_WORK(>extcon_wq, sii8620_extcon_work);
> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, >extcon_nb);
 You better to use devm_extcon_register_notifier().
>>> With devm version I risk that in case of device unbind notification will
>>> be called after .remove callback, it seems to me quite dangerous. Or am
>>> I missing something?
>> If you use the cancel_work_sync() in remove() instead of flush_work(),
>> you can use the 'devm_extcon_*'.
> 
> cancel_work_sync() does not prevent works scheduled later from execution
> [1] and this scenario is possible with 

usb: musb: "(null)" in sysfs mode file after disabling a gadget (and at other times, system hangs)

2018-03-01 Thread Merlijn Wajer
Hi,

I found that the "mode" file in musb sys node will return "(null)" when
one would expect it show b_idle.

Wrong /mode file:

[Fresh boot, cable not connected yet]
root@n900devuan:~# modprobe g_nokia
[Insert cable connected to PC]
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
b_peripheral
[Remove cable connected to PC]
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
b_idle
root@n900devuan:~# rmmod g_nokia
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
(null)
[Insert cable connected to PC]
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
b_idle

I would expect it to state "b_idle" instead of "(null)".
I have also been able to reproduce this "(null)" state using only
configfs (and not deprecated g_nokia module), but this example is more
cumbersome to write up)


Sometimes it crashes the entire system (reproducibly):

[Fresh boot, USB cable connect to PC at all times]
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
b_idle
root@n900devuan:~# modprobe g_nokia
root@n900devuan:~# cat
/sys/devices/platform/6800.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/mode
b_peripheral
root@n900devuan:~# rmmod g_nokia
(Hangs, reboots after a few seconds, probably due to watchdog)


This occurs on Nokia N900 (RX-51). Kernel config has:

CONFIG_USB_LIBCOMPOSITE=m
CONFIG_USB_F_*=m
CONFIG_USB_G_*=m

I don't have a trace because I don't have a serial on my Nokia N900. I
have not yet tested this on other musb devices, but I can do that if
that helps reproducing the issue.

Cheers,
Merlijn



signature.asc
Description: OpenPGP digital signature


bug report : TBR oveflow

2018-03-01 Thread ljoublanc
I'm a user of gentoo linux, and would like to raise a bug report. I hope these
are the correct channels; if not, I apologise in advance.

I'm using a realtek-based USB3 to RJ45 gigabit adapter. This plugs
directly into my laptop, which is a Toshiba Radius P20W-C-103, skylake
based, with the following controller:

```
00:14.0 USB controller: Intel Corporation Sunrise Point-LP USB 3.0
xHCI Controller (rev 21)
```

I am experiencing this on 4.9.79-r1, and also 4.14.22.

When I plug the device in, unless I disable power management on USB
hubs 3 and 4, I get errors saying 'root hub lost power or was reset'.
However, if I disable PM using powertop, I get the device to work
seemingly well. But, as soon as I start heavy transfers (in my case
distributed compile), the network device stops responding

The error messages that I'm receiving are very similar to a bug described
here: https://bugs.launchpad.net/dell-sputnik/+bug/1729674
But I've been told that it's unrelated.

First, these are the logs showing the dongle being plugged in.
```
Feb 26 20:17:09 nizuc kernel: usb usb3: root hub lost power or was reset
Feb 26 20:17:09 nizuc kernel: usb usb4: root hub lost power or was reset
Feb 26 20:17:41 nizuc kernel: usb 4-1: new SuperSpeed USB device
number 2 using xhci_hcd
Feb 26 20:17:41 nizuc kernel: usb 4-1: New USB device found,
idVendor=0bda, idProduct=8153
Feb 26 20:17:41 nizuc kernel: usb 4-1: New USB device strings: Mfr=1,
Product=2, SerialNumber=6
Feb 26 20:17:41 nizuc kernel: usb 4-1: Product: USB 10/100/1000 LAN
Feb 26 20:17:41 nizuc kernel: usb 4-1: Manufacturer: Realtek
Feb 26 20:17:41 nizuc kernel: usb 4-1: SerialNumber: 01
Feb 26 20:17:41 nizuc kernel: usb 4-1: reset SuperSpeed USB device
number 2 using xhci_hcd
Feb 26 20:17:41 nizuc NetworkManager[2049]:   [1519676261.9009]
manager: (eth0): new Ethernet device
(/org/freedesktop/NetworkManager/Devices/5)
Feb 26 20:17:41 nizuc kernel: r8152 4-1:1.0 eth0: v1.09.9
Feb 26 20:17:42 nizuc mtp-probe[3673]: checking bus 4, device 2:
"/sys/devices/pci:00/:00:1c.0/:01:00.0/usb4/4-1"
Feb 26 20:17:42 nizuc mtp-probe[3673]: bus: 4, device: 2 was not an MTP device
Feb 26 20:17:42 nizuc upowerd[2168]: unhandled action 'bind' on
/sys/devices/pci:00/:00:1c.0/:01:00.0/usb4/4-1
Feb 26 20:17:42 nizuc systemd-udevd[3676]: link_config:
autonegotiation is unset or enabled, the speed and duplex are not
writable.
Feb 26 20:17:42 nizuc kernel: r8152 4-1:1.0 enp1s0u1: renamed from eth0
Feb 26 20:17:42 nizuc NetworkManager[2049]:   [1519676262.2645]
device (eth0): interface index 4 renamed iface from 'eth0' to
'enp1s0u1'
Feb 26 20:17:42 nizuc kernel: IPv6: ADDRCONF(NETDEV_UP): enp1s0u1:
link is not ready
Feb 26 20:17:42 nizuc NetworkManager[2049]:   [1519676262.2829]
device (enp1s0u1): state change: unmanaged -> unavailable (reason
'managed', internal state 'external')
Feb 26 20:17:42 nizuc upowerd[2168]: unhandled action 'bind' on
/sys/devices/pci:00/:00:1c.0/:01:00.0/usb4/4-1/4-1:1.0
Feb 26 20:17:42 nizuc kernel: IPv6: ADDRCONF(NETDEV_UP): enp1s0u1:
link is not ready
Feb 26 20:17:46 nizuc kernel: r8152 4-1:1.0 enp1s0u1: carrier on
Feb 26 20:17:46 nizuc kernel: IPv6: ADDRCONF(NETDEV_CHANGE): enp1s0u1:
link becomes ready
```

but once I begin distrubted compile, the network drops, with the following
messages:

```
Feb 26 20:25:32 nizuc distccd[13928]: (dcc_r_token_int) got ARGV000f
Feb 26 20:25:32 nizuc distccd[13928]: (dcc_r_token_string) got '-Wa,-mtune=i686'
Feb 26 20:25:32 nizuc distccd[13928]: (dcc_r_argv) argv[22] = "-Wa,-mtune=i686"
Feb 26 20:25:32 nizuc distccd[13928]: (dcc_r_token_int) got ARGV0011
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: ERROR Transfer
event TRB DMA ptr not part of current TD ep_index 2 comp_code 13
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: Looking for
event-dma 00020c8aea40 trb-start 00020c8aea20 trb-end
00020c8aea20 seg-start 00020c8ae000 s>
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: ERROR Transfer
event TRB DMA ptr not part of current TD ep_index 2 comp_code 13
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: Looking for
event-dma 00020c8aea50 trb-start 00020c8aea20 trb-end
00020c8aea20 seg-start 00020c8ae000 s>
Feb 26 20:25:32 nizuc distccd[14319]: (dcc_check_client) connection
from 192.168.2.1:42290
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: ERROR Transfer
event TRB DMA ptr not part of current TD ep_index 2 comp_code 13
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: Looking for
event-dma 00020c8aea60 trb-start 00020c8aea20 trb-end
00020c8aea20 seg-start 00020c8ae000 s>
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: ERROR Transfer
event TRB DMA ptr not part of current TD ep_index 2 comp_code 13
Feb 26 20:25:32 nizuc kernel: xhci_hcd :01:00.0: Looking for
event-dma 00020c8aea70 trb-start 00020c8aea20 trb-end
00020c8aea20 seg-start 00020c8ae000 s>
Feb 26 20:25:32 nizuc kernel: 

Re: [PATCH 0/3] *** SUBJECT HERE ***

2018-03-01 Thread Greg KH
On Thu, Mar 01, 2018 at 07:08:28PM +0200, Dafna Hirschfeld wrote:
> *** BLURB HERE ***

I think you missed a subject and a blurb, and you sent it to the wrong
mailing list :(

try again?

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


Re: [PATCH] usb: Don't disable Latency tolerance Messaging (LTM) before port reset

2018-03-01 Thread Alan Stern
On Thu, 1 Mar 2018, Mathias Nyman wrote:

> Disabing Latency Tolerance Messaging before port reset is unnecessary.
> LTM is automatically disabled at port reset.
> 
> If host can't communicate with the device the LTM message will fail, and
> the hub driver will unnecessarily do a logical disconnect.
> Broken communication is ofter the reason for a reset in the first place.
> 
> Additionally we can't guarantee device is in a configured state,
> epecially in reset-resume case when root hub lost power.
> LTM can't be modified unless device is in a configured state.
> 
> Just remove LTM disabling before port reset.

That definitely sounds like the right approach.

> Details about LTM and port reset in USB 3 specification:
> 
> USB 3 spec section 9.4.5
> "The LTM Enable field can be modified by the SetFeature() and
> ClearFeature() requests using the LTM_ENABLE feature selector.
> This field is reset to zero when the device is reset."
> 
> USB 3 spec section 9.4.1
> "The device shall process a Clear Feature (U1_Enable or U2_Enable or
> LTM_Enable) only if the device is in the configured state."
> 
> Signed-off-by: Mathias Nyman 

Acked-by: Alan Stern 

> ---
>  drivers/usb/core/hub.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c5c1f6c..ac7bab7 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5505,21 +5505,15 @@ static int usb_reset_and_verify_device(struct 
> usb_device *udev)
>   if (udev->usb2_hw_lpm_enabled == 1)
>   usb_set_usb2_hardware_lpm(udev, 0);
>  
> - /* Disable LPM and LTM while we reset the device and reinstall the alt
> -  * settings.  Device-initiated LPM settings, and system exit latency
> -  * settings are cleared when the device is reset, so we have to set
> -  * them up again.
> + /* Disable LPM while we reset the device and reinstall the alt settings.
> +  * Device-initiated LPM, and system exit latency settings are cleared
> +  * when the device is reset, so we have to set them up again.
>*/

In theory, since you're changing this comment anyway, it could be 
reformatted into the more commonly accepted form:

/*
 * blah, blah, blah
 * blah, blah, blah
 */

But it's not a big deal.

>   ret = usb_unlocked_disable_lpm(udev);
>   if (ret) {
>   dev_err(>dev, "%s Failed to disable LPM\n", __func__);
>   goto re_enumerate_no_bos;
>   }
> - ret = usb_disable_ltm(udev);
> - if (ret) {
> - dev_err(>dev, "%s Failed to disable LTM\n", __func__);
> - goto re_enumerate_no_bos;
> - }
>  
>   bos = udev->bos;
>   udev->bos = NULL;

Alan Stern

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


Re: [PATCH] USB: storage: Add JMicron bridge 152d:2567 to unusual_devs.h

2018-03-01 Thread Alan Stern
On Thu, 1 Mar 2018, Teijo Kinnunen wrote:

> This USB-SATA controller seems to be similar with JMicron bridge
> 152d:2566 already on the list. Adding it here fixes "Invalid
> field in cdb" errors.
> 
> Signed-off-by: Teijo Kinnunen 
> Cc: sta...@vger.kernel.org

Acked-by: Alan Stern 

> ---
>   drivers/usb/storage/unusual_devs.h | 7 +++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/storage/unusual_devs.h 
> b/drivers/usb/storage/unusual_devs.h
> index 264af199aec8..747d3a9596d9 100644
> --- a/drivers/usb/storage/unusual_devs.h
> +++ b/drivers/usb/storage/unusual_devs.h
> @@ -2118,6 +2118,13 @@ UNUSUAL_DEV(  0x152d, 0x2566, 0x0114, 0x0114,
>   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>   US_FL_BROKEN_FUA ),
> 
> +/* Reported by Teijo Kinnunen  */
> +UNUSUAL_DEV(  0x152d, 0x2567, 0x0117, 0x0117,
> + "JMicron",
> + "USB to ATA/ATAPI Bridge",
> + USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> + US_FL_BROKEN_FUA ),
> +
>   /* Reported-by George Cherian  */
>   UNUSUAL_DEV(0x152d, 0x9561, 0x, 0x,
>   "JMicron",
> 

--
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: storage: Add JMicron bridge 152d:2567 to unusual_devs.h

2018-03-01 Thread Teijo Kinnunen

This USB-SATA controller seems to be similar with JMicron bridge
152d:2566 already on the list. Adding it here fixes "Invalid
field in cdb" errors.

Signed-off-by: Teijo Kinnunen 
Cc: sta...@vger.kernel.org
---
 drivers/usb/storage/unusual_devs.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/storage/unusual_devs.h 
b/drivers/usb/storage/unusual_devs.h
index 264af199aec8..747d3a9596d9 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -2118,6 +2118,13 @@ UNUSUAL_DEV(  0x152d, 0x2566, 0x0114, 0x0114,
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_BROKEN_FUA ),

+/* Reported by Teijo Kinnunen  */
+UNUSUAL_DEV(  0x152d, 0x2567, 0x0117, 0x0117,
+   "JMicron",
+   "USB to ATA/ATAPI Bridge",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_BROKEN_FUA ),
+
 /* Reported-by George Cherian  */
 UNUSUAL_DEV(0x152d, 0x9561, 0x, 0x,
"JMicron",
--
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 v2] staging: rtl8188eu: Move a blank line

2018-03-01 Thread Dafna Hirschfeld
Move a blank line from in the middle of a declaration list to
after the declaration list, to improve readability.

Issue found by checkpatch.

Signed-off-by: Dafna Hirschfeld 
---
 drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c 
b/drivers/staging/rtl8188eu/hal/usb_halinit.c
index 17967c9..c3bb183 100644
--- a/drivers/staging/rtl8188eu/hal/usb_halinit.c
+++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c
@@ -1516,8 +1516,8 @@ void rtw_hal_set_hwreg(struct adapter *Adapter, u8 
variable, u8 *val)
case HW_VAR_CAM_WRITE:
{
u32 cmd;
-
u32 *cam_val = (u32 *)val;
+
usb_write32(Adapter, WCAMI, cam_val[0]);
 
cmd = CAM_POLLINIG | CAM_WRITE | cam_val[1];
-- 
2.7.4

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


Re: patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality

2018-03-01 Thread Marc Zyngier
On 01/03/18 08:01, Bockholdt Arne wrote:
> 
> On Thu, 2018-02-15 at 19:29 +, Marc Zyngier wrote:
>> [+ Ard, who helped me chasing the initial issue]
>>
>> On 15/02/18 06:43, Bockholdt Arne wrote:
>>> Hi all,
>>>
>>> on our Intel Atom C2578 server with a SuperMicro A1SAi board and a
>>> Renesas uPD720201 USB 3.0 host controller the controller has
>>> stopped
>>> working since kernel 4.13.x. Before that kernel the dmesg messages
>>> from
>>> XHCI were:
>>>
>>> dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
>>> registered,
>>> assigned bus number 2
>>> dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: hcc params
>>> 0x014051cf
>>> hci version 0x100 quirks 0x0010
>>> dmesg-4.12.1-serverv4.log:usb usb2: Manufacturer: Linux 4.12.1-
>>> serverv4
>>> xhci-hcd
>>> dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
>>> registered,
>>> assigned bus number 3
>>> dmesg-4.12.1-serverv4.log:usb usb3: Manufacturer: Linux 4.12.1-
>>> serverv4
>>> xhci-hcd
>>>
>>> After that the message look like that:
>>>
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Resetting
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Refused to change
>>> power
>>> state, currently in D3
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
>>> Controller
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
>>> registered,
>>> assigned bus number 2
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Host halt failed,
>>> -19
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: can't setup: -19
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: USB bus 2
>>> deregistered
>>> dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: init :03:00.0
>>> fail, -19
>>>
>>> I've tested it with 4.15.3 too, it's still the same. I've narrowed
>>> it
>>> down to the following patch:
>>>
>>> commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7
>>> Author: Marc Zyngier >> .com>>
>>> Date:   Tue Aug 1 20:11:08 2017 -0500
>>>
>>> xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA
>>> issue
>>>
>>> Reverting the patch on top of 4.15.3 restores the USB3
>>> functionality on
>>> our server. Please let me know if there is anything I can do to fix
>>> the
>>> problem. Thank you.
>>
>> Hi Arne,
>>
>> This looks pretty bad. I suspect that once reset, the firmware is
>> lost.
>> I'll try to write a patch dumping some information about it.
>>
>> Ard: Do you know if the Cello board has a SPI flash connected to the
>> Renesas chip, from which it would load the firmware?
>>
>> Another possibility is that power management kicks in, and that the
>> endpoint is stuck there. Could also be firmware related, but not
>> only.
>> I'd welcome any idea on the subject, as I cannot reproduce this issue
>> on
>> the HW I have.
>>
>> It we cannot work out what exactly is causing this, we may have to
>> default to not resetting the part and rely on a command-line option
>> to
>> do it... I can't say I'm a fan.
>>
>> Thanks,
>>
>>  M.
>>
> 
> Hi Marc,
> 
> I've tested it with 4.15.7 and it's still there. Is there anything that
> I can do to help fixing this problem?

Would you mind trying the following patch and let me know if it helps?
It is not exactly pretty, but we can polish it if that solves your issue.

Thanks,

M.

>From 9a253773b289f781b7114e887481939b3021bb30 Mon Sep 17 00:00:00 2001
From: Marc Zyngier 
Date: Thu, 1 Mar 2018 17:27:42 +
Subject: [PATCH] xhci: Only reset uPD72020x if programmed with 64bit DMA
 addresses

The Renesas uPD72020x USB controller misbehaves when switching
from 64bit DMA addresses to 32bit ones, and can only accept a
new programming if hard reset first.

But it also misbehaves (in a different way) if reset for no
good reason. So let's restrict the resetting to the cases where
we detect the 64/32 issue, and leave the device alone in the
other cases.

Fixes: 8466489ef5ba ("xhci: Reset Renesas uPD72020x USB controller for 32-bit 
DMA issue")
Signed-off-by: Marc Zyngier 
---
 drivers/usb/host/pci-quirks.c | 33 ++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 67ad4bb6919a..b05a5c2ec08d 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -1277,13 +1278,39 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
 * at the wrong addresses, as it keeps the top 32bit of some
 * addresses from its previous programming under obscure
 * circumstances.
-* Give it a good wack at probe time. Unfortunately, this
+* Give it a good whack at probe time 

Re: [RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

2018-03-01 Thread Vincent Pelletier
Hello,

On Sun, 7 Jan 2018 12:48:35 +, Vincent Pelletier
 wrote:
> Some update: this patch looks good on DWC3: both unplugging and
> unbinding the UDC while AOI transfers are queued works fine.
> 
> What I intended to do is to also test it on DWC2, so I bought a
> raspberry pi zero W (ie, with embedded wifi chip on SDIO), but the fail
> is strong with me and I just cannot get a custom kernel build to boot
> (blocks right after detecting mmc0 & mmc1 and listing MMC partitions),
> so I could not extend my tests yet. I'll keep trying, but testers with
> a DWC2 or other UDC are very welcome to give that patch a try.

I could get around to build a kernel on the raspberry pi zero,
rpi-4.16.y as of 6793af87063ddb793c9b40c77a0a70bad09375c7 on
  https://github.com/raspberrypi/linux
with proposed patch applied.
My test scenario passed with dwc2 module (dwc_otg, the default
off-tree driver on raspberry-pi kernels, is host-only).

There are a few caveat though, which I believe are unrelated to this
patch:
- upon first binding the UDC to the device, there is a WARNING (see
  the end of this mail) which seems to be a known issue:
  https://lkml.org/lkml/2017/2/24/723
  In ly case, this does not seem to prevent the UDC from working.
- upon unplugging USB, the function does not receive an onDisable
  event. I believe this is because of how the raspberry pi zero board
  is wired: USB power bus is the board main 5V power bus (shorted to
  the power-only micro usb plug), so to keep the raspberry pi powered
  on despite unplugging the fully-wired port, Vbus stays high, so AFAIK
  the bus is in a state indistinguishable from HSidle.
  It is only when re-plugging that onDisable fires (because of
  re-enumeration ?), and the gadget does not seem to recover (no
  onEnable happen). It is after I unbind the UDC (to do things
  cleanly), exit the function process, start it again and re-bind the
  UDC that stuff start working again (so kernel-side can recover
  without a reboot, there is that).
  I of course do get an onUnbind if I unbind the UDC while the function
  implementation is running. No issue when doing this (especially, no
  AIO-related issue).
  No issue either when killing function program while bound to UDC and
  with in-flight AIO requests.

While I would like to test this code with more UDCs, I am not sure I
can afford to buy development boards (or otherwise linux-friendly boards
involving such chips), build linux for each and do comprehensive
testing.

Should I resubmit my patch ? It applies cleanly on linus master as of 
  f3afe530d644488a074291da04a69a296ab63046
  Merge branch 'fixes-v4.16-rc4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

Finally, the WARNING mentioned above, surrounded by UDC binding logs to
illustrate the overall sequence:

Mar 02 00:57:54 sushi kernel: dwc2 2098.usb: bound driver configfs-gadget
Mar 02 00:57:54 sushi kernel: [ cut here ]
Mar 02 00:57:55 sushi kernel: WARNING: CPU: 0 PID: 911 at 
drivers/usb/dwc2/gadget.c:289 dwc2_hsotg_init_fifo+0xf0/0x1d0 [dwc2]
Mar 02 00:57:55 sushi kernel: insufficient fifo memory
Mar 02 00:57:55 sushi kernel: Modules linked in: usb_f_fs libcomposite bnep 
hci_uart btbcm bluetooth ecdh_generic brcmfmac brcmutil sha256_generic cfg80211 
snd_bcm2835(C) rfkill snd_pcm snd_timer snd dwc2 udc_core bcm2835_gpiomem 
uio_pdrv_genirq uio i2c_dev fuse ipv6
Mar 02 00:57:55 sushi kernel: CPU: 0 PID: 911 Comm: bash Tainted: G C   
4.16.0-rc3+ #11
Mar 02 00:57:55 sushi kernel: Hardware name: BCM2835
Mar 02 00:57:55 sushi kernel: [] (unwind_backtrace) from [] 
(show_stack+0x20/0x24)
Mar 02 00:57:55 sushi kernel: [] (show_stack) from [] 
(dump_stack+0x20/0x28)
Mar 02 00:57:55 sushi kernel: [] (dump_stack) from [] 
(__warn+0xe0/0x108)
Mar 02 00:57:55 sushi kernel: [] (__warn) from [] 
(warn_slowpath_fmt+0x44/0x4c)
Mar 02 00:57:55 sushi kernel: [] (warn_slowpath_fmt) from 
[] (dwc2_hsotg_init_fifo+0xf0/0x1d0 [dwc2])
Mar 02 00:57:55 sushi kernel: [] (dwc2_hsotg_init_fifo [dwc2]) from 
[] (dwc2_hsotg_core_init_disconnected+0x88/0x3e4 [dwc2])
Mar 02 00:57:55 sushi kernel: [] (dwc2_hsotg_core_init_disconnected 
[dwc2]) from [] (dwc2_hsotg_pullup+0x108/0x14c [dwc2])
Mar 02 00:57:55 sushi kernel: [] (dwc2_hsotg_pullup [dwc2]) from 
[] (usb_gadget_connect+0x48/0xe0 [udc_core])
Mar 02 00:57:55 sushi kernel: [] (usb_gadget_connect [udc_core]) from 
[] (udc_bind_to_driver+0x100/0x108 [udc_core])
Mar 02 00:57:55 sushi kernel: [] (udc_bind_to_driver [udc_core]) from 
[] (usb_gadget_probe_driver+0xb0/0x168 [udc_core])
Mar 02 00:57:55 sushi kernel: [] (usb_gadget_probe_driver [udc_core]) 
from [] (gadget_dev_desc_UDC_store+0xc0/0xdc [libcomposite])
Mar 02 00:57:55 sushi kernel: [] (gadget_dev_desc_UDC_store 
[libcomposite]) from [] (configfs_write_file+0xe0/0x170)
Mar 02 00:57:55 sushi kernel: [] (configfs_write_file) from 
[] (__vfs_write+0x38/0x130)
Mar 02 00:57:55 sushi kernel: [] 

Re: [PATCH 3/7] usb: dwc3: pci: Store device properties dynamically

2018-03-01 Thread John Youn

On 03/01/2018 12:25 AM, Felipe Balbi wrote:


Hi,

John Youn  writes:

On 02/22/2018 07:20 AM, Andy Shevchenko wrote:

On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen  wrote:

On 2/21/2018 6:46 AM, Andy Shevchenko wrote:

On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen
 wrote:

On 2/17/2018 7:29 AM, Andy Shevchenko wrote:

On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
 wrote:

Add the ability to add device properties dynamically. Currently, device
properties are added to platform device using
platform_device_add_properties(). However, this function does not allow
adding properties incrementally. It is useful to have this ability when
the driver needs to set common device properties across different HW


I'm not sure it's useful anyhow.


or
if IP and FPGA validation test different configurations for different
HW.


Shouldn't be a separate stuff for FPGA exclusively?


Can you clarify/expand your question?


FPGA is the one which might have different properties at run time for
the same device.
So, why do we care on driver / generic level of it?

Shouldn't be FPGA manager take care of it (via DT overlays, for example)?


Normally it is handled using DT overlays. But for using FPGA as PCI
device, I'm not aware of a better solution.


If it's a PCI device, it may utilize PCI (hot plug if you would like
to change IP at run time) with appropriate IDs and stuff, right?


Introduce two new functions to do so:
 * dwc3_pci_add_one_property() - this function adds one property to
   dwc->properties array and increase its size as needed
 * dwc3_pci_add_properties() - this function takes a null terminated
   array of device properties and add them to dwc->properties


So, why you can't use ACPI / DT here?



dwc3_pci_add_properties() is a convenient function that takes statically
allocated array of (quirks) properties for different HW and store them
to dwc->properties. The idea is to add more properties on top of these
required quirks.


Yes, I understand that. What's wrong with DT? The built-in device
properties have the same nature as usual properties for DT.
Whenever driver calls device_property_read_uXX() or alike it would
check all property provides for asked one.


I may not have explained fully in my commit message the purpose of this
change. That's why I think I misinterpreted your previous question.



With this new debugging feature, we want to delay adding device
properties until the user initiates it.


So, see above.
When user wants to enable this IP at run time it will be a PCI hot
plug event which makes device appear behind PCI switch.
When device appears it would have it's own VndrID/DevID + sub pair.

Based on that IDs you may apply hard coded quirks (though I am against
quirks in new hardware) as it's done right now.



Hi Andy,

Using VID/PID is not really feasible for our use case. If we only had
a few concrete devices then it would be ok.


Agreed. VID/PID would not scale at all :-)


But understand that this an IP running on an FPGA validation
platform. The IP is very large and flexible with *many* parameters
that we must test against.  It is also deployed by our customers with
widely varying configurations. So we are constantly testing these as
well.

One of the last pieces for moving our FPGA validation completely to
the in-kernel Linux stack is the ability to configure the driver to
set parameters that are not visible to the driver, or with parameters
that we want to constrain for testing.


I'm very much interested in helping you guys validate your IP with
upstream Linux :-) My concern with this patch is that it makes dwc3-pci
super complex even for users who are not interested in IP validation.

So, instead, how about we introduce dwc3-snps.c where you guys can put
all the controls you need? We remove HAPS from current dwc3-pci.c and
move everything to dwc3-snps.c. Sure, we'd have a little duplication,
but I guess that'd be very minor.

While doing that, we can make dwc3-snps.c depend on EXPERT, so that
distros don't enable it by default.



Sure, we'll make the changes. I seem to recall we came to this same
conclusion a while back... forgot about that!

Regards,
John

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


[PATCH 1/3] staging: rtl8192e: Fix issues regarding blank lines

2018-03-01 Thread Dafna Hirschfeld
Fix multiple blank lines and blank lines after braces.
Issues found with checkpatch.pl

Signed-off-by: Dafna Hirschfeld 
---
 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 27 ---
 1 file changed, 27 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
index f802f60..1d9eb54 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_wx.c
@@ -36,7 +36,6 @@ static int _rtl92e_wx_get_freq(struct net_device *dev,
return rtllib_wx_get_freq(priv->rtllib, a, wrqu, b);
 }
 
-
 static int _rtl92e_wx_get_mode(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
@@ -149,7 +148,6 @@ static int _rtl92e_wx_set_rawtx(struct net_device *dev,
mutex_unlock(>wx_mutex);
 
return ret;
-
 }
 
 static int _rtl92e_wx_force_reset(struct net_device *dev,
@@ -165,7 +163,6 @@ static int _rtl92e_wx_force_reset(struct net_device *dev,
priv->force_reset = *extra;
mutex_unlock(>wx_mutex);
return 0;
-
 }
 
 static int _rtl92e_wx_adapter_power_status(struct net_device *dev,
@@ -231,7 +228,6 @@ static int _rtl92e_wx_set_force_lps(struct net_device *dev,
priv->force_lps = *extra;
mutex_unlock(>wx_mutex);
return 0;
-
 }
 
 static int _rtl92e_wx_set_debug(struct net_device *dev,
@@ -475,12 +471,10 @@ static int _rtl92e_wx_set_scan(struct net_device *dev,
return ret;
 }
 
-
 static int _rtl92e_wx_get_scan(struct net_device *dev,
   struct iw_request_info *a,
   union iwreq_data *wrqu, char *b)
 {
-
int ret;
struct r8192_priv *priv = rtllib_priv(dev);
 
@@ -490,7 +484,6 @@ static int _rtl92e_wx_get_scan(struct net_device *dev,
if (priv->bHwRadioOff)
return 0;
 
-
mutex_lock(>wx_mutex);
 
ret = rtllib_wx_get_scan(priv->rtllib, a, wrqu, b);
@@ -552,7 +545,6 @@ static int _rtl92e_wx_set_nick(struct net_device *dev,
memcpy(priv->nick, extra, wrqu->data.length);
mutex_unlock(>wx_mutex);
return 0;
-
 }
 
 static int _rtl92e_wx_get_nick(struct net_device *dev,
@@ -596,7 +588,6 @@ static int _rtl92e_wx_get_name(struct net_device *dev,
return rtllib_wx_get_name(priv->rtllib, info, wrqu, extra);
 }
 
-
 static int _rtl92e_wx_set_frag(struct net_device *dev,
   struct iw_request_info *info,
   union iwreq_data *wrqu, char *extra)
@@ -619,7 +610,6 @@ static int _rtl92e_wx_set_frag(struct net_device *dev,
return 0;
 }
 
-
 static int _rtl92e_wx_get_frag(struct net_device *dev,
   struct iw_request_info *info,
   union iwreq_data *wrqu, char *extra)
@@ -633,7 +623,6 @@ static int _rtl92e_wx_get_frag(struct net_device *dev,
return 0;
 }
 
-
 static int _rtl92e_wx_set_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *awrq, char *extra)
@@ -651,10 +640,8 @@ static int _rtl92e_wx_set_wap(struct net_device *dev,
mutex_unlock(>wx_mutex);
 
return ret;
-
 }
 
-
 static int _rtl92e_wx_get_wap(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *extra)
@@ -664,7 +651,6 @@ static int _rtl92e_wx_get_wap(struct net_device *dev,
return rtllib_wx_get_wap(priv->rtllib, info, wrqu, extra);
 }
 
-
 static int _rtl92e_wx_get_enc(struct net_device *dev,
  struct iw_request_info *info,
  union iwreq_data *wrqu, char *key)
@@ -707,7 +693,6 @@ static int _rtl92e_wx_set_enc(struct net_device *dev,
ret = rtllib_wx_set_encode(priv->rtllib, info, wrqu, key);
mutex_unlock(>wx_mutex);
 
-
if (wrqu->encoding.flags & IW_ENCODE_DISABLED) {
ieee->pairwise_key_type = ieee->group_key_type = KEY_TYPE_NA;
rtl92e_cam_reset(dev);
@@ -716,7 +701,6 @@ static int _rtl92e_wx_set_enc(struct net_device *dev,
goto end_hw_sec;
}
if (wrqu->encoding.length != 0) {
-
for (i = 0; i < 4; i++) {
hwkey[i] |=  key[4*i+0]
if (i == 1 && (4 * i + 1) == wrqu->encoding.length)
@@ -786,8 +770,6 @@ static int _rtl92e_wx_set_scan_type(struct net_device *dev,
return 1;
 }
 
-
-
 #define R8192_MAX_RETRY 255
 static int _rtl92e_wx_set_retry(struct net_device *dev,
struct iw_request_info *info,
@@ -833,7 +815,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,
 {
struct r8192_priv *priv = rtllib_priv(dev);
 
-
wrqu->retry.disabled = 0; /* can't be disabled */
 
if 

[PATCH 0/3] *** SUBJECT HERE ***

2018-03-01 Thread Dafna Hirschfeld
*** BLURB HERE ***

Dafna Hirschfeld (3):
  staging: rtl8192e: Fix issues regarding blank lines
  staging: rtl8192e: Remove unnecessary parentheses
  staging: rtl8192e: Add spaces around operators.

 drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 51 +++---
 1 file changed, 12 insertions(+), 39 deletions(-)

-- 
2.7.4

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


Re: [PATCH 2/3] usb: xhci: tegra: Add runtime PM support

2018-03-01 Thread Mathias Nyman

On 14.02.2018 18:34, Jon Hunter wrote:

Add runtime PM support to the Tegra XHCI driver and move the function
calls to enable/disable the clocks, regulators and PHY into the runtime
PM callbacks.

Signed-off-by: Jon Hunter 
---
  drivers/usb/host/xhci-tegra.c | 80 ++-
  1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 02b0b24faa58..42aa67858b53 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -18,6 +18,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -1067,22 +1068,12 @@ static int tegra_xusb_probe(struct platform_device 
*pdev)
 */
platform_set_drvdata(pdev, tegra);
  
-	err = tegra_xusb_clk_enable(tegra);

-   if (err) {
-   dev_err(>dev, "failed to enable clocks: %d\n", err);
-   goto put_usb2;
-   }
-
-   err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies);
-   if (err) {
-   dev_err(>dev, "failed to enable regulators: %d\n", err);
-   goto disable_clk;
-   }
+   pm_runtime_enable(>dev);
  
-	err = tegra_xusb_phy_enable(tegra);

+   err = pm_runtime_get_sync(>dev);
if (err < 0) {


Does this mean that if runtime PM is disabled then clocks and regulator will 
never be enabled
for Tegra xhci?

How about keeping the clock and regualtor enabling in probe, and instead add 
something like:

pm_runtime_set_active(>dev);
pm_runtime_enable(>dev);
pm_runtime_get_noresume(>dev);

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


Re: [PATCH] usb: Don't disable Latency tolerance Messaging (LTM) before port reset

2018-03-01 Thread Mathias Nyman

On 01.03.2018 13:56, Greg KH wrote:

On Thu, Mar 01, 2018 at 01:15:28PM +0200, Mathias Nyman wrote:

Disabing Latency Tolerance Messaging before port reset is unnecessary.
LTM is automatically disabled at port reset.

If host can't communicate with the device the LTM message will fail, and
the hub driver will unnecessarily do a logical disconnect.
Broken communication is ofter the reason for a reset in the first place.

Additionally we can't guarantee device is in a configured state,
epecially in reset-resume case when root hub lost power.
LTM can't be modified unless device is in a configured state.

Just remove LTM disabling before port reset.

Details about LTM and port reset in USB 3 specification:

USB 3 spec section 9.4.5
"The LTM Enable field can be modified by the SetFeature() and
ClearFeature() requests using the LTM_ENABLE feature selector.
This field is reset to zero when the device is reset."

USB 3 spec section 9.4.1
"The device shall process a Clear Feature (U1_Enable or U2_Enable or
LTM_Enable) only if the device is in the configured state."

Signed-off-by: Mathias Nyman 
---
  drivers/usb/core/hub.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)


Is this causing problems now such that we should get this to older
kernels as well?



Could be, it can reduce extra logical disconnects on reset on resume.
But this didn't resolve some some real life bug, it was one thing Alan 
commented on
while debugging a failing USB3 device reset.

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


Re: [PATCH] usb: Don't disable Latency tolerance Messaging (LTM) before port reset

2018-03-01 Thread Greg KH
On Thu, Mar 01, 2018 at 01:15:28PM +0200, Mathias Nyman wrote:
> Disabing Latency Tolerance Messaging before port reset is unnecessary.
> LTM is automatically disabled at port reset.
> 
> If host can't communicate with the device the LTM message will fail, and
> the hub driver will unnecessarily do a logical disconnect.
> Broken communication is ofter the reason for a reset in the first place.
> 
> Additionally we can't guarantee device is in a configured state,
> epecially in reset-resume case when root hub lost power.
> LTM can't be modified unless device is in a configured state.
> 
> Just remove LTM disabling before port reset.
> 
> Details about LTM and port reset in USB 3 specification:
> 
> USB 3 spec section 9.4.5
> "The LTM Enable field can be modified by the SetFeature() and
> ClearFeature() requests using the LTM_ENABLE feature selector.
> This field is reset to zero when the device is reset."
> 
> USB 3 spec section 9.4.1
> "The device shall process a Clear Feature (U1_Enable or U2_Enable or
> LTM_Enable) only if the device is in the configured state."
> 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/core/hub.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)

Is this causing problems now such that we should get this to older
kernels 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


RE: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow

2018-03-01 Thread 李書帆
Hi Jun,

> -Original Message-
> From: Jun Li [mailto:jun...@nxp.com]
> Sent: Thursday, March 01, 2018 6:06 PM
> To: shufan_lee(李書帆); ShuFanLee; heikki.kroge...@linux.intel.com; 
> li...@roeck-us.net; g...@kroah.com
> Cc: cy_huang(黃啟原); linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; 
> dl-linux-imx
> Subject: RE: [PATCH] staging: typec: handle vendor defined part and modify 
> drp toggling flow
>
> Hi Shufan
>
> Please don't top posting
>
> -Original Message-
> From: shufan_lee(李書帆) [mailto:shufan_...@richtek.com]
> Sent: 2018年3月1日 16:49
> To: Jun Li ; ShuFanLee ;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net; g...@kroah.com
> Cc: cy_huang(黃啟原) ;
> linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> 
> Subject: RE: [PATCH] staging: typec: handle vendor defined part and
> modify drp toggling flow
>
> Hi Jun,
>
>   The attachment is waveform of the condition we met but I'm not sure
> whether you can download the attachment.
>   I add log in RT1711H it shows as following:
>
> You don't need add log by your own.
> There is tcpm(./drivers/usb/typec/tcpm.c) log for debug already, which can 
> show all the events and state transitions, you can get it by below command as 
> I commented:
>
> cat /sys/kernel/debug/tcpm/x(your tcpc i2c device)
>
After applying your patch[2], TCPM log is as following:

[   53.050602] CC1: 0 -> 2, CC2: 0 -> 0 [state DRP_TOGGLING, polarity 0, 
connected]
[   53.050613] state change DRP_TOGGLING -> SRC_ATTACH_WAIT
[   53.050678] pending state change SRC_ATTACH_WAIT -> SNK_TRY @ 200 ms
=> Rd is plugged out
[   53.178804] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_ATTACH_WAIT, polarity 0, 
disconnected]
[   53.178815] state change SRC_ATTACH_WAIT -> SRC_UNATTACHED
=> Rd is plugged in
[   53.178874] Start DRP toggling
[   53.188472] CC1: 0 -> 0, CC2: 0 -> 0 [state DRP_TOGGLING, polarity 0, 
disconnected]

If TCPM does not enter SRC_ATTACHED state, RC.DRP will not be cleared.
When TCPM writes Rd/Rd or Rp/Rp in the drp_toggling function, it does not take 
effect until LOOK4CONNECTION command is set.
The above condition causes RT1711H reports open/open at [53.188472]

> [ 914.937340] typec_rt1711h 2-004e: __rt1711h_irq_handler [
> 914.943838] typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 = Open
> => Device(Rd) is plugged out
>
> [ 914.958041] typec_rt1711h 2-004e: tcpm_set_pd_rx 0 [ 914.963011]
> typec_rt1711h 2-004e: tcpm_set_vbus vbus = 0 [ 914.968407]
> typec_rt1711h
> 2-004e: tcpm_set_vbus chg is already 0 [ 914.974541] typec_rt1711h 2-004e:
> tcpm_set_vconn vconn is already 0 [ 914.980921] typec_rt1711h 2-004e:
> tcpm_set_current_limit 0 ma, 0 mv (not implemented) [ 914.988894]
> typec_rt1711h 2-004e: tcpm_set_polarity Polarity_CC1 [ 915.003201]
> typec_rt1711h 2-004e: tcpm_set_roles Source Host [ 915.009264]
> typec_rt1711h 2-004e: tcpm_start_drp_toggling => state_machine_work of
> TCPM calls start_drp_toggling function but does not set
> LOOK4CONNECTION command yet => (Note1) Device(Rd) is plugged in
> (RT1711H's internal firmware detects Rd plugged in. cc_change is
> triggered and it will be vRd-connected at this moment) => TCPM writes
> LOOK4CONNECTION command
> - Because RC.DRP is still 1, RT1711H will not pull cc1/cc2 to Rd while
> writing Rd/Rd to RC.CC1/RC.CC2.
> - (Note2) Right after LOOK4CONNECTION command is written, RT1711H
> pulls CC to Rd's level (because RC.Role is Rd/Rd), stop toggling
> (because
> device(Rd) is already connected) and CC status will be open/open now
> (because RT1711H presents Rd and device is connected(Rd))
>
> [ 915.037263] typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 =
> Open => Enter RT1711H's irq handler and it reports open/open
>
> I think the point is to write RC.DRP = 0 at the beginning of
> drp_toggling so that RT1711H will pull cc1/cc2 to Rd while writing
> Rd/Rd to RC.CC1/RC.CC2 This operation will make RT1711H's internal
> firmware restarts from disconnected state and toggles correctly.
>
> According to TCPCI spec (4.4.5.2):
> It is recommended the TCPM write ROLE_CONTROL.DRP=0 before writing to
> POWER_CONTROL.AutoDischargeDisconnect and starting the DRP toggling
> using COMMAND.Look4Connection Restart DRP Toggling => It is
> recommended the TCPM write ROLE_CONTROL.DRP=0 here Set
>
> This statement is_not_ recommend you do this(RC.DRP=0) for start drp 
> toggling, Please carefully check the whole sentence:
> "... as shown in Figure 4-16, "
> If you look at "Figure 4-16. DRP Initialization and Connection Detection"
> It gives clear drp toggling start operations:
>
> Set TCPC to DRP
> - Read PS.TCPCInitializationStatus
> - Write ROLE_CONTROL
> - RC.DRP = 1b
> - RC.CC2=01b or 10b
> - RC.CC1=01b or 10b
> - RC.CC1=RC.CC2
> - Write COMMAND.Look4ConnectionPS.
>
> Above is all operations required to start drp toggling. You also can see 
> RC.CCx = 01b or 10b, not fixed to be Rd, right?
Yes, this one should be 

Re: [PATCH] usb: Don't disable Latency tolerance Messaging (LTM) before port reset

2018-03-01 Thread Sergei Shtylyov
Hello!

On 03/01/2018 02:15 PM, Mathias Nyman wrote:

> Disabing Latency Tolerance Messaging before port reset is unnecessary.
> LTM is automatically disabled at port reset.
> 
> If host can't communicate with the device the LTM message will fail, and
> the hub driver will unnecessarily do a logical disconnect.
> Broken communication is ofter the reason for a reset in the first place.

   Often.

> Additionally we can't guarantee device is in a configured state,
> epecially in reset-resume case when root hub lost power.

   Especially.

> LTM can't be modified unless device is in a configured state.
> 
> Just remove LTM disabling before port reset.
> 
> Details about LTM and port reset in USB 3 specification:
> 
> USB 3 spec section 9.4.5
> "The LTM Enable field can be modified by the SetFeature() and
> ClearFeature() requests using the LTM_ENABLE feature selector.
> This field is reset to zero when the device is reset."
> 
> USB 3 spec section 9.4.1
> "The device shall process a Clear Feature (U1_Enable or U2_Enable or
> LTM_Enable) only if the device is in the configured state."
> 
> Signed-off-by: Mathias Nyman 
[...]

MBR, Sergei
--
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: Don't disable Latency tolerance Messaging (LTM) before port reset

2018-03-01 Thread Mathias Nyman
Disabing Latency Tolerance Messaging before port reset is unnecessary.
LTM is automatically disabled at port reset.

If host can't communicate with the device the LTM message will fail, and
the hub driver will unnecessarily do a logical disconnect.
Broken communication is ofter the reason for a reset in the first place.

Additionally we can't guarantee device is in a configured state,
epecially in reset-resume case when root hub lost power.
LTM can't be modified unless device is in a configured state.

Just remove LTM disabling before port reset.

Details about LTM and port reset in USB 3 specification:

USB 3 spec section 9.4.5
"The LTM Enable field can be modified by the SetFeature() and
ClearFeature() requests using the LTM_ENABLE feature selector.
This field is reset to zero when the device is reset."

USB 3 spec section 9.4.1
"The device shall process a Clear Feature (U1_Enable or U2_Enable or
LTM_Enable) only if the device is in the configured state."

Signed-off-by: Mathias Nyman 
---
 drivers/usb/core/hub.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c5c1f6c..ac7bab7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5505,21 +5505,15 @@ static int usb_reset_and_verify_device(struct 
usb_device *udev)
if (udev->usb2_hw_lpm_enabled == 1)
usb_set_usb2_hardware_lpm(udev, 0);
 
-   /* Disable LPM and LTM while we reset the device and reinstall the alt
-* settings.  Device-initiated LPM settings, and system exit latency
-* settings are cleared when the device is reset, so we have to set
-* them up again.
+   /* Disable LPM while we reset the device and reinstall the alt settings.
+* Device-initiated LPM, and system exit latency settings are cleared
+* when the device is reset, so we have to set them up again.
 */
ret = usb_unlocked_disable_lpm(udev);
if (ret) {
dev_err(>dev, "%s Failed to disable LPM\n", __func__);
goto re_enumerate_no_bos;
}
-   ret = usb_disable_ltm(udev);
-   if (ret) {
-   dev_err(>dev, "%s Failed to disable LTM\n", __func__);
-   goto re_enumerate_no_bos;
-   }
 
bos = udev->bos;
udev->bos = NULL;
-- 
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: Continuous USB resets on NXP i.MX 6ULL device

2018-03-01 Thread Stefan Wahren
Hi Peter,

Am 01.03.2018 um 03:00 schrieb Peter Chen:
> On Wed, Feb 28, 2018 at 06:08:20PM +0100, Stefan Wahren wrote:
>> Hi,
>>
>> i'm not sure this is related, but i noticed something ugly in
>> driver/usb/chipidea/usbmisc_imx.c.
>>
>> The compatible "fsl,imx6ul-usbmisc" uses imx6sx_usbmisc_ops, which uses
>> usbmisc_imx6sx_init (which also calls usbmisc_imx6q_init).
>> Within usbmisc_imx6q_init the flag MX6_BM_NON_BURST_SETTING is enabled.
>>
>> But according to the i.MX6 ULL reference manual this bit is marked as
>> reserved and set to zero at reset.
>>
> For non-core registers, i.mx6ull can use i.mx6q as its base setting, the
> bits you mentioned may also be reserved at imx6qdl RM.
>
just for my understanding: MX6_BM_NON_BURST_SETTING is a no-op on
i.MX6ULL or not properly documented in reference manual?

--
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 RESEND 0/2] Fix STM32F7 DWC2 OTG HS binding

2018-03-01 Thread Amelie Delaunay
This patchset fixes STM32F7 DWC2 OTG HS binding. It actually
re-applies the v2 former patches adding STM32F7 DWC2 OTG HS support.
Bindings reviewed by Rob Herring.

Amelie Delaunay (2):
  dt-bindings: usb: fix the STM32F7 DWC2 OTG HS core binding
  usb: dwc2: fix STM32F7 USB OTG HS compatible

 Documentation/devicetree/bindings/usb/dwc2.txt | 2 +-
 drivers/usb/dwc2/params.c  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.7.4

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


[PATCH RESEND 1/2] dt-bindings: usb: fix the STM32F7 DWC2 OTG HS core binding

2018-03-01 Thread Amelie Delaunay
This patch fixes binding documentation for DWC2 controller in HS mode
found on STMicroelectronics STM32F7 SoC.
The v2 former patch [1] had been acked by Rob Herring, but v1 was merged.

[1] https://patchwork.kernel.org/patch/9925575/

Fixes: 000777dadc7e ("dt-bindings: usb: Document the STM32F7xx DWC2 ...")
Signed-off-by: Amelie Delaunay 
Reviewed-by: Rob Herring 
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index e64d903..46da5f1 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -19,7 +19,7 @@ Required properties:
   configured in FS mode;
   - "st,stm32f4x9-hsotg": The DWC2 USB HS controller instance in STM32F4x9 SoCs
   configured in HS mode;
-  - "st,stm32f7xx-hsotg": The DWC2 USB HS controller instance in STM32F7xx SoCs
+  - "st,stm32f7-hsotg": The DWC2 USB HS controller instance in STM32F7 SoCs
 configured in HS mode;
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
-- 
2.7.4

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


[PATCH RESEND 2/2] usb: dwc2: fix STM32F7 USB OTG HS compatible

2018-03-01 Thread Amelie Delaunay
This patch fixes compatible for STM32F7 USB OTG HS and consistently rename
dw2_set_params function.
The v2 former patch [1] had been acked by Paul Young, but v1 was merged.

[1] https://patchwork.kernel.org/patch/9925573/

Fixes: d8fae8b93682 ("usb: dwc2: add support for STM32F7xx USB OTG HS")
Signed-off-by: Amelie Delaunay 
---
 drivers/usb/dwc2/params.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 03fd20f..c4a4749 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -137,7 +137,7 @@ static void dwc2_set_stm32f4x9_fsotg_params(struct 
dwc2_hsotg *hsotg)
p->activate_stm_fs_transceiver = true;
 }
 
-static void dwc2_set_stm32f7xx_hsotg_params(struct dwc2_hsotg *hsotg)
+static void dwc2_set_stm32f7_hsotg_params(struct dwc2_hsotg *hsotg)
 {
struct dwc2_core_params *p = >params;
 
@@ -164,8 +164,8 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "st,stm32f4x9-fsotg",
  .data = dwc2_set_stm32f4x9_fsotg_params },
{ .compatible = "st,stm32f4x9-hsotg" },
-   { .compatible = "st,stm32f7xx-hsotg",
- .data = dwc2_set_stm32f7xx_hsotg_params },
+   { .compatible = "st,stm32f7-hsotg",
+ .data = dwc2_set_stm32f7_hsotg_params },
{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.7.4

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


RE: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow

2018-03-01 Thread Jun Li
Hi Shufan

Please don't top posting

> -Original Message-
> From: shufan_lee(李書帆) [mailto:shufan_...@richtek.com]
> Sent: 2018年3月1日 16:49
> To: Jun Li ; ShuFanLee ;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net; g...@kroah.com
> Cc: cy_huang(黃啟原) ;
> linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> 
> Subject: RE: [PATCH] staging: typec: handle vendor defined part and modify
> drp toggling flow
> 
> Hi Jun,
> 
>   The attachment is waveform of the condition we met but I'm not sure
> whether you can download the attachment.
>   I add log in RT1711H it shows as following:

You don't need add log by your own.
There is tcpm(./drivers/usb/typec/tcpm.c) log for debug already, which can
show all the events and state transitions, you can get it by below command
as I commented:

cat /sys/kernel/debug/tcpm/x(your tcpc i2c device)

> [ 914.937340] typec_rt1711h 2-004e: __rt1711h_irq_handler [ 914.943838]
> typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 = Open => Device(Rd)
> is plugged out
> 
> [ 914.958041] typec_rt1711h 2-004e: tcpm_set_pd_rx 0 [ 914.963011]
> typec_rt1711h 2-004e: tcpm_set_vbus vbus = 0 [ 914.968407] typec_rt1711h
> 2-004e: tcpm_set_vbus chg is already 0 [ 914.974541] typec_rt1711h 2-004e:
> tcpm_set_vconn vconn is already 0 [ 914.980921] typec_rt1711h 2-004e:
> tcpm_set_current_limit 0 ma, 0 mv (not implemented) [ 914.988894]
> typec_rt1711h 2-004e: tcpm_set_polarity Polarity_CC1 [ 915.003201]
> typec_rt1711h 2-004e: tcpm_set_roles Source Host [ 915.009264]
> typec_rt1711h 2-004e: tcpm_start_drp_toggling => state_machine_work of
> TCPM calls start_drp_toggling function but does not set
> LOOK4CONNECTION command yet => (Note1) Device(Rd) is plugged in
> (RT1711H's internal firmware detects Rd plugged in. cc_change is triggered
> and it will be vRd-connected at this moment) => TCPM writes
> LOOK4CONNECTION command
> - Because RC.DRP is still 1, RT1711H will not pull cc1/cc2 to Rd while writing
> Rd/Rd to RC.CC1/RC.CC2.
> - (Note2) Right after LOOK4CONNECTION command is written, RT1711H
> pulls CC to Rd's level (because RC.Role is Rd/Rd), stop toggling (because
> device(Rd) is already connected) and CC status will be open/open now
> (because RT1711H presents Rd and device is connected(Rd))
> 
> [ 915.037263] typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 =
> Open => Enter RT1711H's irq handler and it reports open/open
> 
> I think the point is to write RC.DRP = 0 at the beginning of drp_toggling so
> that RT1711H will pull cc1/cc2 to Rd while writing Rd/Rd to RC.CC1/RC.CC2
> This operation will make RT1711H's internal firmware restarts from
> disconnected state and toggles correctly.
> 
> According to TCPCI spec (4.4.5.2):
> It is recommended the TCPM write ROLE_CONTROL.DRP=0 before writing to
> POWER_CONTROL.AutoDischargeDisconnect and starting the DRP toggling
> using COMMAND.Look4Connection Restart DRP Toggling => It is
> recommended the TCPM write ROLE_CONTROL.DRP=0 here Set

This statement is_not_ recommend you do this(RC.DRP=0) for start drp toggling,
Please carefully check the whole sentence:
"... as shown in Figure 4-16, "
If you look at "Figure 4-16. DRP Initialization and Connection Detection"
It gives clear drp toggling start operations:

Set TCPC to DRP
- Read PS.TCPCInitializationStatus
- Write ROLE_CONTROL
- RC.DRP = 1b
- RC.CC2=01b or 10b
- RC.CC1=01b or 10b
- RC.CC1=RC.CC2
- Write COMMAND.Look4ConnectionPS.

Above is all operations required to start drp toggling. You also
can see RC.CCx = 01b or 10b, not fixed to be Rd, right?

Go on to check the Figure 4-16 
After debounce, we need do following:

ConnectionDetermine CC & VCONN
- Write RC.CC1 & RC.CC2 per decision
- Write RC.DRP=0
- Write TCPC_CONTROl.PlugOrientation
- Write PC.AutoDischargeDisconnect=1
 & PC.EnableVconnConnection

Current existing tcpm+tcpci will not clear RC.DRP after attach,
That means RC.DRP clear may be done after attach, not in start_drp_toggling.
I am not sure if this can resolve your problem, but I think it deserve
a try, you can follow above operation sequence while entering
attach state, refer to my patch[2]:

[2] https://www.spinics.net/lists/devicetree/msg216852.html

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 530a5d7..7145771 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -184,6 +184,7 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
  enum typec_cc_polarity polarity)
 {
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+   unsigned int reg;
int ret;

ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
@@ -192,6 +193,20 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
if (ret < 0)
return ret;

+   ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, );
+   if (ret < 0)
+   return ret;
+
+   if 

[PATCH v4 1/2] dt-bindings: usb: ehci: add optional external vbus supply property

2018-03-01 Thread Amelie Delaunay
On some boards, especially when vbus supply requires large current,
and the charge pump on the PHY isn't enough, an external vbus power switch
per port may be used.
Add portN_vbus-supply property to usb-ehci bindings. As the number of ports
depends on the ehci controller, and the port on which an external vbus
supply depends on the platform,  is used to make it generic.

Signed-off-by: Amelie Delaunay 
---
 Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt 
b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index 3efde12..cd576db 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -19,6 +19,7 @@ Optional properties:
  - phys : phandle + phy specifier pair
  - phy-names : "usb"
  - resets : phandle + reset specifier pair
+ - portN_vbus-supply : phandle of regulator supplying vbus for port N
 
 Example (Sequoia 440EPx):
 ehci@e300 {
-- 
2.7.4

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


[PATCH v4 2/2] usb: host: ehci-platform: add support for optional external vbus supply

2018-03-01 Thread Amelie Delaunay
On some boards, especially when vbus supply requires large current,
and the charge pump on the PHY isn't enough, an external vbus power switch
may be used.
Add support for optional external vbus supply per port in ehci-platform.

Signed-off-by: Amelie Delaunay 
Reviewed-by: Roger Quadros 
---
 drivers/usb/host/ehci-platform.c | 51 +++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index b065a96..e0dace7 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -46,6 +47,7 @@ struct ehci_platform_priv {
struct reset_control *rsts;
struct phy **phys;
int num_phys;
+   struct regulator **vbus_supplies;
bool reset_on_resume;
 };
 
@@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct usb_ehci_pdata *pdata = dev_get_platdata(>dev);
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-   int retval;
+   struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int portnum, n_ports, retval;
 
ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
 
@@ -71,11 +74,56 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
if (retval)
return retval;
 
+   n_ports = HCS_N_PORTS(ehci->hcs_params);
+   priv->vbus_supplies = devm_kcalloc(>dev, n_ports,
+  sizeof(*priv->vbus_supplies),
+  GFP_KERNEL);
+   if (!priv->vbus_supplies)
+   return -ENOMEM;
+
+   for (portnum = 0; portnum < n_ports; portnum++) {
+   struct regulator *vbus_supply;
+   char id[20];
+
+   sprintf(id, "port%d_vbus", portnum);
+
+   vbus_supply = devm_regulator_get_optional(>dev, id);
+   if (IS_ERR(vbus_supply)) {
+   retval = PTR_ERR(vbus_supply);
+   if (retval != ENODEV)
+   return retval;
+   } else {
+   priv->vbus_supplies[portnum] = vbus_supply;
+   }
+   }
+
if (pdata->no_io_watchdog)
ehci->need_io_watchdog = 0;
return 0;
 }
 
+static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum,
+   bool enable)
+{
+   struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int ret;
+
+   if (!priv->vbus_supplies[portnum])
+   return 0;
+
+   if (enable)
+   ret = regulator_enable(priv->vbus_supplies[portnum]);
+   else
+   ret = regulator_disable(priv->vbus_supplies[portnum]);
+
+   if (ret)
+   dev_err(hcd->self.controller,
+   "failed to %s vbus supply for port %d: %d\n",
+   enable ? "enable" : "disable", portnum, ret);
+
+   return ret;
+}
+
 static int ehci_platform_power_on(struct platform_device *dev)
 {
struct usb_hcd *hcd = platform_get_drvdata(dev);
@@ -134,6 +182,7 @@ static struct hc_driver __read_mostly 
ehci_platform_hc_driver;
 static const struct ehci_driver_overrides platform_overrides __initconst = {
.reset =ehci_platform_reset,
.extra_priv_size =  sizeof(struct ehci_platform_priv),
+   .port_power =   ehci_platform_port_power,
 };
 
 static struct usb_ehci_pdata ehci_platform_defaults = {
-- 
2.7.4

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


[PATCH v4 0/2] usb: host: ehci-platform: add support for optional external vbus supply

2018-03-01 Thread Amelie Delaunay
This patchset adds support for optional external vbus supply per port in
ehci-platform.

On some boards, especially when vbus supply requires large current,
and the charge pump on the PHY isn't enough, an external vbus power switch
per port may be used.

---
Changes in v4:
 * Address Roger Quadros comments: split dt-bindings and driver changes.
 * Address Robin Murphy comments: simplify vbus_spplies initialization.

Changes in v3:
 * Address Felipe Balbi comments: reduce indentation in
   ehci_platform_port_power.
 * Address Roger Quadros and Alan Stern comments: platforms can have one
   external vbus supply per port, so add support to get as many optional
   regulator as implemented ports on the host controller.

Changes in v2:
 * Address Roger Quadros comments: move regulator_enable/disable from
   ehci_platform_power_on/off to ehci_platform_port_power.

Amelie Delaunay (2):
  dt-bindings: usb: ehci: add optional external vbus supply property
  usb: host: ehci-platform: add support for optional external vbus
supply

 Documentation/devicetree/bindings/usb/usb-ehci.txt |  1 +
 drivers/usb/host/ehci-platform.c   | 51 +-
 2 files changed, 51 insertions(+), 1 deletion(-)

-- 
2.7.4

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


Re: [PATCH v5 01/12] drivers: base: Unified device connection lookup

2018-03-01 Thread Hans de Goede

Hi,

On 01-03-18 10:32, Andy Shevchenko wrote:

On Thu, Mar 1, 2018 at 9:28 AM, Heikki Krogerus
 wrote:

Hi,

On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:

+struct device *device_find_connection(struct device *dev, const char
+*con_id) {
+   return __device_find_connection(dev, con_id, generic_match, NULL); }


-   return __device_find_connection(dev, con_id, generic_match, NULL);
+   return __device_find_connection(dev, con_id, NULL, generic_match);


Good catch!


It seems I proposed to put function first parameter followed by opaque
data pointer for it.
In that case it would be exactly like now.


Yes, but as mentioned I decided to keep it as is, so this is really a bug,
will fix for v6.

Regards,

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


Re: [PATCH v5 01/12] drivers: base: Unified device connection lookup

2018-03-01 Thread Andy Shevchenko
On Thu, Mar 1, 2018 at 9:28 AM, Heikki Krogerus
 wrote:
> Hi,
>
> On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:
>> > +struct device *device_find_connection(struct device *dev, const char
>> > +*con_id) {
>> > +   return __device_find_connection(dev, con_id, generic_match, NULL); }
>>
>> -   return __device_find_connection(dev, con_id, generic_match, NULL);
>> +   return __device_find_connection(dev, con_id, NULL, generic_match);
>
> Good catch!

It seems I proposed to put function first parameter followed by opaque
data pointer for it.
In that case it would be exactly like now.

Though, I didn't check if the parameter ordering was changed in the prototype.

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


Re: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply

2018-03-01 Thread Amelie DELAUNAY
Hi Robin,

On 02/28/2018 02:33 PM, Robin Murphy wrote:
> Hi Amelie,
> 
> Just a couple of drive-by coding style comments...
> 
> On 23/02/18 13:46, Amelie Delaunay wrote:
>> On some boards, especially when vbus supply requires large current,
>> and the charge pump on the PHY isn't enough, an external vbus power switch
>> may be used.
>> Add support for optional external vbus supply per port in ehci-platform.
>>
>> Signed-off-by: Amelie Delaunay 
>>
>> ---
>> Changes in v3:
>>* Address Felipe Balbi comments: reduce indentation in
>>  ehci_platform_port_power.
>>* Address Roger Quadros and Alan Stern comments: platforms can have one
>>  external vbus supply per port, so add support to get as many optional
>>  regulator as implemented ports on the host controller.
>>
>> Changes in v2:
>>* Address Roger Quadros comments: move regulator_enable/disable from
>>  ehci_platform_power_on/off to ehci_platform_port_power.
>> ---
>>Documentation/devicetree/bindings/usb/usb-ehci.txt |  1 +
>>drivers/usb/host/ehci-platform.c   | 52 
>> +-
>>2 files changed, 52 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt 
>> b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> index 3efde12..cd576db 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> @@ -19,6 +19,7 @@ Optional properties:
>> - phys : phandle + phy specifier pair
>> - phy-names : "usb"
>> - resets : phandle + reset specifier pair
>> + - portN_vbus-supply : phandle of regulator supplying vbus for port N
>>
>>Example (Sequoia 440EPx):
>>ehci@e300 {
>> diff --git a/drivers/usb/host/ehci-platform.c 
>> b/drivers/usb/host/ehci-platform.c
>> index b065a96..8e9f201 100644
>> --- a/drivers/usb/host/ehci-platform.c
>> +++ b/drivers/usb/host/ehci-platform.c
>> @@ -29,6 +29,7 @@
>>#include 
>>#include 
>>#include 
>> +#include 
>>#include 
>>#include 
>>#include 
>> @@ -46,6 +47,7 @@ struct ehci_platform_priv {
>>  struct reset_control *rsts;
>>  struct phy **phys;
>>  int num_phys;
>> +struct regulator **vbus_supplies;
>>  bool reset_on_resume;
>>};
>>
>> @@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>  struct platform_device *pdev = to_platform_device(hcd->self.controller);
>>  struct usb_ehci_pdata *pdata = dev_get_platdata(>dev);
>>  struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> -int retval;
>> +struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
>> +int portnum, n_ports, retval;
>>
>>  ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
>>
>> @@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>  if (retval)
>>  return retval;
>>
>> +n_ports = HCS_N_PORTS(ehci->hcs_params);
>> +priv->vbus_supplies = devm_kcalloc(>dev, n_ports,
>> +   sizeof(struct regulator *),
> 
> Using "sizeof(*priv->vbus_supplies)" here will prevent people sending
> annoying cleanup patches later.
> 

OK, I will fix it in v4.

>> +   GFP_KERNEL);
>> +if (!priv->vbus_supplies)
>> +return -ENOMEM;
>> +
>> +for (portnum = 0; portnum < n_ports; portnum++) {
>> +struct regulator *vbus_supply;
>> +char id[20];
>> +
>> +sprintf(id, "port%d_vbus", portnum);
>> +
>> +vbus_supply = devm_regulator_get_optional(>dev, id);
>> +if (IS_ERR(vbus_supply)) {
>> +retval = PTR_ERR(vbus_supply);
>> +if (retval == -ENODEV)
>> +priv->vbus_supplies[portnum] = NULL;
> 
> The array element here hasn't yet been assigned to since kcalloc()
> initially zeroed it, so this is entirely redundant - you can simply make
> the comparison a "!=" and remove the "else" case.
> 
> Robin.
> 

Thanks for spotting this! Fix in v4.

Amelie

>> +else
>> +return retval;
>> +} else {
>> +priv->vbus_supplies[portnum] = vbus_supply;
>> +}
>> +}
>> +
>>  if (pdata->no_io_watchdog)
>>  ehci->need_io_watchdog = 0;
>>  return 0;
>>}
>>
>> +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum,
>> +bool enable)
>> +{
>> +struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
>> +int ret;
>> +
>> +if (!priv->vbus_supplies[portnum])
>> +return 0;
>> +
>> +if (enable)
>> +ret = regulator_enable(priv->vbus_supplies[portnum]);
>> +else
>> +ret = regulator_disable(priv->vbus_supplies[portnum]);
>> +if (ret)
>> +dev_err(hcd->self.controller,
>> +"failed to %s 

Re: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply

2018-03-01 Thread Amelie DELAUNAY
Hi Roger,

On 02/28/2018 12:01 PM, Roger Quadros wrote:
> Hi Amelie,
> 
> On 23/02/18 15:46, Amelie Delaunay wrote:
>> On some boards, especially when vbus supply requires large current,
>> and the charge pump on the PHY isn't enough, an external vbus power switch
>> may be used.
>> Add support for optional external vbus supply per port in ehci-platform.
>>
>> Signed-off-by: Amelie Delaunay 
>>
>> ---
>> Changes in v3:
>>   * Address Felipe Balbi comments: reduce indentation in
>> ehci_platform_port_power.
>>   * Address Roger Quadros and Alan Stern comments: platforms can have one
>> external vbus supply per port, so add support to get as many optional
>> regulator as implemented ports on the host controller.
>>
>> Changes in v2:
>>   * Address Roger Quadros comments: move regulator_enable/disable from
>> ehci_platform_power_on/off to ehci_platform_port_power.
>> ---
>>   Documentation/devicetree/bindings/usb/usb-ehci.txt |  1 +
>>   drivers/usb/host/ehci-platform.c   | 52 
>> +-
>>   2 files changed, 52 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt 
>> b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> index 3efde12..cd576db 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> @@ -19,6 +19,7 @@ Optional properties:
>>- phys : phandle + phy specifier pair
>>- phy-names : "usb"
>>- resets : phandle + reset specifier pair
>> + - portN_vbus-supply : phandle of regulator supplying vbus for port N
>>   
>>   Example (Sequoia 440EPx):
>>   ehci@e300 {
> 
> Sorry for not pointing this out earlier but I think patch to DT bindings
> should come separately (before the driver changes) with the following subject.
> 
> "dt-bindings: usb: ehci: "
> 
> 

Ok, I prepare a v4 with this split. Thanks!

>> diff --git a/drivers/usb/host/ehci-platform.c 
>> b/drivers/usb/host/ehci-platform.c
>> index b065a96..8e9f201 100644
>> --- a/drivers/usb/host/ehci-platform.c
>> +++ b/drivers/usb/host/ehci-platform.c
>> @@ -29,6 +29,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -46,6 +47,7 @@ struct ehci_platform_priv {
>>  struct reset_control *rsts;
>>  struct phy **phys;
>>  int num_phys;
>> +struct regulator **vbus_supplies;
>>  bool reset_on_resume;
>>   };
>>   
>> @@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>  struct platform_device *pdev = to_platform_device(hcd->self.controller);
>>  struct usb_ehci_pdata *pdata = dev_get_platdata(>dev);
>>  struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> -int retval;
>> +struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
>> +int portnum, n_ports, retval;
>>   
>>  ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
>>   
>> @@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>  if (retval)
>>  return retval;
>>   
>> +n_ports = HCS_N_PORTS(ehci->hcs_params);
>> +priv->vbus_supplies = devm_kcalloc(>dev, n_ports,
>> +   sizeof(struct regulator *),
>> +   GFP_KERNEL);
>> +if (!priv->vbus_supplies)
>> +return -ENOMEM;
>> +
>> +for (portnum = 0; portnum < n_ports; portnum++) {
>> +struct regulator *vbus_supply;
>> +char id[20];
>> +
>> +sprintf(id, "port%d_vbus", portnum);
>> +
>> +vbus_supply = devm_regulator_get_optional(>dev, id);
>> +if (IS_ERR(vbus_supply)) {
>> +retval = PTR_ERR(vbus_supply);
>> +if (retval == -ENODEV)
>> +priv->vbus_supplies[portnum] = NULL;
>> +else
>> +return retval;
>> +} else {
>> +priv->vbus_supplies[portnum] = vbus_supply;
>> +}
>> +}
>> +
>>  if (pdata->no_io_watchdog)
>>  ehci->need_io_watchdog = 0;
>>  return 0;
>>   }
>>   
>> +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum,
>> +bool enable)
>> +{
>> +struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
>> +int ret;
>> +
>> +if (!priv->vbus_supplies[portnum])
>> +return 0;
>> +
>> +if (enable)
>> +ret = regulator_enable(priv->vbus_supplies[portnum]);
>> +else
>> +ret = regulator_disable(priv->vbus_supplies[portnum]);
> 
> A newline could be used here.
> 

Ok.

>> +if (ret)
>> +dev_err(hcd->self.controller,
>> +"failed to %s vbus supply for port %d: %d\n",
>> +enable ? "enable" : "disable", portnum, ret);
>> +
>> +return ret;
>> +}
>> +
>>   static int ehci_platform_power_on(struct platform_device 

Re: Wakeup from USB on i.MX6S

2018-03-01 Thread Ralf.4MailingLists
Good Morning Peter,
(it's 9:38 am in here, I don't know your time zone :) )

On 1 March 2018 8:33 AM, Peter Chen  wrote:
> On Wed, Feb 28, 2018 at 01:17:09PM -0500, Ralf.4MailingLists wrote:
> > > Would you please try below changes:
> > > 1.  By connecting external HUB at USBOTG0, and only enable its wakeup
> > > to see if the abnormal wakeup occurs?
> > > 2184200.usb -> 2184000.usb
> > > ci_hdrc.1 ->ci_hdrc.0
> > > 2.  Unbind wired USB HUB at host port to see if the abnormal wakeup
> > > occurs?
> > > echo -n "1-1:1.0" > /sys/bus/usb/drivers/hub/unbind
> > 
> > I'm afraid I'm not able to do this at the moment. I only have a HUB with a 
> > Mini-USB-Input and a cable Mini-USB-B to (usual) USB-A. The connector on 
> > the board also is a Mini-A/B-Port, so i would need a 
> > Mini-USB-(AorB)--to--Mini-USB-B--cable. Maybe with OTG capability and an 
> > USB-HUB with OTG capability.
> > If necessary, I could try to order one or solder one by 2 old 
> > Mini-USB-cables.
> 
> But you could try my above suggestion #2 which is unbind wired USB HUB
> from its driver.

You are right, I'm sorry, I misunderstood your word "wired", I thought you were 
talking about the Hub i should connect to Mini-USB-OTG, not the soldered one on 
the board.
I did so right now, the abnormal wakeup occurs.
Also echo -n "1-0:1.0" > /sys/bus/usb/drivers/hub/unbind doesn't change the 
abnormal wakeup.
Note: I previously enabled all wakeup devices as always. 

This is the content of the hub directory:
root@emtrion-mx6:/sys/bus/usb/drivers/hub# ls -l
lrwxrwxrwx1 root root 0 Mar  1 09:23 1-0:1.0 -> 
../../../../devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-0:1.0
lrwxrwxrwx1 root root 0 Mar  1 09:23 1-1:1.0 -> 
../../../../devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1:1.0
--w---1 root root  4096 Mar  1 09:23 bind
lrwxrwxrwx1 root root 0 Mar  1 09:23 module -> 
../../../../module/usbcore
-rw-r--r--1 root root  4096 Mar  1 09:23 new_id
-rw-r--r--1 root root  4096 Mar  1 09:23 remove_id
--w---1 root root  4096 Mar  1 09:23 uevent
--w---1 root root  4096 Mar  1 09:23 unbind

I've noticed that there is no response on unbindung 1-1:1.0, but on unbinding 
1-0:1.0:
echo -n "1-1:1.0" > /sys/bus/usb/drivers/hub/unbind


echo -n "1-0:1.0" > /sys/bus/usb/drivers/hub/unbind
[  212.373519] usb 1-1: USB disconnect, device number 2


The 2nd one removes 
/sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/ 
directory.

And this time, I enabled everything remaining after unbinding both --> it still 
shows abnormal wakeup behaviour.

I've got a meeting in a few minutes. I will continue later.

Best regards,
Ralf
--
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 3/7] usb: dwc3: pci: Store device properties dynamically

2018-03-01 Thread Felipe Balbi

Hi,

John Youn  writes:
> On 02/22/2018 07:20 AM, Andy Shevchenko wrote:
>> On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen  
>> wrote:
>>> On 2/21/2018 6:46 AM, Andy Shevchenko wrote:
 On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen
  wrote:
> On 2/17/2018 7:29 AM, Andy Shevchenko wrote:
>> On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
>>  wrote:
>>> Add the ability to add device properties dynamically. Currently, device
>>> properties are added to platform device using
>>> platform_device_add_properties(). However, this function does not allow
>>> adding properties incrementally. It is useful to have this ability when
>>> the driver needs to set common device properties across different HW
>>
>> I'm not sure it's useful anyhow.
>>
>>> or
>>> if IP and FPGA validation test different configurations for different
>>> HW.
>>
>> Shouldn't be a separate stuff for FPGA exclusively?
>
> Can you clarify/expand your question?

 FPGA is the one which might have different properties at run time for
 the same device.
 So, why do we care on driver / generic level of it?

 Shouldn't be FPGA manager take care of it (via DT overlays, for example)?
>>>
>>> Normally it is handled using DT overlays. But for using FPGA as PCI
>>> device, I'm not aware of a better solution.
>> 
>> If it's a PCI device, it may utilize PCI (hot plug if you would like
>> to change IP at run time) with appropriate IDs and stuff, right?
>> 
>>> Introduce two new functions to do so:
>>> * dwc3_pci_add_one_property() - this function adds one property to
>>>   dwc->properties array and increase its size as needed
>>> * dwc3_pci_add_properties() - this function takes a null terminated
>>>   array of device properties and add them to dwc->properties
>>
>> So, why you can't use ACPI / DT here?
>> 
> dwc3_pci_add_properties() is a convenient function that takes statically
> allocated array of (quirks) properties for different HW and store them
> to dwc->properties. The idea is to add more properties on top of these
> required quirks.

 Yes, I understand that. What's wrong with DT? The built-in device
 properties have the same nature as usual properties for DT.
 Whenever driver calls device_property_read_uXX() or alike it would
 check all property provides for asked one.
>>>
>>> I may not have explained fully in my commit message the purpose of this
>>> change. That's why I think I misinterpreted your previous question.
>> 
>>> With this new debugging feature, we want to delay adding device
>>> properties until the user initiates it.
>> 
>> So, see above.
>> When user wants to enable this IP at run time it will be a PCI hot
>> plug event which makes device appear behind PCI switch.
>> When device appears it would have it's own VndrID/DevID + sub pair.
>> 
>> Based on that IDs you may apply hard coded quirks (though I am against
>> quirks in new hardware) as it's done right now.
>> 
>
> Hi Andy,
>
> Using VID/PID is not really feasible for our use case. If we only had
> a few concrete devices then it would be ok.

Agreed. VID/PID would not scale at all :-)

> But understand that this an IP running on an FPGA validation
> platform. The IP is very large and flexible with *many* parameters
> that we must test against.  It is also deployed by our customers with
> widely varying configurations. So we are constantly testing these as
> well.
>
> One of the last pieces for moving our FPGA validation completely to
> the in-kernel Linux stack is the ability to configure the driver to
> set parameters that are not visible to the driver, or with parameters
> that we want to constrain for testing.

I'm very much interested in helping you guys validate your IP with
upstream Linux :-) My concern with this patch is that it makes dwc3-pci
super complex even for users who are not interested in IP validation.

So, instead, how about we introduce dwc3-snps.c where you guys can put
all the controls you need? We remove HAPS from current dwc3-pci.c and
move everything to dwc3-snps.c. Sure, we'd have a little duplication,
but I guess that'd be very minor.

While doing that, we can make dwc3-snps.c depend on EXPERT, so that
distros don't enable it by default.

I don't mind adding a bunch of properties to dwc3 core as long as it has
an actual use. I guess maintaining that is far from being a problem,
however I don't want to have impacts on actual products since this is
only necessary for validation activities :-)

cheers

-- 
balbi


signature.asc
Description: PGP signature


Re: patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality

2018-03-01 Thread Bockholdt Arne

On Thu, 2018-02-15 at 19:29 +, Marc Zyngier wrote:
> [+ Ard, who helped me chasing the initial issue]
> 
> On 15/02/18 06:43, Bockholdt Arne wrote:
> > Hi all,
> > 
> > on our Intel Atom C2578 server with a SuperMicro A1SAi board and a
> > Renesas uPD720201 USB 3.0 host controller the controller has
> > stopped
> > working since kernel 4.13.x. Before that kernel the dmesg messages
> > from
> > XHCI were:
> > 
> > dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
> > Controller
> > dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
> > registered,
> > assigned bus number 2
> > dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: hcc params
> > 0x014051cf
> > hci version 0x100 quirks 0x0010
> > dmesg-4.12.1-serverv4.log:usb usb2: Manufacturer: Linux 4.12.1-
> > serverv4
> > xhci-hcd
> > dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
> > Controller
> > dmesg-4.12.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
> > registered,
> > assigned bus number 3
> > dmesg-4.12.1-serverv4.log:usb usb3: Manufacturer: Linux 4.12.1-
> > serverv4
> > xhci-hcd
> > 
> > After that the message look like that:
> > 
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Resetting
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Refused to change
> > power
> > state, currently in D3
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: xHCI Host
> > Controller
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: new USB bus
> > registered,
> > assigned bus number 2
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: Host halt failed,
> > -19
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: can't setup: -19
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: USB bus 2
> > deregistered
> > dmesg-4.13.1-serverv4.log:xhci_hcd :03:00.0: init :03:00.0
> > fail, -19
> > 
> > I've tested it with 4.15.3 too, it's still the same. I've narrowed
> > it
> > down to the following patch:
> > 
> > commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7
> > Author: Marc Zyngier  > .com>>
> > Date:   Tue Aug 1 20:11:08 2017 -0500
> > 
> > xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA
> > issue
> > 
> > Reverting the patch on top of 4.15.3 restores the USB3
> > functionality on
> > our server. Please let me know if there is anything I can do to fix
> > the
> > problem. Thank you.
> 
> Hi Arne,
> 
> This looks pretty bad. I suspect that once reset, the firmware is
> lost.
> I'll try to write a patch dumping some information about it.
> 
> Ard: Do you know if the Cello board has a SPI flash connected to the
> Renesas chip, from which it would load the firmware?
> 
> Another possibility is that power management kicks in, and that the
> endpoint is stuck there. Could also be firmware related, but not
> only.
> I'd welcome any idea on the subject, as I cannot reproduce this issue
> on
> the HW I have.
> 
> It we cannot work out what exactly is causing this, we may have to
> default to not resetting the part and rely on a command-line option
> to
> do it... I can't say I'm a fan.
> 
> Thanks,
> 
>   M.
> 

Hi Marc,

I've tested it with 4.15.7 and it's still there. Is there anything that
I can do to help fixing this problem?

Thank you,

   Arne