2014-01-08 06:16:28

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 0/5] USB Host support for OMAP5 uEVM (for 3.14)

Hi Benoit & Tony,

This patchset brings up USB Host ports and Ethernet port on
the OMAP5 uEVM board.

It depends on the TI Clock DT conversion patches [1] and is based
on 3.13-rc7

[1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/289895

Changelog:

v4:
- Updated DT binding document for clock binding

v3:
- Rebased on top of 3.13-rc7

cheers,
-roger

Roger Quadros (5):
mfd: omap-usb-host: Update DT clock binding information
ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
ARM: dts: omap4-panda: Provide USB PHY clock
ARM: dts: omap5-uevm: Provide USB PHY clock
ARM: OMAP2+: Remove legacy_init_ehci_clk()

Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++------
arch/arm/boot/dts/omap5-uevm.dts | 8 ++------
arch/arm/boot/dts/omap5.dtsi | 2 ++
arch/arm/mach-omap2/pdata-quirks.c | 16 ----------------
5 files changed, 10 insertions(+), 28 deletions(-)

--
1.8.3.2


2014-01-08 06:16:42

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 2/5] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module

USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz
reference clock to be named "init_60m_fclk". Provide this
information.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap5.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 2f12a47..e0ab379 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -765,6 +765,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges;
+ clocks = <&l3init_60m_fclk>;
+ clock-names = "init_60m_fclk";

usbhsohci: ohci@4a064800 {
compatible = "ti,ohci-omap3", "usb-ohci";
--
1.8.3.2

2014-01-08 06:16:59

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 4/5] ARM: dts: omap5-uevm: Provide USB PHY clock

The HS USB 2 PHY gets its clock from AUXCLK1. Provide this
information.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap5-uevm.dts | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/omap5-uevm.dts b/arch/arm/boot/dts/omap5-uevm.dts
index 002fa70..3b99ec2 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -31,12 +31,8 @@
hsusb2_phy: hsusb2_phy {
compatible = "usb-nop-xceiv";
reset-gpios = <&gpio3 16 GPIO_ACTIVE_LOW>; /* gpio3_80 HUB_NRESET */
- /**
- * FIXME
- * Put the right clock phandle here when available
- * clocks = <&auxclk1>;
- * clock-names = "main_clk";
- */
+ clocks = <&auxclk1_ck>;
+ clock-names = "main_clk";
clock-frequency = <19200000>;
};

--
1.8.3.2

2014-01-08 06:17:13

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 3/5] ARM: dts: omap4-panda: Provide USB PHY clock

The USB PHY gets its clock from AUXCLK3. Provide this
information.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
index 88c6a05..50b72966 100644
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -83,12 +83,8 @@
compatible = "usb-nop-xceiv";
reset-gpios = <&gpio2 30 GPIO_ACTIVE_LOW>; /* gpio_62 */
vcc-supply = <&hsusb1_power>;
- /**
- * FIXME:
- * put the right clock phandle here when available
- * clocks = <&auxclk3>;
- * clock-names = "main_clk";
- */
+ clocks = <&auxclk3_ck>;
+ clock-names = "main_clk";
clock-frequency = <19200000>;
};

--
1.8.3.2

2014-01-08 06:17:27

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 5/5] ARM: OMAP2+: Remove legacy_init_ehci_clk()

The necessary clock phandle for the EHCI clock is now provided
via device tree so we no longer need this legacy method.

Signed-off-by: Roger Quadros <[email protected]>
---
arch/arm/mach-omap2/pdata-quirks.c | 16 ----------------
1 file changed, 16 deletions(-)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 39f020c..6a4e2d1 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -26,20 +26,6 @@ struct pdata_init {
void (*fn)(void);
};

-/*
- * Create alias for USB host PHY clock.
- * Remove this when clock phandle can be provided via DT
- */
-static void __init __used legacy_init_ehci_clk(char *clkname)
-{
- int ret;
-
- ret = clk_add_alias("main_clk", NULL, clkname, NULL);
- if (ret)
- pr_err("%s:Failed to add main_clk alias to %s :%d\n",
- __func__, clkname, ret);
-}
-
#if IS_ENABLED(CONFIG_WL12XX)

static struct wl12xx_platform_data wl12xx __initdata;
@@ -105,7 +91,6 @@ static void __init omap4_sdp_legacy_init(void)
static void __init omap4_panda_legacy_init(void)
{
omap4_panda_display_init_of();
- legacy_init_ehci_clk("auxclk3_ck");
legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
}
#endif
@@ -113,7 +98,6 @@ static void __init omap4_panda_legacy_init(void)
#ifdef CONFIG_SOC_OMAP5
static void __init omap5_uevm_legacy_init(void)
{
- legacy_init_ehci_clk("auxclk1_ck");
}
#endif

--
1.8.3.2

2014-01-08 06:17:57

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

The omap-usb-host driver expects the 60MHz functional clock
of the USB host module to be named as "init_60m_fclk".
Add this information in the DT binding document.

CC: Lee Jones <[email protected]>
CC: Samuel Ortiz <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
index b381fa6..5635202 100644
--- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
+++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
@@ -32,6 +32,10 @@ Optional properties:
- single-ulpi-bypass: Must be present if the controller contains a single
ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1

+- clocks: phandle to 60MHz functional clock to the USB Host module.
+
+- clock-names: must be "init_60m_fclk"
+
Required properties if child node exists:

- #address-cells: Must be 1
--
1.8.3.2

2014-01-08 09:08:32

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wed, Jan 08, 2014 at 11:45:38AM +0530, Roger Quadros wrote:
> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> index b381fa6..5635202 100644
> --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> @@ -32,6 +32,10 @@ Optional properties:
> - single-ulpi-bypass: Must be present if the controller contains a single
> ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>
> +- clocks: phandle to 60MHz functional clock to the USB Host module.
> +
> +- clock-names: must be "init_60m_fclk"
> +
> Required properties if child node exists:
>
> - #address-cells: Must be 1

I have some questions:

What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
all of those be provided by via the DT phandle?

Should the clk_get be changed to of_clk_get()/of_clk_get_by_name() in the
driver? This would potentially remove the need of the init_60m_fclk name.

$ grep clk_get drivers/mfd/omap-usb-host.c
omap->ehci_logic_fck = clk_get(dev, "ehci_logic_fck");
omap->utmi_p1_gfclk = clk_get(dev, "utmi_p1_gfclk");
omap->utmi_p2_gfclk = clk_get(dev, "utmi_p2_gfclk");
omap->xclk60mhsp1_ck = clk_get(dev, "xclk60mhsp1_ck");
omap->xclk60mhsp2_ck = clk_get(dev, "xclk60mhsp2_ck");
omap->init_60m_fclk = clk_get(dev, "init_60m_fclk");
omap->utmi_clk[i] = clk_get(dev, clkname);
omap->hsic480m_clk[i] = clk_get(dev, clkname);
omap->hsic60m_clk[i] = clk_get(dev, clkname);

-- Sebastian


Attachments:
(No filename) (1.55 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-08 09:57:30

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wed, Jan 08, 2014 at 06:15:38AM +0000, Roger Quadros wrote:
> The omap-usb-host driver expects the 60MHz functional clock
> of the USB host module to be named as "init_60m_fclk".
> Add this information in the DT binding document.
>
> CC: Lee Jones <[email protected]>
> CC: Samuel Ortiz <[email protected]>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> index b381fa6..5635202 100644
> --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
> @@ -32,6 +32,10 @@ Optional properties:
> - single-ulpi-bypass: Must be present if the controller contains a single
> ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>
> +- clocks: phandle to 60MHz functional clock to the USB Host module.
> +
> +- clock-names: must be "init_60m_fclk"
> +

Nit: clocks aren't just phandles, there's a clock-specifier too.

Also, it would be nicer if clocks was defined in terms of clock-names,
as it makes the relationship between clocks and clock-names clear, and
makes it easier to extend the list in future. Something like:

- clocks: a list of phandles + clock-specifiers, one for each entry in
clock-names

- clock-names: should include:
* "init_60m_fclk" - the 60MHz functional clock to the USB host module.

Thanks,
Mark.

2014-01-08 10:10:14

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

+Tero

Hi Sebastian,

On 01/08/2014 02:38 PM, Sebastian Reichel wrote:
> On Wed, Jan 08, 2014 at 11:45:38AM +0530, Roger Quadros wrote:
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> index b381fa6..5635202 100644
>> --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> @@ -32,6 +32,10 @@ Optional properties:
>> - single-ulpi-bypass: Must be present if the controller contains a single
>> ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>>
>> +- clocks: phandle to 60MHz functional clock to the USB Host module.
>> +
>> +- clock-names: must be "init_60m_fclk"
>> +
>> Required properties if child node exists:
>>
>> - #address-cells: Must be 1
>
> I have some questions:
>
> What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
> all of those be provided by via the DT phandle?
>

All those clocks are identically named across the OMAP SoCs and are unique for each
SoC, so providing DT phandle for all of them is not required.

The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
this binding.

> Should the clk_get be changed to of_clk_get()/of_clk_get_by_name() in the
> driver? This would potentially remove the need of the init_60m_fclk name.
>

If we use of_clk_xxx() then we'll need to update DT nodes for OMAP4 and OMAP3 as
well to explicitly provide the clock phandle. For now we make use of the fact that
SoC clock data names the clock rightly i.e. "init_60m_fclk".

> $ grep clk_get drivers/mfd/omap-usb-host.c
> omap->ehci_logic_fck = clk_get(dev, "ehci_logic_fck");
> omap->utmi_p1_gfclk = clk_get(dev, "utmi_p1_gfclk");
> omap->utmi_p2_gfclk = clk_get(dev, "utmi_p2_gfclk");
> omap->xclk60mhsp1_ck = clk_get(dev, "xclk60mhsp1_ck");
> omap->xclk60mhsp2_ck = clk_get(dev, "xclk60mhsp2_ck");
> omap->init_60m_fclk = clk_get(dev, "init_60m_fclk");
> omap->utmi_clk[i] = clk_get(dev, clkname);
> omap->hsic480m_clk[i] = clk_get(dev, clkname);
> omap->hsic60m_clk[i] = clk_get(dev, clkname);
>

cheers,
-roger

2014-01-08 10:18:36

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

Hi Mark,

On 01/08/2014 03:26 PM, Mark Rutland wrote:
> On Wed, Jan 08, 2014 at 06:15:38AM +0000, Roger Quadros wrote:
>> The omap-usb-host driver expects the 60MHz functional clock
>> of the USB host module to be named as "init_60m_fclk".
>> Add this information in the DT binding document.
>>
>> CC: Lee Jones <[email protected]>
>> CC: Samuel Ortiz <[email protected]>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> index b381fa6..5635202 100644
>> --- a/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> +++ b/Documentation/devicetree/bindings/mfd/omap-usb-host.txt
>> @@ -32,6 +32,10 @@ Optional properties:
>> - single-ulpi-bypass: Must be present if the controller contains a single
>> ULPI bypass control bit. e.g. OMAP3 silicon <= ES2.1
>>
>> +- clocks: phandle to 60MHz functional clock to the USB Host module.
>> +
>> +- clock-names: must be "init_60m_fclk"
>> +
>
> Nit: clocks aren't just phandles, there's a clock-specifier too.
>
> Also, it would be nicer if clocks was defined in terms of clock-names,
> as it makes the relationship between clocks and clock-names clear, and
> makes it easier to extend the list in future. Something like:
>
> - clocks: a list of phandles + clock-specifiers, one for each entry in
> clock-names
>
> - clock-names: should include:
> * "init_60m_fclk" - the 60MHz functional clock to the USB host module.
>

OK, I'll update it as per your suggestion.

cheers,
-roger

2014-01-08 10:19:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wednesday 08 January 2014 15:39:36 Roger Quadros wrote:
> > What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
> > all of those be provided by via the DT phandle?
> >
>
> All those clocks are identically named across the OMAP SoCs and are unique for each
> SoC, so providing DT phandle for all of them is not required.
>
> The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
> this binding.

What do you mean it was renamed? Is it a different version of the omap-usb-host
device then?

> > Should the clk_get be changed to of_clk_get()/of_clk_get_by_name() in the
> > driver? This would potentially remove the need of the init_60m_fclk name.
> >
>
> If we use of_clk_xxx() then we'll need to update DT nodes for OMAP4 and OMAP3 as
> well to explicitly provide the clock phandle. For now we make use of the fact that
> SoC clock data names the clock rightly i.e. "init_60m_fclk".

I'm getting the feeling that init_60m_fclk is not the name of the clock input
of the omap-usb-host device at all, but rather the name of a signal on the
omap soc, which would not be an appropriate identifier for the binding.

Arnd

2014-01-08 10:27:57

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On 01/08/2014 03:49 PM, Arnd Bergmann wrote:
> On Wednesday 08 January 2014 15:39:36 Roger Quadros wrote:
>>> What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
>>> all of those be provided by via the DT phandle?
>>>
>>
>> All those clocks are identically named across the OMAP SoCs and are unique for each
>> SoC, so providing DT phandle for all of them is not required.
>>
>> The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
>> this binding.
>
> What do you mean it was renamed? Is it a different version of the omap-usb-host
> device then?

I meant the clock gates name on the SoC was renamed. The omap-usb-host device version
is revised as well.

>>> Should the clk_get be changed to of_clk_get()/of_clk_get_by_name() in the
>>> driver? This would potentially remove the need of the init_60m_fclk name.
>>>
>>
>> If we use of_clk_xxx() then we'll need to update DT nodes for OMAP4 and OMAP3 as
>> well to explicitly provide the clock phandle. For now we make use of the fact that
>> SoC clock data names the clock rightly i.e. "init_60m_fclk".
>
> I'm getting the feeling that init_60m_fclk is not the name of the clock input
> of the omap-usb-host device at all, but rather the name of a signal on the
> omap soc, which would not be an appropriate identifier for the binding.

It is a clock gate defined like so in the DT clock data

on OMAP4
init_60m_fclk: init_60m_fclk {
#clock-cells = <0>;
compatible = "ti,divider-clock";
clocks = <&dpll_usb_m2_ck>;
reg = <0x0104>;
ti,dividers = <1>, <8>;
};

on OMAP5
l3init_60m_fclk: l3init_60m_fclk {
#clock-cells = <0>;
compatible = "ti,divider-clock";
clocks = <&dpll_usb_m2_ck>;
reg = <0x0104>;
ti,dividers = <1>, <8>;
};

So you can see that it is the same thing with a different name.

cheers,
-roger

2014-01-08 10:41:52

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wednesday 08 January 2014 15:57:19 Roger Quadros wrote:
> It is a clock gate defined like so in the DT clock data
>
> on OMAP4
> init_60m_fclk: init_60m_fclk {
> #clock-cells = <0>;
> compatible = "ti,divider-clock";
> clocks = <&dpll_usb_m2_ck>;
> reg = <0x0104>;
> ti,dividers = <1>, <8>;
> };
>
> on OMAP5
> l3init_60m_fclk: l3init_60m_fclk {
> #clock-cells = <0>;
> compatible = "ti,divider-clock";
> clocks = <&dpll_usb_m2_ck>;
> reg = <0x0104>;
> ti,dividers = <1>, <8>;
> };
>
> So you can see that it is the same thing with a different name.

Right, but init_60m_fclk is the name of the clock output of the
divider here, which is /not/ what you should put in the "clock-names"
property of the consumer. The clock input name should reflect what
the clock is used for instead.

Arnd

2014-01-08 10:52:52

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

Hi,

On Wed, Jan 08, 2014 at 03:39:36PM +0530, Roger Quadros wrote:
> > What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
> > all of those be provided by via the DT phandle?
>
> All those clocks are identically named across the OMAP SoCs and are unique for each
> SoC, so providing DT phandle for all of them is not required.
>
> The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
> this binding.

I understand the intention of this patch. I was just wondering if
all the clocks should be referenced from DT even if that is not
strictly needed at the moment. This would make clocks similar to
other resources like regulators, gpios, irqs, ...

Having the clocks referenced from DT looks cleaner to me. It means I
can check the DT file for any resources used by a driver. It also
creates some kind of consistency in the kernel.

> > Should the clk_get be changed to of_clk_get()/of_clk_get_by_name() in the
> > driver? This would potentially remove the need of the init_60m_fclk name.
>
> If we use of_clk_xxx() then we'll need to update DT nodes for OMAP4 and OMAP3 as
> well to explicitly provide the clock phandle.

I'm aware of this.

-- Sebastian


Attachments:
(No filename) (1.19 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-08 10:55:56

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wednesday 08 January 2014 11:52:44 Sebastian Reichel wrote:
>
> On Wed, Jan 08, 2014 at 03:39:36PM +0530, Roger Quadros wrote:
> > > What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
> > > all of those be provided by via the DT phandle?
> >
> > All those clocks are identically named across the OMAP SoCs and are unique for each
> > SoC, so providing DT phandle for all of them is not required.
> >
> > The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
> > this binding.
>
> I understand the intention of this patch. I was just wondering if
> all the clocks should be referenced from DT even if that is not
> strictly needed at the moment. This would make clocks similar to
> other resources like regulators, gpios, irqs, ...
>
> Having the clocks referenced from DT looks cleaner to me. It means I
> can check the DT file for any resources used by a driver. It also
> creates some kind of consistency in the kernel.

I think that would be best, yes. AFAIK most other platforms do this
already, OMAP is a bit behind because it started using clocks when the
infrastructure for doing this right was still incomplete.

Arnd

2014-01-08 11:04:56

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On 01/08/2014 04:10 PM, Arnd Bergmann wrote:
> On Wednesday 08 January 2014 15:57:19 Roger Quadros wrote:
>> It is a clock gate defined like so in the DT clock data
>>
>> on OMAP4
>> init_60m_fclk: init_60m_fclk {
>> #clock-cells = <0>;
>> compatible = "ti,divider-clock";
>> clocks = <&dpll_usb_m2_ck>;
>> reg = <0x0104>;
>> ti,dividers = <1>, <8>;
>> };
>>
>> on OMAP5
>> l3init_60m_fclk: l3init_60m_fclk {
>> #clock-cells = <0>;
>> compatible = "ti,divider-clock";
>> clocks = <&dpll_usb_m2_ck>;
>> reg = <0x0104>;
>> ti,dividers = <1>, <8>;
>> };
>>
>> So you can see that it is the same thing with a different name.
>
> Right, but init_60m_fclk is the name of the clock output of the
> divider here, which is /not/ what you should put in the "clock-names"
> property of the consumer. The clock input name should reflect what
> the clock is used for instead.

Ah, now I get it. :). I agree that the name should reflect the function.

Looking more closely, the driver doesn't enable/disable the init_60m_fclk but just
uses it to configure the parent of utmi_p1_gfclk which is a clock input to the
USB module.

Based on this I would call it "refclk_int" for internal reference clock.

cheers,
-roger

2014-01-08 11:11:51

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On 01/08/2014 04:25 PM, Arnd Bergmann wrote:
> On Wednesday 08 January 2014 11:52:44 Sebastian Reichel wrote:
>>
>> On Wed, Jan 08, 2014 at 03:39:36PM +0530, Roger Quadros wrote:
>>>> What about the other clocks acquired in drivers/mfd/omap-usb-host.c? Shouldn't
>>>> all of those be provided by via the DT phandle?
>>>
>>> All those clocks are identically named across the OMAP SoCs and are unique for each
>>> SoC, so providing DT phandle for all of them is not required.
>>>
>>> The init_60m_fclk was renamed to l3init_60m_fclk in OMAP5, and hence the need for
>>> this binding.
>>
>> I understand the intention of this patch. I was just wondering if
>> all the clocks should be referenced from DT even if that is not
>> strictly needed at the moment. This would make clocks similar to
>> other resources like regulators, gpios, irqs, ...
>>
>> Having the clocks referenced from DT looks cleaner to me. It means I
>> can check the DT file for any resources used by a driver. It also
>> creates some kind of consistency in the kernel.
>
> I think that would be best, yes. AFAIK most other platforms do this
> already, OMAP is a bit behind because it started using clocks when the
> infrastructure for doing this right was still incomplete.
>

OK. I'll update the binding information to reflect all the clocks.

But what about clk_get() vs of_clk_get_by_name() ?

cheers,
-roger

2014-01-08 11:36:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On Wednesday 08 January 2014, Roger Quadros wrote:
> OK. I'll update the binding information to reflect all the clocks.
>
> But what about clk_get() vs of_clk_get_by_name() ?

I think the convention these days is to just use clk_get(), which calls
of_clk_get_by_name() internally. Not sure if that's what you are asking.

Arnd

2014-01-08 11:49:33

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] mfd: omap-usb-host: Update DT clock binding information

On 01/08/2014 05:05 PM, Arnd Bergmann wrote:
> On Wednesday 08 January 2014, Roger Quadros wrote:
>> OK. I'll update the binding information to reflect all the clocks.
>>
>> But what about clk_get() vs of_clk_get_by_name() ?
>
> I think the convention these days is to just use clk_get(), which calls
> of_clk_get_by_name() internally. Not sure if that's what you are asking.
>

OK fine. I'll continue to use clk_get() then.

cheers,
-roger

2014-01-09 11:08:40

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] USB Host support for OMAP5 uEVM (for 3.14)

Hi Michele,

Did you enable CONFIG_USB_EHCI_HCD_OMAP in the kernel config?
It is not enabled by default in omap2plus_defconfig.

cheers,
-roger

On 01/09/2014 04:34 PM, Michele Paolino wrote:
> Hello Roger,
>
> I'm testing your patch on an OMAP5430 EVM board. The ethernet is not working in my case. The kernel I'm using is (Tero Kristo's repo - branch 3.13-rc7-dt-clks-v13). Is there something that I'm missing?
>
> Here below you can find more info about my configuration:
>
> boot error messages:
> eth0: ERROR while getting interface flags: No such device
> SIOCSIFADDR: No such device
> eth0: ERROR while getting interface flags: No such device
> SIOCADDRT: Network is unreachable
>
> root@OMAP5:~# /usr/lib/klibc/bin/ipconfig eth0
> ipconfig: eth0: SIOCGIFINDEX: No such device
> /usr/lib/klibc/bin/ipconfig: no devices to configure
>
> root@OMAP5:~# uname -a
> Linux OMAP5 3.13.0-rc7-g2a4526d-dirty #0 SMP Wed Jan 8 10:41:48 CET 2014x
>
> root@OMAP5:~# dmesg | grep eth
> [ 1.661434] usbcore: registered new interface driver cdc_ether
>
> Regards,
>
>
> On 08/01/2014 07:15, Roger Quadros wrote:
>> Hi Benoit & Tony,
>>
>> This patchset brings up USB Host ports and Ethernet port on
>> the OMAP5 uEVM board.
>>
>> It depends on the TI Clock DT conversion patches [1] and is based
>> on 3.13-rc7
>>
>> [1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/289895
>>
>> Changelog:
>>
>> v4:
>> - Updated DT binding document for clock binding
>>
>> v3:
>> - Rebased on top of 3.13-rc7
>>
>> cheers,
>> -roger
>>
>> Roger Quadros (5):
>> mfd: omap-usb-host: Update DT clock binding information
>> ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
>> ARM: dts: omap4-panda: Provide USB PHY clock
>> ARM: dts: omap5-uevm: Provide USB PHY clock
>> ARM: OMAP2+: Remove legacy_init_ehci_clk()
>>
>> Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
>> arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++------
>> arch/arm/boot/dts/omap5-uevm.dts | 8 ++------
>> arch/arm/boot/dts/omap5.dtsi | 2 ++
>> arch/arm/mach-omap2/pdata-quirks.c | 16 ----------------
>> 5 files changed, 10 insertions(+), 28 deletions(-)
>>
>
>
> --
> *Michele Paolino*, Virtualization R&D Engineer
> Virtual Open Systems
> /Open Source KVM Virtualization Developments/
> /Multicore Systems Virtualization Porting Services/
> Web/:/ www.virtualopensystems. <http://www.virtualopensystems.com/>com <http://www.virtualopensystems.com/>
>
>

2014-01-09 11:22:54

by Michele Paolino

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] USB Host support for OMAP5 uEVM (for 3.14)

On 09/01/2014 12:08, Roger Quadros wrote:
> Hi Michele,
>
> Did you enable CONFIG_USB_EHCI_HCD_OMAP in the kernel config?
> It is not enabled by default in omap2plus_defconfig.
Indeed it works. Thank you!

>
> cheers,
> -roger
>
> On 01/09/2014 04:34 PM, Michele Paolino wrote:
>> Hello Roger,
>>
>> I'm testing your patch on an OMAP5430 EVM board. The ethernet is not working in my case. The kernel I'm using is (Tero Kristo's repo - branch 3.13-rc7-dt-clks-v13). Is there something that I'm missing?
>>
>> Here below you can find more info about my configuration:
>>
>> boot error messages:
>> eth0: ERROR while getting interface flags: No such device
>> SIOCSIFADDR: No such device
>> eth0: ERROR while getting interface flags: No such device
>> SIOCADDRT: Network is unreachable
>>
>> root@OMAP5:~# /usr/lib/klibc/bin/ipconfig eth0
>> ipconfig: eth0: SIOCGIFINDEX: No such device
>> /usr/lib/klibc/bin/ipconfig: no devices to configure
>>
>> root@OMAP5:~# uname -a
>> Linux OMAP5 3.13.0-rc7-g2a4526d-dirty #0 SMP Wed Jan 8 10:41:48 CET 2014x
>>
>> root@OMAP5:~# dmesg | grep eth
>> [ 1.661434] usbcore: registered new interface driver cdc_ether
>>
>> Regards,
>>
>>
>> On 08/01/2014 07:15, Roger Quadros wrote:
>>> Hi Benoit & Tony,
>>>
>>> This patchset brings up USB Host ports and Ethernet port on
>>> the OMAP5 uEVM board.
>>>
>>> It depends on the TI Clock DT conversion patches [1] and is based
>>> on 3.13-rc7
>>>
>>> [1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/289895
>>>
>>> Changelog:
>>>
>>> v4:
>>> - Updated DT binding document for clock binding
>>>
>>> v3:
>>> - Rebased on top of 3.13-rc7
>>>
>>> cheers,
>>> -roger
>>>
>>> Roger Quadros (5):
>>> mfd: omap-usb-host: Update DT clock binding information
>>> ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
>>> ARM: dts: omap4-panda: Provide USB PHY clock
>>> ARM: dts: omap5-uevm: Provide USB PHY clock
>>> ARM: OMAP2+: Remove legacy_init_ehci_clk()
>>>
>>> Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++++
>>> arch/arm/boot/dts/omap4-panda-common.dtsi | 8 ++------
>>> arch/arm/boot/dts/omap5-uevm.dts | 8 ++------
>>> arch/arm/boot/dts/omap5.dtsi | 2 ++
>>> arch/arm/mach-omap2/pdata-quirks.c | 16 ----------------
>>> 5 files changed, 10 insertions(+), 28 deletions(-)
>>>
>>
>> --
>> *Michele Paolino*, Virtualization R&D Engineer
>> Virtual Open Systems
>> /Open Source KVM Virtualization Developments/
>> /Multicore Systems Virtualization Porting Services/
>> Web/:/ www.virtualopensystems. <http://www.virtualopensystems.com/>com <http://www.virtualopensystems.com/>
>>
>>
--
Michele Paolino, Virtualization R&D Engineer
Virtual Open Systems