[PATCH 0/3] usb: add device tree support for fotg2-hcd driver

[PATCH 0/3] usb: add device tree support for fotg2-hcd driver

[Hans Ulli Kroll]

Add device tree binding and DT driver support for fotg2-hcd controller
The device is used on Gemini SoC.

striped down dmesg

fotg210_hcd: FOTG210 Host Controller (EHCI) Driver
fotg210-hcd 68000000.usb: Faraday USB2.0 Host Controller
fotg210-hcd 68000000.usb: new USB bus registered, assigned bus number 1
fotg210-hcd 68000000.usb: irq 27, io mem 0x68000000
fotg210-hcd 68000000.usb: USB 2.0 started, EHCI 1.00
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected
fotg210-hcd 69000000.usb: Faraday USB2.0 Host Controller
fotg210-hcd 69000000.usb: new USB bus registered, assigned bus number 2
fotg210-hcd 69000000.usb: irq 28, io mem 0x69000000
fotg210-hcd 69000000.usb: USB 2.0 started, EHCI 1.00
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 1 port detected

Greetings
Hans Ulli Kroll

[PATCH 1/3] usb: host: fotg2: add devicetree probing

[Hans Ulli Kroll]

Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 drivers/usb/host/fotg210-hcd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 9d0b0518290a..2acc51b0be5a 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -23,6 +23,7 @@
  * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/device.h>
 #include <linux/dmapool.h>
 #include <linux/kernel.h>
@@ -5600,6 +5601,15 @@ static int fotg210_hcd_probe(struct platform_device *pdev)
 	if (usb_disabled())
 		return -ENODEV;
 
+	/* Right now device-tree probed devices don't get dma_mask set.
+	 * Since shared usb code relies on it, set it here for now.
+	 * Once we have dma capability bindings this can go away.
+	 */
+
+	retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (retval)
+		goto fail_create_hcd;
+
 	pdev->dev.power.power_state = PMSG_ON;
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -5676,9 +5686,18 @@ static int fotg210_hcd_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id fotg210_ehci_match[] = {
+	{ .compatible = "faraday,fotg210-hcd" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_ehci_match);
+#endif
+
 static struct platform_driver fotg210_hcd_driver = {
 	.driver = {
 		.name   = "fotg210-hcd",
+		.of_match_table = of_match_ptr(fotg210_ehci_match),
 	},
 	.probe  = fotg210_hcd_probe,
 	.remove = fotg210_hcd_remove,
-- 
2.11.0

Re: [PATCH 1/3] usb: host: fotg2: add devicetree probing

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

Hi Hans,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.10-rc7 next-20170208]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-Ulli-Kroll/usb-host-fotg2-add-devicetree-probing/20170209-042041
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-s4-02090632 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/usb/host/fotg210-hcd.c:25:0:
>> drivers/usb/host/fotg210-hcd.c:5694:25: error: 'exynos_ehci_match' undeclared here (not in a function)
    MODULE_DEVICE_TABLE(of, exynos_ehci_match);
                            ^
   include/linux/module.h:213:21: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern const typeof(name) __mod_##type##__##name##_device_table  \
                        ^~~~
>> include/linux/module.h:213:27: error: '__mod_of__exynos_ehci_match_device_table' aliased to undefined symbol 'exynos_ehci_match'
    extern const typeof(name) __mod_##type##__##name##_device_table  \
                              ^
>> drivers/usb/host/fotg210-hcd.c:5694:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, exynos_ehci_match);
    ^~~~~~~~~~~~~~~~~~~

vim +/exynos_ehci_match +5694 drivers/usb/host/fotg210-hcd.c

  5688	
  5689	#ifdef CONFIG_OF
  5690	static const struct of_device_id fotg210_ehci_match[] = {
  5691		{ .compatible = "faraday,fotg210-hcd" },
  5692		{},
  5693	};
> 5694	MODULE_DEVICE_TABLE(of, exynos_ehci_match);
  5695	#endif
  5696	
  5697	static struct platform_driver fotg210_hcd_driver = {

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29308 bytes --]

Re: [PATCH 1/3] usb: host: fotg2: add devicetree probing

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

Hi Hans,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.10-rc7 next-20170208]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-Ulli-Kroll/usb-host-fotg2-add-devicetree-probing/20170209-042041
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-a0-02091152 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb/host/fotg210-hcd.c:5694: error: 'exynos_ehci_match' undeclared here (not in a function)
>> drivers/usb/host/fotg210-hcd.c:5694: error: type defaults to 'int' in declaration of '__mod_of__exynos_ehci_match_device_table'

vim +5694 drivers/usb/host/fotg210-hcd.c

  5688	
  5689	#ifdef CONFIG_OF
  5690	static const struct of_device_id fotg210_ehci_match[] = {
  5691		{ .compatible = "faraday,fotg210-hcd" },
  5692		{},
  5693	};
> 5694	MODULE_DEVICE_TABLE(of, exynos_ehci_match);
  5695	#endif
  5696	
  5697	static struct platform_driver fotg210_hcd_driver = {

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24046 bytes --]

[PATCH 1/3 v2] usb: host: fotg2: add device tree probing

[Hans Ulli Kroll]

Add device tree probe for fotg2 driver

v2:
fix in wrong MODULE_DEVICE_TABLE

Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 drivers/usb/host/fotg210-hcd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 9d0b0518290a..2acc51b0be5a 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -23,6 +23,7 @@
  * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/device.h>
 #include <linux/dmapool.h>
 #include <linux/kernel.h>
@@ -5600,6 +5601,15 @@ static int fotg210_hcd_probe(struct platform_device *pdev)
 	if (usb_disabled())
 		return -ENODEV;
 
+	/* Right now device-tree probed devices don't get dma_mask set.
+	 * Since shared usb code relies on it, set it here for now.
+	 * Once we have dma capability bindings this can go away.
+	 */
+
+	retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (retval)
+		goto fail_create_hcd;
+
 	pdev->dev.power.power_state = PMSG_ON;
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -5676,9 +5686,18 @@ static int fotg210_hcd_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id fotg210_ehci_match[] = {
+	{ .compatible = "faraday,fotg210-hcd" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, fotg210_ehci_match);
+#endif
+
 static struct platform_driver fotg210_hcd_driver = {
 	.driver = {
 		.name   = "fotg210-hcd",
+		.of_match_table = of_match_ptr(fotg210_ehci_match),
 	},
 	.probe  = fotg210_hcd_probe,
 	.remove = fotg210_hcd_remove,
-- 
2.11.0

Re: [PATCH 1/3 v2] usb: host: fotg2: add device tree probing

[Linus Walleij]

On Thu, Feb 9, 2017 at 3:20 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:

> Add device tree probe for fotg2 driver
>
> v2:
> fix in wrong MODULE_DEVICE_TABLE
>
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

> +       /* Right now device-tree probed devices don't get dma_mask set.
> +        * Since shared usb code relies on it, set it here for now.
> +        * Once we have dma capability bindings this can go away.
> +        */
> +
> +       retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +       if (retval)
> +               goto fail_create_hcd;

Were you able to test this part? I saw the gemini has some DMA engine
but I never saw a driver for it. It looks like a reasonable thing to do to
my untrained DMA eye.

Yours,
Linus Walleij

Re: [PATCH 1/3 v2] usb: host: fotg2: add device tree probing

[Arnd Bergmann]

On Friday, February 10, 2017 2:12:36 PM CET Linus Walleij wrote:
> 
> > +       /* Right now device-tree probed devices don't get dma_mask set.
> > +        * Since shared usb code relies on it, set it here for now.
> > +        * Once we have dma capability bindings this can go away.
> > +        */
> > +
> > +       retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > +       if (retval)
> > +               goto fail_create_hcd;
> 
> Were you able to test this part? I saw the gemini has some DMA engine
> but I never saw a driver for it. It looks like a reasonable thing to do to
> my untrained DMA eye.

We should never use dma_coerce_mask_and_coherent() for new code, it's
only needed for broken platforms without DT.

A 32-bit mask is set by default.

	Arnd

Re: [PATCH 1/3 v2] usb: host: fotg2: add device tree probing

[Hans Ulli Kroll]

Hi Linus

On Fri, 10 Feb 2017, Linus Walleij wrote:

> On Thu, Feb 9, 2017 at 3:20 PM, Hans Ulli Kroll
> <ulli.kroll@googlemail.com> wrote:
> 
> > Add device tree probe for fotg2 driver
> >
> > v2:
> > fix in wrong MODULE_DEVICE_TABLE
> >
> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> > +       /* Right now device-tree probed devices don't get dma_mask set.
> > +        * Since shared usb code relies on it, set it here for now.
> > +        * Once we have dma capability bindings this can go away.
> > +        */
> > +
> > +       retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > +       if (retval)
> > +               goto fail_create_hcd;
> 
> Were you able to test this part? 

The driver itself is tested on my NAS 4220, which has *two* USB Ports. I 
transfered about 4 GB from one USB thumb drive to another one.

I must check my Inventory. I can remember of some Samsung SE-208BW.
It's CD/DVD Wifi device which uses also Gemini SoC, and has one host and 
gadget port.

And with the handling with the gadget port is odd. If this is connected 
with an USB host controller all parts are powerd off, only the CD/DVD part 
is running and the USB lines are passed trough. So there must be some 
switch device here.

There is also some VBUS and USB-ID pins controlled by the Gemini SoC, but 
these are not used on my NAS, maybe the CD/DVD Writer.

> I saw the gemini has some DMA engine
> but I never saw a driver for it. It looks like a reasonable thing to do to
> my untrained DMA eye.

I think the DMA engine is used for other stuff.
Some quick grep with SL2312_GENERAL_DMA_BASE and WRITE_DMA_REG
in the NAS 4420 sources 

drivers/char/gemini_i2s.c
drivers/telephony/gemini_ssp.c
drivers/telephony/gemini_ssp.h
include/asm-arm/arch-sl2312/gemini_i2s.h
include/asm-arm/arch-sl2312/gemini_ssp.h
sound/oss/Storm_ssp.c
sound/oss/storm_ssp.h

sound (via i2s) and telephone ???

Oh and your powercontroler is in
drivers/char/gemini_pwr.c

Greetings
Hans Ulli Kroll

Re: [PATCH 1/3 v2] usb: host: fotg2: add device tree probing

[Greg Kroah-Hartman]

On Thu, Feb 09, 2017 at 03:20:49PM +0100, Hans Ulli Kroll wrote:
> 
> Add device tree probe for fotg2 driver
> 
> v2:
> fix in wrong MODULE_DEVICE_TABLE
> 
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> ---
>  drivers/usb/host/fotg210-hcd.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 9d0b0518290a..2acc51b0be5a 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -23,6 +23,7 @@
>   * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/device.h>
>  #include <linux/dmapool.h>
>  #include <linux/kernel.h>
> @@ -5600,6 +5601,15 @@ static int fotg210_hcd_probe(struct platform_device *pdev)
>  	if (usb_disabled())
>  		return -ENODEV;
>  
> +	/* Right now device-tree probed devices don't get dma_mask set.
> +	 * Since shared usb code relies on it, set it here for now.
> +	 * Once we have dma capability bindings this can go away.
> +	 */
> +
> +	retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (retval)
> +		goto fail_create_hcd;
> +

This change doesn't match up with what the changelog describes :(

[PATCH 1/3 v3] usb: host: fotg2: add device tree probing

[Hans Ulli Kroll]

Add device tree probing for fotg2-hcd driver
This device in a dual role hcd/otg device and it's used on Gemini Soc.

Currently all Gemini Soc based devices uses only the hcd part.

v2:
fix wrong name in MODULE_DEVICE_TABLE()

v3:
better commit message
remove dma_coerce_mask_and_coherent() suggested by Arnd

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 drivers/usb/host/fotg210-hcd.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/host/fotg210-hcd.c 
b/drivers/usb/host/fotg210-hcd.c
index 9d0b0518290a..ff6e4275fcf6 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -23,6 +23,7 @@
  * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/device.h>
 #include <linux/dmapool.h>
 #include <linux/kernel.h>
@@ -5676,9 +5677,18 @@ static int fotg210_hcd_remove(struct 
platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id fotg210_ehci_match[] = {
+	{ .compatible = "faraday,fotg210-hcd" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, fotg210_ehci_match);
+#endif
+
 static struct platform_driver fotg210_hcd_driver = {
 	.driver = {
 		.name   = "fotg210-hcd",
+		.of_match_table = of_match_ptr(fotg210_ehci_match),
 	},
 	.probe  = fotg210_hcd_probe,
 	.remove = fotg210_hcd_remove,
-- 
2.11.0

[PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Hans Ulli Kroll]

This adds DT bindings for the Faraday FOTG2 host controller.

Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt

diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
new file mode 100644
index 000000000000..4c07566a4bf5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
@@ -0,0 +1,15 @@
+Faraday FOTG Host controller
+
+Required properties:
+
+- compatible: should be "faraday,fotg210-hcd"
+- reg: should contain one register range i.e. start and length
+- interrupts: description of the interrupt line
+
+Example for Gemini:
+
+usb@68000000 {
+	compatible = "faraday,fotg210-hcd";
+	reg = <0x68000000 0x1000>;
+	interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
+};
-- 
2.11.0

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Linus Walleij]

On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:

> This adds DT bindings for the Faraday FOTG2 host controller.
>
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Rob Herring]

On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
> This adds DT bindings for the Faraday FOTG2 host controller.
> 
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> ---
>  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> new file mode 100644
> index 000000000000..4c07566a4bf5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> @@ -0,0 +1,15 @@
> +Faraday FOTG Host controller
> +
> +Required properties:
> +
> +- compatible: should be "faraday,fotg210-hcd"

hcd as in "host controller driver"? Bindings describe h/w not drivers.

It's an OTG controller or host controller?

> +- reg: should contain one register range i.e. start and length
> +- interrupts: description of the interrupt line
> +
> +Example for Gemini:
> +
> +usb@68000000 {
> +	compatible = "faraday,fotg210-hcd";
> +	reg = <0x68000000 0x1000>;
> +	interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
> +};
> -- 
> 2.11.0
> 

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Hans Ulli Kroll]

Hi Rob,

On Wed, 15 Feb 2017, Rob Herring wrote:

> On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
> > This adds DT bindings for the Faraday FOTG2 host controller.
> > 
> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> > ---
> >  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> > new file mode 100644
> > index 000000000000..4c07566a4bf5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> > @@ -0,0 +1,15 @@
> > +Faraday FOTG Host controller
> > +
> > +Required properties:
> > +
> > +- compatible: should be "faraday,fotg210-hcd"
> 
> hcd as in "host controller driver"? Bindings describe h/w not drivers.
> 
> It's an OTG controller or host controller?
> 

here only the host controller part used.

faraday fotg2 is a dual role hcd/otg device and here is only the 
host part used.

Greetings
ans Ulli Kroll

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Rob Herring]

On Fri, Feb 17, 2017 at 4:07 AM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:
> Hi Rob,
>
> On Wed, 15 Feb 2017, Rob Herring wrote:
>
>> On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
>> > This adds DT bindings for the Faraday FOTG2 host controller.
>> >
>> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>> > ---
>> >  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
>> >  1 file changed, 15 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>> > new file mode 100644
>> > index 000000000000..4c07566a4bf5
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>> > @@ -0,0 +1,15 @@
>> > +Faraday FOTG Host controller
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: should be "faraday,fotg210-hcd"
>>
>> hcd as in "host controller driver"? Bindings describe h/w not drivers.
>>
>> It's an OTG controller or host controller?
>>
>
> here only the host controller part used.
>
> faraday fotg2 is a dual role hcd/otg device and here is only the
> host part used.

Because you don't care about device mode or restricted by the IP
configuration or SoC integration? The former is a user choice and
shouldn't be part of DT. The latter should be implied by an SoC
specific compatible string. Using only a compatible string for a
licensed IP is not specific enough as vendors use differing versions
and integrate them in different ways.

Rob

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Linus Walleij]

On Tue, Feb 21, 2017 at 3:43 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Feb 17, 2017 at 4:07 AM, Hans Ulli Kroll
> <ulli.kroll@googlemail.com> wrote:
>> Hi Rob,
>>
>> On Wed, 15 Feb 2017, Rob Herring wrote:
>>
>>> On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
>>> > This adds DT bindings for the Faraday FOTG2 host controller.
>>> >
>>> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>>> > ---
>>> >  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
>>> >  1 file changed, 15 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>>> > new file mode 100644
>>> > index 000000000000..4c07566a4bf5
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>>> > @@ -0,0 +1,15 @@
>>> > +Faraday FOTG Host controller
>>> > +
>>> > +Required properties:
>>> > +
>>> > +- compatible: should be "faraday,fotg210-hcd"
>>>
>>> hcd as in "host controller driver"? Bindings describe h/w not drivers.
>>>
>>> It's an OTG controller or host controller?
>>>
>>
>> here only the host controller part used.
>>
>> faraday fotg2 is a dual role hcd/otg device and here is only the
>> host part used.
>
> Because you don't care about device mode or restricted by the IP
> configuration or SoC integration? The former is a user choice and
> shouldn't be part of DT. The latter should be implied by an SoC
> specific compatible string. Using only a compatible string for a
> licensed IP is not specific enough as vendors use differing versions
> and integrate them in different ways.

Hans can you add:

compatible = "cortina,gemini-fotg", "faraday,fotg210-hcd" or something
as composite compatible for our controller?

It's OK for the driver to only match on the latter for now, but for the
future it's nice for the driver to know what silicon it is on.

This is what I've done for most other Cortina/Gemini stuff.

Yours,
Linus Walleij

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Hans Ulli Kroll]

Hi Linus,

On Thu, 30 Mar 2017, Linus Walleij wrote:

> On Tue, Feb 21, 2017 at 3:43 PM, Rob Herring <robh@kernel.org> wrote:
> > On Fri, Feb 17, 2017 at 4:07 AM, Hans Ulli Kroll
> > <ulli.kroll@googlemail.com> wrote:
> >> Hi Rob,
> >>
> >> On Wed, 15 Feb 2017, Rob Herring wrote:
> >>
> >>> On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
> >>> > This adds DT bindings for the Faraday FOTG2 host controller.
> >>> >
> >>> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> >>> > ---
> >>> >  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
> >>> >  1 file changed, 15 insertions(+)
> >>> >  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
> >>> >
> >>> > diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> >>> > new file mode 100644
> >>> > index 000000000000..4c07566a4bf5
> >>> > --- /dev/null
> >>> > +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
> >>> > @@ -0,0 +1,15 @@
> >>> > +Faraday FOTG Host controller
> >>> > +
> >>> > +Required properties:
> >>> > +
> >>> > +- compatible: should be "faraday,fotg210-hcd"
> >>>
> >>> hcd as in "host controller driver"? Bindings describe h/w not drivers.
> >>>
> >>> It's an OTG controller or host controller?
> >>>
> >>
> >> here only the host controller part used.
> >>
> >> faraday fotg2 is a dual role hcd/otg device and here is only the
> >> host part used.
> >
> > Because you don't care about device mode or restricted by the IP
> > configuration or SoC integration? The former is a user choice and
> > shouldn't be part of DT. The latter should be implied by an SoC
> > specific compatible string. Using only a compatible string for a
> > licensed IP is not specific enough as vendors use differing versions
> > and integrate them in different ways.
> 
> Hans can you add:
> 
> compatible = "cortina,gemini-fotg", "faraday,fotg210-hcd" or something
> as composite compatible for our controller?
> 

I prefer 
"faraday,fotg210-usb2"

I've got rejected by Rob due the fact this is an dual role controller, 
which supports both host and device mode. And DT must reflect this desgn 
pattern.

Currently I'm wrappingt my head around the design of the fsl-mph-dr-of.c 
driver to use this as a blueprint for the Faraday driver.

Greetings
Hans Ulli Kroll

Re: [PATCH 2/3] usb: add DT bindings for farady fotg2 host controller

[Linus Walleij]

On Thu, Mar 30, 2017 at 8:31 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:
>> On Tue, Feb 21, 2017 at 3:43 PM, Rob Herring <robh@kernel.org> wrote:
>> > On Fri, Feb 17, 2017 at 4:07 AM, Hans Ulli Kroll
>> > <ulli.kroll@googlemail.com> wrote:
>> >> Hi Rob,
>> >>
>> >> On Wed, 15 Feb 2017, Rob Herring wrote:
>> >>
>> >>> On Wed, Feb 08, 2017 at 09:00:09PM +0100, Hans Ulli Kroll wrote:
>> >>> > This adds DT bindings for the Faraday FOTG2 host controller.
>> >>> >
>> >>> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>> >>> > ---
>> >>> >  Documentation/devicetree/bindings/usb/fotg2-host.txt | 15 +++++++++++++++
>> >>> >  1 file changed, 15 insertions(+)
>> >>> >  create mode 100644 Documentation/devicetree/bindings/usb/fotg2-host.txt
>> >>> >
>> >>> > diff --git a/Documentation/devicetree/bindings/usb/fotg2-host.txt b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>> >>> > new file mode 100644
>> >>> > index 000000000000..4c07566a4bf5
>> >>> > --- /dev/null
>> >>> > +++ b/Documentation/devicetree/bindings/usb/fotg2-host.txt
>> >>> > @@ -0,0 +1,15 @@
>> >>> > +Faraday FOTG Host controller
>> >>> > +
>> >>> > +Required properties:
>> >>> > +
>> >>> > +- compatible: should be "faraday,fotg210-hcd"
>> >>>
>> >>> hcd as in "host controller driver"? Bindings describe h/w not drivers.
>> >>>
>> >>> It's an OTG controller or host controller?
>> >>>
>> >>
>> >> here only the host controller part used.
>> >>
>> >> faraday fotg2 is a dual role hcd/otg device and here is only the
>> >> host part used.
>> >
>> > Because you don't care about device mode or restricted by the IP
>> > configuration or SoC integration? The former is a user choice and
>> > shouldn't be part of DT. The latter should be implied by an SoC
>> > specific compatible string. Using only a compatible string for a
>> > licensed IP is not specific enough as vendors use differing versions
>> > and integrate them in different ways.
>>
>> Hans can you add:
>>
>> compatible = "cortina,gemini-fotg", "faraday,fotg210-hcd" or something
>> as composite compatible for our controller?

I hacked on it a bit and sent out. Hope you don't hate it too much.

> I prefer
> "faraday,fotg210-usb2"

I simply named it after the IP core name, which is just "faraday,fotg210".

> I've got rejected by Rob due the fact this is an dual role controller,
> which supports both host and device mode. And DT must reflect this desgn
> pattern.
>
> Currently I'm wrappingt my head around the design of the fsl-mph-dr-of.c
> driver to use this as a blueprint for the Faraday driver.

I don't know how much more of the dual-mode we need to reflect,
I guess it comes up in OTG mode since we don't parse the
host-only or device-only attributes (yet).

Yours,
Linus Walleij

[PATCH 3/3] DT: add Faraday Tec. as vendor

[Hans Ulli Kroll]

add Faraday Technology Corporation as vendor faraday for DT

Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..489c336e8f80 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -102,6 +102,7 @@ everest	Everest Semiconductor Co. Ltd.
 everspin	Everspin Technologies, Inc.
 excito	Excito
 ezchip	EZchip Semiconductor
+faraday	Faraday Technology Corporation
 fcs	Fairchild Semiconductor
 firefly	Firefly
 focaltech	FocalTech Systems Co.,Ltd
-- 
2.11.0

Re: [PATCH 3/3] DT: add Faraday Tec. as vendor

[Linus Walleij]

On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:

> add Faraday Technology Corporation as vendor faraday for DT
>
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I think I should use this for the PCI block as well, looking over some
code and the root hub is using Faraday's PCI ID.

Yours,
Linus Walleij

Re: [PATCH 3/3] DT: add Faraday Tec. as vendor

[Joel Stanley]

On Fri, Feb 10, 2017 at 11:46 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
> <ulli.kroll@googlemail.com> wrote:
>
>> add Faraday Technology Corporation as vendor faraday for DT
>>
>> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I think I should use this for the PCI block as well, looking over some
> code and the root hub is using Faraday's PCI ID.

Acked-by: Joel Stanley <joel@jms.id.au>

This string is already used by the ftgmac100 Ethernet driver. Thanks
for adding it in.

Cheers,

Joel

Re: [PATCH 3/3] DT: add Faraday Tec. as vendor

[Linus Walleij]

On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:

> add Faraday Technology Corporation as vendor faraday for DT
>
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>

I realized a lot of Gemini IP blocks are actually from Faraday, so now
I'm editing the drivers a bit...

Rob could you pick this patch directly to the DT tree so we need
not worry about the vendor thing?

Yours,
Linus Walleij

Re: [PATCH 3/3] DT: add Faraday Tec. as vendor

[Rob Herring]

On Sun, Feb 12, 2017 at 09:37:37PM +0100, Linus Walleij wrote:
> On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
> <ulli.kroll@googlemail.com> wrote:
> 
> > add Faraday Technology Corporation as vendor faraday for DT
> >
> > Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> 
> I realized a lot of Gemini IP blocks are actually from Faraday, so now
> I'm editing the drivers a bit...
> 
> Rob could you pick this patch directly to the DT tree so we need
> not worry about the vendor thing?

Done.

Rob

Re: [PATCH 0/3] usb: add device tree support for fotg2-hcd driver

[Linus Walleij]

On Wed, Feb 8, 2017 at 9:00 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:

> Add device tree binding and DT driver support for fotg2-hcd controller
> The device is used on Gemini SoC.

All looks good to me.

If you send a patch on top of my git branch adding it to the
gemini.dtsi set as status = "disabled"; and gemini-nas4220b.dts
mainly just setting status = "okay"; I can merge this on top of
the rest of the stuff in my tree that should go upstream to ARM
SoC.

Yours.
Linus Walleij