On Tue, Apr 21, 2015 at 5:02 AM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Tue, Apr 21, 2015 at 05:05:30AM +0800, Frank.Li@xxxxxxxxxxxxx wrote:
>> From: Frank Li <Frank.Li@xxxxxxxxxxxxx>
>>
>> Add GPT system timer support for i.MX7D, it has same hardware
>> as i.MX6DL.
>>
>> Signed-off-by: Frank Li <Frank.Li@xxxxxxxxxxxxx>
>> Signed-off-by: Anson Huang <b20788@xxxxxxxxxxxxx>
>> ---
>> arch/arm/mach-imx/time.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c
>> index 15d18e1..7c1d8a3 100644
>> --- a/arch/arm/mach-imx/time.c
>> +++ b/arch/arm/mach-imx/time.c
>> @@ -321,7 +321,7 @@ static void __init _mxc_timer_init(int irq,
>> tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN;
>> if (clk_get_rate(clk_per) == V2_TIMER_RATE_OSC_DIV8) {
>> tctl_val |= V2_TCTL_CLK_OSC_DIV8;
>> - if (cpu_is_imx6dl() || cpu_is_imx6sx()) {
>> + if (cpu_is_imx6dl() || cpu_is_imx6sx() || cpu_is_imx7d()) {
>> /* 24 / 8 = 3 MHz */
>> __raw_writel(7 << V2_TPRER_PRE24M,
>> timer_base + MXC_TPRER);
>> @@ -383,3 +383,4 @@ CLOCKSOURCE_OF_DECLARE(mx53_timer, "fsl,imx53-gpt", mxc_timer_init_dt);
>> CLOCKSOURCE_OF_DECLARE(mx6q_timer, "fsl,imx6q-gpt", mxc_timer_init_dt);
>> CLOCKSOURCE_OF_DECLARE(mx6sl_timer, "fsl,imx6sl-gpt", mxc_timer_init_dt);
>> CLOCKSOURCE_OF_DECLARE(mx6sx_timer, "fsl,imx6sx-gpt", mxc_timer_init_dt);
>> +CLOCKSOURCE_OF_DECLARE(mx7d_timer, "fsl,imx7d-gpt", mxc_timer_init_dt);
>
> Hmm, this suggests to me that something is very wrong here, especially
> those cpu_is_imx*() things.
>
> Several questions crop up:
>
> * Shouldn't the code be split between a timer v1 and timer v2 driver,
> and the appropriate one be selected via the DT compatible?
>
> * Shouldn't the clock source for the v2 timer be determined from DT?
> (It looks to me like the v2 timer can be clocked by the perclk or
> by a 24MHz clock with a built-in prescaler.)
>
> * Shouldn't the prescaler for the timer be calculated in the driver and
> the associated divisor automatically set, or specified via DT?
>
> Had these been fixed, you probably wouldn't need to modify the driver to
> add iMX7 support here - which is really what DT is supposed to be about.
>
> I'm not expecting you to fix them - but it would be good to get this
> properly sorted out so that when iMX8 comes along, the same process
> doesn't happen again.
Thank you for your suggestion. I will do that after this patches.
>
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
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
