lkml.org 
[lkml]   [2010]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/6 v9] ARM: Add basic architecture support for VIA/WonderMedia 85xx SoC's
    From
    2010/12/21 Alexey Charkov <alchark@gmail.com>:
    > 2010/12/21 Ryan Mallon <ryan@bluewatersys.com>:
    >> On 12/21/2010 12:00 PM, Alexey Charkov wrote:
    >>> 2010/12/21 Ryan Mallon <ryan@bluewatersys.com>:
    >>>> On 12/21/2010 10:48 AM, Alexey Charkov wrote:
    >>>>> 2010/12/20 Ryan Mallon <ryan@bluewatersys.com>:
    >>>>>> On 12/21/2010 08:54 AM, Alexey Charkov wrote:
    >>>>>>> +}
    >>>>>>> +
    >>>>>>> +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
    >>>>>>> +{
    >>>>>>> +     unsigned long long c;
    >>>>>>> +     unsigned long period_cycles, prescale, pv, dc;
    >>>>>>> +
    >>>>>>> +     if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
    >>>>>>> +             return -EINVAL;
    >>>>>>> +
    >>>>>>> +     c = 25000000/2; /* wild guess --- need to implement clocks */
    >>>>>>> +     c = c * period_ns;
    >>>>>>> +     do_div(c, 1000000000);
    >>>>>>> +     period_cycles = c;
    >>>>>>
    >>>>>> This looks like it could be reworked to remove the do_div call.
    >>>>>>
    >>>>>
    >>>>> I just followed PXA implementation in this regard. Are there any
    >>>>> specific suggestions? Note that c should not be a complie-time
    >>>>> constant eventually, as this is the guessed PWM base frequency (should
    >>>>> be read from the hardware, but the code for clocks is not yet in).
    >>>>
    >>>> I didn't have a particular solution in mind, but often by changing the
    >>>> units used and rearranging the math a bit you can often avoid having to
    >>>> do horrible multiplies and divides.
    >>>>
    >>>> For now at least you could avoid the do_div by assigning period_cycles
    >>>> directly.
    >>>>
    >>>
    >>> It depends on period_ns, which is passed in as an argument from
    >>> whatever uses PWM, so I'm not sure it can be assigned directly.
    >>
    >> Ah. How big a number is period_ns? Can you do something like this instead?
    >>
    >>        period_cycles = ((250 / 2) * period_ns) / 10000;
    >>
    >> and still safely avoid overflows?
    >>
    >
    > The only current in-kernel user of PWM is the backlight, and that
    > currently uses period_ns = 250000. At this value it does not overflow.
    > However, in a general case the base frequency will also be returned as
    > a large number (like 12500000) as per CLK infrastructure conventions
    > (once that part is implemented). Further, I can't see any built-in
    > reasons for period_ns to be bounded by anything below sizeof(int). The
    > hardware will work with up to 4096*1024*1000000000/base_frequency (see
    > the code for constraints), so it can in principle overflow with 32 bit
    > arithmetics.
    >

    This discussion led me to look closer at the duty counter calculation:
    if period_ns and duty_ns are both large and close to each other (about
    0.3 seconds, rare but possible use case for PWM), then (pv * duty_ns)
    can overflow in 32 bit multiplication for permissible argument values.
    Should I use do_div((u64)pv * (u64)duty_ns, period_ns) here?

    Best regards,
    Alexey
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-12-21 01:13    [W:0.032 / U:2.456 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site