Thread (96 messages) 96 messages, 9 authors, 2012-06-26
STALE5086d

Re: [RFC PATCH 09/11] ARM: OMAP4+: thermal: introduce bandgap temperature sensor

From: Eduardo Valentin <hidden>
Date: 2012-05-28 10:55:55
Also in: linux-arm-kernel, linux-omap

Hello Konstantin,

On Fri, May 25, 2012 at 08:39:09PM +0400, Konstantin Baydarov wrote:
  Hi.
On 05/25/2012 12:25 PM, Eduardo Valentin wrote:
quoted
In the System Control Module, OMAP supplies a voltage reference
and a temperature sensor feature that are gathered in the band
gap voltage and temperature sensor (VBGAPTS) module. The band
gap provides current and voltage reference for its internal
circuits and other analog IP blocks. The analog-to-digital
converter (ADC) produces an output value that is proportional
to the silicon temperature.

This patch provides a platform driver which expose this feature.
It is moduled as a MFD child of the System Control Module core
MFD driver.

This driver provides only APIs to access the device properties,
like temperature, thresholds and update rate.

Signed-off-by: Eduardo Valentin <redacted>
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 .../devicetree/bindings/thermal/omap_bandgap.txt   |   27 +
 drivers/thermal/Kconfig                            |   13 +
 drivers/thermal/Makefile                           |    4 +-
 drivers/thermal/omap-bandgap.c                     | 1601 ++++++++++++++++++++
 drivers/thermal/omap-bandgap.h                     |   63 +
 5 files changed, 1707 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/omap_bandgap.txt
 create mode 100644 drivers/thermal/omap-bandgap.c
 create mode 100644 drivers/thermal/omap-bandgap.h
Add private spin lock in omap-bandgap driver to prevent blocking of
control module general registers access.
Thanks for sending this out. I will rethink these drivers wrt to locking and will
your proposal.
I wasn't able to test - I have panda 4430 board.
Right, we definetly need to have it probing on 4430, I will check how we can close on that.
TODO:
Prevent over-usage of spin_lock/spin_unlock for sequential calls of
bg_writel().
OK...
quoted hunk ↗ jump to hunk
Signed-off-by: Konstantin Baydarov <redacted>

Index: omap-thermal/drivers/mfd/omap-control-core.c
===================================================================
--- omap-thermal.orig/drivers/mfd/omap-control-core.c
+++ omap-thermal/drivers/mfd/omap-control-core.c
@@ -67,6 +67,19 @@ EXPORT_SYMBOL_GPL(omap_control_readl);
 int omap_control_writel(struct device *dev, u32 val, u32 reg)
 {
 	struct omap_control *omap_control = dev_get_drvdata(dev);
+
+	if (!omap_control)
+		return -EINVAL;
+
+	writel(val, omap_control->base + reg);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(omap_control_writel);
+
+int omap_control_lock_writel(struct device *dev, u32 val, u32 reg)
+{
+	struct omap_control *omap_control = dev_get_drvdata(dev);
 	unsigned long flags;
 
 	if (!omap_control)
@@ -78,7 +91,7 @@ int omap_control_writel(struct device *d
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(omap_control_writel);
+EXPORT_SYMBOL_GPL(omap_control_lock_writel);
 
 /**
  * omap_control_get: returns the control module device pinter
@@ -136,6 +149,9 @@ static int __devinit omap_control_probe(
 	struct device_node *np = dev->of_node;
 	struct omap_control *omap_control;
 
+	printk("\n\t\t **** omap_control_probe(): enter ");
+	dump_stack();
+
 	omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
 	if (!omap_control) {
 		dev_err(dev, "not enough memory for omap_control\n");
Index: omap-thermal/drivers/thermal/omap-bandgap.c
===================================================================
--- omap-thermal.orig/drivers/thermal/omap-bandgap.c
+++ omap-thermal/drivers/thermal/omap-bandgap.c
@@ -154,6 +154,7 @@ struct temp_sensor_registers {
 	u32	status_cold_mask;
 
 	u32	bgap_efuse;
+	spinlock_t	bg_reg_lock;
Indeed. I agree we should have this per sensor. Need to revisit the existing mutex as well though.
quoted hunk ↗ jump to hunk
 };
 
 /**
@@ -579,6 +580,17 @@ omap5430_adc_to_temp[OMAP5430_ADC_END_VA
 	124600, 124900, 125000, 125000, 125000, 125000,
 };
 
+static int bg_writel(struct device *dev, u32 val, u32 reg, spinlock_t *lock)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(lock, flags);
+	ret = omap_control_writel(dev, val, reg);
+	spin_unlock_irqrestore(lock, flags);
In fact this doesn't help much in the bulk writes case. As you have already mentioned,
ideally we should lock, perform the operation, then unlock, instead of several
lock/write/unlock sequences.
quoted hunk ↗ jump to hunk
+	return ret;
+}
+
 static irqreturn_t talert_irq_handler(int irq, void *data)
 {
 	struct omap_bandgap *bg_ptr = data;
@@ -615,7 +627,7 @@ static irqreturn_t talert_irq_handler(in
 			ctrl |= tsr->mask_hot_mask;
 		}
 
-		r |= omap_control_writel(cdev, ctrl, tsr->bgap_mask_ctrl);
+		r |= bg_writel(cdev, ctrl, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
 
 		if (r) {
 			dev_err(bg_ptr->dev, "failed to ack talert interrupt\n");
@@ -705,7 +717,7 @@ static int temp_sensor_unmask_interrupts
 		reg_val |= tsr->mask_cold_mask;
 	else
 		reg_val &= ~tsr->mask_cold_mask;
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
 
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to unmask interrupts\n");
@@ -751,14 +763,14 @@ int temp_sensor_configure_thot(struct om
 		/* write the new t_cold value */
 		reg_val = thresh_val & (~tsr->threshold_tcold_mask);
 		reg_val |= cold << __ffs(tsr->threshold_tcold_mask);
-		err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+		err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 		thresh_val = reg_val;
 	}
 
 	/* write the new t_hot value */
 	reg_val = thresh_val & ~tsr->threshold_thot_mask;
 	reg_val |= (t_hot << __ffs(tsr->threshold_thot_mask));
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
 		return -EIO;
@@ -782,7 +794,7 @@ int temp_sensor_init_talert_thresholds(s
 	/* write the new t_cold value */
 	reg_val = thresh_val & ~tsr->threshold_tcold_mask;
 	reg_val |= (t_cold << __ffs(tsr->threshold_tcold_mask));
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
 		return -EIO;
@@ -793,7 +805,7 @@ int temp_sensor_init_talert_thresholds(s
 	/* write the new t_hot value */
 	reg_val = thresh_val & ~tsr->threshold_thot_mask;
 	reg_val |= (t_hot << __ffs(tsr->threshold_thot_mask));
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
 		return -EIO;
@@ -802,7 +814,7 @@ int temp_sensor_init_talert_thresholds(s
 	err = omap_control_readl(cdev, tsr->bgap_mask_ctrl, &reg_val);
 	reg_val |= tsr->mask_hot_mask;
 	reg_val |= tsr->mask_cold_mask;
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
 		return -EIO;
@@ -833,14 +845,14 @@ int temp_sensor_configure_tcold(struct o
 		/* write the new t_hot value */
 		reg_val = thresh_val & (~tsr->threshold_thot_mask);
 		reg_val |= hot << __ffs(tsr->threshold_thot_mask);
-		err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+		err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 		thresh_val = reg_val;
 	}
 
 	/* write the new t_cold value */
 	reg_val = thresh_val & ~tsr->threshold_tcold_mask;
 	reg_val |= (t_cold << __ffs(tsr->threshold_tcold_mask));
-	err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
 		return -EIO;
@@ -861,7 +873,7 @@ static int temp_sensor_configure_tshut_h
 	err = omap_control_readl(cdev, tsr->tshut_threshold, &reg_val);
 	reg_val &= ~tsr->tshut_hot_mask;
 	reg_val |= tshut_hot << __ffs(tsr->tshut_hot_mask);
-	err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->tshut_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram tshut thot\n");
 		return -EIO;
@@ -882,7 +894,7 @@ static int temp_sensor_configure_tshut_c
 	err = omap_control_readl(cdev, tsr->tshut_threshold, &reg_val);
 	reg_val &= ~tsr->tshut_cold_mask;
 	reg_val |= tshut_cold << __ffs(tsr->tshut_cold_mask);
-	err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
+	err |= bg_writel(cdev, reg_val, tsr->tshut_threshold, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
 		return -EIO;
@@ -903,7 +915,7 @@ static int configure_temp_sensor_counter
 	err = omap_control_readl(cdev, tsr->bgap_counter, &val);
 	val &= ~tsr->counter_mask;
 	val |= counter << __ffs(tsr->counter_mask);
-	err |= omap_control_writel(cdev, val, tsr->bgap_counter);
+	err |= bg_writel(cdev, val, tsr->bgap_counter, &tsr->bg_reg_lock);
 	if (err) {
 		dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
 		return -EIO;
@@ -1124,7 +1136,7 @@ static int enable_continuous_mode(struct
 		tsr = bg_ptr->pdata->sensors[i].registers;
 		r = omap_control_readl(cdev, tsr->bgap_mode_ctrl, &val);
 		val |= 1 << __ffs(tsr->mode_ctrl_mask);
-		r |= omap_control_writel(cdev, val, tsr->bgap_mode_ctrl);
+		r |= bg_writel(cdev, val, tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
 		if (r)
 			dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
 	}
@@ -1342,6 +1354,9 @@ int __devinit omap_bandgap_probe(struct
 		u32 val;
 
 		tsr = bg_ptr->pdata->sensors[i].registers;
+		/* Initialize register lock */
+		spin_lock_init(&tsr->bg_reg_lock);
+
 		/*
 		 * check if the efuse has a non-zero value if not
 		 * it is an untrimmed sample and the temperatures
@@ -1482,12 +1497,12 @@ omap_bandgap_force_single_read(struct om
 	/* Select single conversion mode */
 	err = omap_control_readl(cdev, tsr->bgap_mode_ctrl, &temp);
 	temp &= ~(1 << __ffs(tsr->mode_ctrl_mask));
-	omap_control_writel(cdev, temp, tsr->bgap_mode_ctrl);
+	bg_writel(cdev, temp, tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
 
 	/* Start of Conversion = 1 */
 	err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
 	temp |= 1 << __ffs(tsr->bgap_soc_mask);
-	omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
+	bg_writel(cdev, temp, tsr->temp_sensor_ctrl, &tsr->bg_reg_lock);
 	/* Wait until DTEMP is updated */
 	err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
 	temp &= (tsr->bgap_dtemp_mask);
@@ -1498,7 +1513,7 @@ omap_bandgap_force_single_read(struct om
 	/* Start of Conversion = 0 */
 	err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
 	temp &= ~(1 << __ffs(tsr->bgap_soc_mask));
-	err |= omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
+	err |= bg_writel(cdev, temp, tsr->temp_sensor_ctrl, &tsr->bg_reg_lock);
 
 	return err ? -EIO : 0;
 }
@@ -1519,20 +1534,20 @@ static int omap_bandgap_restore_ctxt(str
 
 		err = omap_control_readl(cdev, tsr->bgap_counter, &val);
 		if (val == 0) {
-			err |= omap_control_writel(cdev, rval->bg_threshold,
-						   tsr->bgap_threshold);
-			err |= omap_control_writel(cdev, rval->tshut_threshold,
-						   tsr->tshut_threshold);
+			err |= bg_writel(cdev, rval->bg_threshold,
+						   tsr->bgap_threshold, &tsr->bg_reg_lock);
+			err |= bg_writel(cdev, rval->tshut_threshold,
+						   tsr->tshut_threshold, &tsr->bg_reg_lock);
 			/* Force immediate temperature measurement and update
 			 * of the DTEMP field
 			 */
 			omap_bandgap_force_single_read(bg_ptr, i);
-			err |= omap_control_writel(cdev, rval->bg_counter,
-						   tsr->bgap_counter);
-			err |= omap_control_writel(cdev, rval->bg_mode_ctrl,
-						   tsr->bgap_mode_ctrl);
-			err |= omap_control_writel(cdev, rval->bg_ctrl,
-						   tsr->bgap_mask_ctrl);
+			err |= bg_writel(cdev, rval->bg_counter,
+						   tsr->bgap_counter, &tsr->bg_reg_lock);
+			err |= bg_writel(cdev, rval->bg_mode_ctrl,
+						   tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
+			err |= bg_writel(cdev, rval->bg_ctrl,
+						   tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
 		} else {
 			err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,
 						 &temp);
@@ -1543,8 +1558,8 @@ static int omap_bandgap_restore_ctxt(str
 							  tsr->bgap_mask_ctrl,
 							  &temp);
 				temp |= 1 << __ffs(tsr->mode_ctrl_mask);
-				err |= omap_control_writel(cdev, temp,
-							   tsr->bgap_mask_ctrl);
+				err |= bg_writel(cdev, temp,
+							   tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
 			}
 		}
 		if (err)
Index: omap-thermal/drivers/usb/otg/omap4-usb-phy.c
===================================================================
--- omap-thermal.orig/drivers/usb/otg/omap4-usb-phy.c
+++ omap-thermal/drivers/usb/otg/omap4-usb-phy.c
@@ -46,13 +46,13 @@ int omap4_usb_phy_power(struct device *d
 	if (on) {
 		ret = omap_control_readl(dev, CONTROL_DEV_CONF, &val);
 		if (!ret && (val & PHY_PD)) {
-			ret = omap_control_writel(dev, ~PHY_PD,
+			ret = omap_control_lock_writel(dev, ~PHY_PD,
 						  CONTROL_DEV_CONF);
 			/* XXX: add proper documentation for this delay */
 			mdelay(200);
 		}
 	} else {
-		ret = omap_control_writel(dev, PHY_PD, CONTROL_DEV_CONF);
+		ret = omap_control_lock_writel(dev, PHY_PD, CONTROL_DEV_CONF);
 	}
 
 	return ret;
@@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(omap4_usb_phy_power);
  */
 int omap4_usb_phy_mailbox(struct device *dev, u32 val)
 {
-	return omap_control_writel(dev, val, CONTROL_USBOTGHS_CONTROL);
+	return omap_control_lock_writel(dev, val, CONTROL_USBOTGHS_CONTROL);
 }
 EXPORT_SYMBOL_GPL(omap4_usb_phy_mailbox);
 
Index: omap-thermal/include/linux/mfd/omap_control.h
===================================================================
--- omap-thermal.orig/include/linux/mfd/omap_control.h
+++ omap-thermal/include/linux/mfd/omap_control.h
@@ -43,6 +43,7 @@ struct omap_control {
 #ifdef CONFIG_MFD_OMAP_CONTROL
 extern int omap_control_readl(struct device *dev, u32 reg, u32 *val);
 extern int omap_control_writel(struct device *dev, u32 val, u32 reg);
+extern int omap_control_lock_writel(struct device *dev, u32 val, u32 reg);
 extern struct device *omap_control_get(void);
 extern void omap_control_put(struct device *dev);
 #else
@@ -55,6 +56,11 @@ static inline int omap_control_writel(st
 {
 	return 0;
 }
+
+static inline int omap_control_lock_writel(struct device *dev, u32 val, u32 reg)
+{
+	return 0;
+}
 
 static inline struct device *omap_control_get(void)
 {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help