Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 1, 2011 at 5:26 AM, Paul Walmsley p...@pwsan.com wrote: Hi, On Wed, 31 Aug 2011, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. I commented in a separate post that this driver should probably use an MFD driver for the System Control Module accesses, and then this hwmon driver should use functions from that to access the BANDGAP temperature sensor registers[1]. But I had another comment on this driver. A similar sensor is available on the OMAP34xx[2], OMAP36xx[3], and OMAP4430[4] chips. There are some register layout differences; the thermal shutdown threshold is configurable on the 4460 but fixed on the 4430; and also I'd assume, without looking, that the temperature mapping table is different on different chips. So it would seem to make sense to move the chip-specific code and data into chip-specific source files. I could see keeping a generic filename like omap_temp_sensor.c if you implemented a common interface to the bandgap sensors, ADCs and comparators, across all those different chips. That might be worth thinking about. But at least, this should probably be named drivers/hwmon/omap4460_temp_sensor.c, or something similar. The same driver can be used for OMAP5 too. That has 3 instances of the same sensor. So omap4460+? - Paul 1. Walmsley, Paul. _Re: [PATCH 4/6 V4] OMAP4: Hwmod: OMAP temperature sensor_. Posted to the linux-omap@vger.kernel.org list on Wed, 31 Aug 2011 17:16:44 -0600. Available from (among others): http://marc.info/?l=linux-omapm=131483260632685w=2 2. Section 7.4.6 Band Gap Voltage and Temperature Sensor. _OMAP34xx Multimedia Device Silicon Revision 3.1.x Version R (SWPU223R)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP34xx_ES3.1.x_PUBLIC_TRM_vZR.zip 3. Section 13.4.6 Band Gap Voltage and Temperature Sensor. _OMAP36xx Multimedia Device Silicon Revision 1.x Version V (SWPU177V)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP36xx_ES1.x_PUBLIC_TRM_vV.zip 4. Section 18.4.10 Band Gap Voltage and Temperature Sensor. _OMAP4430 Multimedia Device Silicon Revision 2.x Version V (SWPU231V)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP4430_ES2.x_PUBLIC_TRM_vV.zip -- Regards and Thanks, Keerthy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 1, 2011 at 6:06 AM, Paul Walmsley p...@pwsan.com wrote:
Hi
Some comments.
On Wed, 31 Aug 2011, Keerthy wrote:
diff --git a/drivers/hwmon/omap_temp_sensor.c
b/drivers/hwmon/omap_temp_sensor.c
new file mode 100644
index 000..67fa424
--- /dev/null
+++ b/drivers/hwmon/omap_temp_sensor.c
@@ -0,0 +1,881 @@
You've done almost all the hard work to create kerneldoc-NANO compliant
structure documentation, which is good. But a few important things are
missing. Please review Documentation/kernel-doc-nano-HOWTO.txt.
Ok
+/*
Should be /** to indicate kerneldoc.
Ok
+ * omap_temp_sensor structure
Should be struct omap_temp_sensor and should include a short
description.
Ok
+ * @hwmon_dev - hwmon device pointer
+ * @pdev_dev - platform device pointer
+ * @clock - Clock pointer
+ * @registers - Pointer to structure with register offsets and bitfields
+ * @sensor_mutex - Mutex for sysfs, irq and PM
+ * @irq - MPU Irq number for thermal alert
+ * @phy_base - Physical base of the temp I/O
+ * @clk_rate - Holds current clock rate
+ * @temp_sensor_ctrl - temp sensor control register value
+ * @bg_ctrl - bandgap ctrl register value
+ * @bg_counter - bandgap counter value
+ * @bg_threshold - bandgap threshold register value
+ * @temp_sensor_tshut_threshold - bandgap tshut register value
+ * @clk_on - Manages the current clock state
+ */
+struct omap_temp_sensor {
+ struct device *hwmon_dev;
+ struct device *pdev_dev;
+ struct clk *clock;
+ struct omap_temp_sensor_registers *registers;
+ struct mutex sensor_mutex; /* Mutex for sysfs, irq and PM */
+ unsigned int irq;
+ void __iomem *phy_base;
+ u32 clk_rate;
+ u32 temp_sensor_ctrl;
+ u32 bg_ctrl;
+ u32 bg_counter;
+ u32 bg_threshold;
+ u32 temp_sensor_tshut_threshold;
+ bool clk_on;
+};
+
+/*
+ * Temperature values in milli degree celsius
+ * ADC code values from 530 to 923
+ */
+static int adc_to_temp[394] = {
+ -4, -4, -4, -4, -39800, -39400, -39000, -38600, -38200,
+ -37800, -37300, -36800, -36400, -36000, -35600, -35200, -34800,
+ -34300, -33800, -33400, -33000, -32600, -32200, -31800, -31300,
+ -30800, -30400, -3, -29600, -29200, -28700, -28200, -27800,
+ -27400, -27000, -26600, -26200, -25700, -25200, -24800, -24400,
+ -24000, -23600, -23200, -22700, -22200, -21800, -21400, -21000,
+ -20600, -20200, -19700, -19200, -18800, -18400, -18000, -17600,
+ -17200, -16700, -16200, -15800, -15400, -15000, -14600, -14200,
+ -13700, -13200, -12800, -12400, -12000, -11600, -11200, -10700,
+ -10200, -9800, -9400, -9000, -8600, -8200, -7700, -7200, -6800,
+ -6400, -6000, -5600, -5200, -4800, -4300, -3800, -3400, -3000,
+ -2600, -2200, -1800, -1300, -800, -400, 0, 400, 800, 1200, 1600,
+ 2100, 2600, 3000, 3400, 3800, 4200, 4600, 5100, 5600, 6000, 6400,
+ 6800, 7200, 7600, 8000, 8500, 9000, 9400, 9800, 10200, 10600, 11000,
+ 11400, 11900, 12400, 12800, 13200, 13600, 14000, 14400, 14800,
+ 15300, 15800, 16200, 16600, 17000, 17400, 17800, 18200, 18700,
+ 19200, 19600, 2, 20400, 20800, 21200, 21600, 22100, 22600,
+ 23000, 23400, 23800, 24200, 24600, 25000, 25400, 25900, 26400,
+ 26800, 27200, 27600, 28000, 28400, 28800, 29300, 29800, 30200,
+ 30600, 31000, 31400, 31800, 32200, 32600, 33100, 33600, 34000,
+ 34400, 34800, 35200, 35600, 36000, 36400, 36800, 37300, 37800,
+ 38200, 38600, 39000, 39400, 39800, 40200, 40600, 41100, 41600,
+ 42000, 42400, 42800, 43200, 43600, 44000, 44400, 44800, 45300,
+ 45800, 46200, 46600, 47000, 47400, 47800, 48200, 48600, 49000,
+ 49500, 5, 50400, 50800, 51200, 51600, 52000, 52400, 52800,
+ 53200, 53700, 54200, 54600, 55000, 55400, 55800, 56200, 56600,
+ 57000, 57400, 57800, 58200, 58700, 59200, 59600, 6, 60400,
+ 60800, 61200, 61600, 62000, 62400, 62800, 63300, 63800, 64200,
+ 64600, 65000, 65400, 65800, 66200, 66600, 67000, 67400, 67800,
+ 68200, 68700, 69200, 69600, 7, 70400, 70800, 71200, 71600,
+ 72000, 72400, 72800, 73200, 73600, 74100, 74600, 75000, 75400,
+ 75800, 76200, 76600, 77000, 77400, 77800, 78200, 78600, 79000,
+ 79400, 79800, 80300, 80800, 81200, 81600, 82000, 82400, 82800,
+ 83200, 83600, 84000, 84400, 84800, 85200, 85600, 86000, 86400,
+ 86800, 87300, 87800, 88200, 88600, 89000, 89400, 89800, 90200,
+ 90600, 91000, 91400, 91800, 92200, 92600, 93000, 93400, 93800,
+ 94200, 94600, 95000, 95500, 96000, 96400, 96800, 97200, 97600,
+ 98000, 98400, 98800, 99200, 99600, 10, 100400, 100800, 101200,
+ 101600, 102000, 102400, 102800, 103200, 103600, 104000, 104400,
+ 104800,
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 1, 2011 at 9:39 AM, Paul Walmsley p...@pwsan.com wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. The TSHUT thresholds are not even exposed through the sysfs nodes. This driver only creates sysfs nodes for TALERT thresholds. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? What part of code should be moved? This driver does just the basic hardware monitoring and exposes configurable thresholds for t_hot and t_cold. This does not include t_shut configuration and handling. This is a simple hardware monitoring driver which does not cater to Thermal management. That is being discussed separately in other thread. - Paul -- Regards and Thanks, Keerthy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 1, 2011 at 10:10 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? This driver is not taking any action on THSUT. This is not doing the thermal management. It is a driver exposing configurable temperature thresholds. Guenter -- Regards and Thanks, Keerthy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Tue, 2011-09-06 at 14:02 -0400, J, KEERTHY wrote: On Thu, Sep 1, 2011 at 10:10 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? This driver is not taking any action on THSUT. This is not doing the thermal management. It is a driver exposing configurable temperature thresholds. What sense would it make, then, to keep the driver around even if SYSFS is not defined ? Note that I am not looking at the code right now, but at the suggestion that the driver would do something useful if SYSFS is not defined. Question is what that is, and if that part of it should reside in a hwmon driver. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Tue, Sep 6, 2011 at 11:42 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Tue, 2011-09-06 at 14:02 -0400, J, KEERTHY wrote: On Thu, Sep 1, 2011 at 10:10 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? This driver is not taking any action on THSUT. This is not doing the thermal management. It is a driver exposing configurable temperature thresholds. What sense would it make, then, to keep the driver around even if SYSFS is not defined ? SYSFS nodes are defined for t_hot and t_cold thresholds. SYSFS nodes are not defined for TSHUT thresholds which are different. Note that I am not looking at the code right now, but at the suggestion that the driver would do something useful if SYSFS is not defined. Question is what that is, and if that part of it should reside in a hwmon driver. Thanks, Guenter -- Regards and Thanks, Keerthy -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
Hi, On Wed, 31 Aug 2011, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. I commented in a separate post that this driver should probably use an MFD driver for the System Control Module accesses, and then this hwmon driver should use functions from that to access the BANDGAP temperature sensor registers[1]. But I had another comment on this driver. A similar sensor is available on the OMAP34xx[2], OMAP36xx[3], and OMAP4430[4] chips. There are some register layout differences; the thermal shutdown threshold is configurable on the 4460 but fixed on the 4430; and also I'd assume, without looking, that the temperature mapping table is different on different chips. So it would seem to make sense to move the chip-specific code and data into chip-specific source files. I could see keeping a generic filename like omap_temp_sensor.c if you implemented a common interface to the bandgap sensors, ADCs and comparators, across all those different chips. That might be worth thinking about. But at least, this should probably be named drivers/hwmon/omap4460_temp_sensor.c, or something similar. - Paul 1. Walmsley, Paul. _Re: [PATCH 4/6 V4] OMAP4: Hwmod: OMAP temperature sensor_. Posted to the linux-omap@vger.kernel.org list on Wed, 31 Aug 2011 17:16:44 -0600. Available from (among others): http://marc.info/?l=linux-omapm=131483260632685w=2 2. Section 7.4.6 Band Gap Voltage and Temperature Sensor. _OMAP34xx Multimedia Device Silicon Revision 3.1.x Version R (SWPU223R)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP34xx_ES3.1.x_PUBLIC_TRM_vZR.zip 3. Section 13.4.6 Band Gap Voltage and Temperature Sensor. _OMAP36xx Multimedia Device Silicon Revision 1.x Version V (SWPU177V)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP36xx_ES1.x_PUBLIC_TRM_vV.zip 4. Section 18.4.10 Band Gap Voltage and Temperature Sensor. _OMAP4430 Multimedia Device Silicon Revision 2.x Version V (SWPU231V)_ (public version). Available from http://focus.ti.com/pdfs/wtbu/OMAP4430_ES2.x_PUBLIC_TRM_vV.zip -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
Hi
Some comments.
On Wed, 31 Aug 2011, Keerthy wrote:
diff --git a/drivers/hwmon/omap_temp_sensor.c
b/drivers/hwmon/omap_temp_sensor.c
new file mode 100644
index 000..67fa424
--- /dev/null
+++ b/drivers/hwmon/omap_temp_sensor.c
@@ -0,0 +1,881 @@
You've done almost all the hard work to create kerneldoc-NANO compliant
structure documentation, which is good. But a few important things are
missing. Please review Documentation/kernel-doc-nano-HOWTO.txt.
+/*
Should be /** to indicate kerneldoc.
+ * omap_temp_sensor structure
Should be struct omap_temp_sensor and should include a short
description.
+ * @hwmon_dev - hwmon device pointer
+ * @pdev_dev - platform device pointer
+ * @clock - Clock pointer
+ * @registers - Pointer to structure with register offsets and bitfields
+ * @sensor_mutex - Mutex for sysfs, irq and PM
+ * @irq - MPU Irq number for thermal alert
+ * @phy_base - Physical base of the temp I/O
+ * @clk_rate - Holds current clock rate
+ * @temp_sensor_ctrl - temp sensor control register value
+ * @bg_ctrl - bandgap ctrl register value
+ * @bg_counter - bandgap counter value
+ * @bg_threshold - bandgap threshold register value
+ * @temp_sensor_tshut_threshold - bandgap tshut register value
+ * @clk_on - Manages the current clock state
+ */
+struct omap_temp_sensor {
+ struct device *hwmon_dev;
+ struct device *pdev_dev;
+ struct clk *clock;
+ struct omap_temp_sensor_registers *registers;
+ struct mutexsensor_mutex; /* Mutex for sysfs, irq and PM */
+ unsigned intirq;
+ void __iomem*phy_base;
+ u32 clk_rate;
+ u32 temp_sensor_ctrl;
+ u32 bg_ctrl;
+ u32 bg_counter;
+ u32 bg_threshold;
+ u32 temp_sensor_tshut_threshold;
+ boolclk_on;
+};
+
+/*
+ * Temperature values in milli degree celsius
+ * ADC code values from 530 to 923
+ */
+static int adc_to_temp[394] = {
+ -4, -4, -4, -4, -39800, -39400, -39000, -38600, -38200,
+ -37800, -37300, -36800, -36400, -36000, -35600, -35200, -34800,
+ -34300, -33800, -33400, -33000, -32600, -32200, -31800, -31300,
+ -30800, -30400, -3, -29600, -29200, -28700, -28200, -27800,
+ -27400, -27000, -26600, -26200, -25700, -25200, -24800, -24400,
+ -24000, -23600, -23200, -22700, -22200, -21800, -21400, -21000,
+ -20600, -20200, -19700, -19200, -18800, -18400, -18000, -17600,
+ -17200, -16700, -16200, -15800, -15400, -15000, -14600, -14200,
+ -13700, -13200, -12800, -12400, -12000, -11600, -11200, -10700,
+ -10200, -9800, -9400, -9000, -8600, -8200, -7700, -7200, -6800,
+ -6400, -6000, -5600, -5200, -4800, -4300, -3800, -3400, -3000,
+ -2600, -2200, -1800, -1300, -800, -400, 0, 400, 800, 1200, 1600,
+ 2100, 2600, 3000, 3400, 3800, 4200, 4600, 5100, 5600, 6000, 6400,
+ 6800, 7200, 7600, 8000, 8500, 9000, 9400, 9800, 10200, 10600, 11000,
+ 11400, 11900, 12400, 12800, 13200, 13600, 14000, 14400, 14800,
+ 15300, 15800, 16200, 16600, 17000, 17400, 17800, 18200, 18700,
+ 19200, 19600, 2, 20400, 20800, 21200, 21600, 22100, 22600,
+ 23000, 23400, 23800, 24200, 24600, 25000, 25400, 25900, 26400,
+ 26800, 27200, 27600, 28000, 28400, 28800, 29300, 29800, 30200,
+ 30600, 31000, 31400, 31800, 32200, 32600, 33100, 33600, 34000,
+ 34400, 34800, 35200, 35600, 36000, 36400, 36800, 37300, 37800,
+ 38200, 38600, 39000, 39400, 39800, 40200, 40600, 41100, 41600,
+ 42000, 42400, 42800, 43200, 43600, 44000, 44400, 44800, 45300,
+ 45800, 46200, 46600, 47000, 47400, 47800, 48200, 48600, 49000,
+ 49500, 5, 50400, 50800, 51200, 51600, 52000, 52400, 52800,
+ 53200, 53700, 54200, 54600, 55000, 55400, 55800, 56200, 56600,
+ 57000, 57400, 57800, 58200, 58700, 59200, 59600, 6, 60400,
+ 60800, 61200, 61600, 62000, 62400, 62800, 63300, 63800, 64200,
+ 64600, 65000, 65400, 65800, 66200, 66600, 67000, 67400, 67800,
+ 68200, 68700, 69200, 69600, 7, 70400, 70800, 71200, 71600,
+ 72000, 72400, 72800, 73200, 73600, 74100, 74600, 75000, 75400,
+ 75800, 76200, 76600, 77000, 77400, 77800, 78200, 78600, 79000,
+ 79400, 79800, 80300, 80800, 81200, 81600, 82000, 82400, 82800,
+ 83200, 83600, 84000, 84400, 84800, 85200, 85600, 86000, 86400,
+ 86800, 87300, 87800, 88200, 88600, 89000, 89400, 89800, 90200,
+ 90600, 91000, 91400, 91800, 92200, 92600, 93000, 93400, 93800,
+ 94200, 94600, 95000, 95500, 96000, 96400, 96800, 97200, 97600,
+ 98000, 98400, 98800, 99200, 99600, 10, 100400, 100800, 101200,
+ 101600, 102000, 102400, 102800, 103200, 103600, 104000, 104400,
+ 104800, 105200, 105600, 106100, 106600, 107000, 107400, 107800,
+ 108200, 108600, 109000,
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 31, 2011 at 01:25:10PM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org --- Documentation/hwmon/omap_temp_sensor | 26 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 881 ++ 4 files changed, 919 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c diff --git a/Documentation/hwmon/omap_temp_sensor b/Documentation/hwmon/omap_temp_sensor new file mode 100644 index 000..357f09a --- /dev/null +++ b/Documentation/hwmon/omap_temp_sensor @@ -0,0 +1,26 @@ +Kernel driver omap_temp_sensor +== + +Supported chips: + * Texas Instruments OMAP4460 +Prefix: 'omap_temp_sensor' + +Author: +J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments OMAP4 family of chips have a bandgap temperature sensor. +The temperature sensor feature is used to convert the temperature of the device +into a decimal value coded on 10 bits. An internal ADC is used for conversion. +The recommended operating temperatures must be in the range -40 degree Celsius +to 123 degree celsius for standard conversion. +The thresholds are programmable and upon crossing the thresholds an interrupt +is generated. The OMAP temperature sensor has a programmable update rate in +milli seconds. +(Currently the driver programs a default of 2000 milliseconds). + +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). + diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5f888f7..9c9cd8b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -323,6 +323,17 @@ config SENSORS_F71805F This driver can also be built as a module. If so, the module will be called f71805f. +config SENSORS_OMAP_BANDGAP_TEMP_SENSOR + bool OMAP on-die temperature sensor hwmon driver + depends on HWMON ARCH_OMAP OMAP_TEMP_SENSOR + help + If you say yes here you get support for hardware + monitoring features of the OMAP on die temperature + sensor. + + Continuous conversion programmable delay + mode is used for temperature conversion. + config SENSORS_F71882FG tristate Fintek F71882FG and compatibles help diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 28061cf..d0f89f5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o obj-$(CONFIG_SENSORS_MAX6642) += max6642.o obj-$(CONFIG_SENSORS_MAX6650) += max6650.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_OMAP_BANDGAP_TEMP_SENSOR) += omap_temp_sensor.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/omap_temp_sensor.c b/drivers/hwmon/omap_temp_sensor.c new file mode 100644 index 000..67fa424 --- /dev/null +++ b/drivers/hwmon/omap_temp_sensor.c @@ -0,0 +1,881 @@ +/* + * OMAP4 Temperature sensor driver file + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * Author: J Keerthy j-keer...@ti.com + * Author: Moiz Sonasath m-sonas...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/interrupt.h +#include linux/clk.h +#include linux/io.h +#include linux/slab.h +#include linux/init.h +#include plat/omap_device.h +#include linux/kernel.h +#include linux/device.h +#include linux/jiffies.h +#include linux/hwmon.h +#include linux/hwmon-sysfs.h +#include linux/stddef.h +#include linux/sysfs.h
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
