linux-kernel - Re: [PATCH 1/6 v9] ARM: Add basic architecture support for VIA/WonderMedia 85xx SoC's

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTik600vg0otkJVVeWCwYv-0=UNnQDr1mCR9Zvpve@mail.gmail.com>
Date:	Tue, 21 Dec 2010 03:09:55 +0300
From:	Alexey Charkov <alchark@...il.com>
To:	Ryan Mallon <ryan@...ewatersys.com>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	linux-arm-kernel@...ts.infradead.org,
	vt8500-wm8505-linux-kernel@...glegroups.com,
	Eric Miao <eric.y.miao@...il.com>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Albin Tonnerre <albin.tonnerre@...e-electrons.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6 v9] ARM: Add basic architecture support for
 VIA/WonderMedia 85xx SoC's

2010/12/21 Alexey Charkov <alchark@...il.com>:
> 2010/12/21 Ryan Mallon <ryan@...ewatersys.com>:
>> On 12/21/2010 12:00 PM, Alexey Charkov wrote:
>>> 2010/12/21 Ryan Mallon <ryan@...ewatersys.com>:
>>>> On 12/21/2010 10:48 AM, Alexey Charkov wrote:
>>>>> 2010/12/20 Ryan Mallon <ryan@...ewatersys.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/