Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1564027 > unrolled thread

Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC

Started byStephen Boyd <sboyd@codeaurora.org>
First post2017-01-21 01:50 +0100
Last post2017-01-24 01:00 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC Stephen Boyd <sboyd@codeaurora.org> - 2017-01-21 01:50 +0100
    Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC Chris Packham <Chris.Packham@alliedtelesis.co.nz> - 2017-01-23 09:00 +0100
      Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC Stephen Boyd <sboyd@codeaurora.org> - 2017-01-24 01:00 +0100

#1564027 — Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC

FromStephen Boyd <sboyd@codeaurora.org>
Date2017-01-21 01:50 +0100
SubjectRe: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC
Message-ID<t1S2t-5PG-1@gated-at.bofh.it>
On 01/13, Chris Packham wrote:
> @@ -158,6 +170,14 @@ static const struct coreclk_soc_desc axp_coreclks = {
>  	.num_ratios = ARRAY_SIZE(axp_coreclk_ratios),
>  };
>  
> +static const struct coreclk_soc_desc mv98dx3236_coreclks = {
> +	.get_tclk_freq = mv98dx3236_get_tclk_freq,
> +	.get_cpu_freq = mv98dx3236_get_cpu_freq,
> +	.get_clk_ratio = NULL,
> +	.ratios = NULL,
> +	.num_ratios = 0,

Are these intentionally initialized to 0 explicitly? Otherwise we
could leave them out and it's all the same.

> +};
> +
>  /*
>   * Clock Gating Control
>   */
[..]
> @@ -243,5 +245,30 @@ static void __init of_cpu_clk_setup(struct device_node *node)
>  	iounmap(clock_complex_base);
>  }
>  
> +/* Use this function to call the generic setup with the correct
> + * clock operation
> + */
> +static void __init of_cpu_clk_setup(struct device_node *node)
> +{
> +	_of_cpu_clk_setup(node, &cpu_ops);
> +}
> +
>  CLK_OF_DECLARE(armada_xp_cpu_clock, "marvell,armada-xp-cpu-clock",
>  					 of_cpu_clk_setup);
> +
> +/* Define the clock and operations for the mv98dx3236 - it cannot perform
> + * any operations.
> + */
> +static const struct clk_ops mv98dx3236_cpu_ops = {
> +	.recalc_rate = NULL,
> +	.round_rate = NULL,
> +	.set_rate = NULL,

But clk_set_rate() works silently? Why not just register a clk
provider that returns a NULL pointer? Then there isn't any
structure to maintain?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]


#1564740

FromChris Packham <Chris.Packham@alliedtelesis.co.nz>
Date2017-01-23 09:00 +0100
Message-ID<t2HHI-3EH-7@gated-at.bofh.it>
In reply to#1564027
On 21/01/17 13:48, Stephen Boyd wrote:
> On 01/13, Chris Packham wrote:
>> @@ -158,6 +170,14 @@ static const struct coreclk_soc_desc axp_coreclks = {
>>  	.num_ratios = ARRAY_SIZE(axp_coreclk_ratios),
>>  };
>>
>> +static const struct coreclk_soc_desc mv98dx3236_coreclks = {
>> +	.get_tclk_freq = mv98dx3236_get_tclk_freq,
>> +	.get_cpu_freq = mv98dx3236_get_cpu_freq,
>> +	.get_clk_ratio = NULL,
>> +	.ratios = NULL,
>> +	.num_ratios = 0,
>
> Are these intentionally initialized to 0 explicitly? Otherwise we
> could leave them out and it's all the same.
>

No reason, just didn't remove the unused members when copying the 
armada-xp example above.

>> +};
>> +
>>  /*
>>   * Clock Gating Control
>>   */
> [..]
>> @@ -243,5 +245,30 @@ static void __init of_cpu_clk_setup(struct device_node *node)
>>  	iounmap(clock_complex_base);
>>  }
>>
>> +/* Use this function to call the generic setup with the correct
>> + * clock operation
>> + */
>> +static void __init of_cpu_clk_setup(struct device_node *node)
>> +{
>> +	_of_cpu_clk_setup(node, &cpu_ops);
>> +}
>> +
>>  CLK_OF_DECLARE(armada_xp_cpu_clock, "marvell,armada-xp-cpu-clock",
>>  					 of_cpu_clk_setup);
>> +
>> +/* Define the clock and operations for the mv98dx3236 - it cannot perform
>> + * any operations.
>> + */
>> +static const struct clk_ops mv98dx3236_cpu_ops = {
>> +	.recalc_rate = NULL,
>> +	.round_rate = NULL,
>> +	.set_rate = NULL,
>
> But clk_set_rate() works silently? Why not just register a clk
> provider that returns a NULL pointer? Then there isn't any
> structure to maintain?
>

Not 100% sure what you mean. Something like this?

+static void __init of_mv98dx3236_cpu_clk_setup(struct device_node *node)
+{
+       of_clk_add_provider(node, of_clk_src_simple_get, NULL);
+}

Seems to work as expected (i.e. does nothing, kernel boots/runs).

[toc] | [prev] | [next] | [standalone]


#1565373

FromStephen Boyd <sboyd@codeaurora.org>
Date2017-01-24 01:00 +0100
Message-ID<t2WGJ-4NH-9@gated-at.bofh.it>
In reply to#1564740
On 01/22/2017 11:53 PM, Chris Packham wrote:
> Not 100% sure what you mean. Something like this?
>
> +static void __init of_mv98dx3236_cpu_clk_setup(struct device_node *node)
> +{
> +       of_clk_add_provider(node, of_clk_src_simple_get, NULL);
> +}
>
> Seems to work as expected (i.e. does nothing, kernel boots/runs).
>

Yep that's what I mean.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web