[PATCH 0/5] USB Device Controller support for BCM281xx

[PATCH 0/5] USB Device Controller support for BCM281xx

[Matt Porter]

This series adds USB Device Controller support for the Broadcom
BCM281xx family of parts. BCM281xx contains a DWC2 OTG block and
s3c-hsotg is used to support UDC operation.

Parts 1-2 allows s3c-hsotg to build on non-Samsung platforms and
adds support for a configurable UTMI PHY width. s3c-hsotg is extended
to also support the dwc2 binding as that binding should describe the
hardware independently of there being two different drivers (dwc2 in
staging and s3c-hsotg) for the same device.

Parts 3-4 add a PHY control driver for the Broadcom USB control block
as well as a PHY driver that calls the exported API from the control
driver. This approach is borrowed directly from the phy-omap-control.c
driver.

Patch 5 adds the DT nodes to enable UDC support on both BCM281xx boards
in the kernel.

This series depends on the "Update Kona drivers to use clocks" series
(https://lkml.org/lkml/2013/10/3/645). The dependencies noted for that
series are already queued for 3.13.

Matt Porter (5):
  usb: gadget: s3c-hsotg: enable build for other platforms
  usb: gadget: s3c-hsotg: support configurable UTMI PHY width
  usb: phy: add Broadcom Kona USB control driver
  usb: phy: add Broadcom Kona USB PHY driver
  ARM: dts: add usb udc support to bcm281xx

 Documentation/devicetree/bindings/staging/dwc2.txt |   4 +
 .../devicetree/bindings/usb/bcm-kona-usb-ctrl.txt  |  12 ++
 .../devicetree/bindings/usb/bcm-kona-usb-phy.txt   |  10 ++
 arch/arm/boot/dts/bcm11351-brt.dts                 |  10 ++
 arch/arm/boot/dts/bcm11351.dtsi                    |  21 +++
 arch/arm/boot/dts/bcm28155-ap.dts                  |  12 ++
 drivers/usb/gadget/Kconfig                         |   7 +-
 drivers/usb/gadget/s3c-hsotg.c                     |  20 ++-
 drivers/usb/gadget/s3c-hsotg.h                     |   1 +
 drivers/usb/phy/Kconfig                            |  12 ++
 drivers/usb/phy/Makefile                           |   2 +
 drivers/usb/phy/bcm-kona-usb.h                     |  31 +++++
 drivers/usb/phy/phy-bcm-kona-ctrl.c                | 151 +++++++++++++++++++++
 drivers/usb/phy/phy-bcm-kona-usb2.c                |  99 ++++++++++++++
 14 files changed, 385 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
 create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-phy.txt
 create mode 100644 drivers/usb/phy/bcm-kona-usb.h
 create mode 100644 drivers/usb/phy/phy-bcm-kona-ctrl.c
 create mode 100644 drivers/usb/phy/phy-bcm-kona-usb2.c

-- 
1.8.4

[PATCH 1/5] usb: gadget: s3c-hsotg: enable build for other platforms

[Matt Porter]

Remove unused Samsung-specific machine include and Kconfig
dependency on S3C.

Signed-off-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
---
 drivers/usb/gadget/Kconfig     | 7 +++----
 drivers/usb/gadget/s3c-hsotg.c | 2 --
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 48cddf3..2e69fa0 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -280,11 +280,10 @@ config USB_PXA27X
 	   gadget drivers to also be dynamically linked.
 
 config USB_S3C_HSOTG
-	tristate "S3C HS/OtG USB Device controller"
-	depends on S3C_DEV_USB_HSOTG
+	tristate "Designware/S3C HS/OtG USB Device controller"
 	help
-	  The Samsung S3C64XX USB2.0 high-speed gadget controller
-	  integrated into the S3C64XX series SoC.
+	  The Designware USB2.0 high-speed gadget controller
+	  integrated into the S3C64XX and BCM281xx series SoC.
 
 config USB_S3C2410
 	tristate "S3C2410 USB Device Controller"
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 6bddf1a..34d80f3 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -36,8 +36,6 @@
 #include <linux/usb/phy.h>
 #include <linux/platform_data/s3c-hsotg.h>
 
-#include <mach/map.h>
-
 #include "s3c-hsotg.h"
 
 static const char * const s3c_hsotg_supply_names[] = {
-- 
1.8.4

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Matt Porter]

Extend dwc2 binding with an optional utmi phy width property.
Enable the s3c-hsotg.c driver to use standard dwc2 binding
and enable configuration of the UTMI phy width based on the
property.

Signed-off-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
---
 Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
 drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
 drivers/usb/gadget/s3c-hsotg.h                     |  1 +
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt
index 1a1b7cf..fb6b8ee 100644
--- a/Documentation/devicetree/bindings/staging/dwc2.txt
+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
@@ -6,10 +6,14 @@ Required properties:
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
 
+Optional properties:
+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
+
 Example:
 
         usb at 101c0000 {
                 compatible = "ralink,rt3050-usb, snps,dwc2";
                 reg = <0x101c0000 40000>;
                 interrupts = <18>;
+		snps,phy-utmi-width = <8>;
         };
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 34d80f3..396afb3 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -137,6 +137,7 @@ struct s3c_hsotg_ep {
  * @regs: The memory area mapped for accessing registers.
  * @irq: The IRQ number we are using
  * @supplies: Definition of USB power supplies
+ * @phyif: PHY interface width
  * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
  * @num_of_eps: Number of available EPs (excluding EP0)
  * @debug_root: root directrory for debugfs.
@@ -164,6 +165,7 @@ struct s3c_hsotg {
 
 	struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
 
+	u32			phyif;
 	unsigned int		dedicated_fifos:1;
 	unsigned char           num_of_eps;
 
@@ -2212,7 +2214,7 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
 	 */
 
 	/* set the PLL on, remove the HNP/SRP and set the PHY */
-	writel(GUSBCFG_PHYIf16 | GUSBCFG_TOutCal(7) |
+	writel(hsotg->phyif | GUSBCFG_TOutCal(7) |
 	       (0x5 << 10), hsotg->regs + GUSBCFG);
 
 	s3c_hsotg_init_fifo(hsotg);
@@ -3447,9 +3449,11 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
 	struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
 	struct usb_phy *phy;
 	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct s3c_hsotg_ep *eps;
 	struct s3c_hsotg *hsotg;
 	struct resource *res;
+	u32 phy_utmi_width = 16;
 	int epnum;
 	int ret;
 	int i;
@@ -3539,6 +3543,17 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
 		goto err_supplies;
 	}
 
+	/* Set default UTMI width */
+	hsotg->phyif = GUSBCFG_PHYIf16;
+	if (np) {
+		of_property_read_u32(np, "snps,phy-utmi-width",
+				     &phy_utmi_width);
+		if (phy_utmi_width == 8)
+			hsotg->phyif = GUSBCFG_PHYIf8;
+		if ((phy_utmi_width != 8) && (phy_utmi_width != 16))
+			dev_warn(dev, "invalid snps,phy-utmi-width value\n");
+	}
+
 	/* usb phy enable */
 	s3c_hsotg_phy_enable(hsotg);
 
@@ -3645,6 +3660,7 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
 #ifdef CONFIG_OF
 static const struct of_device_id s3c_hsotg_of_ids[] = {
 	{ .compatible = "samsung,s3c6400-hsotg", },
+	{ .compatible = "snps,dwc2", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
diff --git a/drivers/usb/gadget/s3c-hsotg.h b/drivers/usb/gadget/s3c-hsotg.h
index d650b12..85f549f 100644
--- a/drivers/usb/gadget/s3c-hsotg.h
+++ b/drivers/usb/gadget/s3c-hsotg.h
@@ -55,6 +55,7 @@
 #define GUSBCFG_HNPCap				(1 << 9)
 #define GUSBCFG_SRPCap				(1 << 8)
 #define GUSBCFG_PHYIf16			(1 << 3)
+#define GUSBCFG_PHYIf8				(0 << 3)
 #define GUSBCFG_TOutCal_MASK			(0x7 << 0)
 #define GUSBCFG_TOutCal_SHIFT			(0)
 #define GUSBCFG_TOutCal_LIMIT			(0x7)
-- 
1.8.4

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Felipe Balbi]

On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> Extend dwc2 binding with an optional utmi phy width property.
> Enable the s3c-hsotg.c driver to use standard dwc2 binding
> and enable configuration of the UTMI phy width based on the
> property.
> 
> Signed-off-by: Matt Porter <matt.porter@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> ---
>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt
> index 1a1b7cf..fb6b8ee 100644
> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> @@ -6,10 +6,14 @@ Required properties:
>  - reg : Should contain 1 register range (address and length)
>  - interrupts : Should contain 1 interrupt
>  
> +Optional properties:
> +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)

isn't this available in any of the configuration registers ?

>  Example:
>  
>          usb at 101c0000 {
>                  compatible = "ralink,rt3050-usb, snps,dwc2";
>                  reg = <0x101c0000 40000>;
>                  interrupts = <18>;
> +		snps,phy-utmi-width = <8>;

indentation.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131010/6f24fc09/attachment.sig>

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Matt Porter]

On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
>> Extend dwc2 binding with an optional utmi phy width property.
>> Enable the s3c-hsotg.c driver to use standard dwc2 binding
>> and enable configuration of the UTMI phy width based on the
>> property.
>>
>> Signed-off-by: Matt Porter <matt.porter@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
>>   drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
>>   drivers/usb/gadget/s3c-hsotg.h                     |  1 +
>>   3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt
>> index 1a1b7cf..fb6b8ee 100644
>> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
>> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
>> @@ -6,10 +6,14 @@ Required properties:
>>   - reg : Should contain 1 register range (address and length)
>>   - interrupts : Should contain 1 interrupt
>>
>> +Optional properties:
>> +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
>
> isn't this available in any of the configuration registers ?

Yes and no. HWCFG4 has a UTMI data width field. However, it has 3 valid 
states, "8", "16", or "8 or 16". The BCM281xx implementation is set to 
the latter and the attached phy is 8-bit.

Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts 
validating the value of phy_utmi_width in that driver, the pci.c 
dwc2_module_params .phy_utmi_width field there even had the comment, "/* 
16 bits - NOT DETECTABLE */". The autodetect code in 
dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16" 
option as it just decides to default to a phy width of 16 if nothing is 
configured by the platform glue. This property would also allow this 
issue to be addressed in that driver.

>>   Example:
>>
>>           usb at 101c0000 {
>>                   compatible = "ralink,rt3050-usb, snps,dwc2";
>>                   reg = <0x101c0000 40000>;
>>                   interrupts = <18>;
>> +		snps,phy-utmi-width = <8>;
>
> indentation.

will fix.

-Matt

[1] de4a193 staging: dwc2: validate the value for phy_utmi_width

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Felipe Balbi]

Hi,

On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> >On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> >>Extend dwc2 binding with an optional utmi phy width property.
> >>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> >>and enable configuration of the UTMI phy width based on the
> >>property.
> >>
> >>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> >>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> >>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> >>---
> >>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> >>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
> >>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> >>  3 files changed, 22 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt
> >>index 1a1b7cf..fb6b8ee 100644
> >>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> >>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> >>@@ -6,10 +6,14 @@ Required properties:
> >>  - reg : Should contain 1 register range (address and length)
> >>  - interrupts : Should contain 1 interrupt
> >>
> >>+Optional properties:
> >>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> >
> >isn't this available in any of the configuration registers ?
> 
> Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> set to the latter and the attached phy is 8-bit.
> 
> Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> validating the value of phy_utmi_width in that driver, the pci.c
> dwc2_module_params .phy_utmi_width field there even had the comment,
> "/* 16 bits - NOT DETECTABLE */". The autodetect code in
> dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> option as it just decides to default to a phy width of 16 if nothing
> is configured by the platform glue. This property would also allow
> this issue to be addressed in that driver.

fair enough, but I'd really like to hear from DT folks if your suggested
binding is acceptable. It seems like we can equally argue that it's a SW
configuration or HW description.

cheers

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131010/50c83362/attachment.sig>

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Paul Zimmerman]

> From: Felipe Balbi [mailto:balbi at ti.com]
> Sent: Thursday, October 10, 2013 10:46 AM
> 
> On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> > On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> > >On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> > >>Extend dwc2 binding with an optional utmi phy width property.
> > >>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> > >>and enable configuration of the UTMI phy width based on the
> > >>property.
> > >>
> > >>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> > >>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> > >>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> > >>---
> > >>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> > >>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
> > >>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> > >>  3 files changed, 22 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> b/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>index 1a1b7cf..fb6b8ee 100644
> > >>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>@@ -6,10 +6,14 @@ Required properties:
> > >>  - reg : Should contain 1 register range (address and length)
> > >>  - interrupts : Should contain 1 interrupt
> > >>
> > >>+Optional properties:
> > >>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> > >
> > >isn't this available in any of the configuration registers ?
> >
> > Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> > valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> > set to the latter and the attached phy is 8-bit.
> >
> > Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> > validating the value of phy_utmi_width in that driver, the pci.c
> > dwc2_module_params .phy_utmi_width field there even had the comment,
> > "/* 16 bits - NOT DETECTABLE */". The autodetect code in
> > dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> > option as it just decides to default to a phy width of 16 if nothing
> > is configured by the platform glue. This property would also allow
> > this issue to be addressed in that driver.
> 
> fair enough, but I'd really like to hear from DT folks if your suggested
> binding is acceptable. It seems like we can equally argue that it's a SW
> configuration or HW description.

It's definitely a HW description - the width of the UTMI data connection.

But, which PHY is this? Does it have a register that could tell what
the data width is? The dwc2 core has an (optional) PHY Vendor Control
Register that allows reading the PHY registers.

-- 
Paul

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Felipe Balbi]

Hi,

On Thu, Oct 10, 2013 at 05:57:54PM +0000, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:balbi at ti.com]
> > Sent: Thursday, October 10, 2013 10:46 AM
> > 
> > On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> > > On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> > > >On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> > > >>Extend dwc2 binding with an optional utmi phy width property.
> > > >>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> > > >>and enable configuration of the UTMI phy width based on the
> > > >>property.
> > > >>
> > > >>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> > > >>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> > > >>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> > > >>---
> > > >>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> > > >>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
> > > >>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> > > >>  3 files changed, 22 insertions(+), 1 deletion(-)
> > > >>
> > > >>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> > b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > >>index 1a1b7cf..fb6b8ee 100644
> > > >>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> > > >>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > >>@@ -6,10 +6,14 @@ Required properties:
> > > >>  - reg : Should contain 1 register range (address and length)
> > > >>  - interrupts : Should contain 1 interrupt
> > > >>
> > > >>+Optional properties:
> > > >>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> > > >
> > > >isn't this available in any of the configuration registers ?
> > >
> > > Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> > > valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> > > set to the latter and the attached phy is 8-bit.
> > >
> > > Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> > > validating the value of phy_utmi_width in that driver, the pci.c
> > > dwc2_module_params .phy_utmi_width field there even had the comment,
> > > "/* 16 bits - NOT DETECTABLE */". The autodetect code in
> > > dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> > > option as it just decides to default to a phy width of 16 if nothing
> > > is configured by the platform glue. This property would also allow
> > > this issue to be addressed in that driver.
> > 
> > fair enough, but I'd really like to hear from DT folks if your suggested
> > binding is acceptable. It seems like we can equally argue that it's a SW
> > configuration or HW description.
> 
> It's definitely a HW description - the width of the UTMI data connection.

right, but that description should be passed to the PHY, right ? DWC2
works with both widths (8 or 16).

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131010/6207ef25/attachment.sig>

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Paul Zimmerman]

> From: Felipe Balbi [mailto:balbi at ti.com]
> Sent: Thursday, October 10, 2013 11:07 AM
> 
> On Thu, Oct 10, 2013 at 05:57:54PM +0000, Paul Zimmerman wrote:
> > > From: Felipe Balbi [mailto:balbi at ti.com]
> > > Sent: Thursday, October 10, 2013 10:46 AM
> > >
> > > On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> > > > On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> > > > >On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> > > > >>Extend dwc2 binding with an optional utmi phy width property.
> > > > >>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> > > > >>and enable configuration of the UTMI phy width based on the
> > > > >>property.
> > > > >>
> > > > >>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> > > > >>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> > > > >>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> > > > >>---
> > > > >>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> > > > >>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
> > > > >>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> > > > >>  3 files changed, 22 insertions(+), 1 deletion(-)
> > > > >>
> > > > >>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> > > b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > >>index 1a1b7cf..fb6b8ee 100644
> > > > >>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > >>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > >>@@ -6,10 +6,14 @@ Required properties:
> > > > >>  - reg : Should contain 1 register range (address and length)
> > > > >>  - interrupts : Should contain 1 interrupt
> > > > >>
> > > > >>+Optional properties:
> > > > >>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> > > > >
> > > > >isn't this available in any of the configuration registers ?
> > > >
> > > > Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> > > > valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> > > > set to the latter and the attached phy is 8-bit.
> > > >
> > > > Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> > > > validating the value of phy_utmi_width in that driver, the pci.c
> > > > dwc2_module_params .phy_utmi_width field there even had the comment,
> > > > "/* 16 bits - NOT DETECTABLE */". The autodetect code in
> > > > dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> > > > option as it just decides to default to a phy width of 16 if nothing
> > > > is configured by the platform glue. This property would also allow
> > > > this issue to be addressed in that driver.
> > >
> > > fair enough, but I'd really like to hear from DT folks if your suggested
> > > binding is acceptable. It seems like we can equally argue that it's a SW
> > > configuration or HW description.
> >
> > It's definitely a HW description - the width of the UTMI data connection.
> 
> right, but that description should be passed to the PHY, right ? DWC2
> works with both widths (8 or 16).

I don't know what you mean by "passed to the PHY". The data width is the
number of physical wires connecting the PHY to the DWC2 controller. If
that value can be read from the PHY, fine. Otherwise, I think a DT
binding is the way to go.

-- 
Paul

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Felipe Balbi]

Hi,

On Thu, Oct 10, 2013 at 06:14:19PM +0000, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:balbi at ti.com]
> > Sent: Thursday, October 10, 2013 11:07 AM
> > 
> > On Thu, Oct 10, 2013 at 05:57:54PM +0000, Paul Zimmerman wrote:
> > > > From: Felipe Balbi [mailto:balbi at ti.com]
> > > > Sent: Thursday, October 10, 2013 10:46 AM
> > > >
> > > > On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> > > > > On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> > > > > >On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> > > > > >>Extend dwc2 binding with an optional utmi phy width property.
> > > > > >>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> > > > > >>and enable configuration of the UTMI phy width based on the
> > > > > >>property.
> > > > > >>
> > > > > >>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> > > > > >>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> > > > > >>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> > > > > >>---
> > > > > >>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> > > > > >>  drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
> > > > > >>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> > > > > >>  3 files changed, 22 insertions(+), 1 deletion(-)
> > > > > >>
> > > > > >>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > > >>index 1a1b7cf..fb6b8ee 100644
> > > > > >>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > > >>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> > > > > >>@@ -6,10 +6,14 @@ Required properties:
> > > > > >>  - reg : Should contain 1 register range (address and length)
> > > > > >>  - interrupts : Should contain 1 interrupt
> > > > > >>
> > > > > >>+Optional properties:
> > > > > >>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
> > > > > >
> > > > > >isn't this available in any of the configuration registers ?
> > > > >
> > > > > Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> > > > > valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> > > > > set to the latter and the attached phy is 8-bit.
> > > > >
> > > > > Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> > > > > validating the value of phy_utmi_width in that driver, the pci.c
> > > > > dwc2_module_params .phy_utmi_width field there even had the comment,
> > > > > "/* 16 bits - NOT DETECTABLE */". The autodetect code in
> > > > > dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> > > > > option as it just decides to default to a phy width of 16 if nothing
> > > > > is configured by the platform glue. This property would also allow
> > > > > this issue to be addressed in that driver.
> > > >
> > > > fair enough, but I'd really like to hear from DT folks if your suggested
> > > > binding is acceptable. It seems like we can equally argue that it's a SW
> > > > configuration or HW description.
> > >
> > > It's definitely a HW description - the width of the UTMI data connection.
> > 
> > right, but that description should be passed to the PHY, right ? DWC2
> > works with both widths (8 or 16).
> 
> I don't know what you mean by "passed to the PHY". The data width is the
> number of physical wires connecting the PHY to the DWC2 controller. If

right, that's well understood.

> that value can be read from the PHY, fine. Otherwise, I think a DT
> binding is the way to go.

Let's see what the DT folks have to say.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131010/7f9b9088/attachment.sig>

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Matt Porter]

On 10/10/2013 01:57 PM, Paul Zimmerman wrote:
>> From: Felipe Balbi [mailto:balbi at ti.com]
>> Sent: Thursday, October 10, 2013 10:46 AM
>>
>> On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
>>> On 10/10/2013 11:29 AM, Felipe Balbi wrote:
>>>> On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
>>>>> Extend dwc2 binding with an optional utmi phy width property.
>>>>> Enable the s3c-hsotg.c driver to use standard dwc2 binding
>>>>> and enable configuration of the UTMI phy width based on the
>>>>> property.
>>>>>
>>>>> Signed-off-by: Matt Porter <matt.porter@linaro.org>
>>>>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>>>>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
>>>>>   drivers/usb/gadget/s3c-hsotg.c                     | 18 +++++++++++++++++-
>>>>>   drivers/usb/gadget/s3c-hsotg.h                     |  1 +
>>>>>   3 files changed, 22 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
>> b/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>> index 1a1b7cf..fb6b8ee 100644
>>>>> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>> @@ -6,10 +6,14 @@ Required properties:
>>>>>   - reg : Should contain 1 register range (address and length)
>>>>>   - interrupts : Should contain 1 interrupt
>>>>>
>>>>> +Optional properties:
>>>>> +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16)
>>>>
>>>> isn't this available in any of the configuration registers ?
>>>
>>> Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
>>> valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
>>> set to the latter and the attached phy is 8-bit.
>>>
>>> Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
>>> validating the value of phy_utmi_width in that driver, the pci.c
>>> dwc2_module_params .phy_utmi_width field there even had the comment,
>>> "/* 16 bits - NOT DETECTABLE */". The autodetect code in
>>> dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
>>> option as it just decides to default to a phy width of 16 if nothing
>>> is configured by the platform glue. This property would also allow
>>> this issue to be addressed in that driver.
>>
>> fair enough, but I'd really like to hear from DT folks if your suggested
>> binding is acceptable. It seems like we can equally argue that it's a SW
>> configuration or HW description.
>
> It's definitely a HW description - the width of the UTMI data connection.
>
> But, which PHY is this? Does it have a register that could tell what
> the data width is? The dwc2 core has an (optional) PHY Vendor Control
> Register that allows reading the PHY registers.

This is the integrated PHY on the bcm28155 part (the entire 
bcm281xx/11351 family shares it). My register doc doesn't show the 
optional PHY vendor control register as being present in this dwc2 
implementation. I'll track down with the design team if this is 
accurate...but it appears we can't go that route.

-Matt

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Matt Porter]

On 10/10/2013 03:07 PM, Matt Porter wrote:
> On 10/10/2013 01:57 PM, Paul Zimmerman wrote:
>>> From: Felipe Balbi [mailto:balbi at ti.com]
>>> Sent: Thursday, October 10, 2013 10:46 AM
>>>
>>> On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
>>>> On 10/10/2013 11:29 AM, Felipe Balbi wrote:
>>>>> On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
>>>>>> Extend dwc2 binding with an optional utmi phy width property.
>>>>>> Enable the s3c-hsotg.c driver to use standard dwc2 binding
>>>>>> and enable configuration of the UTMI phy width based on the
>>>>>> property.
>>>>>>
>>>>>> Signed-off-by: Matt Porter <matt.porter@linaro.org>
>>>>>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>>>>>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>>>>>> ---
>>>>>>   Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
>>>>>>   drivers/usb/gadget/s3c-hsotg.c                     | 18
>>>>>> +++++++++++++++++-
>>>>>>   drivers/usb/gadget/s3c-hsotg.h                     |  1 +
>>>>>>   3 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
>>> b/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>>> index 1a1b7cf..fb6b8ee 100644
>>>>>> --- a/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>>> +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
>>>>>> @@ -6,10 +6,14 @@ Required properties:
>>>>>>   - reg : Should contain 1 register range (address and length)
>>>>>>   - interrupts : Should contain 1 interrupt
>>>>>>
>>>>>> +Optional properties:
>>>>>> +- snps,phy-utmi-width: Must contain the UTMI data width (either 8
>>>>>> or 16)
>>>>>
>>>>> isn't this available in any of the configuration registers ?
>>>>
>>>> Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
>>>> valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
>>>> set to the latter and the attached phy is 8-bit.
>>>>
>>>> Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
>>>> validating the value of phy_utmi_width in that driver, the pci.c
>>>> dwc2_module_params .phy_utmi_width field there even had the comment,
>>>> "/* 16 bits - NOT DETECTABLE */". The autodetect code in
>>>> dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
>>>> option as it just decides to default to a phy width of 16 if nothing
>>>> is configured by the platform glue. This property would also allow
>>>> this issue to be addressed in that driver.
>>>
>>> fair enough, but I'd really like to hear from DT folks if your suggested
>>> binding is acceptable. It seems like we can equally argue that it's a SW
>>> configuration or HW description.
>>
>> It's definitely a HW description - the width of the UTMI data connection.
>>
>> But, which PHY is this? Does it have a register that could tell what
>> the data width is? The dwc2 core has an (optional) PHY Vendor Control
>> Register that allows reading the PHY registers.
>
> This is the integrated PHY on the bcm28155 part (the entire
> bcm281xx/11351 family shares it). My register doc doesn't show the
> optional PHY vendor control register as being present in this dwc2
> implementation. I'll track down with the design team if this is
> accurate...but it appears we can't go that route.

The design team confirmed that we do not have any registers that can be 
accessed via the PHY Vendor Control Register. All PHY control registers 
are implemented in the MMIO control block which is supported in the PHY 
control driver later in this series.They also confirmed there's no other 
way to detect that it's an 8-bit data path.

-Matt

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Felipe Balbi]

Hi,

On Thu, Oct 10, 2013 at 11:21:30PM -0400, Matt Porter wrote:
> On 10/10/2013 03:07 PM, Matt Porter wrote:
> >On 10/10/2013 01:57 PM, Paul Zimmerman wrote:
> >>>From: Felipe Balbi [mailto:balbi at ti.com]
> >>>Sent: Thursday, October 10, 2013 10:46 AM
> >>>
> >>>On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> >>>>On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> >>>>>On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> >>>>>>Extend dwc2 binding with an optional utmi phy width property.
> >>>>>>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> >>>>>>and enable configuration of the UTMI phy width based on the
> >>>>>>property.
> >>>>>>
> >>>>>>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> >>>>>>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> >>>>>>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> >>>>>>---
> >>>>>>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> >>>>>>  drivers/usb/gadget/s3c-hsotg.c                     | 18
> >>>>>>+++++++++++++++++-
> >>>>>>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> >>>>>>  3 files changed, 22 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> >>>b/Documentation/devicetree/bindings/staging/dwc2.txt
> >>>>>>index 1a1b7cf..fb6b8ee 100644
> >>>>>>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> >>>>>>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> >>>>>>@@ -6,10 +6,14 @@ Required properties:
> >>>>>>  - reg : Should contain 1 register range (address and length)
> >>>>>>  - interrupts : Should contain 1 interrupt
> >>>>>>
> >>>>>>+Optional properties:
> >>>>>>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8
> >>>>>>or 16)
> >>>>>
> >>>>>isn't this available in any of the configuration registers ?
> >>>>
> >>>>Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> >>>>valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> >>>>set to the latter and the attached phy is 8-bit.
> >>>>
> >>>>Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> >>>>validating the value of phy_utmi_width in that driver, the pci.c
> >>>>dwc2_module_params .phy_utmi_width field there even had the comment,
> >>>>"/* 16 bits - NOT DETECTABLE */". The autodetect code in
> >>>>dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> >>>>option as it just decides to default to a phy width of 16 if nothing
> >>>>is configured by the platform glue. This property would also allow
> >>>>this issue to be addressed in that driver.
> >>>
> >>>fair enough, but I'd really like to hear from DT folks if your suggested
> >>>binding is acceptable. It seems like we can equally argue that it's a SW
> >>>configuration or HW description.
> >>
> >>It's definitely a HW description - the width of the UTMI data connection.
> >>
> >>But, which PHY is this? Does it have a register that could tell what
> >>the data width is? The dwc2 core has an (optional) PHY Vendor Control
> >>Register that allows reading the PHY registers.
> >
> >This is the integrated PHY on the bcm28155 part (the entire
> >bcm281xx/11351 family shares it). My register doc doesn't show the
> >optional PHY vendor control register as being present in this dwc2
> >implementation. I'll track down with the design team if this is
> >accurate...but it appears we can't go that route.
> 
> The design team confirmed that we do not have any registers that can
> be accessed via the PHY Vendor Control Register. All PHY control
> registers are implemented in the MMIO control block which is
> supported in the PHY control driver later in this series.They also
> confirmed there's no other way to detect that it's an 8-bit data
> path.

then we need the DT binding, let's see what DT maintainers say.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131011/e050f9b3/attachment.sig>

[PATCH 2/5] usb: gadget: s3c-hsotg: support configurable UTMI PHY width

[Matt Porter]

On Fri, Oct 11, 2013 at 08:37:31AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Oct 10, 2013 at 11:21:30PM -0400, Matt Porter wrote:
> > On 10/10/2013 03:07 PM, Matt Porter wrote:
> > >On 10/10/2013 01:57 PM, Paul Zimmerman wrote:
> > >>>From: Felipe Balbi [mailto:balbi at ti.com]
> > >>>Sent: Thursday, October 10, 2013 10:46 AM
> > >>>
> > >>>On Thu, Oct 10, 2013 at 12:54:40PM -0400, Matt Porter wrote:
> > >>>>On 10/10/2013 11:29 AM, Felipe Balbi wrote:
> > >>>>>On Mon, Oct 07, 2013 at 06:12:29AM -0400, Matt Porter wrote:
> > >>>>>>Extend dwc2 binding with an optional utmi phy width property.
> > >>>>>>Enable the s3c-hsotg.c driver to use standard dwc2 binding
> > >>>>>>and enable configuration of the UTMI phy width based on the
> > >>>>>>property.
> > >>>>>>
> > >>>>>>Signed-off-by: Matt Porter <matt.porter@linaro.org>
> > >>>>>>Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> > >>>>>>Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> > >>>>>>---
> > >>>>>>  Documentation/devicetree/bindings/staging/dwc2.txt |  4 ++++
> > >>>>>>  drivers/usb/gadget/s3c-hsotg.c                     | 18
> > >>>>>>+++++++++++++++++-
> > >>>>>>  drivers/usb/gadget/s3c-hsotg.h                     |  1 +
> > >>>>>>  3 files changed, 22 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>>diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>>b/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>>>>>index 1a1b7cf..fb6b8ee 100644
> > >>>>>>--- a/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>>>>>+++ b/Documentation/devicetree/bindings/staging/dwc2.txt
> > >>>>>>@@ -6,10 +6,14 @@ Required properties:
> > >>>>>>  - reg : Should contain 1 register range (address and length)
> > >>>>>>  - interrupts : Should contain 1 interrupt
> > >>>>>>
> > >>>>>>+Optional properties:
> > >>>>>>+- snps,phy-utmi-width: Must contain the UTMI data width (either 8
> > >>>>>>or 16)
> > >>>>>
> > >>>>>isn't this available in any of the configuration registers ?
> > >>>>
> > >>>>Yes and no. HWCFG4 has a UTMI data width field. However, it has 3
> > >>>>valid states, "8", "16", or "8 or 16". The BCM281xx implementation is
> > >>>>set to the latter and the attached phy is 8-bit.
> > >>>>
> > >>>>Looking at dwc2 prior to Matthijs Kooijman's patch [1] which starts
> > >>>>validating the value of phy_utmi_width in that driver, the pci.c
> > >>>>dwc2_module_params .phy_utmi_width field there even had the comment,
> > >>>>"/* 16 bits - NOT DETECTABLE */". The autodetect code in
> > >>>>dwc2_set_param_phy_utmi_width() will fail if HWCFG4 has the "8 or 16"
> > >>>>option as it just decides to default to a phy width of 16 if nothing
> > >>>>is configured by the platform glue. This property would also allow
> > >>>>this issue to be addressed in that driver.
> > >>>
> > >>>fair enough, but I'd really like to hear from DT folks if your suggested
> > >>>binding is acceptable. It seems like we can equally argue that it's a SW
> > >>>configuration or HW description.
> > >>
> > >>It's definitely a HW description - the width of the UTMI data connection.
> > >>
> > >>But, which PHY is this? Does it have a register that could tell what
> > >>the data width is? The dwc2 core has an (optional) PHY Vendor Control
> > >>Register that allows reading the PHY registers.
> > >
> > >This is the integrated PHY on the bcm28155 part (the entire
> > >bcm281xx/11351 family shares it). My register doc doesn't show the
> > >optional PHY vendor control register as being present in this dwc2
> > >implementation. I'll track down with the design team if this is
> > >accurate...but it appears we can't go that route.
> > 
> > The design team confirmed that we do not have any registers that can
> > be accessed via the PHY Vendor Control Register. All PHY control
> > registers are implemented in the MMIO control block which is
> > supported in the PHY control driver later in this series.They also
> > confirmed there's no other way to detect that it's an 8-bit data
> > path.
> 
> then we need the DT binding, let's see what DT maintainers say.

I've reworked all of this against the generic phy layer. Things are
simplified, of course, but this needs to be clarified to move forward
on either the phy driver or the s3c-hsotg support.

DT maintainers: can you weigh in here please?

Summary is that we have a 8-bit data path UTMI phy attached to a dwc2.
The width is undetectable and a fixed h/w configuration. I would
actually view this as a property of the phy itself. It's 8-bit only and
thus puts the constraint on the controller it's attached to (internally)
to operate in 8-bit UTMI mode. However, I see no way with the generic
phy binding to provide this information to the controller h/w.

The original approach above adds a phy-utmi-width property to the dwc2
binding which is simple, but in retrospect may not reflect where that
constraint actually comes into play.

-Matt

[PATCH 3/5] usb: phy: add Broadcom Kona USB control driver

[Matt Porter]

Broadcom BCM281xx parts have a PHY control block that
operates in conjunction with the DWC2 USB OTG. This driver
exposes an API that allows control of power/reset for a
connected USB PHY.

Signed-off-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
---
 .../devicetree/bindings/usb/bcm-kona-usb-ctrl.txt  |  12 ++
 drivers/usb/phy/Kconfig                            |   5 +
 drivers/usb/phy/Makefile                           |   1 +
 drivers/usb/phy/bcm-kona-usb.h                     |  31 +++++
 drivers/usb/phy/phy-bcm-kona-ctrl.c                | 151 +++++++++++++++++++++
 5 files changed, 200 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
 create mode 100644 drivers/usb/phy/bcm-kona-usb.h
 create mode 100644 drivers/usb/phy/phy-bcm-kona-ctrl.c

diff --git a/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
new file mode 100644
index 0000000..98163f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
@@ -0,0 +1,12 @@
+BROADCOM KONA USB CONTROL
+
+Required properties:
+ - compatible: brcm,kona-ctrl-usb
+ - reg: offset and length of the USB control registers
+
+Example:
+
+	usbctl: usbctl at 3f130000 {
+		compatible = "brcm,kona-ctrl-usb";
+		reg = <0x3f130000 0x28>;
+	};
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index d5589f9..2b7b4f1 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -100,6 +100,11 @@ config AM335X_PHY_USB
 	  This driver provides PHY support for that phy which part for the
 	  AM335x SoC.
 
+config BCM_KONA_CTRL_USB
+	tristate
+	help
+	  Enable this to support the Broadcom Kona USB control block
+
 config SAMSUNG_USBPHY
 	tristate
 	help
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 2135e85..98c545e 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-generic.o
 obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
 obj-$(CONFIG_AM335X_CONTROL_USB)	+= phy-am335x-control.o
 obj-$(CONFIG_AM335X_PHY_USB)		+= phy-am335x.o
+obj-$(CONFIG_BCM_KONA_CTRL_USB)		+= phy-bcm-kona-ctrl.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
 obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
diff --git a/drivers/usb/phy/bcm-kona-usb.h b/drivers/usb/phy/bcm-kona-usb.h
new file mode 100644
index 0000000..4563dd6
--- /dev/null
+++ b/drivers/usb/phy/bcm-kona-usb.h
@@ -0,0 +1,31 @@
+/*
+ * bcm_kona_usb.h -- Broadcom Kona USB header file
+ *
+ * Copyright (C) 2013 Linaro Limited
+ * Matt Porter <matt.porter@linaro.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __BCM_KONA_USB_H
+#define __BCM_KONA_USB_H
+
+#include <linux/usb/phy.h>
+
+struct bcm_kona_usb {
+	struct usb_phy phy;
+	struct device *dev;
+	struct device *ctrl_dev;
+};
+
+extern struct device *bcm_kona_get_ctrl_dev(void);
+extern void bcm_kona_ctrl_usb_phy_power(struct device *, int);
+
+#endif /* __BCM_KONA_USB_H */
diff --git a/drivers/usb/phy/phy-bcm-kona-ctrl.c b/drivers/usb/phy/phy-bcm-kona-ctrl.c
new file mode 100644
index 0000000..d2936b6
--- /dev/null
+++ b/drivers/usb/phy/phy-bcm-kona-ctrl.c
@@ -0,0 +1,151 @@
+/*
+ * phy-bcm-kona-ctrl.c - Broadcom Kona USB Control Driver
+ *
+ * Based on phy-omap-control.c
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2013 Linaro Limited
+ * Matt Porter <matt.porter@linaro.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+
+#include "bcm-kona-usb.h"
+
+#define OTGCTL_OTGSTAT2		(1 << 31)
+#define OTGCTL_OTGSTAT1		(1 << 30)
+#define OTGCTL_PRST_N_SW	(1 << 11)
+#define OTGCTL_HRESET_N		(1 << 10)
+#define OTGCTL_UTMI_LINE_STATE1	(1 << 9)
+#define OTGCTL_UTMI_LINE_STATE0	(1 << 8)
+
+#define P1CTL_SOFT_RESET	(1 << 1)
+#define P1CTL_NON_DRIVING	(1 << 0)
+
+struct bcm_kona_usb_ctrl_regs {
+	u32 ctrl;
+	u32 cfg;
+	u32 p1ctl;
+	u32 status;
+	u32 bc_cfg;
+	u32 tp_in;
+	u32 tp_out;
+	u32 phy_ctrl;
+	u32 usbreg;
+	u32 usbproben;
+};
+
+struct bcm_kona_ctrl_usb {
+	struct device *dev;
+	struct bcm_kona_usb_ctrl_regs *regs;
+};
+
+static struct bcm_kona_ctrl_usb *ctrl_usb;
+
+/**
+ * bcm_kona_ctrl_dev - returns the device pointer for this control device
+ *
+ * This API should be called to get the device pointer for the Kona USB
+ * control device. This device pointer should be used when calling the
+ * exported bcm_kona_ctrl_usb_phy_power() API.
+ */
+struct device *bcm_kona_get_ctrl_dev(void)
+{
+	if (!ctrl_usb)
+		return ERR_PTR(-ENODEV);
+
+	return ctrl_usb->dev;
+}
+EXPORT_SYMBOL_GPL(bcm_kona_get_ctrl_dev);
+
+/**
+ * bcm_kona_ctrl_usb_phy_power - power on/off the phy using control module reg
+ * @dev: the control module device
+ * @on: 0 or 1, based on powering on or off the PHY
+ */
+void bcm_kona_ctrl_usb_phy_power(struct device *dev, int on)
+{
+	struct bcm_kona_ctrl_usb *ctrl = dev_get_drvdata(dev);
+	u32 val;
+
+	val = readl(&ctrl->regs->ctrl);
+	if (on) {
+		/* Configure and power PHY */
+		val &= ~(OTGCTL_OTGSTAT2 | OTGCTL_OTGSTAT1 |
+			 OTGCTL_UTMI_LINE_STATE1 | OTGCTL_UTMI_LINE_STATE0);
+		val |= OTGCTL_PRST_N_SW | OTGCTL_HRESET_N;
+		writel(val, &ctrl->regs->ctrl);
+
+		/* Soft reset PHY */
+		val = readl(&ctrl->regs->p1ctl);
+		val &= ~P1CTL_NON_DRIVING;
+		val |= P1CTL_SOFT_RESET;
+		writel(val, &ctrl->regs->p1ctl);
+		writel(val & ~P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
+		/* Reset needs to be asserted for 2ms */
+		mdelay(2);
+		writel(val | P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
+	} else {
+		val &= ~(OTGCTL_PRST_N_SW | OTGCTL_HRESET_N);
+		writel(val, &ctrl->regs->ctrl);
+	}
+}
+EXPORT_SYMBOL_GPL(bcm_kona_ctrl_usb_phy_power);
+
+static int bcm_kona_ctrl_usb_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+
+	ctrl_usb = devm_kzalloc(&pdev->dev, sizeof(*ctrl_usb), GFP_KERNEL);
+	if (!ctrl_usb) {
+		dev_err(&pdev->dev, "unable to alloc memory for control usb\n");
+		return -ENOMEM;
+	}
+
+	ctrl_usb->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctrl_usb->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ctrl_usb->regs))
+		return PTR_ERR(ctrl_usb->regs);
+
+	dev_set_drvdata(ctrl_usb->dev, ctrl_usb);
+
+	return 0;
+}
+
+static const struct of_device_id bcm_kona_ctrl_usb_id_table[] = {
+	{ .compatible = "brcm,kona-ctrl-usb" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, bcm_kona_ctrl_usb_id_table);
+
+static struct platform_driver bcm_kona_ctrl_usb_driver = {
+	.probe		= bcm_kona_ctrl_usb_probe,
+	.driver		= {
+		.name	= "bcm-kona-ctrl-usb",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(bcm_kona_ctrl_usb_id_table),
+	},
+};
+
+module_platform_driver(bcm_kona_ctrl_usb_driver);
+
+MODULE_ALIAS("platform:bcm-kona-ctrl-usb");
+MODULE_AUTHOR("Matt Porter");
+MODULE_DESCRIPTION("Broadcom Kona USB Control Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.4

[PATCH 3/5] usb: phy: add Broadcom Kona USB control driver

[Felipe Balbi]

Hi,

On Mon, Oct 07, 2013 at 06:12:30AM -0400, Matt Porter wrote:
> Broadcom BCM281xx parts have a PHY control block that
> operates in conjunction with the DWC2 USB OTG. This driver
> exposes an API that allows control of power/reset for a
> connected USB PHY.
> 
> Signed-off-by: Matt Porter <matt.porter@linaro.org>
> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> ---
>  .../devicetree/bindings/usb/bcm-kona-usb-ctrl.txt  |  12 ++
>  drivers/usb/phy/Kconfig                            |   5 +
>  drivers/usb/phy/Makefile                           |   1 +
>  drivers/usb/phy/bcm-kona-usb.h                     |  31 +++++
>  drivers/usb/phy/phy-bcm-kona-ctrl.c                | 151 +++++++++++++++++++++
>  5 files changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
>  create mode 100644 drivers/usb/phy/bcm-kona-usb.h
>  create mode 100644 drivers/usb/phy/phy-bcm-kona-ctrl.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
> new file mode 100644
> index 0000000..98163f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
> @@ -0,0 +1,12 @@
> +BROADCOM KONA USB CONTROL
> +
> +Required properties:
> + - compatible: brcm,kona-ctrl-usb
> + - reg: offset and length of the USB control registers
> +
> +Example:
> +
> +	usbctl: usbctl at 3f130000 {
> +		compatible = "brcm,kona-ctrl-usb";
> +		reg = <0x3f130000 0x28>;
> +	};
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index d5589f9..2b7b4f1 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -100,6 +100,11 @@ config AM335X_PHY_USB
>  	  This driver provides PHY support for that phy which part for the
>  	  AM335x SoC.
>  
> +config BCM_KONA_CTRL_USB
> +	tristate
> +	help
> +	  Enable this to support the Broadcom Kona USB control block
> +
>  config SAMSUNG_USBPHY
>  	tristate
>  	help
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 2135e85..98c545e 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-generic.o
>  obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
>  obj-$(CONFIG_AM335X_CONTROL_USB)	+= phy-am335x-control.o
>  obj-$(CONFIG_AM335X_PHY_USB)		+= phy-am335x.o
> +obj-$(CONFIG_BCM_KONA_CTRL_USB)		+= phy-bcm-kona-ctrl.o
>  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>  obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
>  obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
> diff --git a/drivers/usb/phy/bcm-kona-usb.h b/drivers/usb/phy/bcm-kona-usb.h
> new file mode 100644
> index 0000000..4563dd6
> --- /dev/null
> +++ b/drivers/usb/phy/bcm-kona-usb.h
> @@ -0,0 +1,31 @@
> +/*
> + * bcm_kona_usb.h -- Broadcom Kona USB header file
> + *
> + * Copyright (C) 2013 Linaro Limited
> + * Matt Porter <matt.porter@linaro.org>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __BCM_KONA_USB_H
> +#define __BCM_KONA_USB_H
> +
> +#include <linux/usb/phy.h>
> +
> +struct bcm_kona_usb {
> +	struct usb_phy phy;
> +	struct device *dev;
> +	struct device *ctrl_dev;
> +};
> +
> +extern struct device *bcm_kona_get_ctrl_dev(void);
> +extern void bcm_kona_ctrl_usb_phy_power(struct device *, int);

looks like a job for extcon fwk.

> diff --git a/drivers/usb/phy/phy-bcm-kona-ctrl.c b/drivers/usb/phy/phy-bcm-kona-ctrl.c
> new file mode 100644
> index 0000000..d2936b6
> --- /dev/null
> +++ b/drivers/usb/phy/phy-bcm-kona-ctrl.c
> @@ -0,0 +1,151 @@
> +/*
> + * phy-bcm-kona-ctrl.c - Broadcom Kona USB Control Driver
> + *
> + * Based on phy-omap-control.c
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + * Copyright (C) 2013 Linaro Limited
> + * Matt Porter <matt.porter@linaro.org>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +
> +#include "bcm-kona-usb.h"
> +
> +#define OTGCTL_OTGSTAT2		(1 << 31)
> +#define OTGCTL_OTGSTAT1		(1 << 30)
> +#define OTGCTL_PRST_N_SW	(1 << 11)
> +#define OTGCTL_HRESET_N		(1 << 10)
> +#define OTGCTL_UTMI_LINE_STATE1	(1 << 9)
> +#define OTGCTL_UTMI_LINE_STATE0	(1 << 8)
> +
> +#define P1CTL_SOFT_RESET	(1 << 1)
> +#define P1CTL_NON_DRIVING	(1 << 0)
> +
> +struct bcm_kona_usb_ctrl_regs {
> +	u32 ctrl;
> +	u32 cfg;
> +	u32 p1ctl;
> +	u32 status;
> +	u32 bc_cfg;
> +	u32 tp_in;
> +	u32 tp_out;
> +	u32 phy_ctrl;
> +	u32 usbreg;
> +	u32 usbproben;
> +};
> +
> +struct bcm_kona_ctrl_usb {
> +	struct device *dev;
> +	struct bcm_kona_usb_ctrl_regs *regs;
> +};
> +
> +static struct bcm_kona_ctrl_usb *ctrl_usb;
> +
> +/**
> + * bcm_kona_ctrl_dev - returns the device pointer for this control device
> + *
> + * This API should be called to get the device pointer for the Kona USB
> + * control device. This device pointer should be used when calling the
> + * exported bcm_kona_ctrl_usb_phy_power() API.
> + */
> +struct device *bcm_kona_get_ctrl_dev(void)
> +{
> +	if (!ctrl_usb)
> +		return ERR_PTR(-ENODEV);
> +
> +	return ctrl_usb->dev;
> +}
> +EXPORT_SYMBOL_GPL(bcm_kona_get_ctrl_dev);

definitely a job for extcon ;-)

> +void bcm_kona_ctrl_usb_phy_power(struct device *dev, int on)
> +{
> +	struct bcm_kona_ctrl_usb *ctrl = dev_get_drvdata(dev);
> +	u32 val;
> +
> +	val = readl(&ctrl->regs->ctrl);
> +	if (on) {
> +		/* Configure and power PHY */
> +		val &= ~(OTGCTL_OTGSTAT2 | OTGCTL_OTGSTAT1 |
> +			 OTGCTL_UTMI_LINE_STATE1 | OTGCTL_UTMI_LINE_STATE0);
> +		val |= OTGCTL_PRST_N_SW | OTGCTL_HRESET_N;
> +		writel(val, &ctrl->regs->ctrl);
> +
> +		/* Soft reset PHY */
> +		val = readl(&ctrl->regs->p1ctl);
> +		val &= ~P1CTL_NON_DRIVING;
> +		val |= P1CTL_SOFT_RESET;
> +		writel(val, &ctrl->regs->p1ctl);
> +		writel(val & ~P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
> +		/* Reset needs to be asserted for 2ms */
> +		mdelay(2);
> +		writel(val | P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
> +	} else {
> +		val &= ~(OTGCTL_PRST_N_SW | OTGCTL_HRESET_N);
> +		writel(val, &ctrl->regs->ctrl);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(bcm_kona_ctrl_usb_phy_power);

what a hack :-)

Please use phy->init() and phy->exit() callbacks. In fact, please use
Kishon's generic PHY layer.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131010/e8adbff6/attachment.sig>

[PATCH 3/5] usb: phy: add Broadcom Kona USB control driver

[Matt Porter]

On 10/10/2013 11:31 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Oct 07, 2013 at 06:12:30AM -0400, Matt Porter wrote:
>> Broadcom BCM281xx parts have a PHY control block that
>> operates in conjunction with the DWC2 USB OTG. This driver
>> exposes an API that allows control of power/reset for a
>> connected USB PHY.
>>
>> Signed-off-by: Matt Porter <matt.porter@linaro.org>
>> Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
>> ---
>>   .../devicetree/bindings/usb/bcm-kona-usb-ctrl.txt  |  12 ++
>>   drivers/usb/phy/Kconfig                            |   5 +
>>   drivers/usb/phy/Makefile                           |   1 +
>>   drivers/usb/phy/bcm-kona-usb.h                     |  31 +++++
>>   drivers/usb/phy/phy-bcm-kona-ctrl.c                | 151 +++++++++++++++++++++
>>   5 files changed, 200 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
>>   create mode 100644 drivers/usb/phy/bcm-kona-usb.h
>>   create mode 100644 drivers/usb/phy/phy-bcm-kona-ctrl.c
>>
>> diff --git a/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
>> new file mode 100644
>> index 0000000..98163f6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/bcm-kona-usb-ctrl.txt
>> @@ -0,0 +1,12 @@
>> +BROADCOM KONA USB CONTROL
>> +
>> +Required properties:
>> + - compatible: brcm,kona-ctrl-usb
>> + - reg: offset and length of the USB control registers
>> +
>> +Example:
>> +
>> +	usbctl: usbctl at 3f130000 {
>> +		compatible = "brcm,kona-ctrl-usb";
>> +		reg = <0x3f130000 0x28>;
>> +	};
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index d5589f9..2b7b4f1 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -100,6 +100,11 @@ config AM335X_PHY_USB
>>   	  This driver provides PHY support for that phy which part for the
>>   	  AM335x SoC.
>>
>> +config BCM_KONA_CTRL_USB
>> +	tristate
>> +	help
>> +	  Enable this to support the Broadcom Kona USB control block
>> +
>>   config SAMSUNG_USBPHY
>>   	tristate
>>   	help
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index 2135e85..98c545e 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-generic.o
>>   obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
>>   obj-$(CONFIG_AM335X_CONTROL_USB)	+= phy-am335x-control.o
>>   obj-$(CONFIG_AM335X_PHY_USB)		+= phy-am335x.o
>> +obj-$(CONFIG_BCM_KONA_CTRL_USB)		+= phy-bcm-kona-ctrl.o
>>   obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>>   obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
>>   obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
>> diff --git a/drivers/usb/phy/bcm-kona-usb.h b/drivers/usb/phy/bcm-kona-usb.h
>> new file mode 100644
>> index 0000000..4563dd6
>> --- /dev/null
>> +++ b/drivers/usb/phy/bcm-kona-usb.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * bcm_kona_usb.h -- Broadcom Kona USB header file
>> + *
>> + * Copyright (C) 2013 Linaro Limited
>> + * Matt Porter <matt.porter@linaro.org>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __BCM_KONA_USB_H
>> +#define __BCM_KONA_USB_H
>> +
>> +#include <linux/usb/phy.h>
>> +
>> +struct bcm_kona_usb {
>> +	struct usb_phy phy;
>> +	struct device *dev;
>> +	struct device *ctrl_dev;
>> +};
>> +
>> +extern struct device *bcm_kona_get_ctrl_dev(void);
>> +extern void bcm_kona_ctrl_usb_phy_power(struct device *, int);
>
> looks like a job for extcon fwk.
>
>> diff --git a/drivers/usb/phy/phy-bcm-kona-ctrl.c b/drivers/usb/phy/phy-bcm-kona-ctrl.c
>> new file mode 100644
>> index 0000000..d2936b6
>> --- /dev/null
>> +++ b/drivers/usb/phy/phy-bcm-kona-ctrl.c
>> @@ -0,0 +1,151 @@
>> +/*
>> + * phy-bcm-kona-ctrl.c - Broadcom Kona USB Control Driver
>> + *
>> + * Based on phy-omap-control.c
>> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
>> + * Copyright (C) 2013 Linaro Limited
>> + * Matt Porter <matt.porter@linaro.org>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/clk.h>
>> +
>> +#include "bcm-kona-usb.h"
>> +
>> +#define OTGCTL_OTGSTAT2		(1 << 31)
>> +#define OTGCTL_OTGSTAT1		(1 << 30)
>> +#define OTGCTL_PRST_N_SW	(1 << 11)
>> +#define OTGCTL_HRESET_N		(1 << 10)
>> +#define OTGCTL_UTMI_LINE_STATE1	(1 << 9)
>> +#define OTGCTL_UTMI_LINE_STATE0	(1 << 8)
>> +
>> +#define P1CTL_SOFT_RESET	(1 << 1)
>> +#define P1CTL_NON_DRIVING	(1 << 0)
>> +
>> +struct bcm_kona_usb_ctrl_regs {
>> +	u32 ctrl;
>> +	u32 cfg;
>> +	u32 p1ctl;
>> +	u32 status;
>> +	u32 bc_cfg;
>> +	u32 tp_in;
>> +	u32 tp_out;
>> +	u32 phy_ctrl;
>> +	u32 usbreg;
>> +	u32 usbproben;
>> +};
>> +
>> +struct bcm_kona_ctrl_usb {
>> +	struct device *dev;
>> +	struct bcm_kona_usb_ctrl_regs *regs;
>> +};
>> +
>> +static struct bcm_kona_ctrl_usb *ctrl_usb;
>> +
>> +/**
>> + * bcm_kona_ctrl_dev - returns the device pointer for this control device
>> + *
>> + * This API should be called to get the device pointer for the Kona USB
>> + * control device. This device pointer should be used when calling the
>> + * exported bcm_kona_ctrl_usb_phy_power() API.
>> + */
>> +struct device *bcm_kona_get_ctrl_dev(void)
>> +{
>> +	if (!ctrl_usb)
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	return ctrl_usb->dev;
>> +}
>> +EXPORT_SYMBOL_GPL(bcm_kona_get_ctrl_dev);
>
> definitely a job for extcon ;-)
>
>> +void bcm_kona_ctrl_usb_phy_power(struct device *dev, int on)
>> +{
>> +	struct bcm_kona_ctrl_usb *ctrl = dev_get_drvdata(dev);
>> +	u32 val;
>> +
>> +	val = readl(&ctrl->regs->ctrl);
>> +	if (on) {
>> +		/* Configure and power PHY */
>> +		val &= ~(OTGCTL_OTGSTAT2 | OTGCTL_OTGSTAT1 |
>> +			 OTGCTL_UTMI_LINE_STATE1 | OTGCTL_UTMI_LINE_STATE0);
>> +		val |= OTGCTL_PRST_N_SW | OTGCTL_HRESET_N;
>> +		writel(val, &ctrl->regs->ctrl);
>> +
>> +		/* Soft reset PHY */
>> +		val = readl(&ctrl->regs->p1ctl);
>> +		val &= ~P1CTL_NON_DRIVING;
>> +		val |= P1CTL_SOFT_RESET;
>> +		writel(val, &ctrl->regs->p1ctl);
>> +		writel(val & ~P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
>> +		/* Reset needs to be asserted for 2ms */
>> +		mdelay(2);
>> +		writel(val | P1CTL_SOFT_RESET, &ctrl->regs->p1ctl);
>> +	} else {
>> +		val &= ~(OTGCTL_PRST_N_SW | OTGCTL_HRESET_N);
>> +		writel(val, &ctrl->regs->ctrl);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(bcm_kona_ctrl_usb_phy_power);
>
> what a hack :-)

:)

> Please use phy->init() and phy->exit() callbacks. In fact, please use
> Kishon's generic PHY layer.

Ok, reworking on top of his series...looks to be straightforward.

Thanks,
Matt

[PATCH 4/5] usb: phy: add Broadcom Kona USB PHY driver

[Matt Porter]

Add a USB PHY driver for BCM281xx devices. This driver makes use of the
Broadcom Kona USB control driver to control init/shutdown of the PHY.

Signed-off-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
---
 .../devicetree/bindings/usb/bcm-kona-usb-phy.txt   | 10 +++
 drivers/usb/phy/Kconfig                            |  7 ++
 drivers/usb/phy/Makefile                           |  1 +
 drivers/usb/phy/phy-bcm-kona-usb2.c                | 99 ++++++++++++++++++++++
 4 files changed, 117 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/bcm-kona-usb-phy.txt
 create mode 100644 drivers/usb/phy/phy-bcm-kona-usb2.c

diff --git a/Documentation/devicetree/bindings/usb/bcm-kona-usb-phy.txt b/Documentation/devicetree/bindings/usb/bcm-kona-usb-phy.txt
new file mode 100644
index 0000000..a935fe2
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/bcm-kona-usb-phy.txt
@@ -0,0 +1,10 @@
+BROADCOM KONA USB PHY
+
+Required properties:
+ - compatible: brcm,kona-usb2
+
+Example:
+
+	usbphy: usbphy {
+		compatible = "brcm,kona-usb2";
+	};
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 2b7b4f1..d063204 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -105,6 +105,13 @@ config BCM_KONA_CTRL_USB
 	help
 	  Enable this to support the Broadcom Kona USB control block
 
+config BCM_KONA_PHY_USB2
+	tristate "Broadcom Kona USB2 PHY Driver"
+	select USB_PHY
+	select BCM_KONA_CTRL_USB
+	help
+	  Enable this to support the Broadcom Kona USB 2.0 PHY.
+
 config SAMSUNG_USBPHY
 	tristate
 	help
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 98c545e..227fba5 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_OMAP_CONTROL_USB)		+= phy-omap-control.o
 obj-$(CONFIG_AM335X_CONTROL_USB)	+= phy-am335x-control.o
 obj-$(CONFIG_AM335X_PHY_USB)		+= phy-am335x.o
 obj-$(CONFIG_BCM_KONA_CTRL_USB)		+= phy-bcm-kona-ctrl.o
+obj-$(CONFIG_BCM_KONA_PHY_USB2)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)			+= phy-omap-usb3.o
 obj-$(CONFIG_SAMSUNG_USBPHY)		+= phy-samsung-usb.o
diff --git a/drivers/usb/phy/phy-bcm-kona-usb2.c b/drivers/usb/phy/phy-bcm-kona-usb2.c
new file mode 100644
index 0000000..21aee4a
--- /dev/null
+++ b/drivers/usb/phy/phy-bcm-kona-usb2.c
@@ -0,0 +1,99 @@
+/*
+ * phy-bcm-kona-control.c - Broadcom Kona USB2 Phy Driver
+ *
+ * Copyright (C) 2013 Linaro Limited
+ * Matt Porter <matt.porter@linaro.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "bcm-kona-usb.h"
+
+static int bcm_kona_phy_init(struct usb_phy *uphy)
+{
+	struct bcm_kona_usb *phy = dev_get_drvdata(uphy->dev);
+
+	bcm_kona_ctrl_usb_phy_power(phy->ctrl_dev, 1);
+
+	return 0;
+}
+
+static void bcm_kona_phy_shutdown(struct usb_phy *uphy)
+{
+	struct bcm_kona_usb *phy = dev_get_drvdata(uphy->dev);
+
+	bcm_kona_ctrl_usb_phy_power(phy->ctrl_dev, 0);
+}
+
+static int bcm_kona_usb2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bcm_kona_usb *phy;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->ctrl_dev = bcm_kona_get_ctrl_dev();
+	if (IS_ERR(phy->ctrl_dev)) {
+		dev_dbg(&pdev->dev, "Failed to get control device\n");
+		return -ENODEV;
+	}
+
+	phy->dev		= dev;
+	phy->phy.dev		= phy->dev;
+	phy->phy.label		= "bcm-kona-usb2";
+	phy->phy.init		= bcm_kona_phy_init;
+	phy->phy.shutdown	= bcm_kona_phy_shutdown;
+	phy->phy.type		= USB_PHY_TYPE_USB2;
+
+	platform_set_drvdata(pdev, phy);
+
+	usb_add_phy_dev(&phy->phy);
+
+	return 0;
+}
+
+static int bcm_kona_usb2_remove(struct platform_device *pdev)
+{
+	struct bcm_kona_usb *phy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&phy->phy);
+
+	return 0;
+}
+
+static const struct of_device_id bcm_kona_usb2_dt_ids[] = {
+	{ .compatible = "brcm,kona-usb2" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, bcm_kona_usb2_dt_ids);
+
+static struct platform_driver bcm_kona_usb2_driver = {
+	.probe		= bcm_kona_usb2_probe,
+	.remove		= bcm_kona_usb2_remove,
+	.driver		= {
+		.name	= "bcm-kona-usb2",
+		.owner	= THIS_MODULE,
+		.of_match_table = bcm_kona_usb2_dt_ids,
+	},
+};
+
+module_platform_driver(bcm_kona_usb2_driver);
+
+MODULE_ALIAS("platform:bcm-kona-usb2");
+MODULE_AUTHOR("Matt Porter");
+MODULE_DESCRIPTION("BCM Kona USB 2.0 PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.4

[PATCH 5/5] ARM: dts: add usb udc support to bcm281xx

[Matt Porter]

Adds USB OTG/Control/PHY and clock support to BCM281xx and enables
UDC support on the bcm11351-brt and bcm28155-ap boards.

Signed-off-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
---
 arch/arm/boot/dts/bcm11351-brt.dts | 10 ++++++++++
 arch/arm/boot/dts/bcm11351.dtsi    | 21 +++++++++++++++++++++
 arch/arm/boot/dts/bcm28155-ap.dts  | 12 ++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/arch/arm/boot/dts/bcm11351-brt.dts b/arch/arm/boot/dts/bcm11351-brt.dts
index 9d36eb4..5a269c7 100644
--- a/arch/arm/boot/dts/bcm11351-brt.dts
+++ b/arch/arm/boot/dts/bcm11351-brt.dts
@@ -43,5 +43,15 @@
 		status = "okay";
 	};
 
+	usbotg at 3f120000 {
+		status = "okay";
+	};
 
+	usbctl at 3f130000 {
+		status = "okay";
+	};
+
+	usbphy {
+		status = "okay";
+	};
 };
diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 22aef21..1c647f2 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -227,4 +227,25 @@
 			#clock-cells = <0>;
 		};
 	};
+
+	usbotg: usbotg at 3f120000 {
+		compatible = "snps,dwc2";
+		reg = <0x3f120000 0x10000>;
+		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&usb_otg_ahb_clk>;
+		clock-names = "otg";
+		snps,phy-utmi-width = <8>;
+		status = "disabled";
+	};
+
+	usbctl: usbctl at 3f130000 {
+		compatible = "brcm,kona-ctrl-usb";
+		reg = <0x3f130000 0x28>;
+		status = "disabled";
+	};
+
+	usbphy: usbphy {
+		compatible = "brcm,kona-usb2";
+		status = "disabled";
+	};
 };
diff --git a/arch/arm/boot/dts/bcm28155-ap.dts b/arch/arm/boot/dts/bcm28155-ap.dts
index 96ae67a..7b1b741 100644
--- a/arch/arm/boot/dts/bcm28155-ap.dts
+++ b/arch/arm/boot/dts/bcm28155-ap.dts
@@ -42,4 +42,16 @@
 		max-frequency = <48000000>;
 		status = "okay";
 	};
+
+	usbotg at 3f120000 {
+		status = "okay";
+	};
+
+	usbctl at 3f130000 {
+		status = "okay";
+	};
+
+	usbphy {
+		status = "okay";
+	};
 };
-- 
1.8.4