| Message ID | 1363352438-15935-48-git-send-email-eduardo.valentin@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@ti.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. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@ti.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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Mar 16, 2013 at 08:41:30AM -0400, Eduardo Valentin wrote: > On 16-03-2013 04:59, Dan Carpenter wrote: > >On Fri, Mar 15, 2013 at 09:00:35AM -0400, Eduardo Valentin wrote: > >>@@ -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.. Yes. It still applies. We need to disable IRQs from the current CPU while we are holding a spin_lock which they will need. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); omap_bandgap_update_alert_threshold(bg_ptr, id, adc_val, hot); - mutex_unlock(&bg_ptr->bg_mutex); + spin_unlock(&bg_ptr->lock); exit: return ret; @@ -666,9 +668,9 @@ int omap_bandgap_write_update_interval(struct omap_bandgap *bg_ptr, return -ENOTSUPP; interval = interval * bg_ptr->clk_rate / 1000; - mutex_lock(&bg_ptr->bg_mutex); + spin_lock(&bg_ptr->lock); RMW_BITS(bg_ptr, id, bgap_counter, counter_mask, interval); - mutex_unlock(&bg_ptr->bg_mutex); + spin_unlock(&bg_ptr->lock); return 0; } @@ -691,9 +693,9 @@ int omap_bandgap_read_temperature(struct omap_bandgap *bg_ptr, int id, if (ret) return ret; - mutex_lock(&bg_ptr->bg_mutex); + spin_lock(&bg_ptr->lock); temp = omap_bandgap_read_temp(bg_ptr, id); - mutex_unlock(&bg_ptr->bg_mutex); + spin_unlock(&bg_ptr->lock); ret |= omap_bandgap_adc_to_mcelsius(bg_ptr, temp, &temp); if (ret) @@ -1016,7 +1018,7 @@ int omap_bandgap_probe(struct platform_device *pdev) clk_prepare_enable(bg_ptr->fclock); - mutex_init(&bg_ptr->bg_mutex); + spin_lock_init(&bg_ptr->lock); bg_ptr->dev = &pdev->dev; platform_set_drvdata(pdev, bg_ptr); diff --git a/drivers/staging/omap-thermal/omap-bandgap.h b/drivers/staging/omap-thermal/omap-bandgap.h index edcc965..5700586 100644 --- a/drivers/staging/omap-thermal/omap-bandgap.h +++ b/drivers/staging/omap-thermal/omap-bandgap.h @@ -23,7 +23,7 @@ #ifndef __OMAP_BANDGAP_H #define __OMAP_BANDGAP_H -#include <linux/mutex.h> +#include <linux/spinlock.h> #include <linux/types.h> #include <linux/err.h> @@ -211,7 +211,7 @@ struct omap_bandgap { struct omap_bandgap_data *conf; struct clk *fclock; struct clk *div_clk; - struct mutex bg_mutex; /* shields this struct */ + spinlock_t lock; /* shields this struct */ int irq; int tshut_gpio; u32 clk_rate;
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@ti.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(-)