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: > > 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 <[email protected]> > > Signed-off-by: Keerthy <[email protected]> > > --- > > .../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... > > Signed-off-by: Konstantin Baydarov <[email protected]> > > 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. > }; > > /** > @@ -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. > + 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, ®_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, ®_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, ®_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) > { > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
