Hi Tomasz,
On Wed, Jul 16, 2014 at 4:33 PM, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote:
> Hi Abhilash, Andrew,
>
> Please see my comments inline.
>
> On 15.07.2014 20:35, Abhilash Kesavan wrote:
>> From: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
>>
>> Use the common-clock framework to scale frequencies for the various
>> peripheral clocks on the Exynos5250.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
>> ---
>> drivers/devfreq/exynos/exynos5_bus.c | 131 ++++++++++++++++++++++++++++++----
>> 1 file changed, 119 insertions(+), 12 deletions(-)
>
> [snip]
>
>> +
>> static struct int_bus_opp_table exynos5_int_opp_table[] = {
>> {LV_0, 266000, 1025000},
>> {LV_1, 200000, 1025000},
>> @@ -75,6 +85,98 @@ static struct int_bus_opp_table exynos5_int_opp_table[] = {
>> {0, 0, 0},
>> };
>>
>> +static struct int_clk_table aclk_166[] = {
>> + {LV_0, 167000},
>> + {LV_1, 111000},
>> + {LV_2, 84000},
>> + {LV_3, 84000},
>> + {LV_4, 42000},
>> +};
>
> [snip]
>
>> +static struct int_clk_table aclk_300_gscl[] = {
>> + {LV_0, 267000},
>> + {LV_1, 267000},
>> + {LV_2, 267000},
>> + {LV_3, 200000},
>> + {LV_4, 100000},
>> +};
>
> Shouldn't you manage this using OPP framework and parse this from DT?
OK, one of the questions I had was about the handling of virtual INT
bus levels (frequencies and voltages). I have consolidated my
understanding of how the bindings should look and questions I had in
the driver thread.
>
>> +
>> +#define EXYNOS5_INT_CLK(name, tbl) { \
>> + .clk_name = name, \
>> + .freq_table = tbl, \
>> +}
>
> [snip]
>
>> @@ -320,16 +427,16 @@ static int exynos5_busfreq_int_probe(struct platform_device *pdev)
>> rcu_read_unlock();
>> data->curr_freq = initial_freq;
>>
>> - err = clk_set_rate(data->int_clk, initial_freq * 1000);
>> + err = exynos5_int_setvolt(data, initial_volt);
>> + if (err)
>> + return err;
>> +
>> + err = exynos5_int_set_rate(data, initial_freq);
>> if (err) {
>> dev_err(dev, "Failed to set initial frequency\n");
>> return err;
>> }
>>
>> - err = exynos5_int_setvolt(data, initial_volt);
>> - if (err)
>> - return err;
>> -
>
> Whether voltage or rate should be set first depends on current settings
> and initial_{freq,volt}. Also it might be more convenient to simply call
> exynos5_busfreq_int_target() here.
Wouldn't setting voltage first always be safe ? Yes, just calling
target would be better. Will modify.
Regards,
Abhilash
>
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
