Re: [RFC PATH 2/3] usb: dwc3: add ULPI interface support

2013-12-02 Thread Kishon Vijay Abraham I
Hi,

On Thursday 28 November 2013 09:29 PM, Heikki Krogerus wrote:
 Registers ULPI interface with the ULPI abstraction layer if
 the HSPHY type is ULPI, which will create phy instance for
 usb2.
 
 Depends on Kishon's patch set adding support for generic PHY
 framework.
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 ---
  drivers/usb/dwc3/Kconfig  |   7 +++
  drivers/usb/dwc3/Makefile |   4 ++
  drivers/usb/dwc3/core.c   |   8 
  drivers/usb/dwc3/core.h   |  21 +
  drivers/usb/dwc3/ulpi.c   | 110 
 ++
  5 files changed, 150 insertions(+)
  create mode 100644 drivers/usb/dwc3/ulpi.c
 
 diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
 index 64eed6f..a97b294 100644
 --- a/drivers/usb/dwc3/Kconfig
 +++ b/drivers/usb/dwc3/Kconfig
 @@ -13,6 +13,13 @@ config USB_DWC3
  
  if USB_DWC3
  
 +config USB_DWC3_ULPI
 + bool Provide ULPI PHY Interface
 + depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
 + help
 +   Select this if you have ULPI type PHY attached to your DWC3
 +   controller.
 +
  choice
   bool DWC3 Mode Selection
   default USB_DWC3_DUAL_ROLE if (USB  USB_GADGET)
 diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
 index dd17601..8bb82bc 100644
 --- a/drivers/usb/dwc3/Makefile
 +++ b/drivers/usb/dwc3/Makefile
 @@ -13,6 +13,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
 $(CONFIG_USB_DWC3_DUAL_ROLE)),)
   dwc3-y  += gadget.o ep0.o
  endif
  
 +ifneq ($(CONFIG_USB_DWC3_ULPI),)
 + dwc3-y  += ulpi.o
 +endif
 +
  ifneq ($(CONFIG_DEBUG_FS),)
   dwc3-y  += debugfs.o
  endif
 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 1c0a69a..94927b2 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -505,6 +505,12 @@ static int dwc3_probe(struct platform_device *pdev)
   dwc-regs_size  = resource_size(res);
   dwc-dev= dev;
  
 + if (!dwc-usb2_generic_phy) {
 + ret = dwc3_ulpi_init(dwc);

shouldn't this be called from dwc3-pci (or any other dwc3 glue)?
 + if (ret)
 + return ret;
 + }
 +
   dev-dma_mask   = dev-parent-dma_mask;
   dev-dma_parms  = dev-parent-dma_parms;
   dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);
 @@ -613,6 +619,7 @@ err1:
  
  err0:
   dwc3_free_event_buffers(dwc);
 + dwc3_ulpi_exit(dwc);
  
   return ret;
  }
 @@ -653,6 +660,7 @@ static int dwc3_remove(struct platform_device *pdev)
   dwc3_event_buffers_cleanup(dwc);
   dwc3_free_event_buffers(dwc);
   dwc3_core_exit(dwc);
 + dwc3_ulpi_exit(dwc);
  
   return 0;
  }
 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
 index 01ec7d7..6df398a 100644
 --- a/drivers/usb/dwc3/core.h
 +++ b/drivers/usb/dwc3/core.h
 @@ -169,6 +169,14 @@
  #define DWC3_GUSB2PHYCFG_PHYSOFTRST  (1  31)
  #define DWC3_GUSB2PHYCFG_SUSPHY  (1  6)
  
 +/* Global USB2 PHY Vendor Control Register */
 +#define DWC3_GUSB2PHYACC_NEWREGREQ   (1  25)
 +#define DWC3_GUSB2PHYACC_BUSY(1  23)
 +#define DWC3_GUSB2PHYACC_WRITE   (1  22)
 +#define DWC3_GUSB2PHYACC_ADDR(n) (n  16)
 +#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n)  (n  8)
 +#define DWC3_GUSB2PHYACC_DATA(n) (n  0xff)
 +
  /* Global USB3 PIPE Control Register */
  #define DWC3_GUSB3PIPECTL_PHYSOFTRST (1  31)
  #define DWC3_GUSB3PIPECTL_SUSPHY (1  17)
 @@ -557,6 +565,7 @@ struct dwc3_hwparams {
  #define DWC3_NUM_INT(n)  (((n)  (0x3f  15))  15)
  
  /* HWPARAMS3 */
 +#define DWC3_ULPI_HSPHY  (1  3)
  #define DWC3_NUM_IN_EPS_MASK (0x1f  18)
  #define DWC3_NUM_EPS_MASK(0x3f  12)
  #define DWC3_NUM_EPS(p)  (((p)-hwparams3   \
 @@ -672,6 +681,8 @@ struct dwc3 {
   struct phy  *usb2_generic_phy;
   struct phy  *usb3_generic_phy;
  
 + struct ulpi_interface   *ulpi;
 +
   void __iomem*regs;
   size_t  regs_size;
  
 @@ -922,4 +933,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
  }
  #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
  
 +#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
 +int dwc3_ulpi_init(struct dwc3 *dwc);
 +void dwc3_ulpi_exit(struct dwc3 *dwc);
 +#else
 +static inline int dwc3_ulpi_init(struct dwc3 *dwc)
 +{ return 0; }
 +static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
 +{ }
 +#endif
 +
  #endif /* __DRIVERS_USB_DWC3_CORE_H */
 diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
 new file mode 100644
 index 000..e1f152b
 --- /dev/null
 +++ b/drivers/usb/dwc3/ulpi.c
 @@ -0,0 +1,110 @@
 +/**
 + * ulpi.c - DesignWare USB3 ULPI PHY interface
 + *
 + * Copyright (C) 2013 Intel Corporation
 + *
 + * Author: Heikki Krogerus heikki.kroge...@linux.intel.com
 + *
 + * This program is free software; you can redistribute  it and/or modify it
 + * under 

Re: [RFC PATH 2/3] usb: dwc3: add ULPI interface support

2013-12-02 Thread Heikki Krogerus
Hi,

On Mon, Dec 02, 2013 at 04:24:31PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Thursday 28 November 2013 09:29 PM, Heikki Krogerus wrote:

snip

  diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
  index dd17601..8bb82bc 100644
  --- a/drivers/usb/dwc3/Makefile
  +++ b/drivers/usb/dwc3/Makefile
  @@ -13,6 +13,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
  $(CONFIG_USB_DWC3_DUAL_ROLE)),)
  dwc3-y  += gadget.o ep0.o
   endif
   
  +ifneq ($(CONFIG_USB_DWC3_ULPI),)
  +   dwc3-y  += ulpi.o
  +endif
  +
   ifneq ($(CONFIG_DEBUG_FS),)
  dwc3-y  += debugfs.o
   endif
  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
  index 1c0a69a..94927b2 100644
  --- a/drivers/usb/dwc3/core.c
  +++ b/drivers/usb/dwc3/core.c
  @@ -505,6 +505,12 @@ static int dwc3_probe(struct platform_device *pdev)
  dwc-regs_size  = resource_size(res);
  dwc-dev= dev;
   
  +   if (!dwc-usb2_generic_phy) {
  +   ret = dwc3_ulpi_init(dwc);
 
 shouldn't this be called from dwc3-pci (or any other dwc3 glue)?

It's not going to be possible to put it to the dwc3-pci, but my
motivation for that is actually up and coming ACPI support. I do not
want to create glue driver for it just because this. With ACPI DSDT
there is no guarantee to get device entries for PHYs I'm afraid. You
will have them on some platforms, but on others you don't :(.

The issue with PCI is the device name, which is used when matching the
phy. We don't know it before the device is registered. We can't live
with the assumption there is only one instance in case of PCI. The
damn thing can be hotpluged from some weird dock, like exchangeable
back cover or something similar. So you will be able to register the
phy only after the core.c has probed, and requested the phys, which is
too late.

And besides. The glue drivers should only have platform specific code.
This is definitely platform agnostic feature of the controller.

  +   if (ret)
  +   return ret;
  +   }
  +
  dev-dma_mask   = dev-parent-dma_mask;
  dev-dma_parms  = dev-parent-dma_parms;
  dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);

snip

  +{
  +   int ret;
  +
  +   /* First check if there is ULPI PHY */
  +   ret = dwc3_readl(dwc-regs, DWC3_GHWPARAMS3);
  +   if (!(ret  DWC3_ULPI_HSPHY))
  +   return 0;
  +
  +   /* Register the interface */
  +   dwc-ulpi = ulpi_new_interface(dwc-dev,
  +  dwc3_ulpi_read, dwc3_ulpi_write, dwc);
  +   if (IS_ERR(dwc-ulpi)) {
  +   dev_err(dwc-dev, failed to register ULPI interface);
  +   return PTR_ERR(dwc-ulpi);
  +   }
  +
  +   /* Get the ULPI phy instance
  +* REVISIT: There should be no need to get it separately here.
  +*/
  +   dwc-usb2_generic_phy = devm_phy_get(dwc-dev, ULPI_PORT_NAME);
 
 You shouldn't be needing this. It should get the phy from dwc3 core probe 
 itself.

You don't need this here. I made it like this now just because we
don't yet have your final version of the patches where you introduce
the generic phy support in dwc3.

I want to change the core.c so it set's the dwc-dev and maps the
iomem before requesting the phys. And of course put dwc3_ulpi_init in
between.

Thanks,

-- 
heikki
--
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 PATH 2/3] usb: dwc3: add ULPI interface support

2013-11-28 Thread Heikki Krogerus
Registers ULPI interface with the ULPI abstraction layer if
the HSPHY type is ULPI, which will create phy instance for
usb2.

Depends on Kishon's patch set adding support for generic PHY
framework.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
 drivers/usb/dwc3/Kconfig  |   7 +++
 drivers/usb/dwc3/Makefile |   4 ++
 drivers/usb/dwc3/core.c   |   8 
 drivers/usb/dwc3/core.h   |  21 +
 drivers/usb/dwc3/ulpi.c   | 110 ++
 5 files changed, 150 insertions(+)
 create mode 100644 drivers/usb/dwc3/ulpi.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 64eed6f..a97b294 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -13,6 +13,13 @@ config USB_DWC3
 
 if USB_DWC3
 
+config USB_DWC3_ULPI
+   bool Provide ULPI PHY Interface
+   depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
+   help
+ Select this if you have ULPI type PHY attached to your DWC3
+ controller.
+
 choice
bool DWC3 Mode Selection
default USB_DWC3_DUAL_ROLE if (USB  USB_GADGET)
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index dd17601..8bb82bc 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -13,6 +13,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
$(CONFIG_USB_DWC3_DUAL_ROLE)),)
dwc3-y  += gadget.o ep0.o
 endif
 
+ifneq ($(CONFIG_USB_DWC3_ULPI),)
+   dwc3-y  += ulpi.o
+endif
+
 ifneq ($(CONFIG_DEBUG_FS),)
dwc3-y  += debugfs.o
 endif
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1c0a69a..94927b2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -505,6 +505,12 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-regs_size  = resource_size(res);
dwc-dev= dev;
 
+   if (!dwc-usb2_generic_phy) {
+   ret = dwc3_ulpi_init(dwc);
+   if (ret)
+   return ret;
+   }
+
dev-dma_mask   = dev-parent-dma_mask;
dev-dma_parms  = dev-parent-dma_parms;
dma_set_coherent_mask(dev, dev-parent-coherent_dma_mask);
@@ -613,6 +619,7 @@ err1:
 
 err0:
dwc3_free_event_buffers(dwc);
+   dwc3_ulpi_exit(dwc);
 
return ret;
 }
@@ -653,6 +660,7 @@ static int dwc3_remove(struct platform_device *pdev)
dwc3_event_buffers_cleanup(dwc);
dwc3_free_event_buffers(dwc);
dwc3_core_exit(dwc);
+   dwc3_ulpi_exit(dwc);
 
return 0;
 }
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 01ec7d7..6df398a 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -169,6 +169,14 @@
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1  31)
 #define DWC3_GUSB2PHYCFG_SUSPHY(1  6)
 
+/* Global USB2 PHY Vendor Control Register */
+#define DWC3_GUSB2PHYACC_NEWREGREQ (1  25)
+#define DWC3_GUSB2PHYACC_BUSY  (1  23)
+#define DWC3_GUSB2PHYACC_WRITE (1  22)
+#define DWC3_GUSB2PHYACC_ADDR(n)   (n  16)
+#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n)(n  8)
+#define DWC3_GUSB2PHYACC_DATA(n)   (n  0xff)
+
 /* Global USB3 PIPE Control Register */
 #define DWC3_GUSB3PIPECTL_PHYSOFTRST   (1  31)
 #define DWC3_GUSB3PIPECTL_SUSPHY   (1  17)
@@ -557,6 +565,7 @@ struct dwc3_hwparams {
 #define DWC3_NUM_INT(n)(((n)  (0x3f  15))  15)
 
 /* HWPARAMS3 */
+#define DWC3_ULPI_HSPHY(1  3)
 #define DWC3_NUM_IN_EPS_MASK   (0x1f  18)
 #define DWC3_NUM_EPS_MASK  (0x3f  12)
 #define DWC3_NUM_EPS(p)(((p)-hwparams3   \
@@ -672,6 +681,8 @@ struct dwc3 {
struct phy  *usb2_generic_phy;
struct phy  *usb3_generic_phy;
 
+   struct ulpi_interface   *ulpi;
+
void __iomem*regs;
size_t  regs_size;
 
@@ -922,4 +933,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
 }
 #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
 
+#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
+int dwc3_ulpi_init(struct dwc3 *dwc);
+void dwc3_ulpi_exit(struct dwc3 *dwc);
+#else
+static inline int dwc3_ulpi_init(struct dwc3 *dwc)
+{ return 0; }
+static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
+{ }
+#endif
+
 #endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
new file mode 100644
index 000..e1f152b
--- /dev/null
+++ b/drivers/usb/dwc3/ulpi.c
@@ -0,0 +1,110 @@
+/**
+ * ulpi.c - DesignWare USB3 ULPI PHY interface
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Author: Heikki Krogerus heikki.kroge...@linux.intel.com
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include