Re: [PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver

2016-08-31 Thread Alan Cooper
On Wed, Aug 31, 2016 at 5:57 AM, Kishon Vijay Abraham I  wrote:
> Hi,
>
> On Tuesday 30 August 2016 08:00 PM, Al Cooper wrote:
>> Add a new USB Phy driver for Broadcom STB SoCs. This driver
>> supports all Broadcom STB ARM SoCs. This driver in combination
>> with the generic ohci, ehci and xhci platform drivers will enable
>> USB1.1, USB2.0 and USB3.0 support. This Phy driver also supports
>> the Broadcom UDC gadget driver.
>
> Remove usb: from $subject

Ok


>>
>> Signed-off-by: Al Cooper 
>> ---
>>  .../bindings/phy/brcm,brcmstb-usb-phy.txt  |  39 +
>>  MAINTAINERS|   7 +
>>  drivers/phy/Kconfig|  10 +
>>  drivers/phy/Makefile   |   2 +
>>  drivers/phy/phy-brcm-usb-init.c| 792 
>> +
>
> other broadcom phy drivers use only "bcm" in the file name.

We have a mixed use of both names throughout the kernel. Notice
"phy-brcm-sata.c in the phy directory. "brcm" seems a little better
because it matches the device tree prefix used by all drivers and is
used by many of the latest drivers.


>> diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt 
>> b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
>> new file mode 100644
>> index 000..34fa9dd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
>> @@ -0,0 +1,39 @@
>> +Broadcom STB USB PHY
>> +
>> +Required properties:
>> + - compatible: brcm,brcmstb-usb-phy
>> + - reg: two offset and length pairs. The second pair specifies the
>> +USB 3.0 related registers and is only required for PHYs
>> +that support USB 3.0
>
> I think the right way to model this should be to have separate subnodes for
> usb2 and usb3.

The first register block contains both the 2.0 and 3.0 phy registers.
The second register block contains an optional "workaround" register
set used on just a few SoCs to workaround a bug in the XHCI phy. Also,
this approach allows us to use the already parsed resource and saves
some additional code to get and map the registers.


>> + - #phy-cells: Shall be 1 as it expects one argument for setting
>> +the type of the PHY. Possible values are 0 (1.1 and 2.0),
>> +1 (3.0)
>> +
>> +
> spurious blank space.

Ok


>> +Optional Properties:
>> +- clocks : phandle + clock specifier for the phy clocks
>> +- clock-names: string, clock name
>> +- ipp: Invert Port Power
>> +- ioc: Invert Over Current detection
>> +- has_xhci: Contains an optional 3.0 PHY
>
> prefix all the broadcom specific properties with "bcm," or whatever is used 
> for
> broadcom specific properties.

Ok


>> +- device: PHY Device mode. Possible values are: 0 (Host), 1 (Device)
>> +  or 2 (DRD)
>> +
>> +
>> +
>
> spurious blank spaces..

Ok


>> +Example:
>> +
>> +usbphy_0: usb-phy@f0470200 {
>> + reg = <0xf0470200 0xb8>,
>> + <0xf0471940 0x6c0>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>
> Why do you need address cells, size cells? Include it in the documentation.

I don't, I'll remove them.


>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 19bff3a..5ff5e47 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -443,6 +443,16 @@ config PHY_CYGNUS_PCIE
>> Enable this to support the Broadcom Cygnus PCIe PHY.
>> If unsure, say N.
>>
>> +config BRCM_USB_PHY
>> + tristate "Broadcom USB PHY driver"
>> + depends on OF && USB && ARCH_BRCMSTB
>
> Please also include COMPILE_TEST

Ok


>> + select GENERIC_PHY
>> + default y
>> + help
>> +   Enable this to support the Broadcom USB PHY on
>> +   Broadcom STB SoCs.
>> +   If unsure, say Y.
>
> Generally it is N.

Since this driver should work on any "ARCH_BRCMSTB" system and
"ARCH_BRCMSTB" is included in the "depends", it seems like this should
be defaulted to Y.


>> diff --git a/drivers/phy/phy-brcm-usb-init.c 
>> b/drivers/phy/phy-brcm-usb-init.c
>> new file mode 100644
>> index 000..f5d8c32
>> --- /dev/null
>> +++ b/drivers/phy/phy-brcm-usb-init.c
>> @@ -0,0 +1,792 @@
>> +/*
>> + * Copyright (C) 2014-2016 Broadcom
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *notice, this list of conditions and the following disclaimer in the
>> + *documentation and/or other materials provided with the distribution.
>> + * 3. Neither the name of the project nor the names of its contributors
>> + *may be used to endorse or promote products derived from this software
>> + *without specific prior written permission.
>> + *
>> + * T

Re: [PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver

2016-08-31 Thread Kishon Vijay Abraham I
Hi,

On Tuesday 30 August 2016 08:00 PM, Al Cooper wrote:
> Add a new USB Phy driver for Broadcom STB SoCs. This driver
> supports all Broadcom STB ARM SoCs. This driver in combination
> with the generic ohci, ehci and xhci platform drivers will enable
> USB1.1, USB2.0 and USB3.0 support. This Phy driver also supports
> the Broadcom UDC gadget driver.

Remove usb: from $subject
> 
> Signed-off-by: Al Cooper 
> ---
>  .../bindings/phy/brcm,brcmstb-usb-phy.txt  |  39 +
>  MAINTAINERS|   7 +
>  drivers/phy/Kconfig|  10 +
>  drivers/phy/Makefile   |   2 +
>  drivers/phy/phy-brcm-usb-init.c| 792 
> +

other broadcom phy drivers use only "bcm" in the file name.
>  drivers/phy/phy-brcm-usb-init.h|  49 ++
>  drivers/phy/phy-brcm-usb.c | 206 ++
>  7 files changed, 1105 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
>  create mode 100644 drivers/phy/phy-brcm-usb-init.c
>  create mode 100644 drivers/phy/phy-brcm-usb-init.h
>  create mode 100644 drivers/phy/phy-brcm-usb.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt 
> b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
> new file mode 100644
> index 000..34fa9dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
> @@ -0,0 +1,39 @@
> +Broadcom STB USB PHY
> +
> +Required properties:
> + - compatible: brcm,brcmstb-usb-phy
> + - reg: two offset and length pairs. The second pair specifies the
> +USB 3.0 related registers and is only required for PHYs
> +that support USB 3.0

I think the right way to model this should be to have separate subnodes for
usb2 and usb3.
> + - #phy-cells: Shall be 1 as it expects one argument for setting
> +the type of the PHY. Possible values are 0 (1.1 and 2.0),
> +1 (3.0)
> +
> +
spurious blank space.
> +Optional Properties:
> +- clocks : phandle + clock specifier for the phy clocks
> +- clock-names: string, clock name
> +- ipp: Invert Port Power
> +- ioc: Invert Over Current detection
> +- has_xhci: Contains an optional 3.0 PHY

prefix all the broadcom specific properties with "bcm," or whatever is used for
broadcom specific properties.
> +- device: PHY Device mode. Possible values are: 0 (Host), 1 (Device)
> +  or 2 (DRD)
> +
> +
> +

spurious blank spaces..
> +Example:
> +
> +usbphy_0: usb-phy@f0470200 {
> + reg = <0xf0470200 0xb8>,
> + <0xf0471940 0x6c0>;
> + #address-cells = <1>;
> + #size-cells = <1>;

Why do you need address cells, size cells? Include it in the documentation.
> + compatible = "brcm,brcmstb-usb-phy";
> + ioc = <1>;
> + ipp = <1>;
> + #phy-cells = <1>;
> + ranges;
> + has_xhci;
> + clocks = <&sw_usb20>;
> + clock-names = "sw_usb";
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0bbe4b1..d58b124 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2697,6 +2697,13 @@ S: Maintained
>  F:   drivers/bcma/
>  F:   include/linux/bcma/
>  
> +BROADCOM STB USB PHY DRIVER
> +M:   Al Cooper 
> +L:   linux-...@vger.kernel.org
> +L:   bcm-kernel-feedback-l...@broadcom.com
> +S:   Supported
> +F:   drivers/phy/phy-brcm-usb*
> +
>  BROADCOM SYSTEMPORT ETHERNET DRIVER
>  M:   Florian Fainelli 
>  L:   net...@vger.kernel.org
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 19bff3a..5ff5e47 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -443,6 +443,16 @@ config PHY_CYGNUS_PCIE
> Enable this to support the Broadcom Cygnus PCIe PHY.
> If unsure, say N.
>  
> +config BRCM_USB_PHY
> + tristate "Broadcom USB PHY driver"
> + depends on OF && USB && ARCH_BRCMSTB

Please also include COMPILE_TEST
> + select GENERIC_PHY
> + default y
> + help
> +   Enable this to support the Broadcom USB PHY on
> +   Broadcom STB SoCs.
> +   If unsure, say Y.

Generally it is N.
> +
>  source "drivers/phy/tegra/Kconfig"
>  
>  config PHY_NS2_PCIE
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 90ae198..f4ff6c7 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -10,6 +10,8 @@ obj-$(CONFIG_PHY_DA8XX_USB) += phy-da8xx-usb.o
>  obj-$(CONFIG_PHY_DM816X_USB) += phy-dm816x-usb.o
>  obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)   += phy-armada375-usb2.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)  += phy-bcm-kona-usb2.o
> +obj-$(CONFIG_BRCM_USB_PHY)   += phy-brcm-usb.o
> +obj-$(CONFIG_BRCM_USB_PHY)   += phy-brcm-usb-init.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)  += phy-exynos-mipi-video.o
>  obj-$(CONFIG_PHY_LPC18XX_USB_OTG)+= phy-lpc18xx-usb-otg.o
> diff --git a/drivers/phy/phy-brcm-usb-init.c b/drivers/

Re: [PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver

2016-08-30 Thread kbuild test robot
Hi Al,

[auto build test ERROR on phy/next]
[also build test ERROR on next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]
[Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for 
convenience) to record what (public, well-known) commit your patch series was 
built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:
https://github.com/0day-ci/linux/commits/Al-Cooper/soc-brcmstb-Add-Product-ID-and-Family-ID-helper-functions/20160830-224057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git 
next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "brcm_usb_common_init" [drivers/phy/phy-brcm-usb.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data


[PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver

2016-08-30 Thread Al Cooper
Add a new USB Phy driver for Broadcom STB SoCs. This driver
supports all Broadcom STB ARM SoCs. This driver in combination
with the generic ohci, ehci and xhci platform drivers will enable
USB1.1, USB2.0 and USB3.0 support. This Phy driver also supports
the Broadcom UDC gadget driver.

Signed-off-by: Al Cooper 
---
 .../bindings/phy/brcm,brcmstb-usb-phy.txt  |  39 +
 MAINTAINERS|   7 +
 drivers/phy/Kconfig|  10 +
 drivers/phy/Makefile   |   2 +
 drivers/phy/phy-brcm-usb-init.c| 792 +
 drivers/phy/phy-brcm-usb-init.h|  49 ++
 drivers/phy/phy-brcm-usb.c | 206 ++
 7 files changed, 1105 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
 create mode 100644 drivers/phy/phy-brcm-usb-init.c
 create mode 100644 drivers/phy/phy-brcm-usb-init.h
 create mode 100644 drivers/phy/phy-brcm-usb.c

diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt 
b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
new file mode 100644
index 000..34fa9dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
@@ -0,0 +1,39 @@
+Broadcom STB USB PHY
+
+Required properties:
+ - compatible: brcm,brcmstb-usb-phy
+ - reg: two offset and length pairs. The second pair specifies the
+USB 3.0 related registers and is only required for PHYs
+that support USB 3.0
+ - #phy-cells: Shall be 1 as it expects one argument for setting
+  the type of the PHY. Possible values are 0 (1.1 and 2.0),
+  1 (3.0)
+
+
+Optional Properties:
+- clocks : phandle + clock specifier for the phy clocks
+- clock-names: string, clock name
+- ipp: Invert Port Power
+- ioc: Invert Over Current detection
+- has_xhci: Contains an optional 3.0 PHY
+- device: PHY Device mode. Possible values are: 0 (Host), 1 (Device)
+  or 2 (DRD)
+
+
+
+Example:
+
+usbphy_0: usb-phy@f0470200 {
+   reg = <0xf0470200 0xb8>,
+   <0xf0471940 0x6c0>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   compatible = "brcm,brcmstb-usb-phy";
+   ioc = <1>;
+   ipp = <1>;
+   #phy-cells = <1>;
+   ranges;
+   has_xhci;
+   clocks = <&sw_usb20>;
+   clock-names = "sw_usb";
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..d58b124 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2697,6 +2697,13 @@ S:   Maintained
 F: drivers/bcma/
 F: include/linux/bcma/
 
+BROADCOM STB USB PHY DRIVER
+M: Al Cooper 
+L: linux-...@vger.kernel.org
+L: bcm-kernel-feedback-l...@broadcom.com
+S: Supported
+F: drivers/phy/phy-brcm-usb*
+
 BROADCOM SYSTEMPORT ETHERNET DRIVER
 M: Florian Fainelli 
 L: net...@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19bff3a..5ff5e47 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -443,6 +443,16 @@ config PHY_CYGNUS_PCIE
  Enable this to support the Broadcom Cygnus PCIe PHY.
  If unsure, say N.
 
+config BRCM_USB_PHY
+   tristate "Broadcom USB PHY driver"
+   depends on OF && USB && ARCH_BRCMSTB
+   select GENERIC_PHY
+   default y
+   help
+ Enable this to support the Broadcom USB PHY on
+ Broadcom STB SoCs.
+ If unsure, say Y.
+
 source "drivers/phy/tegra/Kconfig"
 
 config PHY_NS2_PCIE
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 90ae198..f4ff6c7 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_PHY_DA8XX_USB)   += phy-da8xx-usb.o
 obj-$(CONFIG_PHY_DM816X_USB)   += phy-dm816x-usb.o
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb-init.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)  += phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
 obj-$(CONFIG_PHY_LPC18XX_USB_OTG)  += phy-lpc18xx-usb-otg.o
diff --git a/drivers/phy/phy-brcm-usb-init.c b/drivers/phy/phy-brcm-usb-init.c
new file mode 100644
index 000..f5d8c32
--- /dev/null
+++ b/drivers/phy/phy-brcm-usb-init.c
@@ -0,0 +1,792 @@
+/*
+ * Copyright (C) 2014-2016 Broadcom
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distrib