lkml.org 
[lkml]   [2013]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 03/11] ARM: timer-sp: convert to use CLKSRC_OF init
    On Wed, Mar 20, 2013 at 05:54:03PM -0500, Rob Herring wrote:
    > + clk0 = of_clk_get(np, 0);
    > + if (IS_ERR(clk0))
    > + clk0 = NULL;
    > +
    > + /* Get the 2nd clock if the timer has 2 timer clocks */
    > + if (of_count_phandle_with_args(np, "clocks", "#clock-cells") == 3) {
    > + clk1 = of_clk_get(np, 1);
    > + if (IS_ERR(clk1)) {
    > + pr_err("sp804: %s clock not found: %d\n", np->name,
    > + (int)PTR_ERR(clk1));
    > + return;
    > + }
    > + } else
    > + clk1 = clk0;
    > +
    > + irq = irq_of_parse_and_map(np, 0);
    > + if (irq <= 0)
    > + return;
    > +
    > + of_property_read_u32(np, "arm,sp804-has-irq", &irq_num);
    > + if (irq_num == 2)
    > + tmr2_evt = true;
    > +
    > + __sp804_clockevents_init(base + (tmr2_evt ? TIMER_2_BASE : 0),
    > + irq, tmr2_evt ? clk1 : clk0, name);
    > + __sp804_clocksource_and_sched_clock_init(base + (tmr2_evt ? 0 : TIMER_2_BASE),
    > + name, tmr2_evt ? clk0 : clk1, 1);

    This just looks totally screwed to me.

    A SP804 cell has two timers, and has one clock input and two clock
    enable inputs. The clock input is common to both timers. The timers
    only count on the rising edge of the clock input when the enable
    input is high. (the APB PCLK also matters too...)

    Now, the clock enable inputs are controlled by the SP810 system
    controller to achieve different clock rates for each. So, we *can*
    view an SP804 as having two clock inputs.

    However, the two clock inputs do not depend on whether one or the
    other has an IRQ or not. Timer 1 is always clocked by TIMCLK &
    TIMCLKEN1. Timer 2 is always clocked by TIMCLK & TIMCLKEN2.

    Using the logic above, the clocks depend on how the IRQs are wired
    up... really? Can you see from my description above why that is
    screwed? What bearing does the IRQ have on the wiring of the
    clock inputs?

    And, by doing this aren't you embedding into the DT description
    something specific to the Linux implementation for this device -
    namely the decision by you that the IRQs determine how the clocks
    get used?

    So... NAK.


    \
     
     \ /
      Last update: 2013-03-22 02:21    [W:2.461 / U:2.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site