linux-kernel - Re: [PATCH 47/50] staging: omap-thermal: switch mutex to spinlock inside omap-bandgap

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]

Message-ID: <5144687A.7030007@ti.com>
Date:	Sat, 16 Mar 2013 08:41:30 -0400
From:	Eduardo Valentin <eduardo.valentin@...com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
CC:	<gregkh@...uxfoundation.org>, <devel@...verdev.osuosl.org>,
	<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-pm@...r.kernel.org>
Subject: Re: [PATCH 47/50] staging: omap-thermal: switch mutex to spinlock
 inside omap-bandgap

On 16-03-2013 04:59, Dan Carpenter wrote:
> On Fri, Mar 15, 2013 at 09:00:35AM -0400, Eduardo Valentin wrote:
>> Because there is a need to lock inside IRQ handler, this patch
>> changes the locking mechanism inside the omap-bandgap.[c,h] to
>> spinlocks. Now this lock is used to protect omap_bandgap struct
>> during APIs exposed (possibly used in sysfs handling functions)
>> and inside the ALERT IRQ handler.
>>
>> Because there are registers shared among the sensors, this lock
>> is global, not per sensor.
>>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@...com>
>> ---
>>   drivers/staging/omap-thermal/TODO           |    1 -
>>   drivers/staging/omap-thermal/omap-bandgap.c |   18 ++++++++++--------
>>   drivers/staging/omap-thermal/omap-bandgap.h |    4 ++--
>>   3 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/omap-thermal/TODO b/drivers/staging/omap-thermal/TODO
>> index 77b761b..0f24e9b 100644
>> --- a/drivers/staging/omap-thermal/TODO
>> +++ b/drivers/staging/omap-thermal/TODO
>> @@ -1,7 +1,6 @@
>>   List of TODOs (by Eduardo Valentin)
>>
>>   on omap-bandgap.c:
>> -- Rework locking
>>   - Improve driver code by adding usage of regmap-mmio
>>   - Test every exposed API to userland
>>   - Add support to hwmon
>> diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
>> index 4b631fd..846ced6 100644
>> --- a/drivers/staging/omap-thermal/omap-bandgap.c
>> +++ b/drivers/staging/omap-thermal/omap-bandgap.c
>> @@ -33,7 +33,7 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/err.h>
>>   #include <linux/types.h>
>> -#include <linux/mutex.h>
>> +#include <linux/spinlock.h>
>>   #include <linux/reboot.h>
>>   #include <linux/of_device.h>
>>   #include <linux/of_platform.h>
>> @@ -170,6 +170,7 @@ static irqreturn_t omap_bandgap_talert_irq_handler(int irq, void *data)
>>   	u32 t_hot = 0, t_cold = 0, ctrl;
>>   	int i;
>>
>> +	spin_lock(&bg_ptr->lock);
>>   	for (i = 0; i < bg_ptr->conf->sensor_count; i++) {
>>   		tsr = bg_ptr->conf->sensors[i].registers;
>>   		ctrl = omap_bandgap_readl(bg_ptr, tsr->bgap_status);
>> @@ -208,6 +209,7 @@ static irqreturn_t omap_bandgap_talert_irq_handler(int irq, void *data)
>>   		if (bg_ptr->conf->report_temperature)
>>   			bg_ptr->conf->report_temperature(bg_ptr, i);
>>   	}
>> +	spin_unlock(&bg_ptr->lock);
>>
>>   	return IRQ_HANDLED;
>>   }
>> @@ -502,9 +504,9 @@ int _omap_bandgap_write_threshold(struct omap_bandgap *bg_ptr, int id, int val,
>>   	if (ret < 0)
>>   		goto exit;
>>
>> -	mutex_lock(&bg_ptr->bg_mutex);
>> +	spin_lock(&bg_ptr->lock);
>
> These need to disable interrupts because we take the spin lock in
> the IRQ handler.

This IRQ gets masked at the IRQ controller level when served (ONE_SHOT). 
Not sure if your comment is applicable in this case..

>
> regards,
> dan carpenter
>
>
>
>

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