[PATCH 0/1] ARM: OMAP: add external clock provider support

[PATCH 0/1] ARM: OMAP: add external clock provider support

[Tero Kristo]

Hi,

This patch adds possibility to register external clocks (outside the main
SoC) on TI boards through device tree. Clock sources as such include for
example twl-6030 / twl-6040 chips and variants which can be used to clock
for example audio / WLAN chips.

This patch can be queued once someone has a use-case + patches that requires
usage of such clocks.

-Tero

[PATCH 1/1] clk: ti: add support for external clock provider

[Tero Kristo]

External clock provider can now be used to register external clocks under
it. This is needed as the TI clock driver only registers clocks
hierarchically under clock providers, and external clocks do not belong
under any of the current ones.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 .../devicetree/bindings/arm/omap/ext-clocks.txt    |   32 ++++++++++++++++++++
 arch/arm/mach-omap2/io.c                           |   12 ++++++--
 drivers/clk/ti/clk.c                               |   23 ++++++++++++++
 include/linux/clk/ti.h                             |    2 ++
 4 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/ext-clocks.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
new file mode 100644
index 0000000..8e784bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
@@ -0,0 +1,32 @@
+TI external clock provider properties
+
+External clock provider is used to group SoC external board specific
+clock nodes. A separate provider node is required as the TI clock
+driver registers clocks hierarchically.
+
+Required properties:
+- compatible:		Shall be: "ti,external-clocks"
+- clocks:		Contains the external clocks for the board
+- clockdomains:		Contains the external clockdomains for the board
+
+Example:
+
+ext_clocks {
+	compatible = "ti,external-clocks";
+
+	ext_clocks: clocks {
+	};
+
+	ext_clockdomains: clockdomains {
+	};
+};
+
+...
+
+&ext_clocks {
+	gpio_test_clock {
+		compatible = "gpio-clock";
+		#clock-cells = <0>;
+		enable-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>;
+	};
+};
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 8f55945..77be18b 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -21,6 +21,8 @@
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clk/ti.h>
 
 #include <asm/tlb.h>
 #include <asm/mach/map.h>
@@ -729,8 +731,14 @@ int __init omap_clk_init(void)
 		return 0;
 
 	ret = of_prcm_init();
-	if (!ret)
-		ret = omap_clk_soc_init();
+	if (ret)
+		return ret;
+
+	ret = ti_dt_clk_ext_init();
+	if (ret)
+		return ret;
+
+	ret = omap_clk_soc_init();
 
 	return ret;
 }
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
index b1a6f71..c84ce4d 100644
--- a/drivers/clk/ti/clk.c
+++ b/drivers/clk/ti/clk.c
@@ -165,3 +165,26 @@ void ti_dt_clk_init_provider(struct device_node *parent, int index)
 		kfree(retry);
 	}
 }
+
+static struct of_device_id ti_ext_clk_match_table[] = {
+	{ .compatible = "ti,external-clocks" },
+	{ }
+};
+
+/**
+ * ti_dt_clk_ext_init - initialize external clocks from DT
+ *
+ * Some clocks are provided from external chips outside the main SoC.
+ * The details for these are given under a special DT node, which will
+ * be parsed by this function.
+ */
+int ti_dt_clk_ext_init(void)
+{
+	struct device_node *np;
+
+	for_each_matching_node(np, ti_ext_clk_match_table) {
+		ti_dt_clk_init_provider(np, -1);
+	}
+
+	return 0;
+}
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index e8d8a35..188270c 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -310,6 +310,8 @@ int am43xx_dt_clk_init(void);
 int omap2420_dt_clk_init(void);
 int omap2430_dt_clk_init(void);
 
+int ti_dt_clk_ext_init(void);
+
 #ifdef CONFIG_OF
 void of_ti_clk_allow_autoidle_all(void);
 void of_ti_clk_deny_autoidle_all(void);
-- 
1.7.9.5

Re: [PATCH 1/1] clk: ti: add support for external clock provider

[Jyri Sarha]

On 08/01/2014 04:15 PM, Tero Kristo wrote:
> External clock provider can now be used to register external clocks under
> it. This is needed as the TI clock driver only registers clocks
> hierarchically under clock providers, and external clocks do not belong
> under any of the current ones.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Tested-by: Jyri Sarha <jsahra@ti.com>

Re: [PATCH 1/1] clk: ti: add support for external clock provider

[Mark Rutland]

Hi Tero,

On Fri, Aug 01, 2014 at 02:15:48PM +0100, Tero Kristo wrote:
> External clock provider can now be used to register external clocks under
> it. This is needed as the TI clock driver only registers clocks
> hierarchically under clock providers, and external clocks do not belong
> under any of the current ones.

I must admit that I don't understand the justification for this binding.

Why can't these clocks be descrbied elsewhere and wired up as usual? Why
must they be under the TI clock provide node?

>From the limited description above it sounds like the only reason this
exists is to work around a deficiency in the TI clock driver. That does
not sound like a very good reason for having this.

Thanks,
Mark.

> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  .../devicetree/bindings/arm/omap/ext-clocks.txt    |   32 ++++++++++++++++++++
>  arch/arm/mach-omap2/io.c                           |   12 ++++++--
>  drivers/clk/ti/clk.c                               |   23 ++++++++++++++
>  include/linux/clk/ti.h                             |    2 ++
>  4 files changed, 67 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
> new file mode 100644
> index 0000000..8e784bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
> @@ -0,0 +1,32 @@
> +TI external clock provider properties
> +
> +External clock provider is used to group SoC external board specific
> +clock nodes. A separate provider node is required as the TI clock
> +driver registers clocks hierarchically.
> +
> +Required properties:
> +- compatible:		Shall be: "ti,external-clocks"
> +- clocks:		Contains the external clocks for the board
> +- clockdomains:		Contains the external clockdomains for the board
> +
> +Example:
> +
> +ext_clocks {
> +	compatible = "ti,external-clocks";
> +
> +	ext_clocks: clocks {
> +	};
> +
> +	ext_clockdomains: clockdomains {
> +	};
> +};
> +
> +...
> +
> +&ext_clocks {
> +	gpio_test_clock {
> +		compatible = "gpio-clock";
> +		#clock-cells = <0>;
> +		enable-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> index 8f55945..77be18b 100644
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -21,6 +21,8 @@
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk/ti.h>
>  
>  #include <asm/tlb.h>
>  #include <asm/mach/map.h>
> @@ -729,8 +731,14 @@ int __init omap_clk_init(void)
>  		return 0;
>  
>  	ret = of_prcm_init();
> -	if (!ret)
> -		ret = omap_clk_soc_init();
> +	if (ret)
> +		return ret;
> +
> +	ret = ti_dt_clk_ext_init();
> +	if (ret)
> +		return ret;
> +
> +	ret = omap_clk_soc_init();
>  
>  	return ret;
>  }
> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
> index b1a6f71..c84ce4d 100644
> --- a/drivers/clk/ti/clk.c
> +++ b/drivers/clk/ti/clk.c
> @@ -165,3 +165,26 @@ void ti_dt_clk_init_provider(struct device_node *parent, int index)
>  		kfree(retry);
>  	}
>  }
> +
> +static struct of_device_id ti_ext_clk_match_table[] = {
> +	{ .compatible = "ti,external-clocks" },
> +	{ }
> +};
> +
> +/**
> + * ti_dt_clk_ext_init - initialize external clocks from DT
> + *
> + * Some clocks are provided from external chips outside the main SoC.
> + * The details for these are given under a special DT node, which will
> + * be parsed by this function.
> + */
> +int ti_dt_clk_ext_init(void)
> +{
> +	struct device_node *np;
> +
> +	for_each_matching_node(np, ti_ext_clk_match_table) {
> +		ti_dt_clk_init_provider(np, -1);
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
> index e8d8a35..188270c 100644
> --- a/include/linux/clk/ti.h
> +++ b/include/linux/clk/ti.h
> @@ -310,6 +310,8 @@ int am43xx_dt_clk_init(void);
>  int omap2420_dt_clk_init(void);
>  int omap2430_dt_clk_init(void);
>  
> +int ti_dt_clk_ext_init(void);
> +
>  #ifdef CONFIG_OF
>  void of_ti_clk_allow_autoidle_all(void);
>  void of_ti_clk_deny_autoidle_all(void);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/1] clk: ti: add support for external clock provider

[Tero Kristo]

On 08/19/2014 04:22 PM, Mark Rutland wrote:
> Hi Tero,
>
> On Fri, Aug 01, 2014 at 02:15:48PM +0100, Tero Kristo wrote:
>> External clock provider can now be used to register external clocks under
>> it. This is needed as the TI clock driver only registers clocks
>> hierarchically under clock providers, and external clocks do not belong
>> under any of the current ones.
>
> I must admit that I don't understand the justification for this binding.
>
> Why can't these clocks be descrbied elsewhere and wired up as usual? Why
> must they be under the TI clock provide node?
>
>  From the limited description above it sounds like the only reason this
> exists is to work around a deficiency in the TI clock driver. That does
> not sound like a very good reason for having this.

I wouldn't call it a deficiency, but its by design due to need for 
registering clocks and clock providers hierarchically.

I guess it might be possible to re-work the TI clock init to check for 
the parent node, and if it is a clock provider, see if it has been 
initialized or not. The clock provider maps the IO range so it must be 
initialized before any clocks under it are initialized. I'll experiment 
with this a bit and see how it works out.

-Tero

>
> Thanks,
> Mark.
>
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   .../devicetree/bindings/arm/omap/ext-clocks.txt    |   32 ++++++++++++++++++++
>>   arch/arm/mach-omap2/io.c                           |   12 ++++++--
>>   drivers/clk/ti/clk.c                               |   23 ++++++++++++++
>>   include/linux/clk/ti.h                             |    2 ++
>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>> new file mode 100644
>> index 0000000..8e784bb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>> @@ -0,0 +1,32 @@
>> +TI external clock provider properties
>> +
>> +External clock provider is used to group SoC external board specific
>> +clock nodes. A separate provider node is required as the TI clock
>> +driver registers clocks hierarchically.
>> +
>> +Required properties:
>> +- compatible:		Shall be: "ti,external-clocks"
>> +- clocks:		Contains the external clocks for the board
>> +- clockdomains:		Contains the external clockdomains for the board
>> +
>> +Example:
>> +
>> +ext_clocks {
>> +	compatible = "ti,external-clocks";
>> +
>> +	ext_clocks: clocks {
>> +	};
>> +
>> +	ext_clockdomains: clockdomains {
>> +	};
>> +};
>> +
>> +...
>> +
>> +&ext_clocks {
>> +	gpio_test_clock {
>> +		compatible = "gpio-clock";
>> +		#clock-cells = <0>;
>> +		enable-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>;
>> +	};
>> +};
>> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
>> index 8f55945..77be18b 100644
>> --- a/arch/arm/mach-omap2/io.c
>> +++ b/arch/arm/mach-omap2/io.c
>> @@ -21,6 +21,8 @@
>>   #include <linux/init.h>
>>   #include <linux/io.h>
>>   #include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/clk/ti.h>
>>
>>   #include <asm/tlb.h>
>>   #include <asm/mach/map.h>
>> @@ -729,8 +731,14 @@ int __init omap_clk_init(void)
>>   		return 0;
>>
>>   	ret = of_prcm_init();
>> -	if (!ret)
>> -		ret = omap_clk_soc_init();
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = ti_dt_clk_ext_init();
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = omap_clk_soc_init();
>>
>>   	return ret;
>>   }
>> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
>> index b1a6f71..c84ce4d 100644
>> --- a/drivers/clk/ti/clk.c
>> +++ b/drivers/clk/ti/clk.c
>> @@ -165,3 +165,26 @@ void ti_dt_clk_init_provider(struct device_node *parent, int index)
>>   		kfree(retry);
>>   	}
>>   }
>> +
>> +static struct of_device_id ti_ext_clk_match_table[] = {
>> +	{ .compatible = "ti,external-clocks" },
>> +	{ }
>> +};
>> +
>> +/**
>> + * ti_dt_clk_ext_init - initialize external clocks from DT
>> + *
>> + * Some clocks are provided from external chips outside the main SoC.
>> + * The details for these are given under a special DT node, which will
>> + * be parsed by this function.
>> + */
>> +int ti_dt_clk_ext_init(void)
>> +{
>> +	struct device_node *np;
>> +
>> +	for_each_matching_node(np, ti_ext_clk_match_table) {
>> +		ti_dt_clk_init_provider(np, -1);
>> +	}
>> +
>> +	return 0;
>> +}
>> diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
>> index e8d8a35..188270c 100644
>> --- a/include/linux/clk/ti.h
>> +++ b/include/linux/clk/ti.h
>> @@ -310,6 +310,8 @@ int am43xx_dt_clk_init(void);
>>   int omap2420_dt_clk_init(void);
>>   int omap2430_dt_clk_init(void);
>>
>> +int ti_dt_clk_ext_init(void);
>> +
>>   #ifdef CONFIG_OF
>>   void of_ti_clk_allow_autoidle_all(void);
>>   void of_ti_clk_deny_autoidle_all(void);
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH 1/1] clk: ti: add support for external clock provider

[Tero Kristo]

On 08/20/2014 10:39 AM, Tero Kristo wrote:
> On 08/19/2014 04:22 PM, Mark Rutland wrote:
>> Hi Tero,
>>
>> On Fri, Aug 01, 2014 at 02:15:48PM +0100, Tero Kristo wrote:
>>> External clock provider can now be used to register external clocks
>>> under
>>> it. This is needed as the TI clock driver only registers clocks
>>> hierarchically under clock providers, and external clocks do not belong
>>> under any of the current ones.
>>
>> I must admit that I don't understand the justification for this binding.
>>
>> Why can't these clocks be descrbied elsewhere and wired up as usual? Why
>> must they be under the TI clock provide node?
>>
>>  From the limited description above it sounds like the only reason this
>> exists is to work around a deficiency in the TI clock driver. That does
>> not sound like a very good reason for having this.
>
> I wouldn't call it a deficiency, but its by design due to need for
> registering clocks and clock providers hierarchically.
>
> I guess it might be possible to re-work the TI clock init to check for
> the parent node, and if it is a clock provider, see if it has been
> initialized or not. The clock provider maps the IO range so it must be
> initialized before any clocks under it are initialized. I'll experiment
> with this a bit and see how it works out.

I have an alternative approach for this now, posting a patch in a bit.

-Tero

>
>>
>> Thanks,
>> Mark.
>>
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> ---
>>>   .../devicetree/bindings/arm/omap/ext-clocks.txt    |   32
>>> ++++++++++++++++++++
>>>   arch/arm/mach-omap2/io.c                           |   12 ++++++--
>>>   drivers/clk/ti/clk.c                               |   23
>>> ++++++++++++++
>>>   include/linux/clk/ti.h                             |    2 ++
>>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>>> b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>>> new file mode 100644
>>> index 0000000..8e784bb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/omap/ext-clocks.txt
>>> @@ -0,0 +1,32 @@
>>> +TI external clock provider properties
>>> +
>>> +External clock provider is used to group SoC external board specific
>>> +clock nodes. A separate provider node is required as the TI clock
>>> +driver registers clocks hierarchically.
>>> +
>>> +Required properties:
>>> +- compatible:        Shall be: "ti,external-clocks"
>>> +- clocks:        Contains the external clocks for the board
>>> +- clockdomains:        Contains the external clockdomains for the board
>>> +
>>> +Example:
>>> +
>>> +ext_clocks {
>>> +    compatible = "ti,external-clocks";
>>> +
>>> +    ext_clocks: clocks {
>>> +    };
>>> +
>>> +    ext_clockdomains: clockdomains {
>>> +    };
>>> +};
>>> +
>>> +...
>>> +
>>> +&ext_clocks {
>>> +    gpio_test_clock {
>>> +        compatible = "gpio-clock";
>>> +        #clock-cells = <0>;
>>> +        enable-gpios = <&gpio5 25 GPIO_ACTIVE_HIGH>;
>>> +    };
>>> +};
>>> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
>>> index 8f55945..77be18b 100644
>>> --- a/arch/arm/mach-omap2/io.c
>>> +++ b/arch/arm/mach-omap2/io.c
>>> @@ -21,6 +21,8 @@
>>>   #include <linux/init.h>
>>>   #include <linux/io.h>
>>>   #include <linux/clk.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/clk/ti.h>
>>>
>>>   #include <asm/tlb.h>
>>>   #include <asm/mach/map.h>
>>> @@ -729,8 +731,14 @@ int __init omap_clk_init(void)
>>>           return 0;
>>>
>>>       ret = of_prcm_init();
>>> -    if (!ret)
>>> -        ret = omap_clk_soc_init();
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    ret = ti_dt_clk_ext_init();
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    ret = omap_clk_soc_init();
>>>
>>>       return ret;
>>>   }
>>> diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c
>>> index b1a6f71..c84ce4d 100644
>>> --- a/drivers/clk/ti/clk.c
>>> +++ b/drivers/clk/ti/clk.c
>>> @@ -165,3 +165,26 @@ void ti_dt_clk_init_provider(struct device_node
>>> *parent, int index)
>>>           kfree(retry);
>>>       }
>>>   }
>>> +
>>> +static struct of_device_id ti_ext_clk_match_table[] = {
>>> +    { .compatible = "ti,external-clocks" },
>>> +    { }
>>> +};
>>> +
>>> +/**
>>> + * ti_dt_clk_ext_init - initialize external clocks from DT
>>> + *
>>> + * Some clocks are provided from external chips outside the main SoC.
>>> + * The details for these are given under a special DT node, which will
>>> + * be parsed by this function.
>>> + */
>>> +int ti_dt_clk_ext_init(void)
>>> +{
>>> +    struct device_node *np;
>>> +
>>> +    for_each_matching_node(np, ti_ext_clk_match_table) {
>>> +        ti_dt_clk_init_provider(np, -1);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
>>> index e8d8a35..188270c 100644
>>> --- a/include/linux/clk/ti.h
>>> +++ b/include/linux/clk/ti.h
>>> @@ -310,6 +310,8 @@ int am43xx_dt_clk_init(void);
>>>   int omap2420_dt_clk_init(void);
>>>   int omap2430_dt_clk_init(void);
>>>
>>> +int ti_dt_clk_ext_init(void);
>>> +
>>>   #ifdef CONFIG_OF
>>>   void of_ti_clk_allow_autoidle_all(void);
>>>   void of_ti_clk_deny_autoidle_all(void);
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>

Re: [PATCH 0/1] ARM: OMAP: add external clock provider support

[Peter Ujfalusi]

On 08/01/2014 04:15 PM, Tero Kristo wrote:
> Hi,
> 
> This patch adds possibility to register external clocks (outside the main
> SoC) on TI boards through device tree. Clock sources as such include for
> example twl-6030 / twl-6040 chips and variants which can be used to clock
> for example audio / WLAN chips.

Just one question to Mike and Tero:
would it be possible to have generic binding for such an external clocks?
We have the palmas clock driver already upstream which handles the 32K clocks
from the PMIC. Palmas class of PMICs can be used with TI/nVidia(/Intel?)
platforms. We use Palmas on omap5-uevm, DRA7-EVM also uses Palmas compatible
PMIC and some nVidia platform also uses this class of devices (and they all
need to have control over the 32K clock(s)).

> This patch can be queued once someone has a use-case + patches that requires
> usage of such clocks.
> 
> -Tero
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/1] ARM: OMAP: add external clock provider support

[Tero Kristo]

On 08/04/2014 02:37 PM, Peter Ujfalusi wrote:
> On 08/01/2014 04:15 PM, Tero Kristo wrote:
>> Hi,
>>
>> This patch adds possibility to register external clocks (outside the main
>> SoC) on TI boards through device tree. Clock sources as such include for
>> example twl-6030 / twl-6040 chips and variants which can be used to clock
>> for example audio / WLAN chips.
>
> Just one question to Mike and Tero:
> would it be possible to have generic binding for such an external clocks?
> We have the palmas clock driver already upstream which handles the 32K clocks
> from the PMIC. Palmas class of PMICs can be used with TI/nVidia(/Intel?)
> platforms. We use Palmas on omap5-uevm, DRA7-EVM also uses Palmas compatible
> PMIC and some nVidia platform also uses this class of devices (and they all
> need to have control over the 32K clock(s)).

Other platforms initialize their clocks in different manner, they can 
use generic of_clk_init I believe. If they can't use that for some 
reason, then they need to implement something similar to this.

-Tero

>
>> This patch can be queued once someone has a use-case + patches that requires
>> usage of such clocks.
>>
>> -Tero
>>
>
>

Re: [PATCH 0/1] ARM: OMAP: add external clock provider support

[Mike Turquette]

Quoting Tero Kristo (2014-08-04 05:36:19)
> On 08/04/2014 02:37 PM, Peter Ujfalusi wrote:
> > On 08/01/2014 04:15 PM, Tero Kristo wrote:
> >> Hi,
> >>
> >> This patch adds possibility to register external clocks (outside the main
> >> SoC) on TI boards through device tree. Clock sources as such include for
> >> example twl-6030 / twl-6040 chips and variants which can be used to clock
> >> for example audio / WLAN chips.
> >
> > Just one question to Mike and Tero:
> > would it be possible to have generic binding for such an external clocks?
> > We have the palmas clock driver already upstream which handles the 32K clocks
> > from the PMIC. Palmas class of PMICs can be used with TI/nVidia(/Intel?)
> > platforms. We use Palmas on omap5-uevm, DRA7-EVM also uses Palmas compatible
> > PMIC and some nVidia platform also uses this class of devices (and they all
> > need to have control over the 32K clock(s)).
> 
> Other platforms initialize their clocks in different manner, they can 
> use generic of_clk_init I believe. If they can't use that for some 
> reason, then they need to implement something similar to this.

Right. To answer Peter's question, we do have a generic binding, it is
the clock binding ;-)

Even the idea of having a "TI clock provider" isn't really a good way to
do things. IP blocks that generate clocks are clock providers, not
companies. As such if Palmas or Gaia or whatever generate clocks then we
don't need any new infrastructure in the clock core to accomodate this.
Those drivers simply need to include clk-provider.h and register their
clocks with the framework.

Some good examples of this are the omap3-isp.c driver, and qcom's hdmi
phy driver (sorry, don't have the path handy) which both consume clocks
generated by other clock providers, and they provide their own clocks
which are generated within their own IP/module.

A big part of the design of the ccf (and later, the DT bindings) is that
we do not need a centralized place to store every piece of clock
generation knowledge and clock routing knowledge.

Regards,
Mike

> 
> -Tero
> 
> >
> >> This patch can be queued once someone has a use-case + patches that requires
> >> usage of such clocks.
> >>
> >> -Tero
> >>
> >
> >
> 

Re: [PATCH 0/1] ARM: OMAP: add external clock provider support

[Tero Kristo]

On 09/03/2014 10:28 PM, Mike Turquette wrote:
> Quoting Tero Kristo (2014-08-04 05:36:19)
>> On 08/04/2014 02:37 PM, Peter Ujfalusi wrote:
>>> On 08/01/2014 04:15 PM, Tero Kristo wrote:
>>>> Hi,
>>>>
>>>> This patch adds possibility to register external clocks (outside the main
>>>> SoC) on TI boards through device tree. Clock sources as such include for
>>>> example twl-6030 / twl-6040 chips and variants which can be used to clock
>>>> for example audio / WLAN chips.
>>>
>>> Just one question to Mike and Tero:
>>> would it be possible to have generic binding for such an external clocks?
>>> We have the palmas clock driver already upstream which handles the 32K clocks
>>> from the PMIC. Palmas class of PMICs can be used with TI/nVidia(/Intel?)
>>> platforms. We use Palmas on omap5-uevm, DRA7-EVM also uses Palmas compatible
>>> PMIC and some nVidia platform also uses this class of devices (and they all
>>> need to have control over the 32K clock(s)).
>>
>> Other platforms initialize their clocks in different manner, they can
>> use generic of_clk_init I believe. If they can't use that for some
>> reason, then they need to implement something similar to this.
>
> Right. To answer Peter's question, we do have a generic binding, it is
> the clock binding ;-)
>
> Even the idea of having a "TI clock provider" isn't really a good way to
> do things. IP blocks that generate clocks are clock providers, not
> companies. As such if Palmas or Gaia or whatever generate clocks then we
> don't need any new infrastructure in the clock core to accomodate this.
> Those drivers simply need to include clk-provider.h and register their
> clocks with the framework.
>
> Some good examples of this are the omap3-isp.c driver, and qcom's hdmi
> phy driver (sorry, don't have the path handy) which both consume clocks
> generated by other clock providers, and they provide their own clocks
> which are generated within their own IP/module.
>
> A big part of the design of the ccf (and later, the DT bindings) is that
> we do not need a centralized place to store every piece of clock
> generation knowledge and clock routing knowledge.

This patch is obsolete now, I posted the patch which changes the TI 
clock driver to use generic of_clk_init instead of a custom one. This 
now initializes all the external clocks also without any issues.

The patch itself can potentially cause some problems with DPLL clocks 
though (as they use the retry init mechanism and this is overlooked in 
v1), so I am working on v2 atm.

-Tero

>
> Regards,
> Mike
>
>>
>> -Tero
>>
>>>
>>>> This patch can be queued once someone has a use-case + patches that requires
>>>> usage of such clocks.
>>>>
>>>> -Tero
>>>>
>>>
>>>
>>