linux-kernel - Re: [PATCH] OMAP: add pwm driver using dmtimers.

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 12 Dec 2012 10:20:34 -0600
From:	Jon Hunter <jon-hunter@...com>
To:	Thierry Reding <thierry.reding@...onic-design.de>
CC:	NeilBrown <neilb@...e.de>, Grant Erickson <marathon96@...il.com>,
	<linux-omap@...r.kernel.org>, lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] OMAP: add pwm driver using dmtimers.

On 12/12/2012 05:31 AM, Thierry Reding wrote:
> On Wed, Dec 12, 2012 at 07:24:30PM +1100, NeilBrown wrote:

[snip]

>> +static int omap_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>> +{
>> +	struct omap_chip *omap = to_omap_chip(chip);
>> +	int status = 0;
>> +
>> +	/* Enable the counter--always--before attempting to write its
>> +	 * registers and then set the timer to its minimum load value to
>> +	 * ensure we get an overflow event right away once we start it.
>> +	 */
> 
> Block comments should be in the following format:
> 
> 	/*
> 	 * foo...
> 	 * bar...
> 	 */
> 
>> +
>> +	omap_dm_timer_enable(omap->dm_timer);
>> +	omap_dm_timer_write_counter(omap->dm_timer, DM_TIMER_LOAD_MIN);
>> +	omap_dm_timer_start(omap->dm_timer);
>> +	omap_dm_timer_disable(omap->dm_timer);
> 
> So omap_dm_timer_disable() doesn't actually stop the timer? It just
> disables the access to the registers?

I thought this looked odd too ;-)

So what is going on here is that omap_dm_timer_start() calls
omap_dm_timer_enable() but does not call omap_dm_timer_disable(). So the
last disable really just complements the first enable (ie. decrements
the use count), but the timer will not actually be disabled, because the
start has called an extra enable.

These four function calls can be replaced by one call to
omap_dm_timer_set_load_start() and I think that will be much clearer and
concise.

In general, it should not be necessary to call these
omap_dm_timer_enable/disable APIs directly. I am not sure what the
history is or if there is a use-case that really requires this. So in
the future may be I should make them static so they cannot be used
directly :-)

Cheers
Jon
--
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/