Re: [PATCH] USB: serial: ftdi_sio: Add MTP NVM support

[Srinivas Kandagatla]

Overall nvmem side looks good!
Minor nits below.

On 14/06/18 21:08, Loic Poulain wrote:

Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...

This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.

This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.

Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.

Signed-off-by: Loic Poulain 
---
  drivers/usb/serial/Kconfig|  11 +
  drivers/usb/serial/ftdi_sio.c | 108 ++
  drivers/usb/serial/ftdi_sio.h |  28 +++
  3 files changed, 147 insertions(+)

diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
index 533f127..2dd2f5d 100644
--- a/drivers/usb/serial/Kconfig
+++ b/drivers/usb/serial/Kconfig
@@ -181,6 +181,17 @@ config USB_SERIAL_FTDI_SIO
  To compile this driver as a module, choose M here: the
  module will be called ftdi_sio.
  
+config USB_SERIAL_FTDI_SIO_NVMEM

+bool "FTDI MTP non-volatile memory support"
+   depends on USB_SERIAL_FTDI_SIO

Looks inconsistent tab and spaces here.


+select NVMEM
+default y

??

Does that mean all the FTDIs have EEPROM?


+help



+ Say yes here to add support for the MTP non-volatile memory
+ present on FTDI. Most of FTDI's devices have an EEPROM which
+ records FTDI device's configuration setting as well as user
+ data.
+
  config USB_SERIAL_VISOR
tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
help
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7ea221d..03c9c75 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -40,6 +40,7 @@
  #include 
  #include 
  #include 
+#include 
  #include "ftdi_sio.h"
  #include "ftdi_sio_ids.h"
  
@@ -73,6 +74,8 @@ struct ftdi_private {

unsigned int latency;   /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config 
ioctl() and change_speed() */
+
+   struct nvmem_device *eeprom;
  };
  
  /* struct ftdi_sio_quirk is used by devices requiring special attention. */

@@ -1529,6 +1532,104 @@ static int get_lsr_info(struct usb_serial_port *port,
return 0;
  }
  
+#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)

I think only ifdef should be Okay here as this Kconfig is just bool

+


...


+static int register_eeprom(struct usb_serial_port *port)
+{
+   struct ftdi_private *priv = usb_get_serial_port_data(port);
+   struct nvmem_config nvmconf = {};
+
+   switch (priv->chip_type) {
+   case FTX:
+   nvmconf.size = 2048;
+   break;
+   case FT232RL:
+   nvmconf.size = 128;
+   break;
+   default:
+   return 0;
+   }
+
+   nvmconf.word_size = 2;
+   nvmconf.stride = 2;
+   nvmconf.read_only = false;
+   nvmconf.priv = port;
+   nvmconf.dev = >dev;
+   nvmconf.reg_read = read_eeprom;
+   nvmconf.reg_write = write_eeprom;
+   nvmconf.owner = THIS_MODULE;
+
+   priv->eeprom = nvmem_register();
+   if (IS_ERR(priv->eeprom)) {
+   priv->eeprom = NULL;
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+static void unregister_eeprom(struct usb_serial_port *port)
+{
+   struct ftdi_private *priv = usb_get_serial_port_data(port);
+
+   if (priv->eeprom)
+   nvmem_unregister(priv->eeprom);
+}
+
+#endif /* CONFIG_USB_SERIAL_FTDI_SIO_NVMEM */
  
  /* Determine type of FTDI chip based on USB config and descriptor. */

  static void ftdi_determine_type(struct usb_serial_port *port)
@@ -1814,6 +1915,10 @@ static int ftdi_sio_port_probe(struct usb_serial_port 
*port)
priv->latency = 16;
write_latency_timer(port);
create_sysfs_attrs(port);
+#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)
+   register_eeprom(port);


Users will be clueless if the register_eeprom fails here.




+#endif
+
return 0;
  }
  
@@ -1931,6 +2036,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port *port)

  {
struct ftdi_private *priv = usb_get_serial_port_data(port);
  
+#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)

+   unregister_eeprom(port);
+#endif
remove_sysfs_attrs(port);
  
  	kfree(priv);


thanks,
srini
--
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 

Re: [PATCH] USB: serial: ftdi_sio: Add MTP NVM support

[Srinivas Kandagatla]

On 18/06/18 09:46, Johan Hovold wrote:

On Thu, Jun 14, 2018 at 10:08:46PM +0200, Loic Poulain wrote:

Most of FTDI's devices have an EEPROM which records FTDI devices
configuration setting (e.g. the VID, PID, I/O config...) and user
data. FT230R chip integrates a 128-byte eeprom, FT230X a 2048-byte
eeprom...

This patch adds support for FTDI EEPROM read/write via USB control
transfers and register a new nvm device to the nvmem core.

This permits to expose the eeprom as a sysfs file, allowing userspace
to read/modify FTDI configuration and its user data without having to
rely on a specific userspace USB driver.

Moreover, any upcoming new tentative to add CBUS GPIO support could
integrate CBUS EEPROM configuration reading in order to determine
which of the CBUS pins are available as GPIO.


I'm not necessarily against the idea, but nvmem core needs to be fixed
so that it can handle hotplugging before this can be considered for
merging.

Right now it just returns -EBUSY from nvmem_unregister(),


Yes, this is the behavior if there are active users/references for the 
nvmem provider.


As this use case seems to have come up more than once. I will take a 
closer look at making nvmem_unregister() return void, but with a BIG 
warn when there are active users.


This is already done in devm_nvmem_register/unregister apis.

I can also suggest you to try devm_nvmem_register().


thanks,
srini


 which results

in all kinds of memory leaks, use-after-frees and crashes when user
space holds the character device open while the device is being
unplugged.


+static void unregister_eeprom(struct usb_serial_port *port)
+{
+   struct ftdi_private *priv = usb_get_serial_port_data(port);
+
+   if (priv->eeprom)
+   nvmem_unregister(priv->eeprom);
+}



@@ -1931,6 +2036,9 @@ static int ftdi_sio_port_remove(struct usb_serial_port 
*port)
  {
struct ftdi_private *priv = usb_get_serial_port_data(port);
  
+#if IS_ENABLED(CONFIG_USB_SERIAL_FTDI_SIO_NVMEM)

+   unregister_eeprom(port);
+#endif
remove_sysfs_attrs(port);
  
  	kfree(priv);


Thanks,
Johan


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

Re: [PATCH] usb: echi-hcd: Add register access check in shutdown

[Srinivas Kandagatla]

On 18/05/16 15:56, Alan Stern wrote:

This doesn't seem like the right place.  What you really should do is
skip calling ehci_silence_controller() if the hardware isn't
accessible.  That's where the hardware gets touched, not in
ehci_shutdown().


Just tried this suggestion, this would not work as well, Its not just 
the hardware registers, which are of concern here, but also the rest of 
the things like ehci->hrtimer pointer which are allocated or initialized 
as part of ehci_setup().


Either the msm controller driver is not correct or we should have a way 
to stop calling ehci_shutdown() if there was no ehci_setup() done yet.


Any suggestions?


--srini
--
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: echi-hcd: Add register access check in shutdown

[Srinivas Kandagatla]

On 18/05/16 17:15, Alan Stern wrote:

On Wed, 18 May 2016, Srinivas Kandagatla wrote:


On 18/05/16 15:56, Alan Stern wrote:

On Wed, 18 May 2016, Srinivas Kandagatla wrote:


This patch adds a check in ehci_shutdown(), to make sure
that the register access is available before accessing registers.

The use case is simple, for boards like DB410c where the usb host
or device functionality is decided based on the micro-usb cable
presence. If the board boots up with micro-usb connected and the
host driver is probed, but the ehci_setup() has not been done yet,
then a system shutdown would trigger below NULL pointer exception
without this patch.


How can that happen?  While the host driver is probed, the probing
thread holds the device lock.  But the system shutdown routine acquires
the device lock before invoking the ->shutdown callback.  Therefore the
two things cannot happen concurrently.


No, I did not mean them happening concurrently, I mean that the host
driver is up, however ehci_setup() is not done yet.


I don't understand.  ehci_setup() is called as part of the probe
procedure.  How can the host driver be up if ehci_setup() is not done
yet?

Yes, this is true in ehci-msm driver, The driver does not add usb host 
by default in probe when phy is otg capable.


The usb host is added dynamically by the msm_otg driver depending on the 
the micro USB cable plug/un-plug events via extcon.



Are you saying that when the system is plugged into the "B" end of an
OTG cable, ehci_setup() doesn't get called at all?


Yes, for echi-msm driver, not sure about other host controller drivers.


And would the same thing happen if the system started out as the host
but then used HNP to change into the peripheral?
I don't think so, As the ehci->regs get populated once we enter the 
ehci_setup(), so ehci_halt() will never get chance to dereference null 
in this case.


Fault occurs only if the driver did not enter into host mode and system 
reboot/shutdown is requested.


--srini




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: echi-hcd: Add register access check in shutdown

[Srinivas Kandagatla]

On 18/05/16 15:56, Alan Stern wrote:

On Wed, 18 May 2016, Srinivas Kandagatla wrote:


This patch adds a check in ehci_shutdown(), to make sure
that the register access is available before accessing registers.

The use case is simple, for boards like DB410c where the usb host
or device functionality is decided based on the micro-usb cable
presence. If the board boots up with micro-usb connected and the
host driver is probed, but the ehci_setup() has not been done yet,
then a system shutdown would trigger below NULL pointer exception
without this patch.


How can that happen?  While the host driver is probed, the probing
thread holds the device lock.  But the system shutdown routine acquires
the device lock before invoking the ->shutdown callback.  Therefore the
two things cannot happen concurrently.


No, I did not mean them happening concurrently, I mean that the host 
driver is up, however ehci_setup() is not done yet.


Will change the comments if its misleading the reader.




Unable to handle kernel NULL pointer dereference at virtual address
0008


...


--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -368,6 +368,9 @@ static void ehci_shutdown(struct usb_hcd *hcd)
  {
struct ehci_hcd *ehci = hcd_to_ehci(hcd);

+   if (!HCD_HW_ACCESSIBLE(hcd))
+   return;
+
spin_lock_irq(>lock);
ehci->shutdown = true;
ehci->rh_state = EHCI_RH_STOPPING;


This doesn't seem like the right place.  What you really should do is
skip calling ehci_silence_controller() if the hardware isn't
accessible.  That's where the hardware gets touched, not in
ehci_shutdown().

Yep , that should work as well.

Will send a v2 patch.

thanks,
srini



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

[PATCH] usb: echi-hcd: Add register access check in shutdown

[Srinivas Kandagatla]

This patch adds a check in ehci_shutdown(), to make sure
that the register access is available before accessing registers.

The use case is simple, for boards like DB410c where the usb host
or device functionality is decided based on the micro-usb cable
presence. If the board boots up with micro-usb connected and the
host driver is probed, but the ehci_setup() has not been done yet,
then a system shutdown would trigger below NULL pointer exception
without this patch.

Unable to handle kernel NULL pointer dereference at virtual address
0008

pgd = ffc034581000
[0008] *pgd=, *pud=
CPU: 2 PID: 1957 Comm: reboot Not tainted 4.6.0+ #99
task: ffc034bc ti: ffc0345cc000 task.ti: ffc0345cc000
PC is at ehci_halt+0x54/0x108
LR is at ehci_halt+0x38/0x108
pc : [] lr : [] pstate: a1c5
sp : ffc0345cfc60
x29: ffc0345cfc60 x28: ffc0345cc000
x27: ff8008a4d000 x26: 008e
x25: ff8008d86cb0 x24: ff800908b040
x23: ffc036068870 x22: ff8009d0a000
x21: ffc03512a410 x20: ffc03512a410
x19: ffc03512a338 x18: 65ba
x17: ff8009b16b80 x16: 0003
x15: 65b9 x14: 65b6
x13:  x12: 
x11: 003d x10: ffc0345cf9e0
x9 : 0001 x8 : ffc0345cc000
x7 : ff8008698360 x6 : 
x5 : 0080 x4 : 0001
x3 :  x2 : 
x1 : 0008 x0 : ffc034bc

Process reboot (pid: 1957, stack limit = 0xffc0345cc020)
Stack: (0xffc0345cfc60 to 0xffc0345d)
fc60: ffc0345cfc90 ff8008698448 ffc03512a338 ffc03512a338
fc80: ffc03512a410 ff8008a3bbfc ffc0345cfcc0 ff8008698548
fca0: ffc03512a338 ffc03512a000 ffc03512a410 ff8009d0a000
fcc0: ffc0345cfcf0 ff800865d2bc ffc036068828 ffc036068810
fce0: ffc036003810 ff800853f43c ffc0345cfd00 ff800854338c
fd00: ffc0345cfd10 ff800853f45c ffc0345cfd60 ff80080e0f48
fd20:  01234567 ff8008f8c000 ff8008f8c060
fd40:  0015 0120 ff80080e0f30
fd60: ffc0345cfd70 ff80080e1020 ffc0345cfd90 ff80080e12fc
fd80:  01234567  ff8008085e70
fda0:  005592905000  007f79daf1cc
fdc0:   007ffcbb1198 000a
fde0: 0055928d3f58 0001 ffc03490 fffe
fe00: ffc03490 007f79da902c ffc0345cfe40 ff800820af38
fe20:  007ffcbb1078  ff80081e9b38
fe40: ffc0345cfe60 ff80081eb410 ffc0345cfe60 ff80081eb444
fe60: ffc0345cfec0 ff80081ec4f4  007ffcbb1078
fe80:  0015 ffc0345cfec0 007ffcbb1078
fea0: 0002 000a  
fec0:  ff8008085e70 fee1dead 28121969
fee0: 01234567   80808080
ff00: 80808080 007ffcbb10f0 008e fefeff54918cb8c7
ff20: 7f7f7f7f 0101010101010101 0010 
ff40:  007f79e33588 005592905eb8 007f79daf1b0
ff60: 007ffcbb1340 005592906000 005592905000 005592906000
ff80: 005592907000 0002 007ffcbb1d98 005592906000
ffa0: 0055928d2000   007ffcbb1aa0
ffc0: 0055928b819c 007ffcbb1aa0 007f79daf1cc 
ffe0: fee1dead 008e 05ef55505715 d44d55d775d3
Call trace:
Exception stack(0xffc0345cfaa0 to 0xffc0345cfbc0)
Set corner to 6
faa0: ffc03512a338 ffc03512a410 ffc0345cfc60 ff800869837c
fac0: ff8008114210 00010001 ff8009ce1b20 ff8009ce5f20
fae0: ffc0345cfb80 ff80081145a8 ffc0345cfc10 ff800810b924
fb00: ffc0345cc000 01c0 ffc03512a410 ff8009d0a000
fb20: ffc036068870 ff800908b040 ff8008d86cb0 008e
fb40: ffc034bc 0008  
fb60: 0001 0080  ff8008698360
fb80: ffc0345cc000 0001 ffc0345cf9e0 003d
fba0:   65b6 65b9
[] ehci_halt+0x54/0x108
[] ehci_silence_controller+0x18/0xcc
[] ehci_shutdown+0x4c/0x64
[] usb_hcd_platform_shutdown+0x1c/0x24
[] platform_drv_shutdown+0x20/0x28
[] device_shutdown+0xf4/0x1b0
[] kernel_restart_prepare+0x34/0x3c
[] kernel_restart+0x14/0x74
[] SyS_reboot+0x110/0x21c
[] el0_svc_naked+0x24/0x28
Code: 53001c42 35a2 d5033e9f 91002021 (b922)

Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
---
 drivers/usb/hos

Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT

[Srinivas Kandagatla]

On 22/02/16 05:32, Bjorn Andersson wrote:

On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
to be able to do DMA allocations, so use the of_dma_configure() helper
to populate the dma properties and assign an appropriate dma_ops.

Signed-off-by: Bjorn Andersson 
---
  drivers/usb/chipidea/core.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7404064b9bbc..047b9d4e67aa 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -62,6 +62,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -834,6 +835,9 @@ struct platform_device *ci_hdrc_add_device(struct device 
*dev,
pdev->dev.dma_parms = dev->dma_parms;
dma_set_coherent_mask(>dev, dev->coherent_dma_mask);

+   if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+   of_dma_configure(>dev, dev->of_node);
+
Would we hit the same issue if we are on non Device tree platforms like 
ACPI or platform device style itself?




ret = platform_device_add_resources(pdev, res, nres);
if (ret)
goto err;



I think this is the side effect of commit 
1dccb598df549d892b6450c261da54cdd7af44b4(arm64: simplify dma_get_ops)


None of the drivers call of_dma_configure() explicitly, which makes me 
feel that we are doing something wrong. TBH, this should be handled in 
more generic way rather than driver like this having an explicit call to 
of_dma_configure().



On the other hand, I think we could also solve the issue by using 
correct device(parent device) while allocating dma/dma-pool.



>cut<-

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 6e53c24..0293ed5 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1844,13 +1844,13 @@ static int udc_start(struct ci_hdrc *ci)
INIT_LIST_HEAD(>gadget.ep_list);

/* alloc resources */
-   ci->qh_pool = dma_pool_create("ci_hw_qh", dev,
+   ci->qh_pool = dma_pool_create("ci_hw_qh", dev->parent,
   sizeof(struct ci_hw_qh),
   64, CI_HDRC_PAGE_SIZE);
if (ci->qh_pool == NULL)
return -ENOMEM;

-   ci->td_pool = dma_pool_create("ci_hw_td", dev,
+   ci->td_pool = dma_pool_create("ci_hw_td", dev->parent,
   sizeof(struct ci_hw_td),
   64, CI_HDRC_PAGE_SIZE);
if (ci->td_pool == NULL) {
>cut<-


--srini
--
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: phy: msm: Trigger USB state detection work in DRD mode

[Srinivas Kandagatla]

From: "Ivan T. Ivanov" <ivan.iva...@linaro.org>

When working in Dual Role Device mode, USB state machine is not kicked,
when host or gadget drivers are loaded. Fix this be explicitly triggering
state detection on client driver load.

Issue is that if the board is booted without micro usb cable and usb
device attached, kernel fails to populate the usb host and device.
The reason for this is that the state machine worker logic only checks
for USB_DR_MODE_PERIPHERAL and USB_DR_MODE_HOST modes to run worker
thread. However if the phy is configured in OTG mode it would fail
to run the state machine, resulting in failure to detect for very
first time.

This patch fixes the issue by removing the explicit checks.

Issue is noticed on Qualcomm Dragon board DB410C.

[srinivas.kandaga...@linaro.org: Added more details to log]
Signed-off-by: Ivan T. Ivanov <ivan.iva...@linaro.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
---
 drivers/usb/phy/phy-msm-usb.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 970a30e..72b387d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -757,14 +757,8 @@ static int msm_otg_set_host(struct usb_otg *otg, struct 
usb_bus *host)
otg->host = host;
dev_dbg(otg->usb_phy->dev, "host driver registered w/ tranceiver\n");
 
-   /*
-* Kick the state machine work, if peripheral is not supported
-* or peripheral is already registered with us.
-*/
-   if (motg->pdata->mode == USB_DR_MODE_HOST || otg->gadget) {
-   pm_runtime_get_sync(otg->usb_phy->dev);
-   schedule_work(>sm_work);
-   }
+   pm_runtime_get_sync(otg->usb_phy->dev);
+   schedule_work(>sm_work);
 
return 0;
 }
@@ -827,14 +821,8 @@ static int msm_otg_set_peripheral(struct usb_otg *otg,
dev_dbg(otg->usb_phy->dev,
"peripheral driver registered w/ tranceiver\n");
 
-   /*
-* Kick the state machine work, if host is not supported
-* or host is already registered with us.
-*/
-   if (motg->pdata->mode == USB_DR_MODE_PERIPHERAL || otg->host) {
-   pm_runtime_get_sync(otg->usb_phy->dev);
-   schedule_work(>sm_work);
-   }
+   pm_runtime_get_sync(otg->usb_phy->dev);
+   schedule_work(>sm_work);
 
return 0;
 }
-- 
1.9.1

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

Re: [PATCH v2] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

Thanks Felipe,

On 20/08/14 16:57, Felipe Balbi wrote:

On Thu, Jul 17, 2014 at 09:16:40PM +0100, Srinivas Kandagatla wrote:

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

phy-reset clk is only used as argument to the mach level callbacks, so
this patch adds condition before clk_get calls so that the driver
wouldn't fail on SOCs which do not have this support.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
Hi Felipe,

With this new patch now the error message is only printed if the SOC actually 
supports
the phy reset clk, for SOCs like APQ8064 where there is no phy reset clock or
the callback which takes it there is no point in doing a clk_get call in the 
first place.


doesn't apply. Please rebase on top of v3.17-rc1


this is because a previous version of the same patch got applied.
Anyway I will rebase this patch and send v3.

thanks,
srini
--
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 v3] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

phy-reset clk is only used as argument to the mach level callbacks, so
this patch adds condition before clk_get calls so that the driver
wouldn't fail on SOCs which do not have this support.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
Hi Felipe,

With this new patch now the error message is only printed if the SOC actually 
supports
the phy reset clk, for SOCs like APQ8064 where there is no phy reset clock or
the callback which takes it there is no point in doing a clk_get call in the 
first place.

Changes since v2:
- rebased patch on top of v3.17-rc1


Thanks,
srini

 drivers/usb/phy/phy-msm-usb.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index e4108ee..7f6aa32 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -281,7 +281,7 @@ static int msm_otg_phy_clk_reset(struct msm_otg *motg)
 {
int ret = 0;
 
-   if (motg-pdata-phy_clk_reset  motg-phy_reset_clk)
+   if (motg-pdata-phy_clk_reset)
ret = motg-pdata-phy_clk_reset(motg-phy_reset_clk);
else if (motg-phy_rst)
ret = reset_control_reset(motg-phy_rst);
@@ -1554,11 +1554,14 @@ static int msm_otg_probe(struct platform_device *pdev)
phy = motg-phy;
phy-dev = pdev-dev;
 
-   motg-phy_reset_clk = devm_clk_get(pdev-dev,
+   if (motg-pdata-phy_clk_reset) {
+   motg-phy_reset_clk = devm_clk_get(pdev-dev,
   np ? phy : usb_phy_clk);
-   if (IS_ERR(motg-phy_reset_clk)) {
-   dev_err(pdev-dev, failed to get usb_phy_clk\n);
-   motg-phy_reset_clk = NULL;
+
+   if (IS_ERR(motg-phy_reset_clk)) {
+   dev_err(pdev-dev, failed to get usb_phy_clk\n);
+   return PTR_ERR(motg-phy_reset_clk);
+   }
}
 
motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
-- 
1.9.1

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

Re: [RFC PATCH 2/3 ] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

On 17/07/14 13:54, pramod gurav wrote:

Hi Srini,

On Thu, Jul 17, 2014 at 6:19 PM,  pramod.gurav@gmail.com wrote:

From: Srinivas Kandagatla srinivas.kandaga...@linaro.org

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.


.
[snip]
.


 pdata-mode = of_usb_get_dr_mode(node);
 if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
np ? phy : usb_phy_clk);
 if (IS_ERR(motg-phy_reset_clk)) {
 dev_err(pdev-dev, failed to get usb_phy_clk\n);


I keep getting this error on IFC6410. Cant we suppress it?
IMO, We should ignore this message for IFC 6410 board as they do not 
have phy reset clk.






-   return PTR_ERR(motg-phy_reset_clk);
+   motg-phy_reset_clk = NULL;


for non-ifc boards(having this clk), if they have this clock should
not code return on failure to get the usb_phy_clk?

I agree, Its a catch 22 situation here.
Unless we introduce more SOC level awareness into this driver. Which 
would be a bit overdo for printing this message.


The error message should be considered more seriously for non IFC board.

Thanks,
srini



 }

 motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
--
1.7.9.5






--
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: [RFC PATCH 2/3 ] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

On 17/07/14 15:39, Felipe Balbi wrote:

Hi,

On Thu, Jul 17, 2014 at 02:14:15PM +0100, Srinivas Kandagatla wrote:

On 17/07/14 13:54, pramod gurav wrote:

Hi Srini,

On Thu, Jul 17, 2014 at 6:19 PM,  pramod.gurav@gmail.com wrote:

From: Srinivas Kandagatla srinivas.kandaga...@linaro.org

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.


.
[snip]
.


 pdata-mode = of_usb_get_dr_mode(node);
 if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
np ? phy : usb_phy_clk);
 if (IS_ERR(motg-phy_reset_clk)) {
 dev_err(pdev-dev, failed to get usb_phy_clk\n);


I keep getting this error on IFC6410. Cant we suppress it?

IMO, We should ignore this message for IFC 6410 board as they do not have
phy reset clk.


looks like that should become a dev_dbg() then ?


Sure, I agree.

I will resend the patch with dev_dbg.

--srini
--
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] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

phy-reset clk is only used as argument to the mach level callbacks, so
this patch adds condition before clk_get calls so that the driver
wouldn't fail on SOCs which do not have this support.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
Hi Felipe,

With this new patch now the error message is only printed if the SOC actually 
supports
the phy reset clk, for SOCs like APQ8064 where there is no phy reset clock or
the callback which takes it there is no point in doing a clk_get call in the 
first place.


Thanks,
srini




 drivers/usb/phy/phy-msm-usb.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c929370..aa8e2b9 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -279,11 +279,11 @@ static int msm_otg_link_clk_reset(struct msm_otg *motg, 
bool assert)
 
 static int msm_otg_phy_clk_reset(struct msm_otg *motg)
 {
-   int ret;
+   int ret = 0;
 
if (motg-pdata-phy_clk_reset)
ret = motg-pdata-phy_clk_reset(motg-phy_reset_clk);
-   else
+   else if (motg-phy_rst)
ret = reset_control_reset(motg-phy_rst);
 
if (ret)
@@ -1466,7 +1466,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
 
motg-phy_rst = devm_reset_control_get(pdev-dev, phy);
if (IS_ERR(motg-phy_rst))
-   return PTR_ERR(motg-phy_rst);
+   motg-phy_rst = NULL;
 
pdata-mode = of_usb_get_dr_mode(node);
if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1554,11 +1554,14 @@ static int msm_otg_probe(struct platform_device *pdev)
phy = motg-phy;
phy-dev = pdev-dev;
 
-   motg-phy_reset_clk = devm_clk_get(pdev-dev,
+   if (motg-pdata-phy_clk_reset) {
+   motg-phy_reset_clk = devm_clk_get(pdev-dev,
   np ? phy : usb_phy_clk);
-   if (IS_ERR(motg-phy_reset_clk)) {
-   dev_err(pdev-dev, failed to get usb_phy_clk\n);
-   return PTR_ERR(motg-phy_reset_clk);
+
+   if (IS_ERR(motg-phy_reset_clk)) {
+   dev_err(pdev-dev, failed to get usb_phy_clk\n);
+   return PTR_ERR(motg-phy_reset_clk);
+   }
}
 
motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
-- 
1.9.1

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

Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Srinivas Kandagatla]

Hi Felipe,

On 27/06/14 16:54, Felipe Balbi wrote:

Hi,

On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote:

Use case is when the phy is configured in host mode and a usb device is
attached to board before bootup. On bootup, with the existing code and
runtime pm enabled, the driver would decrement the pm usage count
without checking the current state of the phy. This pm usage count
decrement would trigger the runtime pm which than would abort the
usb enumeration which was in progress. In my case a usb stick gets
detected and then immediatly the driver goes to low power mode which is
not correct.

log:
[1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
[1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
bus number 1
[1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
[1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
[1.659473] hub 1-0:1.0: USB hub found
[1.663415] hub 1-0:1.0: 1 port detected
...
[1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
[2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
[2.108993] scsi0 : usb-storage 1-1:1.0
[2.678341] msm_otg 1252.phy: USB in low power mode
[3.168977] usb 1-1: USB disconnect, device number 2

This issue was detected on IFC6410 board.

This patch fixes the intial runtime pm trigger by checking the phy
state and decrementing the pm use count only when the phy state is IDLE.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
  drivers/usb/phy/phy-msm-usb.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 3bb559d..78cc870 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
motg-chg_state = USB_CHG_STATE_UNDEFINED;
motg-chg_type = USB_INVALID_CHARGER;
}
-   pm_runtime_put_sync(otg-phy-dev);
+
+   if (otg-phy-state == OTG_STATE_B_IDLE)
+   pm_runtime_put_sync(otg-phy-dev);


instead, you might as well just use autosuspend.


autosuspend is a good idea and will provide a delay before rumtime 
suspend, however the bug which is fixed here still needs to be fixed anyway.


runtime PM in this driver is not that great, the driver just increments 
the pm use count if the phy is configured in host or deivce mode. This 
patch fixes a bug in its state-machine which decrements pm use-count 
without checking the state.


As this is a PHY driver, am not sure moving to autosuspend means we 
would end up just adding some static inactivity delay? Is it really 
going to add any value to this PHY driver?




--srini



--
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: [RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.

[Srinivas Kandagatla]

Thanks Felipe,


On 30/06/14 18:13, Felipe Balbi wrote:

Hi,

On Wed, Jun 18, 2014 at 06:00:01PM +0100, Srinivas Kandagatla wrote:

While testing usb host on Qualcomm APQ8064, I encountered few issues.
These patches fixes those issues.

Without these patches USB is not functional on AQ8064.
All the patches are tested on IFC6410.


please resend without RFC so I can apply.


I will resend the patches.

--srini
--
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 v1 0/3] ehci_msm fixes for APQ8064 USB host support.

[Srinivas Kandagatla]

While testing usb host on Qualcomm APQ8064, I encountered few issues.
These patches fixes those issues.

Without these patches USB is not functional on AQ8064.
All the patches are tested on IFC6410.

Resending the patches by removing the RFC in the subject.

Can you please consider these patches for next rc, as they are just fixes.

Thanks,
srini

Srinivas Kandagatla (3):
  usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
  usb: phy: msm: Make phy_reset clk and reset line optional.
  usb: phy: msm: Do not do runtime pm if the phy is not idle

 drivers/usb/host/Kconfig  |  2 +-
 drivers/usb/phy/phy-msm-usb.c | 14 --
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
1.9.1

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

[PATCH v1 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs

[Srinivas Kandagatla]

This patch makes the msm ehci driver available to use on QCOM SOCs,
which have the same IP.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 61b7817..03314f8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91
 
 config USB_EHCI_MSM
tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller
-   depends on ARCH_MSM
+   depends on ARCH_MSM || ARCH_QCOM
select USB_EHCI_ROOT_HUB_TT
---help---
  Enables support for the USB Host controller present on the
-- 
1.9.1

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

[PATCH v1 2/3] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/phy/phy-msm-usb.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ced34f3..3bb559d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -279,11 +279,11 @@ static int msm_otg_link_clk_reset(struct msm_otg *motg, 
bool assert)
 
 static int msm_otg_phy_clk_reset(struct msm_otg *motg)
 {
-   int ret;
+   int ret = 0;
 
-   if (motg-pdata-phy_clk_reset)
+   if (motg-pdata-phy_clk_reset  motg-phy_reset_clk)
ret = motg-pdata-phy_clk_reset(motg-phy_reset_clk);
-   else
+   else if (motg-phy_rst)
ret = reset_control_reset(motg-phy_rst);
 
if (ret)
@@ -1464,7 +1464,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
 
motg-phy_rst = devm_reset_control_get(pdev-dev, phy);
if (IS_ERR(motg-phy_rst))
-   return PTR_ERR(motg-phy_rst);
+   motg-phy_rst = NULL;
 
pdata-mode = of_usb_get_dr_mode(node);
if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
   np ? phy : usb_phy_clk);
if (IS_ERR(motg-phy_reset_clk)) {
dev_err(pdev-dev, failed to get usb_phy_clk\n);
-   return PTR_ERR(motg-phy_reset_clk);
+   motg-phy_reset_clk = NULL;
}
 
motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
-- 
1.9.1

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

[PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Srinivas Kandagatla]

Use case is when the phy is configured in host mode and a usb device is
attached to board before bootup. On bootup, with the existing code and
runtime pm enabled, the driver would decrement the pm usage count
without checking the current state of the phy. This pm usage count
decrement would trigger the runtime pm which than would abort the
usb enumeration which was in progress. In my case a usb stick gets
detected and then immediatly the driver goes to low power mode which is
not correct.

log:
[1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
[1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
bus number 1
[1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
[1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
[1.659473] hub 1-0:1.0: USB hub found
[1.663415] hub 1-0:1.0: 1 port detected
...
[1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
[2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
[2.108993] scsi0 : usb-storage 1-1:1.0
[2.678341] msm_otg 1252.phy: USB in low power mode
[3.168977] usb 1-1: USB disconnect, device number 2

This issue was detected on IFC6410 board.

This patch fixes the intial runtime pm trigger by checking the phy
state and decrementing the pm use count only when the phy state is IDLE.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/phy/phy-msm-usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 3bb559d..78cc870 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
motg-chg_state = USB_CHG_STATE_UNDEFINED;
motg-chg_type = USB_INVALID_CHARGER;
}
-   pm_runtime_put_sync(otg-phy-dev);
+
+   if (otg-phy-state == OTG_STATE_B_IDLE)
+   pm_runtime_put_sync(otg-phy-dev);
break;
case OTG_STATE_B_PERIPHERAL:
dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n);
-- 
1.9.1

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

Re: [PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Srinivas Kandagatla]

On 30/06/14 18:56, Felipe Balbi wrote:

On Mon, Jun 30, 2014 at 06:29:57PM +0100, Srinivas Kandagatla wrote:

Use case is when the phy is configured in host mode and a usb device is
attached to board before bootup. On bootup, with the existing code and
runtime pm enabled, the driver would decrement the pm usage count
without checking the current state of the phy. This pm usage count
decrement would trigger the runtime pm which than would abort the
usb enumeration which was in progress. In my case a usb stick gets
detected and then immediatly the driver goes to low power mode which is
not correct.

log:
[1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
[1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
bus number 1
[1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
[1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
[1.659473] hub 1-0:1.0: USB hub found
[1.663415] hub 1-0:1.0: 1 port detected
...
[1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
[2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
[2.108993] scsi0 : usb-storage 1-1:1.0
[2.678341] msm_otg 1252.phy: USB in low power mode
[3.168977] usb 1-1: USB disconnect, device number 2

This issue was detected on IFC6410 board.

This patch fixes the intial runtime pm trigger by checking the phy
state and decrementing the pm use count only when the phy state is IDLE.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
  drivers/usb/phy/phy-msm-usb.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 3bb559d..78cc870 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
motg-chg_state = USB_CHG_STATE_UNDEFINED;
motg-chg_type = USB_INVALID_CHARGER;
}
-   pm_runtime_put_sync(otg-phy-dev);
+
+   if (otg-phy-state == OTG_STATE_B_IDLE)
+   pm_runtime_put_sync(otg-phy-dev);
break;
case OTG_STATE_B_PERIPHERAL:
dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n);


does this need to go on this -rc or can it wait until v3.17 merge


Getting it to rc would be nice :-), but I will leave it up to you.


window? Also, how far back should this be backported ?


This patch is required if runtime pm is activated.

--srini





--
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: [RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.

[Srinivas Kandagatla]

Is it possible to queue these patches for next rc?


Thanks,
srini
On 18/06/14 18:00, Srinivas Kandagatla wrote:

While testing usb host on Qualcomm APQ8064, I encountered few issues.
These patches fixes those issues.

Without these patches USB is not functional on AQ8064.
All the patches are tested on IFC6410.

Thanks,
srini

Srinivas Kandagatla (3):
   usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
   usb: phy: msm: Make phy_reset clk and reset line optional.
   usb: phy: msm: Do not do runtime pm if the phy is not idle

  drivers/usb/host/Kconfig  |  2 +-
  drivers/usb/phy/phy-msm-usb.c | 14 --
  2 files changed, 9 insertions(+), 7 deletions(-)


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

[RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.

[Srinivas Kandagatla]

While testing usb host on Qualcomm APQ8064, I encountered few issues.
These patches fixes those issues.

Without these patches USB is not functional on AQ8064.
All the patches are tested on IFC6410.

Thanks,
srini

Srinivas Kandagatla (3):
  usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
  usb: phy: msm: Make phy_reset clk and reset line optional.
  usb: phy: msm: Do not do runtime pm if the phy is not idle

 drivers/usb/host/Kconfig  |  2 +-
 drivers/usb/phy/phy-msm-usb.c | 14 --
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
1.9.1

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

[RFC PATCH 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs

[Srinivas Kandagatla]

This patch makes the msm ehci driver available to use on QCOM SOCs,
which have the same IP.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 61b7817..03314f8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91
 
 config USB_EHCI_MSM
tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller
-   depends on ARCH_MSM
+   depends on ARCH_MSM || ARCH_QCOM
select USB_EHCI_ROOT_HUB_TT
---help---
  Enables support for the USB Host controller present on the
-- 
1.9.1

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

[RFC PATCH 2/3] usb: phy: msm: Make phy_reset clk and reset line optional.

[Srinivas Kandagatla]

This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/phy/phy-msm-usb.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ced34f3..3bb559d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -279,11 +279,11 @@ static int msm_otg_link_clk_reset(struct msm_otg *motg, 
bool assert)
 
 static int msm_otg_phy_clk_reset(struct msm_otg *motg)
 {
-   int ret;
+   int ret = 0;
 
-   if (motg-pdata-phy_clk_reset)
+   if (motg-pdata-phy_clk_reset  motg-phy_reset_clk)
ret = motg-pdata-phy_clk_reset(motg-phy_reset_clk);
-   else
+   else if (motg-phy_rst)
ret = reset_control_reset(motg-phy_rst);
 
if (ret)
@@ -1464,7 +1464,7 @@ static int msm_otg_read_dt(struct platform_device *pdev, 
struct msm_otg *motg)
 
motg-phy_rst = devm_reset_control_get(pdev-dev, phy);
if (IS_ERR(motg-phy_rst))
-   return PTR_ERR(motg-phy_rst);
+   motg-phy_rst = NULL;
 
pdata-mode = of_usb_get_dr_mode(node);
if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
   np ? phy : usb_phy_clk);
if (IS_ERR(motg-phy_reset_clk)) {
dev_err(pdev-dev, failed to get usb_phy_clk\n);
-   return PTR_ERR(motg-phy_reset_clk);
+   motg-phy_reset_clk = NULL;
}
 
motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
-- 
1.9.1

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

[RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle

[Srinivas Kandagatla]

Use case is when the phy is configured in host mode and a usb device is
attached to board before bootup. On bootup, with the existing code and
runtime pm enabled, the driver would decrement the pm usage count
without checking the current state of the phy. This pm usage count
decrement would trigger the runtime pm which than would abort the
usb enumeration which was in progress. In my case a usb stick gets
detected and then immediatly the driver goes to low power mode which is
not correct.

log:
[1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller
[1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned 
bus number 1
[1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252
[1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00
[1.659473] hub 1-0:1.0: USB hub found
[1.663415] hub 1-0:1.0: 1 port detected
...
[1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host
[2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected
[2.108993] scsi0 : usb-storage 1-1:1.0
[2.678341] msm_otg 1252.phy: USB in low power mode
[3.168977] usb 1-1: USB disconnect, device number 2

This issue was detected on IFC6410 board.

This patch fixes the intial runtime pm trigger by checking the phy
state and decrementing the pm use count only when the phy state is IDLE.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
 drivers/usb/phy/phy-msm-usb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 3bb559d..78cc870 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w)
motg-chg_state = USB_CHG_STATE_UNDEFINED;
motg-chg_type = USB_INVALID_CHARGER;
}
-   pm_runtime_put_sync(otg-phy-dev);
+
+   if (otg-phy-state == OTG_STATE_B_IDLE)
+   pm_runtime_put_sync(otg-phy-dev);
break;
case OTG_STATE_B_PERIPHERAL:
dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n);
-- 
1.9.1

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

Re: [PATCH v3 1/3] usb: chipidea: msm: Add device tree binding information

[Srinivas Kandagatla]

On 22/04/14 10:43, Ivan T. Ivanov wrote:

+- interrupts:   interrupt-specifier for the controller interrupt.
+- usb-phy:  phandle for the PHY device
+- dr_mode:  Sould be peripheral

s/Sould/Should/

+

--
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 3.6.0- 0/7] USB: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

Running below Coccinelle lookup pattern like below on the 
latest kernel showed about 52 hits. This patch series is a subset 
of those 52 patches, so that it will be easy for maintainers to review.
Hopefully these patches will get rid of some code duplication in kernel.


@  @
- initfunc(void)
- { return platform_driver_register(dr); }

...

- module_init(initfunc);
...

- exitfunc(void)
- { platform_driver_unregister(dr); }

...

- module_exit(exitfunc);
+ module_platform_driver(dr); 


Srinivas Kandagatla (7):
  usb/am35x: use module_platform_driver macro
  usb/blackfin: use module_platform_driver macro
  usb/da8xx: use module_platform_driver macro
  usb/davinci: use module_platform_driver macro
  usb/tusb6010: use module_platform_driver macro
  usb/ux500: use module_platform_driver macro
  usb/mv_otg: use module_platform_driver macro

 drivers/usb/musb/am35x.c|   13 +
 drivers/usb/musb/blackfin.c |   13 +
 drivers/usb/musb/da8xx.c|   13 +
 drivers/usb/musb/davinci.c  |   13 +
 drivers/usb/musb/tusb6010.c |   13 +
 drivers/usb/musb/ux500.c|   13 +
 drivers/usb/otg/mv_otg.c|   14 +-
 7 files changed, 7 insertions(+), 85 deletions(-)

--
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 3.6.0- 1/7] usb/am35x: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

This patch removes some code duplication by using
module_platform_driver.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
 drivers/usb/musb/am35x.c |   13 +
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 457f25e..89b128b 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -648,15 +648,4 @@ static struct platform_driver am35x_driver = {
 MODULE_DESCRIPTION(AM35x MUSB Glue Layer);
 MODULE_AUTHOR(Ajay Kumar Gupta ajay.gu...@ti.com);
 MODULE_LICENSE(GPL v2);
-
-static int __init am35x_init(void)
-{
-   return platform_driver_register(am35x_driver);
-}
-module_init(am35x_init);
-
-static void __exit am35x_exit(void)
-{
-   platform_driver_unregister(am35x_driver);
-}
-module_exit(am35x_exit);
+module_platform_driver(am35x_driver);
-- 
1.7.0.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 3.6.0- 2/7] usb/blackfin: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

This patch removes some code duplication by using
module_platform_driver.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
 drivers/usb/musb/blackfin.c |   13 +
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
index e8cff9b..8c16a22 100644
--- a/drivers/usb/musb/blackfin.c
+++ b/drivers/usb/musb/blackfin.c
@@ -585,15 +585,4 @@ static struct platform_driver bfin_driver = {
 MODULE_DESCRIPTION(Blackfin MUSB Glue Layer);
 MODULE_AUTHOR(Bryan Wy coolo...@kernel.org);
 MODULE_LICENSE(GPL v2);
-
-static int __init bfin_init(void)
-{
-   return platform_driver_register(bfin_driver);
-}
-module_init(bfin_init);
-
-static void __exit bfin_exit(void)
-{
-   platform_driver_unregister(bfin_driver);
-}
-module_exit(bfin_exit);
+module_platform_driver(bfin_driver);
-- 
1.7.0.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 3.6.0- 4/7] usb/davinci: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

This patch removes some code duplication by using
module_platform_driver.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
 drivers/usb/musb/davinci.c |   13 +
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 606bfd0..62785bf 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -625,15 +625,4 @@ static struct platform_driver davinci_driver = {
 MODULE_DESCRIPTION(DaVinci MUSB Glue Layer);
 MODULE_AUTHOR(Felipe Balbi ba...@ti.com);
 MODULE_LICENSE(GPL v2);
-
-static int __init davinci_init(void)
-{
-   return platform_driver_register(davinci_driver);
-}
-module_init(davinci_init);
-
-static void __exit davinci_exit(void)
-{
-   platform_driver_unregister(davinci_driver);
-}
-module_exit(davinci_exit);
+module_platform_driver(davinci_driver);
-- 
1.7.0.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 3.6.0- 7/7] usb/mv_otg: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

This patch removes some code duplication by using
module_platform_driver.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
 drivers/usb/otg/mv_otg.c |   14 +-
 1 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/otg/mv_otg.c b/drivers/usb/otg/mv_otg.c
index 3f124e8..1dd5750 100644
--- a/drivers/usb/otg/mv_otg.c
+++ b/drivers/usb/otg/mv_otg.c
@@ -958,16 +958,4 @@ static struct platform_driver mv_otg_driver = {
.resume = mv_otg_resume,
 #endif
 };
-
-static int __init mv_otg_init(void)
-{
-   return platform_driver_register(mv_otg_driver);
-}
-
-static void __exit mv_otg_exit(void)
-{
-   platform_driver_unregister(mv_otg_driver);
-}
-
-module_init(mv_otg_init);
-module_exit(mv_otg_exit);
+module_platform_driver(mv_otg_driver);
-- 
1.7.0.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 3.6.0- 5/7] usb/tusb6010: use module_platform_driver macro

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla srinivas.kandaga...@st.com

This patch removes some code duplication by using
module_platform_driver.

Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
 drivers/usb/musb/tusb6010.c |   13 +
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index dc4d75e..39ece28 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -1251,15 +1251,4 @@ static struct platform_driver tusb_driver = {
 MODULE_DESCRIPTION(TUSB6010 MUSB Glue Layer);
 MODULE_AUTHOR(Felipe Balbi ba...@ti.com);
 MODULE_LICENSE(GPL v2);
-
-static int __init tusb_init(void)
-{
-   return platform_driver_register(tusb_driver);
-}
-module_init(tusb_init);
-
-static void __exit tusb_exit(void)
-{
-   platform_driver_unregister(tusb_driver);
-}
-module_exit(tusb_exit);
+module_platform_driver(tusb_driver);
-- 
1.7.0.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