[xhci-hcd][linux 4.20.13] autosuspend on causes a single cpu core to stay in kernel mode using 100% of said cpu core.

2019-03-03 Thread Farelka kek
usbcore.autosuspend higher than -1, causes 1 cpu core to never step
down from kernel mode.

It happens on a core i5-7200U laptop, an acer f5-573g-50ec
Disabling usb autosuspend causes the cpu not to idle at, minimum of
25% cpu usage.


Re: [xhci-hcd][linux 4.20.13] autosuspend on causes a single cpu core to stay in kernel mode using 100% of said cpu core.

2019-03-03 Thread Greg KH
On Sun, Mar 03, 2019 at 03:15:27PM +0100, Farelka kek wrote:
> usbcore.autosuspend higher than -1, causes 1 cpu core to never step
> down from kernel mode.

Some hardware does not aupport autosuspend, so leaving it at -1 is good.

> It happens on a core i5-7200U laptop, an acer f5-573g-50ec
> Disabling usb autosuspend causes the cpu not to idle at, minimum of
> 25% cpu usage.

Is this a regression from earlier kernel versions?  If so, any chance
you can run 'git bisect' to find the offending commit?

thanks,

greg k-h


Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread John Stultz
On Sat, Mar 2, 2019 at 4:05 AM Yu Chen  wrote:
> +static int hisi_hikey_usb_probe(struct platform_device *pdev)
> +{
> +   struct device *dev = &pdev->dev;
> +   struct device_node *root = dev->of_node;

Minor nit: root is unused and generates build warnings.

thanks
-john


Re: [BUG] mt76x0u: Probing issues on Raspberry Pi 3 B+

2019-03-03 Thread Stefan Wahren
Hi,

> Stanislaw Gruszka  hat am 20. Februar 2019 um 17:32 
> geschrieben:
> 
> 
> On Wed, Feb 20, 2019 at 05:22:18PM +0100, Lorenzo Bianconi wrote:
> > > On Wed, Feb 20, 2019 at 02:22:08PM +0100, Lorenzo Bianconi wrote:
> > > > > On Tue, Feb 19, 2019 at 01:19:26PM +0100, Felix Fietkau wrote:
> > > > > > >> >> The way I see it, we have two choices.
> > > > > > >> >> 1. Fix dwc2 to do its alignment quirk for the urb->sg != NULL 
> > > > > > >> >> case
> > > > > > >> >> 2. Rely on urb->transfer_buffer and keep urb->sg NULL
> > > > > > >> > 
> > > > > > >> > I agree, if this is only needed for dwc2. Though I would 
> > > > > > >> > investigate
> > > > > > >> > if this is not a bug on other platforms as well.
> > > > > > >> >From what I can see, using Lorenzo's patches seems to be the 
> > > > > > >> >better
> > > > > > >> solution, since they avoid these corner cases in dwc2 (and maybe 
> > > > > > >> other
> > > > > > >> drivers as well). I will apply them and then we'll see if we 
> > > > > > >> need to do
> > > > > > >> any further improvements later on.
> > > > > > > 
> > > > > > > They work on rpi dwc2, but they do not address root of the 
> > > > > > > problem.
> > > > > > > There is clearly something wrong how mt76usb handle SG, what is 
> > > > > > > not
> > > > > > > fixed. And adding disable_usb_sg module parameter for hcd's 
> > > > > > > supporting
> > > > > > > SG should be red flag.
> > > > > > I think we're simply dealing with multiple issues here, only some of
> > > > > > which are fixed by Lorenzo's patches.
> > > > > > I'm pretty sure it's still wrong for mt76 to try to align its 
> > > > > > buffers,
> > > > > > since the Linux USB API supports non-aligned transfer buffers and it
> > > > > > should be up to the controller driver to deal with that.
> > > > > 
> > > > > Agree.
> > > > > 
> > > > > > dwc2 tries to do that, but that has limitations which I already 
> > > > > > pointed
> > > > > > out and which are properly dealt with by Lorenzo's patches.
> > > > > 
> > > > > I planed to just accept current solution, but I started to work on 
> > > > > patch
> > > > > that remove len, sglen arguments from mt76u_buf_alloc() and use
> > > > > q->buf_size and SKB_WITH_OVERHEAD(q->buf_size) directly and realized 
> > > > > how
> > > > > related code is now tangled.
> > > > > 
> > > > > Would be ok to send this patch with proper changelog as fix for RPI
> > > > > against wireless-drivers and cc:stable (assuming it works and really
> > > > > fix things on RPI) and revert Lorenzo's patches in -next ?
> > > > 
> > > > Hi Stanislaw,
> > > > 
> > > > what is the advantage of doing so?
> > > To have small fix proper for -stable to fix the problem in 4.20 and 4.19,
> > > and have simpler code.
> > 
> > merging the series I sent we will have a pretty simple approach, just a
> > single routine that allocates the rx buffers in the control path according 
> > to
> > the operating mode.
> 

FWIW i tested current linux-next and it also works with arm64/defconfig on 
Raspberry Pi 3B.

Thanks

> 
> Stanislaw


Re: [PATCH] usb: typec: pi3usb30532: Keep orientation when setting mux to safe mode

2019-03-03 Thread Guenter Roeck

On 2/22/19 11:22 AM, Hans de Goede wrote:

Keep the orientation value when setting the mux to safe mode, this
fixes the orientation getting reset when switching alt-modes.

Signed-off-by: Hans de Goede 


Reviewed-by: Guenter Roeck 


---
  drivers/usb/typec/mux/pi3usb30532.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/mux/pi3usb30532.c 
b/drivers/usb/typec/mux/pi3usb30532.c
index 64eb5983e17a..9294e85fd34b 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -84,7 +84,8 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int 
state)
  
  	switch (state) {

case TYPEC_STATE_SAFE:
-   new_conf = PI3USB30532_CONF_OPEN;
+   new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
+  PI3USB30532_CONF_OPEN;
break;
case TYPEC_STATE_USB:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |





Re: [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)

2019-03-03 Thread Charles Yeh
Hi Johan,

Is it free to check out the patch file provided before?
If you can, please see first
1. [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
2. [PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support
PL2303HXD (TYPE_HX)

1 & 2 are independent..

Charles Yeh  於 2019年2月19日 週二 下午2:47寫道:
>
> Hi Johan & Greg,
>
>
> Do you have received a new patch"[PATCH] USB:serial:pl2303:Add new PID
> to support PL2303HXN (TYPE_HXN)"?
>
> If you have received a new patch, has the content been confirmed?
>
> Or tell me where needs to be modified.
>
> Thanks!
>
> Charles.
>
>
> Charles Yeh  於 2019年2月13日 週三 下午8:30寫道:
>
>
> >
> > Prolific has developed a new USB to UART chip: PL2303HXN 
> > (PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE)
> > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from the 
> > existing PL2303 series (TYPE_HX & TYPE_01).
> > Therefore, different Vendor requests are used to issue related commands.
> >
> > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes new 
> > Vendor request,
> >new flow control and other related instructions if TYPE_HXN is 
> > recognized.
> >
> > 2. Because the new PL2303HXN can only accept the new Vendor request,
> >the old Vendor request cannot be accepted (the error message will be 
> > returned)
> >So first determine the TYPE_HX or TYPE_HXN through 
> > TYPE_HX_READ_STATUS_REG in pl2303_startup.
> >
> >   2.1 If the return message is "1", then the PL2303 is the existing 
> > TYPE_HX/ TYPE_01 series.
> >   The other settings in pl2303_startup are to continue execution.
> >   2.2 If the return message is "not 1", then the PL2303 is the new TYPE_HXN 
> > series.
> >   The other settings in pl2303_startup are ignored.
> >   (PL2303HXN will directly use the default value in the hardware,
> >no need to add additional settings through the software)
> >
> > 3. In pl2303_open: Because TYPE_HXN is different from the instruction of 
> > down/up stream used by TYPE_HX.
> >Therefore, we will also execute different instructions here.
> >
> > 4. In pl2303_set_termios: The UART flow control instructions used by 
> > TYPE_HXN/TYPE_HX/TYPE_01 are different.
> >Therefore, we will also execute different instructions here.
> >
> > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is different 
> > from the vendor request
> >instruction used by TYPE_HX/TYPE_01, it will also execute different 
> > instructions here.
> >
> > Signed-off-by: Charles Yeh 
> > ---
> >  drivers/usb/serial/pl2303.c | 131 +---
> >  drivers/usb/serial/pl2303.h |   7 ++
> >  2 files changed, 113 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> > index bb3f9aa4a909..d7d557e01390 100644
> > --- a/drivers/usb/serial/pl2303.c
> > +++ b/drivers/usb/serial/pl2303.c
> > @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = {
> > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_MOTOROLA) },
> > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_ZTEK) },
> > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_TB) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GC) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GB) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GT) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GL) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GE) },
> > +   { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GS) },
> > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID) },
> > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID_RSAQ5) },
> > { USB_DEVICE(ATEN_VENDOR_ID, ATEN_PRODUCT_ID),
> > @@ -129,9 +135,11 @@ MODULE_DEVICE_TABLE(usb, id_table);
> >
> >  #define VENDOR_WRITE_REQUEST_TYPE  0x40
> >  #define VENDOR_WRITE_REQUEST   0x01
> > +#define VENDOR_WRITE_NREQUEST  0x80
> >
> >  #define VENDOR_READ_REQUEST_TYPE   0xc0
> >  #define VENDOR_READ_REQUEST0x01
> > +#define VENDOR_READ_NREQUEST   0x81
> >
> >  #define UART_STATE_INDEX   8
> >  #define UART_STATE_MSR_MASK0x8b
> > @@ -145,11 +153,30 @@ MODULE_DEVICE_TABLE(usb, id_table);
> >  #define UART_OVERRUN_ERROR 0x40
> >  #define UART_CTS   0x80
> >
> > +#define TYPE_HX_READ_STATUS_REG0x8080
> > +#define TYPE_HXN_FLOWCONTROL_REG   0x0A
> > +#define TYPE_HXN_HARDWAREFLOW_DATA 0xFA
> > +#define TYPE_HXN_SOFTWAREFLOW_DATA 0xEE
> > +#define TYPE_HXN_NOFLOW_DATA   0xFF
> > +#define TYPE_HX_01_FLOWCONTROL_REG 0x00
> > +#define TYPE_01_HARDWAREFLOW_DATA  0x41
> > +#define TYPE_HX_HARDWAREFLOW_DATA  0x61
> > +#define TYPE_HX_01_SOFTWAREFLOW_DATA   0xC0
> > +#define TYPE_HX_01_NOFLOW_DATA 0x00
> > +#define UART_XON_CHAR  0x11
> > +#define UART_XOFF_CHAR

Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chen Yu



On 2019/3/4 4:17, John Stultz wrote:
> On Sat, Mar 2, 2019 at 4:05 AM Yu Chen  wrote:
>> +static int hisi_hikey_usb_probe(struct platform_device *pdev)
>> +{
>> +   struct device *dev = &pdev->dev;
>> +   struct device_node *root = dev->of_node;
> 
> Minor nit: root is unused and generates build warnings.
> 
OK. Thanks!

> thanks
> -john
> 
> .
> 

Thanks
Yu Chen



Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chunfeng Yun
hi,
On Sat, 2019-03-02 at 17:05 +0800, Yu Chen wrote:
> This driver handles usb hub power on and typeC port event of HiKey960 board:
> 1)DP&DM switching between usb hub and typeC port base on typeC port
> state
> 2)Control power of usb hub on Hikey960
> 3)Control vbus of typeC port
> 
> Cc: Chunfeng Yun 
> Cc: Andy Shevchenko 
> Cc: Arnd Bergmann 
> Cc: Greg Kroah-Hartman 
> Cc: John Stultz 
> Cc: Binghui Wang 
> Cc: Heikki Krogerus 
> Signed-off-by: Yu Chen 
> ---
> v1:
> * Using gpiod API with the gpios.
> * Removing registering usb role switch.
> * Registering usb role switch notifier.
> v2:
> * Fix license declaration.
> * Add configuration of  gpio direction.
> * Remove some log print.
> v3:
> * Remove property of "typec_vbus_enable_val".
> * Remove gpiod_direction_output and set initial value of gpio by 
> devm_gpiod_get.
> ---
> ---
>  drivers/misc/Kconfig  |   6 ++
>  drivers/misc/Makefile |   1 +
>  drivers/misc/hisi_hikey_usb.c | 167 
> ++
>  3 files changed, 174 insertions(+)
>  create mode 100644 drivers/misc/hisi_hikey_usb.c
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f417b06e11c5..8d8b717759e2 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -521,6 +521,12 @@ config PVPANIC
> a paravirtualized device provided by QEMU; it lets a virtual machine
> (guest) communicate panic events to the host.
>  
> +config HISI_HIKEY_USB
> + tristate "USB functionality of HiSilicon Hikey Platform"
> + depends on OF && GPIOLIB
> + help
> +   If you say yes here you get support for usb functionality of 
> HiSilicon Hikey Platform.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e39ccbbc1b3a..dc8892b13a1a 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -59,3 +59,4 @@ obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
>  obj-$(CONFIG_OCXL)   += ocxl/
>  obj-y+= cardreader/
>  obj-$(CONFIG_PVPANIC)+= pvpanic.o
> +obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
> new file mode 100644
> index ..85d6fee468c0
> --- /dev/null
> +++ b/drivers/misc/hisi_hikey_usb.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Support for usb functionality of Hikey series boards
> + * based on Hisilicon Kirin Soc.
> + *
> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
> + *   http://www.huawei.com
> + *
> + * Authors: Yu Chen 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DEVICE_DRIVER_NAME "hisi_hikey_usb"
> +
> +#define HUB_VBUS_POWER_ON 1
> +#define HUB_VBUS_POWER_OFF 0
> +#define USB_SWITCH_TO_HUB 1
> +#define USB_SWITCH_TO_TYPEC 0
> +#define TYPEC_VBUS_POWER_ON 1
> +#define TYPEC_VBUS_POWER_OFF 0
> +
> +struct hisi_hikey_usb {
> + struct gpio_desc *otg_switch;
> + struct gpio_desc *typec_vbus;
> + struct gpio_desc *hub_vbus;
> +
> + struct usb_role_switch *role_sw;
> + struct notifier_block nb;
> +};
> +
> +static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
> +{
> + gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
> +}
> +
> +static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
> + int switch_to)
> +{
> + gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
> +}
> +
> +static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
> + int value)
> +{
> + gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
> +}
> +
> +static int hisi_hikey_role_switch(struct notifier_block *nb,
> + unsigned long state, void *data)
> +{
> + struct hisi_hikey_usb *hisi_hikey_usb;
> +
> + hisi_hikey_usb = container_of(nb, struct hisi_hikey_usb, nb);
> +
> + switch (state) {
> + case USB_ROLE_NONE:
> + usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB);
> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON);
> + break;
> + case USB_ROLE_HOST:
> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
> + usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_ON);
> + break;
> + case USB_ROLE_DEVICE:
> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
> + usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int hisi_hikey_usb_probe(struct platform_device *

Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chen Yu
Hi Chunfeng Yun,

On 2019/3/4 9:47, Chunfeng Yun wrote:

>> +
>> +hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
>> +if (!hisi_hikey_usb->role_sw)
>> +return -EPROBE_DEFER;
> Here return EPROBE_DEFFER means the related device_connection is
> registered after this probe is called, right?
> if not, use IS_ERR_OR_NULL then return PTR_ERR is enough
Yes, the driver which register the usb_role_switch may finish probe after
this driver is probed for the first time.
>> +else if (IS_ERR(hisi_hikey_usb->role_sw))
>> +return PTR_ERR(hisi_hikey_usb->role_sw);
>> +
>> +ret = usb_role_switch_register_notifier(hisi_hikey_usb->role_sw,
>> +&hisi_hikey_usb->nb);
>> +if (ret) {
>> +usb_role_switch_put(hisi_hikey_usb->role_sw);
>> +return ret;
>> +}
>> +
>> +platform_set_drvdata(pdev, hisi_hikey_usb);
>> +
>> +return 0;
>> +}
>> +

> 
> 
> .
> 



Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chen Yu
Hi Andy,

On 2019/3/3 0:01, Andy Shevchenko wrote:
> On Sat, Mar 2, 2019 at 11:05 AM Yu Chen  wrote:
>>
>> This driver handles usb hub power on and typeC port event of HiKey960 board:
>> 1)DP&DM switching between usb hub and typeC port base on typeC port
>> state
>> 2)Control power of usb hub on Hikey960
>> 3)Control vbus of typeC port
> 
>> +config HISI_HIKEY_USB
>> +   tristate "USB functionality of HiSilicon Hikey Platform"
>> +   depends on OF && GPIOLIB
>> +   help
>> + If you say yes here you get support for usb functionality of 
>> HiSilicon Hikey Platform.
> 
>> +#include 
> 
> It's hard to see why this have
> depends on OF followed by above header inclusion.
> 
This driver depends on devicetree, so I add "depends on OF".
But is seems that "#include " can be removed after "of_" API
have been removed. Thanks for your reminder!
>> +   hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
>> +   GPIOD_OUT_LOW);
> 
>> +   if (!hisi_hikey_usb->typec_vbus)
>> +   return -ENOENT;
> 
> Hmm... Is it possible to get NULL pointer from gpiod_get() at all?
> 
>> +   if (!hisi_hikey_usb->otg_switch)
>> +   return -ENOENT;
> 
> Ditto.
> 
I check the comments of devm_gpio_get API, it will not return NULL pointer.
But is it more safe to keep the NULL checking? What is your advice?
>> +   /* hub-vdd33-en is optional */
>> +   hisi_hikey_usb->hub_vbus = devm_gpiod_get(dev, "hub-vdd33-en",
>> +   GPIOD_OUT_HIGH);
> 
> devm_gpio_get_optional() if it's indeed optional.
> 
OK.Thanks!
>> +   hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
>> +   if (!hisi_hikey_usb->role_sw)
>> +   return -EPROBE_DEFER;
> 
>> +   else if (IS_ERR(hisi_hikey_usb->role_sw))
> 
> Redundant 'else'
> 
OK.
>> +   return PTR_ERR(hisi_hikey_usb->role_sw);
> 
>> +static const struct of_device_id id_table_hisi_hikey_usb[] = {
>> +   {.compatible = "hisilicon,gpio_hubv1"},
>> +   {.compatible = "hisilicon,hikey960_usb"},
>> +   {}
>> +};
> 
> MODULE_DEVICE_TABLE()?
> 
OK.

Thanks
Yu Chen



Re: [PATCH v3 07/12] phy: Add usb phy support for hi3660 Soc of Hisilicon

2019-03-03 Thread Chen Yu
Hi Andy,

On 2019/3/2 23:56, Andy Shevchenko wrote:
> On Sat, Mar 2, 2019 at 11:06 AM Yu Chen  wrote:
>>
>> This driver handles usb phy power on and shutdown for hi3660 Soc of
>> Hisilicon.
> 
> Few comments below. After fixing them, FWIW
> Reviewed-by: Andy Shevchenko 
> 
>> +#define HI3660_USB_DEFAULT_PHY_PARAM   0x1c466e3
> 
> A bit of description would be nice to have what this value means.
> 
OK.
>> +   /* delay for exit from IDDQ mode */
>> +   usleep_range(100, 100);
> 
> 100,100 ? I think you need to give a room to scheduler, at least 20%
> margin would be good to have.
> 
>> +   /* delay for vbus valid */
>> +   usleep_range(100, 100);
> 
> Ditto.
> 
> 
OK.

Thanks
Yu Chen



//Re: [PATCH v3 06/12] usb: dwc3: Increase timeout for CmdAct cleared by device controller

2019-03-03 Thread Chen Yu
Hi Zhang Fei,

On 2019/3/2 23:47, Andy Shevchenko wrote:
> On Sat, Mar 2, 2019 at 11:06 AM Yu Chen  wrote:
>>
>> It needs more time for the device controller to clear the CmdAct of
>> DEPCMD on Hisilicon Kirin Soc.
>>
> 
> 5x times more? Can you provide more specific details on that?
> 
Can you explain the details about the timeout expansion on Hisilicon Kirin Soc?
Thanks!

>> Cc: Andy Shevchenko 
>> Cc: Felipe Balbi 
>> Cc: Greg Kroah-Hartman 
>> Cc: John Stultz 
>> Cc: Binghui Wang 
>> Signed-off-by: Yu Chen 
>> ---
>>  drivers/usb/dwc3/gadget.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 6c9b76bcc2e1..84d701b37171 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -270,7 +270,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, 
>> unsigned cmd,
>>  {
>> const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
>> struct dwc3 *dwc = dep->dwc;
>> -   u32 timeout = 1000;
>> +   u32 timeout = 5000;
>> u32 saved_config = 0;
>> u32 reg;
>>
>> --
>> 2.15.0-rc2
>>
> 
> 



Re: [PATCH] usb: uas: fix usb subsystem hang after power off hub port

2019-03-03 Thread Greg KH
On Mon, Mar 04, 2019 at 02:08:54AM +, jacky@sony.com wrote:
> 
> 
> This email is confidential and intended only for the use of the individual or 
> entity named above and may contain information that is privileged. If you are 
> not the intended recipient, you are notified that any dissemination, 
> distribution or copying of this email is strictly prohibited. If you have 
> received this email in error, please notify us immediately by return email or 
> telephone and destroy the original message. - This mail is sent via Sony Asia 
> Pacific Mail Gateway..

This footer requires me to delete this email as it is not compatible
with doing Linux kernel development.

greg k-h


Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Andy Shevchenko
On Mon, Mar 4, 2019 at 3:47 AM Chunfeng Yun  wrote:
> On Sat, 2019-03-02 at 17:05 +0800, Yu Chen wrote:

> > + hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
> > + if (!hisi_hikey_usb->role_sw)
> > + return -EPROBE_DEFER;
> Here return EPROBE_DEFFER means the related device_connection is
> registered after this probe is called, right?
> if not, use IS_ERR_OR_NULL then return PTR_ERR is enough

How enough? If return value is NULL it would be transformered to 0,
which is success return code from the ->probe() which means we will
have ->probed() and not functional device.

Am I missing something?

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Andy Shevchenko
On Mon, Mar 4, 2019 at 4:35 AM Chen Yu  wrote:
> On 2019/3/3 0:01, Andy Shevchenko wrote:
> > On Sat, Mar 2, 2019 at 11:05 AM Yu Chen  wrote:

> >> +config HISI_HIKEY_USB
> >> +   tristate "USB functionality of HiSilicon Hikey Platform"
> >> +   depends on OF && GPIOLIB
> >> +   help
> >> + If you say yes here you get support for usb functionality of 
> >> HiSilicon Hikey Platform.
> >
> >> +#include 
> >
> > It's hard to see why this have
> > depends on OF followed by above header inclusion.
> >
> This driver depends on devicetree, so I add "depends on OF".
> But is seems that "#include " can be removed after "of_" API
> have been removed. Thanks for your reminder!

So, it means that technically there is no such dependency, rather
administratively.

> >> +   hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
> >> +   GPIOD_OUT_LOW);
> >
> >> +   if (!hisi_hikey_usb->typec_vbus)
> >> +   return -ENOENT;
> >
> > Hmm... Is it possible to get NULL pointer from gpiod_get() at all?
> >
> >> +   if (!hisi_hikey_usb->otg_switch)
> >> +   return -ENOENT;
> >
> > Ditto.
> >
> I check the comments of devm_gpio_get API, it will not return NULL pointer.
> But is it more safe to keep the NULL checking? What is your advice?

Why do we need dead code?

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chunfeng Yun
hi,

On Mon, 2019-03-04 at 08:50 +0200, Andy Shevchenko wrote:
> On Mon, Mar 4, 2019 at 3:47 AM Chunfeng Yun  wrote:
> > On Sat, 2019-03-02 at 17:05 +0800, Yu Chen wrote:
> 
> > > + hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
> > > + if (!hisi_hikey_usb->role_sw)
> > > + return -EPROBE_DEFER;
> > Here return EPROBE_DEFFER means the related device_connection is
> > registered after this probe is called, right?
> > if not, use IS_ERR_OR_NULL then return PTR_ERR is enough
> 
> How enough? If return value is NULL it would be transformered to 0,
> which is success return code from the ->probe() which means we will
> have ->probed() and not functional device.
> 
You are right:)

> Am I missing something?
> 




RE: [PATCH] usb: uas: fix usb subsystem hang after power off hub port

2019-03-03 Thread Jacky.Cao
Dear Greg-san:

I confirmed our company email related counter person that below footer is added 
automatically
after we send email to who doesn't work in our company.
I am very sorry that I didn't notice this issue and make you confuse.
Now I am confirming with our counter person whether there is solution to not 
add below footer.
After that, I will re-send patch mail to you.

Best Regards
Jacky

-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org]
Sent: Monday, March 04, 2019 2:23 PM
To: Cao, Jacky
Cc: oneu...@suse.com; st...@rowland.harvard.edu; linux-usb@vger.kernel.org; 
linux-s...@vger.kernel.org; usb-stor...@lists.one-eyed-alien.net; 
linux-ker...@vger.kernel.org
Subject: Re: [PATCH] usb: uas: fix usb subsystem hang after power off hub port

On Mon, Mar 04, 2019 at 02:08:54AM +, jacky@sony.com wrote:
> 
>
> This email is confidential and intended only for the use of the individual or 
> entity named above and may contain information that is privileged. If you are 
> not the intended recipient, you are notified that any dissemination, 
> distribution or copying of this email is strictly prohibited. If you have 
> received this email in error, please notify us immediately by return email or 
> telephone and destroy the original message. - This mail is sent via Sony Asia 
> Pacific Mail Gateway..

This footer requires me to delete this email as it is not compatible
with doing Linux kernel development.

greg k-h



This email is confidential and intended only for the use of the individual or 
entity named above and may contain information that is privileged. If you are 
not the intended recipient, you are notified that any dissemination, 
distribution or copying of this email is strictly prohibited. If you have 
received this email in error, please notify us immediately by return email or 
telephone and destroy the original message. - This mail is sent via Sony Asia 
Pacific Mail Gateway..


Re: [PATCH v3 10/12] hikey960: Support usb functionality of Hikey960

2019-03-03 Thread Chen Yu
Hi,

On 2019/3/4 14:55, Andy Shevchenko wrote:
> On Mon, Mar 4, 2019 at 4:35 AM Chen Yu  wrote:
>> On 2019/3/3 0:01, Andy Shevchenko wrote:
>>> On Sat, Mar 2, 2019 at 11:05 AM Yu Chen  wrote:
> 
 +config HISI_HIKEY_USB
 +   tristate "USB functionality of HiSilicon Hikey Platform"
 +   depends on OF && GPIOLIB
 +   help
 + If you say yes here you get support for usb functionality of 
 HiSilicon Hikey Platform.
>>>
 +#include 
>>>
>>> It's hard to see why this have
>>> depends on OF followed by above header inclusion.
>>>
>> This driver depends on devicetree, so I add "depends on OF".
>> But is seems that "#include " can be removed after "of_" API
>> have been removed. Thanks for your reminder!
> 
> So, it means that technically there is no such dependency, rather
> administratively.
> 
OK. I will remove the dependent next version.
 +   hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
 +   GPIOD_OUT_LOW);
>>>
 +   if (!hisi_hikey_usb->typec_vbus)
 +   return -ENOENT;
>>>
>>> Hmm... Is it possible to get NULL pointer from gpiod_get() at all?
>>>
 +   if (!hisi_hikey_usb->otg_switch)
 +   return -ENOENT;
>>>
>>> Ditto.
>>>
>> I check the comments of devm_gpio_get API, it will not return NULL pointer.
>> But is it more safe to keep the NULL checking? What is your advice?
> 
> Why do we need dead code?
> 
OK.I will remove it.

Thanks
Yu Chen