[PATCH 1/2] arm/dt: add basic usb nodes to tegra device trees

[PATCH 1/2] arm/dt: add basic usb nodes to tegra device trees

[Olof Johansson]

For now they are a minimal binding. It needs to be amended with
vendor-specific settings for phy setup and link tuning, etc.

Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
---
 arch/arm/boot/dts/tegra-seaboard.dts |    4 ++++
 arch/arm/boot/dts/tegra20.dtsi       |   18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
index a72299b..88c682a 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -29,4 +29,8 @@
 	sdhci@c8000600 {
 		support-8bit;
 	};
+
+	usb@c5000000 {
+		nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
+	};
 };
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 65d7e6a..2ad1107 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -143,5 +143,23 @@
 		reg = <0xc8000600 0x200>;
 		interrupts = < 63 >;
 	};
+
+	usb@c5000000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5000000 0x4000>;
+		interrupts = < 52 >;
+	};
+
+	usb@c5004000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5004000 0x4000>;
+		interrupts = < 53 >;
+	};
+
+	usb@c5008000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5008000 0x4000>;
+		interrupts = < 129 >;
+	};
 };
 
-- 
1.7.4.1

[PATCH 2/2] USB: ehci-tegra: add probing through device tree

[Olof Johansson]

Rely on platform_data being passed through auxdata for now; more elaborate
bindings for phy config and tunings to be added.

Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
---
 arch/arm/mach-tegra/board-dt.c |    8 +++++
 drivers/usb/host/ehci-tegra.c  |   62 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/board-dt.c b/arch/arm/mach-tegra/board-dt.c
index 74743ad..808a12e 100644
--- a/arch/arm/mach-tegra/board-dt.c
+++ b/arch/arm/mach-tegra/board-dt.c
@@ -61,12 +61,20 @@ struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("nvidia,tegra20-i2s", TEGRA_I2S1_BASE, "tegra-i2s.0", NULL),
 	OF_DEV_AUXDATA("nvidia,tegra20-i2s", TEGRA_I2S1_BASE, "tegra-i2s.1", NULL),
 	OF_DEV_AUXDATA("nvidia,tegra20-das", TEGRA_APB_MISC_DAS_BASE, "tegra-das", NULL),
+	OF_DEV_AUXDATA("nvidia,tegra20-ehci", TEGRA_USB_BASE, "tegra-ehci.0",
+		       &tegra_ehci1_device.dev.platform_data),
+	OF_DEV_AUXDATA("nvidia,tegra20-ehci", TEGRA_USB2_BASE, "tegra-ehci.1",
+		       &tegra_ehci2_device.dev.platform_data),
+	OF_DEV_AUXDATA("nvidia,tegra20-ehci", TEGRA_USB3_BASE, "tegra-ehci.2",
+		       &tegra_ehci3_device.dev.platform_data),
 	{}
 };
 
 static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
 	/* name		parent		rate		enabled */
 	{ "uartd",	"pll_p",	216000000,	true },
+	{ "usbd",	"clk_m",	12000000,	false },
+	{ "usb3",	"clk_m",	12000000,	false },
 	{ NULL,		NULL,		0,		0},
 };
 
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index db9d1b4..58c3ccc 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -21,7 +21,12 @@
 #include <linux/platform_data/tegra_usb.h>
 #include <linux/irq.h>
 #include <linux/usb/otg.h>
+#include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+
 #include <mach/usb_phy.h>
+#include <mach/iomap.h>
 
 #define TEGRA_USB_DMA_ALIGN 32
 
@@ -574,6 +579,27 @@ static const struct hc_driver tegra_ehci_hc_driver = {
 	.port_handed_over	= ehci_port_handed_over,
 };
 
+static int setup_vbus_gpio(struct platform_device *pdev, int gpio)
+{
+	int err = 0;
+
+	err = gpio_request(gpio, "vbus_gpio");
+	if (err) {
+		dev_err(&pdev->dev, "can't request vbus gpio %d", gpio);
+		return err;
+	}
+	err = gpio_direction_output(gpio, 1);
+	if (err) {
+		dev_err(&pdev->dev, "can't enable vbus\n");
+		return err;
+	}
+	gpio_set_value(gpio, 1);
+
+	return err;
+}
+
+static u64 tegra_ehci_dma_mask = DMA_BIT_MASK(32);
+
 static int tegra_ehci_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -583,6 +609,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	int err = 0;
 	int irq;
 	int instance = pdev->id;
+	int vbus_gpio;
 
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
@@ -590,6 +617,13 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (!pdev->dev.dma_mask)
+		pdev->dev.dma_mask = &tegra_ehci_dma_mask;
+
+	vbus_gpio = of_get_named_gpio(pdev->dev.of_node, "nvidia,vbus-gpio", 0);
+	if (vbus_gpio > 0)
+		setup_vbus_gpio(pdev, vbus_gpio);
+
 	tegra = kzalloc(sizeof(struct tegra_ehci_hcd), GFP_KERNEL);
 	if (!tegra)
 		return -ENOMEM;
@@ -640,6 +674,28 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 		goto fail_io;
 	}
 
+	/* This is pretty ugly and needs to be fixed when we do only
+	 * device-tree probing. Old code relies on the platform_device
+	 * numbering that we lack for device-tree-instantiated devices.
+	 */
+	if (instance < 0) {
+		switch (res->start) {
+		case TEGRA_USB_BASE:
+			instance = 0;
+			break;
+		case TEGRA_USB2_BASE:
+			instance = 1;
+			break;
+		case TEGRA_USB3_BASE:
+			instance = 2;
+			break;
+		default:
+			err = -ENODEV;
+			dev_err(&pdev->dev, "unknown usb instance\n");
+			goto fail_phy;
+		}
+	}
+
 	tegra->phy = tegra_usb_phy_open(instance, hcd->regs, pdata->phy_config,
 						TEGRA_USB_PHY_MODE_HOST);
 	if (IS_ERR(tegra->phy)) {
@@ -773,6 +829,11 @@ static void tegra_ehci_hcd_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
+static struct of_device_id tegra_ehci_of_match[] __devinitdata = {
+	{ .compatible = "nvidia,tegra20-ehci", },
+	{ },
+};
+
 static struct platform_driver tegra_ehci_driver = {
 	.probe		= tegra_ehci_probe,
 	.remove		= tegra_ehci_remove,
@@ -783,5 +844,6 @@ static struct platform_driver tegra_ehci_driver = {
 	.shutdown	= tegra_ehci_hcd_shutdown,
 	.driver		= {
 		.name	= "tegra-ehci",
+		.of_match_table = tegra_ehci_of_match,
 	}
 };
-- 
1.7.4.1

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

RE: [PATCH 2/2] USB: ehci-tegra: add probing through device tree

[Stephen Warren]

Olof Johansson wrote at Thursday, November 03, 2011 9:26 PM:
> Rely on platform_data being passed through auxdata for now; more elaborate
> bindings for phy config and tunings to be added.
...
> diff --git a/arch/arm/mach-tegra/board-dt.c b/arch/arm/mach-tegra/board-dt.c
...
>  static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
>  	/* name		parent		rate		enabled */
>  	{ "uartd",	"pll_p",	216000000,	true },
> +	{ "usbd",	"clk_m",	12000000,	false },
> +	{ "usb3",	"clk_m",	12000000,	false },
>  	{ NULL,		NULL,		0,		0},
>  };

As woglinde mentioned on IRC, I think you do want to add "usb2" to the
table here; it's in board-paz00.c at least. It shouldn't hurt other boards.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
...
> @@ -590,6 +617,13 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
> 
> +	if (!pdev->dev.dma_mask)
> +		pdev->dev.dma_mask = &tegra_ehci_dma_mask;

Should this come from DT, or is it some more system-level/internal thing
that doesn't make sense to represent there?

> +
> +	vbus_gpio = of_get_named_gpio(pdev->dev.of_node, "nvidia,vbus-gpio", 0);

of_get_named_gpio() does check for NULL node pointer in practice, but is
it defined to? I wonder if this needs to be conditional of of_node!=NULL
or not?

> +	if (vbus_gpio > 0)

Should that use gpio_is_valid()?

> +		setup_vbus_gpio(pdev, vbus_gpio);

-- 
nvpublic

Re: [PATCH 2/2] USB: ehci-tegra: add probing through device tree

[Olof Johansson]

On Fri, Nov 4, 2011 at 8:37 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Olof Johansson wrote at Thursday, November 03, 2011 9:26 PM:
>> Rely on platform_data being passed through auxdata for now; more elaborate
>> bindings for phy config and tunings to be added.
> ...
>> diff --git a/arch/arm/mach-tegra/board-dt.c b/arch/arm/mach-tegra/board-dt.c
> ...
>>  static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
>>       /* name         parent          rate            enabled */
>>       { "uartd",      "pll_p",        216000000,      true },
>> +     { "usbd",       "clk_m",        12000000,       false },
>> +     { "usb3",       "clk_m",        12000000,       false },
>>       { NULL,         NULL,           0,              0},
>>  };
>
> As woglinde mentioned on IRC, I think you do want to add "usb2" to the
> table here; it's in board-paz00.c at least. It shouldn't hurt other boards.

Yeah, I didn't catch it before posting this.

It might make sense to just move them to common.c then, especially
since the clocks aren't enabled in the table. But I'll add usb2 for
now, and we can move them later if it makes sense.

>
>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> ...
>> @@ -590,6 +617,13 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>>               return -EINVAL;
>>       }
>>
>> +     if (!pdev->dev.dma_mask)
>> +             pdev->dev.dma_mask = &tegra_ehci_dma_mask;
>
> Should this come from DT, or is it some more system-level/internal thing
> that doesn't make sense to represent there?

Ideally it should come from the device tree based on dma window
information, but there's currently no way to encode in there what the
dma address limitations of a device is, or at least nothing that is
used generically -- IBM has some custom extensions to encode what part
of the dma address space is mapped for a specific device through the
iommu.

The auxdata function has a comment saying that it is intentionally not
setting up the dma mapping functions and that it should be handled
through a system notifier instead (but it does set the
coherent_dma_mask). As a stepping stone on getting that all sorted out
I just did the simple solution above. I'll add a comment saying it
should go away.

>> +
>> +     vbus_gpio = of_get_named_gpio(pdev->dev.of_node, "nvidia,vbus-gpio", 0);
>
> of_get_named_gpio() does check for NULL node pointer in practice, but is
> it defined to? I wonder if this needs to be conditional of of_node!=NULL
> or not?

Sure, I can wrap it with a test of of_node.

>> +     if (vbus_gpio > 0)
>
> Should that use gpio_is_valid()?

Yep, fixing.



-Olof

Re: [PATCH 1/2] arm/dt: add basic usb nodes to tegra device trees

[Grant Likely]

[-- Attachment #1.1: Type: text/plain, Size: 1885 bytes --]

On Nov 3, 2011 11:26 PM, "Olof Johansson" <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org> wrote:
>
> For now they are a minimal binding. It needs to be amended with
> vendor-specific settings for phy setup and link tuning, etc.
>
> Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

> ---
>  arch/arm/boot/dts/tegra-seaboard.dts |    4 ++++
>  arch/arm/boot/dts/tegra20.dtsi       |   18 ++++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra-seaboard.dts
b/arch/arm/boot/dts/tegra-seaboard.dts
> index a72299b..88c682a 100644
> --- a/arch/arm/boot/dts/tegra-seaboard.dts
> +++ b/arch/arm/boot/dts/tegra-seaboard.dts
> @@ -29,4 +29,8 @@
>        sdhci@c8000600 {
>                support-8bit;
>        };
> +
> +       usb@c5000000 {
> +               nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
> +       };
>  };
> diff --git a/arch/arm/boot/dts/tegra20.dtsi
b/arch/arm/boot/dts/tegra20.dtsi
> index 65d7e6a..2ad1107 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -143,5 +143,23 @@
>                reg = <0xc8000600 0x200>;
>                interrupts = < 63 >;
>        };
> +
> +       usb@c5000000 {
> +               compatible = "nvidia,tegra20-ehci", "usb-ehci";
> +               reg = <0xc5000000 0x4000>;
> +               interrupts = < 52 >;
> +       };
> +
> +       usb@c5004000 {
> +               compatible = "nvidia,tegra20-ehci", "usb-ehci";
> +               reg = <0xc5004000 0x4000>;
> +               interrupts = < 53 >;
> +       };
> +
> +       usb@c5008000 {
> +               compatible = "nvidia,tegra20-ehci", "usb-ehci";
> +               reg = <0xc5008000 0x4000>;
> +               interrupts = < 129 >;
> +       };
>  };
>
> --
> 1.7.4.1
>

[-- Attachment #1.2: Type: text/html, Size: 2698 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: [PATCH 1/2] arm/dt: add basic usb nodes to tegra device trees

[Stephen Warren]

Olof Johansson wrote at Thursday, November 03, 2011 9:26 PM:
> For now they are a minimal binding. It needs to be amended with
> vendor-specific settings for phy setup and link tuning, etc.
...
> diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
...
> +	usb@c5000000 {
> +		nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
> +	};

Given that's a custom property, don't we need bindings documentation in
Documentation/devictree/bindings/usb/? Even if the binding is pretty self-
explanatory.

-- 
nvpublic

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

Re: [PATCH 1/2] arm/dt: add basic usb nodes to tegra device trees

[Grant Likely]

On Fri, Nov 4, 2011 at 11:27 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Olof Johansson wrote at Thursday, November 03, 2011 9:26 PM:
>> For now they are a minimal binding. It needs to be amended with
>> vendor-specific settings for phy setup and link tuning, etc.
> ...
>> diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
> ...
>> +     usb@c5000000 {
>> +             nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
>> +     };
>
> Given that's a custom property, don't we need bindings documentation in
> Documentation/devictree/bindings/usb/? Even if the binding is pretty self-
> explanatory.

Yes, you're right.  Please add a binding, but you can leave my ack in
place when reposted.

g.