Re: [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer

2012-01-03 Thread Guenter Roeck
On Wed, 2011-12-21 at 06:59 -0500, Amit Daniel Kachhap wrote:
> Export and register information from the hwmon tmu sensor to the samsung
> exynos kernel thermal framework where different cooling devices and thermal
> zone are binded. The exported information is based according to the data
> structure thermal_sensor_conf present in exynos_thermal.h. HWMON sysfs
> functions are currently left although all of them are present in generic
> linux thermal layer.
> Also the platform data structure is modified to pass frequency cooling
> in percentages for each thermal level.
> 
Hi Amit,

wouldn't it make more sense to merge the code as necessary from
hwmon/exynos4_tmu.c into the new thermal/exynos_thermal.c, and drop
hwmon/exynos4_tmu.c entirely ?

With that, you should get the hwmon entries for free, and we would not
have to maintain two drivers with overlapping functionality.

Thanks,
Guenter

> Signed-off-by: Amit Daniel Kachhap 
> ---
>  drivers/hwmon/exynos4_tmu.c   |   34 ++--
>  include/linux/platform_data/exynos4_tmu.h |7 ++
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/exynos4_tmu.c b/drivers/hwmon/exynos4_tmu.c
> index f2359a0..6912a7f 100644
> --- a/drivers/hwmon/exynos4_tmu.c
> +++ b/drivers/hwmon/exynos4_tmu.c
> @@ -37,6 +37,9 @@
>  #include 
>  
>  #include 
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> +#include 
> +#endif
>  
>  #define EXYNOS4_TMU_REG_TRIMINFO 0x0
>  #define EXYNOS4_TMU_REG_CONTROL  0x20
> @@ -248,10 +251,13 @@ static void exynos4_tmu_work(struct work_struct *work)
>  
>   kobject_uevent(&data->hwmon_dev->kobj, KOBJ_CHANGE);
>  
> - enable_irq(data->irq);
>  
>   clk_disable(data->clk);
>   mutex_unlock(&data->lock);
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> + exynos4_report_trigger();
> +#endif
> + enable_irq(data->irq);
>  }
>  
>  static irqreturn_t exynos4_tmu_irq(int irq, void *id)
> @@ -345,6 +351,14 @@ static const struct attribute_group 
> exynos4_tmu_attr_group = {
>   .attrs = exynos4_tmu_attributes,
>  };
>  
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> +static struct thermal_sensor_conf exynos4_sensor_conf = {
> + .name   = "exynos4-therm",
> + .read_temperature   = (int (*)(void *))exynos4_tmu_read,
> +};
> +#endif
> +/*CONFIG_SAMSUNG_THERMAL_INTERFACE*/
> +
>  static int __devinit exynos4_tmu_probe(struct platform_device *pdev)
>  {
>   struct exynos4_tmu_data *data;
> @@ -432,9 +446,20 @@ static int __devinit exynos4_tmu_probe(struct 
> platform_device *pdev)
>   }
>  
>   exynos4_tmu_control(pdev, true);
> -
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> + (&exynos4_sensor_conf)->private_data = data;
> + (&exynos4_sensor_conf)->sensor_data = pdata;
> + ret = exynos4_register_thermal(&exynos4_sensor_conf);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register thermal interface\n");
> + goto err_hwmon_device;
> + }
> +#endif
>   return 0;
> -
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> +err_hwmon_device:
> + hwmon_device_unregister(data->hwmon_dev);
> +#endif
>  err_create_group:
>   sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
>  err_clk:
> @@ -458,6 +483,9 @@ static int __devexit exynos4_tmu_remove(struct 
> platform_device *pdev)
>  
>   exynos4_tmu_control(pdev, false);
>  
> +#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
> + exynos4_unregister_thermal();
> +#endif
>   hwmon_device_unregister(data->hwmon_dev);
>   sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
>  
> diff --git a/include/linux/platform_data/exynos4_tmu.h 
> b/include/linux/platform_data/exynos4_tmu.h
> index 39e038c..642c508 100644
> --- a/include/linux/platform_data/exynos4_tmu.h
> +++ b/include/linux/platform_data/exynos4_tmu.h
> @@ -21,6 +21,7 @@
>  
>  #ifndef _LINUX_EXYNOS4_TMU_H
>  #define _LINUX_EXYNOS4_TMU_H
> +#include 
>  
>  enum calibration_type {
>   TYPE_ONE_POINT_TRIMMING,
> @@ -64,6 +65,9 @@ enum calibration_type {
>   *   in the positive-TC generator block
>   *   0 <= reference_voltage <= 31
>   * @cal_type: calibration type for temperature
> + * @freq_pctg_table: Table representing frequency reduction percentage.
> + * @freq_tab_count: Count of the above table as frequency reduction may
> + *   applicable to only some of the trigger levels.
>   *
>   * This structure is required for configuration of exynos4_tmu driver.
>   */
> @@ -79,5 +83,8 @@ struct exynos4_tmu_platform_data {
>   u8 reference_voltage;
>  
>   enum calibration_type cal_type;
> +
> + struct freq_pctg_table freq_tab[4];
> + unsigned int freq_tab_count;
>  };
>  #endif /* _LINUX_EXYNOS4_TMU_H */



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


RE: [linux-pm] [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer

2012-01-04 Thread Guenter Roeck
On Wed, 2012-01-04 at 05:23 -0500, R, Durgadoss wrote:
> Hi Amit Daniel,
> 
> > Hi Guenter,
> > 
> > The main idea of this work is to leave the current userspace based
> > notification scheme and add the kernel based cooling scheme on top of
> > it. Anyway, It is a good idea to move the file hwmon/exynos4_tmu.c as
> 
> But, What I feel is, kernel based cooling scheme will work only for
> Controlling 'CPU' frequency. But in SoC's there are other devices that
> Contribute to Thermal. For example, GPU, Display, Battery (during charging)
> etc.. In this case, we need a user space to control these devices. So, in a
> way, the user space notification mechanism is a unified solution for
> throttling all devices and keeps the kernel code light weight.
> 
> I am also curious to know why the existing mechanism did not work for you ?
> 
That is one question. 

For me, the main concern is that the proposed implementation creates
duplicate hwmon entries for the same device, which is simply messy. If
both the kernel thermal mechanism and the userspace mechanism are
needed, I think it would make more sense to use a thermal driver and
have that thermal driver generate the necessary userspace events.

Thanks,
Guenter

> Thanks,
> Durga
> 
> > this creates 2 hwmon entries.
> > Adding CC: Donggeun Kim to know his opinion.
> > 
> > Thanks,
> > Amit Daniel
> [snip.]



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 02/07] HWMON: DA9052/53 hwmon driver v1

2012-02-06 Thread Guenter Roeck
On Mon, 2012-02-06 at 07:39 -0500, Ashish Jangam wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch montiors the DA9052 PMIC's ADC channels mostly for battery
> parameters like battery temperature, junction temperature, battery
> current etc.
> 
> This patch is functionally tested on Samsung SMDKV6410.
> 
Hi Ashish,

Looks pretty good. Couple of comments below.

> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 
> ---
>  Documentation/hwmon/da9052   |   59 +++
>  drivers/hwmon/Kconfig|   10 ++
>  drivers/hwmon/Makefile   |1 +
>  drivers/hwmon/da9052-hwmon.c |  343 
> ++

Can you consider adding yourself or David as maintainer ? Not a must,
but no one else would really be able to support the driver.

>  4 files changed, 413 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/da9052
>  create mode 100644 drivers/hwmon/da9052-hwmon.c
> diff --git a/Documentation/hwmon/da9052 b/Documentation/hwmon/da9052
> new file mode 100644
> index 000..3035bae
> --- /dev/null
> +++ b/Documentation/hwmon/da9052
> @@ -0,0 +1,59 @@
> +Supported chips:
> +  * Dialog Semiconductors DA9052-BC and DA9053-AA/Bx PMICs
> +Prefix: 'da9052'
> +Datasheet: Kindly visit www.dialog-semiconductor.com and request for the
> +   official datasheet.
> +
Maybe, but that doesn't seem to be straightforward. The web site has no
search field, and I am not sure who to contact in the contact list. And
since there does not seem to be support in the US, I am not sure if one
is expected to contact someone in Europe or Taiwan or some other place. 

Maybe you can let me know how exactly to get the datasheets - I like to
keep those around for chips we are supporting.

Just saying ... for now I gave up trying, meaning some my feedback is
guesswork and may be wrong.

> +Authors: David Dajun Chen 
> +
> +Description
> +---
> +
> +The DA9052/53 provides an Analogue to Digital Converter (ADC) with 10 bits
> +resolution and track and hold circuitry combined with an analogue input
> +multiplexer. The analogue input multiplexer will allow conversion of up to 10
> +different inputs. The track and hold circuit ensures stable input voltages at
> +the input of the ADC during the conversion.
> +
> +The ADC is used to measure the following inputs:
> +Channel 0: VDDOUT - measurement of the system voltage
> +Channel 1: ICH - internal battery charger current measurement
> +Channel 2: TBAT - output from the battery NTC
> +Channel 3: VBAT - measurement of the battery voltage
> +Channel 4: ADC_IN4 - high impedance input (0 - 2.5V)
> +Channel 5: ADC_IN5 - high impedance input (0 - 2.5V)
> +Channel 6: ADC_IN6 - high impedance input (0 - 2.5V)
> +Channel 7: XY - TSI interface to measure the X and Y voltage of the touch
> +screen resistive potentiometers
> +Channel 8: Internal Tjunc. - sense (internal temp. sensor)
> +Channel 9: VBBAT - measurement of the backup battery voltage
> +
> +By using sysfs attributes we can measure the system voltage VDDOUT, the 
> battery
> +charging current ICH, battery temperature TBAT, battery junction temperature
> +TJUNC, battery voltage VBAT and the back up battery voltage VBBAT.
> +
> +Voltage Monitoring
> +--
> +
> +Voltages are sampled by a 10 bit ADC.

ERROR: trailing whitespace
#121: FILE: Documentation/hwmon/da9052:38:
+Voltages are sampled by a 10 bit ADC. $

> +
> +The battery voltage is calculated as:
> +   Milli volt = ((ADC value * 1000) / 512) + 2500
> +
> +The voltages on ADC channels 4, 5 and 6 are calculated as:
> +   Milli volt = (ADC value * 2500) / 1023
> +
> +Temperature Monitoring
> +--
> +
> +Temperatures are sampled by a 10 bit ADC.  Junction and battery temperatures
> +are monitored by the ADC channels.
> +
> +The junction temperature is calculated:
> +   Degrees celsius = 1.708 * (TJUNC_RES - T_OFFSET) - 108.8
> +The junction temperature attribute is supported by the driver.
> +
> +The battery temperature is calculated:
> +   Degree Celcius = 1 / (t1 + 1/298)- 273
> +where t1 = (1/B)* ln(( ADCval * 2.5)/(R25*ITBAT*255))
> +Default values of R25, B, ITBAT are 10e3, 3380 and 50e-6 respectively.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 24d584a..a35d914 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -313,6 +313,16 @@ config SENSORS_DS1621
>   This driver can also be built as a module.  If so, the module
>   will be called ds1621.
> 
> +config SENSORS_DA9052_ADC
> +   tristate "Dialog DA9052/DA9053 HWMON"

"HWMON" is a bit redundant here. Should probably be "ADC".

> +   depends on PMIC_DA9052
> +   help
> + Say y here to support the ADC found on Dialog Semiconductor
> + DA9052-BC and DA9053-AA/Bx PMICs.
> +
> + This driver can also be built as module. If so, the module
> + 

Re: FW: [PATCH 02/07] HWMON: DA9052/53 hwmon driver v1

2012-02-07 Thread Guenter Roeck
Hi Ashish,

On Tue, Feb 07, 2012 at 08:07:10AM -0500, Ashish Jangam wrote:
> On Tue, 2012-02-07 at 17:52 +0530, Ashish Jangam wrote:
> > Can you consider adding yourself or David as maintainer ? Not a must,
> > but no one else would really be able to support the driver.
> Thank for this, you can add my name. 

You would add the entry in the MAINTAINERS file yourself and submit the change
as part of your patch.

> > > +Supported chips:
> > > +  * Dialog Semiconductors DA9052-BC and DA9053-AA/Bx PMICs
> > > +Prefix: 'da9052'
> > > +Datasheet: Kindly visit www.dialog-semiconductor.com and request for 
> > > the
> > > +   official datasheet.
> > > +
> > Maybe, but that doesn't seem to be straightforward. The web site has no
> > search field, and I am not sure who to contact in the contact list. And
> > since there does not seem to be support in the US, I am not sure if one
> > is expected to contact someone in Europe or Taiwan or some other place. 
> > 
> > Maybe you can let me know how exactly to get the datasheets - I like to
> > keep those around for chips we are supporting.
> You need to request for DA9052/53 datasheet from
> www.dialog-semiconductor.com/DA9053.php

Done.

Thanks,
Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] HWMON: DA9052/53 hwmon driver v2

2012-02-10 Thread Guenter Roeck
On Fri, Feb 10, 2012 at 07:58:22AM -0500, Ashish Jangam wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch montiors the DA9052 PMIC's ADC channels mostly for battery
> parameters like battery temperature, junction temperature, battery
> current etc.
> 
> This patch is functionally tested on Samsung SMDKV6410.
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 

Ashish,

at first glance looks good. I am about to be travelling for a week;
I'll get back afterwards unless Jean picks it up from here. No hurry,
though, since the next release is still a bit out.

Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH] HWMON: DA9052/53 hwmon driver v2

2012-02-21 Thread Guenter Roeck
On Fri, 2012-02-10 at 07:58 -0500, Ashish Jangam wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch montiors the DA9052 PMIC's ADC channels mostly for battery

s/montiors/monitors/

> parameters like battery temperature, junction temperature, battery
> current etc.
> 
> This patch is functionally tested on Samsung SMDKV6410.
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 
> ---
> Changes since v1
> - Update documentation file for url
> - Modify the entry in Kconfig file
> - Correct reporting of junction temperature
> - Add conversion function for backup battery voltage
> - Change labels for temperatures in the device attributes to follow
> sensor index
> ---
>  Documentation/hwmon/da9052   |   62 
>  drivers/hwmon/Kconfig|   10 ++
>  drivers/hwmon/Makefile   |1 +
>  drivers/hwmon/da9052-hwmon.c |  344 
> ++
>  4 files changed, 417 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/da9052
>  create mode 100644 drivers/hwmon/da9052-hwmon.c
> diff --git a/Documentation/hwmon/da9052 b/Documentation/hwmon/da9052
> new file mode 100644
> index 000..7caa066
> --- /dev/null
> +++ b/Documentation/hwmon/da9052
> @@ -0,0 +1,62 @@
> +Supported chips:
> +  * Dialog Semiconductors DA9052-BC and DA9053-AA/Bx PMICs
> +Prefix: 'da9052'
> +Datasheet: Kindly visit www.dialog-semiconductor.com/product_table.php
> +  and request for the official datasheet.
> +
Not really. From practical experience, the datasheet is not available
except for Dialog Semiconductor customers - not even for me to review
the driver (and that appears to be independent of my employer's
relationship with Dialog Semiconductor). I think this should read "not
publicly available". It is a bit misleading to suggest that it would be
available if that is not the case.

> +Authors: David Dajun Chen 
> +
> +Description
> +---
> +
> +The DA9052/53 provides an Analogue to Digital Converter (ADC) with 10 bits
> +resolution and track and hold circuitry combined with an analogue input
> +multiplexer. The analogue input multiplexer will allow conversion of up to 10
> +different inputs. The track and hold circuit ensures stable input voltages at
> +the input of the ADC during the conversion.
> +
> +The ADC is used to measure the following inputs:
> +Channel 0: VDDOUT - measurement of the system voltage
> +Channel 1: ICH - internal battery charger current measurement
> +Channel 2: TBAT - output from the battery NTC
> +Channel 3: VBAT - measurement of the battery voltage
> +Channel 4: ADC_IN4 - high impedance input (0 - 2.5V)
> +Channel 5: ADC_IN5 - high impedance input (0 - 2.5V)
> +Channel 6: ADC_IN6 - high impedance input (0 - 2.5V)
> +Channel 7: XY - TSI interface to measure the X and Y voltage of the touch
> +screen resistive potentiometers

Formatting is a bit off here.

> +Channel 8: Internal Tjunc. - sense (internal temp. sensor)
> +Channel 9: VBBAT - measurement of the backup battery voltage
> +
> +By using sysfs attributes we can measure the system voltage VDDOUT, the 
> battery
> +charging current ICH, battery temperature TBAT, battery junction temperature
> +TJUNC, battery voltage VBAT and the back up battery voltage VBBAT.
> +
> +Voltage Monitoring
> +--
> +
> +Voltages are sampled by a 10 bit ADC.
> +
> +The battery voltage is calculated as:
> +   Milli volt = ((ADC value * 1000) / 512) + 2500
> +
> +The backup battery voltage is calculated as:
> +   Milli volt = (ADC value * 2500) / 512;
> +
> +The voltages on ADC channels 4, 5 and 6 are calculated as:
> +   Milli volt = (ADC value * 2500) / 1023
> +
> +Temperature Monitoring
> +--
> +
> +Temperatures are sampled by a 10 bit ADC.  Junction and battery temperatures
> +are monitored by the ADC channels.
> +
> +The junction temperature is calculated:
> +   Degrees celsius = 1.708 * (TJUNC_RES - T_OFFSET) - 108.8
> +The junction temperature attribute is supported by the driver.
> +
> +The battery temperature is calculated:
> +   Degree Celcius = 1 / (t1 + 1/298)- 273
> +where t1 = (1/B)* ln(( ADCval * 2.5)/(R25*ITBAT*255))
> +Default values of R25, B, ITBAT are 10e3, 3380 and 50e-6 respectively.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 24d584a..42b5f1a 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -313,6 +313,16 @@ config SENSORS_DS1621
>   This driver can also be built as a module.  If so, the module
>   will be called ds1621.
> 
> +config SENSORS_DA9052_ADC
> +   tristate "Dialog DA9052/DA9053 ADC"
> +   depends on PMIC_DA9052
> +   help
> + Say y here to support the ADC found on Dialog Semiconductor
> + DA9052-BC and DA9053-AA/Bx PMICs.
> +
> + This driver can also be built as module. If so, the module
> + will be called da9052-hw

RE: [lm-sensors] [PATCH 02/03] HWMON: HWMON driver for DA9052/53 PMIC v3

2012-03-02 Thread Guenter Roeck
Hi Durga,

On Fri, 2012-03-02 at 11:44 -0500, R, Durgadoss wrote:
> Hi,
> 
> > -Original Message-
> > From: lm-sensors-boun...@lm-sensors.org [mailto:lm-sensors-bounces@lm-
> > sensors.org] On Behalf Of Ashish Jangam
> > Sent: Friday, March 02, 2012 7:59 PM
> > To: Guenter Roeck
> > Cc: randy.dun...@oracle.com; linaro-dev@lists.linaro.org;
> > sa...@linux.intel.com; linux-ker...@vger.kernel.org; 
> > lm-sens...@lm-sensors.org;
> > dc...@diasemi.com
> > Subject: [lm-sensors] [PATCH 02/03] HWMON: HWMON driver for DA9052/53 PMIC 
> > v3
> >
> > The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> > resolution and 10 channels.
> >
> > This patch monitors the DA9052 PMIC's ADC channels mostly for battery
> > parameters like battery temperature, junction temperature, battery
> > current etc.
> >
> > This patch is functionally tested on Samsung SMDKV6410
> >
> > Signed-off-by: David Dajun Chen 
> > Signed-off-by: Ashish Jangam 

[ ... ]

> > +static int __init da9052_hwmon_probe(struct platform_device *pdev)
> > +{
> > + struct da9052_hwmon *hwmon;
> > + int ret;
> > +
> > + hwmon = devm_kzalloc(&pdev->dev, sizeof(struct da9052_hwmon),
> > +  GFP_KERNEL);
> > + if (!hwmon)
> > + return -ENOMEM;
> > +
> > + mutex_init(&hwmon->hwmon_lock);
> > + hwmon->da9052 = dev_get_drvdata(pdev->dev.parent);
> > +
> > + platform_set_drvdata(pdev, hwmon);
> > +
> > + ret = sysfs_create_group(&pdev->dev.kobj, &da9052_attr_group);
> > + if (ret)
> > + goto err_mem;
> > +
> > + hwmon->class_device = hwmon_device_register(&pdev->dev);
> > + if (IS_ERR(hwmon->class_device)) {
> > + ret = PTR_ERR(hwmon->class_device);
> > + goto err_sysfs;
> > + }
> > +
> > + return 0;
> > +
> > +err_sysfs:
> > + sysfs_remove_group(&pdev->dev.kobj, &da9052_attr_group);
> > +err_mem:
> 
> I think you should do a kfree(hwmon) here.
> 

Not needed because devm_kzalloc() is used above.

> > + return ret;
> > +}
> > +
> > +static int __devexit da9052_hwmon_remove(struct platform_device *pdev)
> > +{
> > + struct da9052_hwmon *hwmon = platform_get_drvdata(pdev);
> > +
> > + hwmon_device_unregister(hwmon->class_device);
> > + sysfs_remove_group(&pdev->dev.kobj, &da9052_attr_group);
> 
>  Kfree(hwmon);
> 
Same here.

Guenter



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [lm-sensors] [PATCH 2/4] hwmon: exynos4: Move thermal sensor driver to driver/mfd directory

2012-03-03 Thread Guenter Roeck
On Sat, Mar 03, 2012 at 06:06:05AM -0500, Amit Daniel Kachhap wrote:
> This movement is needed because the hwmon entries and corresponding
> sysfs interface is a duplicate of utilities already provided by
> driver/thermal/thermal_sys.c. The goal is to place it in mfd folder
> and add necessary calls to get the temperature information.
> 
> Signed-off-by: Amit Daniel Kachhap 
> Signed-off-by: Donggeun Kim 
> ---
>  Documentation/hwmon/exynos4_tmu |   81 --
>  Documentation/mfd/exynos4_tmu   |   81 ++
>  drivers/hwmon/Kconfig   |   10 -
>  drivers/hwmon/Makefile  |1 -
>  drivers/hwmon/exynos4_tmu.c |  514 
> ---
>  drivers/mfd/Kconfig |   10 +
>  drivers/mfd/Makefile|1 +
>  drivers/mfd/exynos4_tmu.c   |  514 
> +++
>  8 files changed, 606 insertions(+), 606 deletions(-)
>  delete mode 100644 Documentation/hwmon/exynos4_tmu
>  create mode 100644 Documentation/mfd/exynos4_tmu
>  delete mode 100644 drivers/hwmon/exynos4_tmu.c
>  create mode 100644 drivers/mfd/exynos4_tmu.c
> 

You are just moving the driver from hwmon to mfd. It is still a hwmon driver
and registers itself as hwmon driver. That does not make sense. If you want it
as mfd driver, it should not register itself as hwmon driver. It might have 
sub-devices
which are thermal and/or hwmon devices in the respective trees, or it might 
just be
a thermal driver (which then registers itself as hwmon device automatically).

Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [lm-sensors] [PATCH 3/4] thermal: exynos4: Register the tmu sensor with the thermal interface layer

2012-03-03 Thread Guenter Roeck
On Sat, Mar 03, 2012 at 06:06:06AM -0500, Amit Daniel Kachhap wrote:
> Export and register information from the tmu temperature sensor to the samsung
> exynos kernel thermal framework where different cooling devices and thermal
> zone are binded. The exported information is based according to the data
> structure thermal_sensor_conf present in exynos_thermal.h. HWMON sysfs
> functions are removed as all of them are present in generic linux thermal 
> layer.
> 
> Also the platform data structure is modified to pass frequency cooling
> in percentages for each thermal level.
> 
> Signed-off-by: Amit Daniel Kachhap 

Ah, that is what I meant. I don't think it makes sense to have this as separate 
patch.

Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [lm-sensors] [linux-pm] [PATCH 2/4] hwmon: exynos4: Move thermal sensor driver to driver/mfd directory

2012-03-03 Thread Guenter Roeck
On Sat, Mar 03, 2012 at 11:44:10AM -0500, Mark Brown wrote:
> On Sat, Mar 03, 2012 at 04:36:05PM +0530, Amit Daniel Kachhap wrote:
> > This movement is needed because the hwmon entries and corresponding
> > sysfs interface is a duplicate of utilities already provided by
> > driver/thermal/thermal_sys.c. The goal is to place it in mfd folder
> > and add necessary calls to get the temperature information.
> 
> > --- a/Documentation/hwmon/exynos4_tmu
> > +++ /dev/null
> 
> Moving this seems to be a failure, the device is exposing a hwmon
> interface even if you've moved the code to mfd (though it doesn't
> actually look like a multi-function device at all as far as I can see -
> usually a MFD would have a bunch of unrelated functionality while this
> has one function used by two subsystems).
> 
> If anything it looks like the ADC driver ought to be moved into IIO with
> either generic or Exynos specific function drivers layered on top of it
> in hwmon and thermal making use of the values that are read.
> 
I would agree. Or maybe move it all to thermal, since thermal devices register
the hwmon subsystem.

Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 02/03] HWMON: HWMON driver for DA9052/53 PMIC v3

2012-03-07 Thread Guenter Roeck
On Fri, 2012-03-02 at 09:29 -0500, Ashish Jangam wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch monitors the DA9052 PMIC's ADC channels mostly for battery
> parameters like battery temperature, junction temperature, battery
> current etc.
> 
> This patch is functionally tested on Samsung SMDKV6410
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 

Only comment I would have is that it might make sense to use
DIV_ROUND_CLOSEST(). That is a minor issue, though, so feel free to add

Acked-by: Guenter Roeck 

Question is where this is going. I can not add it to hwmon-next without
the matching mfd changes. Any idea ?

Thanks,
Guenter



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 02/02] HWMON: HWMON driver for DA9052/53 PMIC v4

2012-03-17 Thread Guenter Roeck
On Sat, Mar 17, 2012 at 06:04:41AM -0400, Ashish Jangam wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch monitors the DA9052 PMIC's ADC channels mostly for battery
> parameters like battery temperature, junction temperature, battery
> current etc.
> 
> This patch is functionally tested on Samsung SMDKV6410
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 

Acked-by: Guenter Roeck 

> ---
> Changes since V3
> - Add DIV_ROUND_CLOSEST() macro
> Changes since v2
> - Update documentation file
> Changes since v1
> - Update documentation file for url
> - Modify the entry in Kconfig file
> - Correct reporting of junction temperature
> - Add conversion function for backup battery voltage
> - Change labels for temperatures in the device attributes to follow
> sensor index
> ---
>  Documentation/hwmon/da9052   |   61 
>  drivers/hwmon/Kconfig|   10 ++
>  drivers/hwmon/Makefile   |1 +
>  drivers/hwmon/da9052-hwmon.c |  344 
> ++
>  4 files changed, 416 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/hwmon/da9052
>  create mode 100644 drivers/hwmon/da9052-hwmon.c
> 
> diff --git a/Documentation/hwmon/da9052 b/Documentation/hwmon/da9052
> new file mode 100644
> index 000..ef89855
> --- /dev/null
> +++ b/Documentation/hwmon/da9052
> @@ -0,0 +1,61 @@
> +Supported chips:
> +  * Dialog Semiconductors DA9052-BC and DA9053-AA/Bx PMICs
> +Prefix: 'da9052'
> +Datasheet: Datasheet is not publicly available.
> +
> +Authors: David Dajun Chen 
> +
> +Description
> +---
> +
> +The DA9052/53 provides an Analogue to Digital Converter (ADC) with 10 bits
> +resolution and track and hold circuitry combined with an analogue input
> +multiplexer. The analogue input multiplexer will allow conversion of up to 10
> +different inputs. The track and hold circuit ensures stable input voltages at
> +the input of the ADC during the conversion.
> +
> +The ADC is used to measure the following inputs:
> +Channel 0: VDDOUT - measurement of the system voltage
> +Channel 1: ICH - internal battery charger current measurement
> +Channel 2: TBAT - output from the battery NTC
> +Channel 3: VBAT - measurement of the battery voltage
> +Channel 4: ADC_IN4 - high impedance input (0 - 2.5V)
> +Channel 5: ADC_IN5 - high impedance input (0 - 2.5V)
> +Channel 6: ADC_IN6 - high impedance input (0 - 2.5V)
> +Channel 7: XY - TSI interface to measure the X and Y voltage of the touch
> +  screen resistive potentiometers
> +Channel 8: Internal Tjunc. - sense (internal temp. sensor)
> +Channel 9: VBBAT - measurement of the backup battery voltage
> +
> +By using sysfs attributes we can measure the system voltage VDDOUT, the 
> battery
> +charging current ICH, battery temperature TBAT, battery junction temperature
> +TJUNC, battery voltage VBAT and the back up battery voltage VBBAT.
> +
> +Voltage Monitoring
> +--
> +
> +Voltages are sampled by a 10 bit ADC.
> +
> +The battery voltage is calculated as:
> +   Milli volt = ((ADC value * 1000) / 512) + 2500
> +
> +The backup battery voltage is calculated as:
> +   Milli volt = (ADC value * 2500) / 512;
> +
> +The voltages on ADC channels 4, 5 and 6 are calculated as:
> +   Milli volt = (ADC value * 2500) / 1023
> +
> +Temperature Monitoring
> +--
> +
> +Temperatures are sampled by a 10 bit ADC.  Junction and battery temperatures
> +are monitored by the ADC channels.
> +
> +The junction temperature is calculated:
> +   Degrees celsius = 1.708 * (TJUNC_RES - T_OFFSET) - 108.8
> +The junction temperature attribute is supported by the driver.
> +
> +The battery temperature is calculated:
> +   Degree Celcius = 1 / (t1 + 1/298)- 273
> +where t1 = (1/B)* ln(( ADCval * 2.5)/(R25*ITBAT*255))
> +Default values of R25, B, ITBAT are 10e3, 3380 and 50e-6 respectively.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 12bc500..5549c5b 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -313,6 +313,16 @@ config SENSORS_DS1621
>   This driver can also be built as a module.  If so, the module
>   will be called ds1621.
> 
> +config SENSORS_DA9052_ADC
> +   tristate "Dialog DA9052/DA9053 ADC"
> +   depends on PMIC_DA9052
> +   help
> + Say y here to support the ADC found on Dialog Semiconductor
> + DA9052-BC and DA9053-AA/Bx PMICs.
> +
> + This driver can also be built as module. If so, the module
> + will be call

Re: [PATCH 6/11] HWMON: DA9052 hwmon driver v1

2011-06-28 Thread Guenter Roeck
On Tue, 2011-06-28 at 10:24 -0400, ashishj3 wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch montiors the DA9052 PMIC's ADC channels mostly for battery 
> parameters
> like battery temperature, junction temperature, battery current etc.
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 

[ ... ]

> +static ssize_t da9052_read_vddout(struct device *dev,
> +  struct device_attribute *devattr, char 
> *buf)
> +{
> +   struct da9052_hwmon *hwmon = dev_get_drvdata(dev);
> +   int ret, vdd = -1;
> +
> +   mutex_lock(&hwmon->hwmon_lock);
> +
> +   ret = da9052_enable_vddout_channel(hwmon->da9052);
> +   if (ret < 0)
> +   goto hwmon_err;
> +
> +   ret = da9052_reg_read(hwmon->da9052, DA9052_VDD_RES_REG);
> +   if (ret < 0)
> +   pr_err("failed to read VDD_RES_REG\n");
> +   else
> +   vdd = ret;
> +
> +   ret = da9052_disable_vddout_channel(hwmon->da9052);
> +   if (ret < 0)
> +   goto hwmon_err;
> +
> +   if (vdd >= 0) {
> +   mutex_unlock(&hwmon->hwmon_lock);
> +   return sprintf(buf, "%d\n", vdd);
> +   }
> +
> +hwmon_err:
> +   mutex_unlock(&hwmon->hwmon_lock);
> +   return ret;
> +}

This function still produces a bad result if the call to
da9052_reg_read() fails and the call to da9052_disable_vddout_channel()
doesn't. 

I would suggest to replace pr_err() with "goto hwmon_err". If you do
that, you don't need to initialize vdd, you don't need the else case for
its assignment, and you don't need "if (vdd >= 0)" either.

Guenter



___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 6/11] HWMON: DA9052 hwmon driver v1

2011-06-30 Thread Guenter Roeck
On Wed, Jun 29, 2011 at 07:44:08AM -0400, Ashish Jangam wrote:
> On Tue, 2011-06-28 at 23:05 +0530, Guenter Roeck wrote:
> > On Tue, 2011-06-28 at 10:24 -0400, ashish jangam wrote:
> > > +static ssize_t da9052_read_vddout(struct device *dev,
> > > +  struct device_attribute *devattr, char 
> > > *buf)
> > > +{
> > > +   struct da9052_hwmon *hwmon = dev_get_drvdata(dev);
> > > +   int ret, vdd = -1;
> > > +
> > > +   mutex_lock(&hwmon->hwmon_lock);
> > > +
> > > +   ret = da9052_enable_vddout_channel(hwmon->da9052);
> > > +   if (ret < 0)
> > > +   goto hwmon_err;
> > > +
> > > +   ret = da9052_reg_read(hwmon->da9052, DA9052_VDD_RES_REG);
> > > +   if (ret < 0)
> > > +   pr_err("failed to read VDD_RES_REG\n");
> > > +   else
> > > +   vdd = ret;
> > > +
> > > +   ret = da9052_disable_vddout_channel(hwmon->da9052);
> > > +   if (ret < 0)
> > > +   goto hwmon_err;
> > > +
> > > +   if (vdd >= 0) {
> > > +   mutex_unlock(&hwmon->hwmon_lock);
> > > +   return sprintf(buf, "%d\n", vdd);
> > > +   }
> > > +
> > > +hwmon_err:
> > > +   mutex_unlock(&hwmon->hwmon_lock);
> > > +   return ret;
> > > +}
> > 
> > This function still produces a bad result if the call to
> > da9052_reg_read() fails and the call to da9052_disable_vddout_channel()
> > doesn't. 
> Thanks much for comments and patience. When vddout channel is enabled
> and then read from this channel fails then, in this case should vddout
> channel get disabled? Is this correct understanding.

Hmm, yes, you are right there. You should try to disable it. But you would have 
to do that such that you don't override the original error. One possibility 
would
be to add an unconditional call to da9052_disable_vddout_channel() into the 
error 
path.

ret = da9052_reg_read(hwmon->da9052, DA9052_VDD_RES_REG);
if (ret < 0)
goto hwmon_err_release;

...
hwmon_err_release:
da9052_disable_vddout_channel(hwmon->da9052);
hwmon_err:
...

Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH 6/11] HWMON: DA9052 hwmon driver v2

2011-07-01 Thread Guenter Roeck
On Thu, Jun 30, 2011 at 09:15:27AM -0400, ashishj3 wrote:
> The DA9052 PMIC provides an Analogue to Digital Converter with 10 bits
> resolution and 10 channels.
> 
> This patch montiors the DA9052 PMIC's ADC channels mostly for battery 
> parameters
> like battery temperature, junction temperature, battery current etc.
> 
> Signed-off-by: David Dajun Chen 
> Signed-off-by: Ashish Jangam 

Acked-by: Guenter Roeck 

I assume this patch will be handled as part of a patchset, correct ?

Thanks,
Guenter

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [RFC] Energy/power monitoring within the kernel

2012-10-23 Thread Guenter Roeck
On Tue, Oct 23, 2012 at 06:30:49PM +0100, Pawel Moll wrote:
> Greetings All,
> 
> More and more of people are getting interested in the subject of power
> (energy) consumption monitoring. We have some external tools like
> "battery simulators", energy probes etc., but some targets can measure
> their power usage on their own.
> 
> Traditionally such data should be exposed to the user via hwmon sysfs
> interface, and that's exactly what I did for "my" platform - I have
> a /sys/class/hwmon/hwmon*/device/energy*_input and this was good
> enough to draw pretty graphs in userspace. Everyone was happy...
> 
Only driver supporting "energy" output so far is ibmaem, and the reported energy
is supposed to be cumulative, as in energy = power * time. Do you mean power,
possibly ?

> Now I am getting new requests to do more with this data. In particular
> I'm asked how to add such information to ftrace/perf output. The second
> most frequent request is about providing it to a "energy aware"
> cpufreq governor.
> 

Anything energy related would have to be along the line of "do something after a
certain amount of work has been performed", which at least at the surface does
not make much sense to me, unless you mean something along the line of a
process scheduler which schedules a process not based on time slices but based
on energy consumed, ie if you want to define a time slice not in milli-seconds
but in Joule.

If so, I would argue that a similar behavior could be achieved by varying the
duration of time slices with the current CPU speed, or simply by using cycle
count instead of time as time slice parameter. Not that I am sure if such an
approach would really be of interest for anyone. 

Or do you really mean power, not energy, such as in "reduce CPU speed if its
power consumption is above X Watt" ?

> I've came up with three (non-mutually exclusive) options. I will
> appreciate any other ideas and comments (including "it makes not sense
> whatsoever" ones, with justification). Of course I am more than willing
> to spend time on prototyping anything that seems reasonable and propose
> patches.
> 
> 
> 
> === Option 1: Trace event ===
> 
> This seems to be the "cheapest" option. Simply defining a trace event
> that can be generated by a hwmon (or any other) driver makes the
> interesting data immediately available to any ftrace/perf user. Of
> course it doesn't really help with the cpufreq case, but seems to be
> a good place to start with.
> 
> The question is how to define it... I've came up with two prototypes:
> 
> = Generic hwmon trace event =
> 
> This one allows any driver to generate a trace event whenever any
> "hwmon attribute" (measured value) gets updated. The rate at which the
> updates happen can be controlled by already existing "update_interval"
> attribute.
> 
> 8<---
> TRACE_EVENT(hwmon_attr_update,
>   TP_PROTO(struct device *dev, struct attribute *attr, long long input),
>   TP_ARGS(dev, attr, input),
> 
>   TP_STRUCT__entry(
>   __string(   dev,dev_name(dev))
>   __string(   attr,   attr->name)
>   __field(long long,  input)
>   ),
> 
>   TP_fast_assign(
>   __assign_str(dev, dev_name(dev));
>   __assign_str(attr, attr->name);
>   __entry->input = input;
>   ),
> 
>   TP_printk("%s %s %lld", __get_str(dev), __get_str(attr), __entry->input)
> );
> 8<---
> 
> It generates such ftrace message:
> 
> <...>212.673126: hwmon_attr_update: hwmon4 temp1_input 34361
> 
> One issue with this is that some external knowledge is required to
> relate a number to a processor core. Or maybe it's not an issue at all
> because it should be left for the user(space)?
> 
> = CPU power/energy/temperature trace event =
> 
> This one is designed to emphasize the relation between the measured
> value (whether it is energy, temperature or any other physical
> phenomena, really) and CPUs, so it is quite specific (too specific?)
> 
> 8<---
> TRACE_EVENT(cpus_environment,
>   TP_PROTO(const struct cpumask *cpus, long long value, char unit),
>   TP_ARGS(cpus, value, unit),
> 
>   TP_STRUCT__entry(
>   __array(unsigned char,  cpus,   sizeof(struct cpumask))
>   __field(long long,  value)
>   __field(char,   unit)
>   ),
> 
>   TP_fast_assign(
>   memcpy(__entry->cpus, cpus, sizeof(struct cpumask));
>   __entry->value = value;
>   __entry->unit = unit;
>   ),
> 
>   TP_printk("cpus %s %lld[%c]",
>   __print_cpumask((struct cpumask *)__entry->cpus),
>   __entry->value, __entry->unit)
> );
> 8<---
> 
> And the equivalent ftrace message is:
> 
> <...>127.063107: cpus_environment: c

Re: [RFC] Energy/power monitoring within the kernel

2012-10-24 Thread Guenter Roeck
On Wed, Oct 24, 2012 at 05:37:27PM +0100, Pawel Moll wrote:
> On Tue, 2012-10-23 at 23:02 +0100, Guenter Roeck wrote:
> > > Traditionally such data should be exposed to the user via hwmon sysfs
> > > interface, and that's exactly what I did for "my" platform - I have
> > > a /sys/class/hwmon/hwmon*/device/energy*_input and this was good
> > > enough to draw pretty graphs in userspace. Everyone was happy...
> > > 
> > Only driver supporting "energy" output so far is ibmaem, and the reported 
> > energy
> > is supposed to be cumulative, as in energy = power * time. Do you mean 
> > power,
> > possibly ?
> 
> So the vexpress would be the second one, than :-) as the energy
> "monitor" actually on the latest tiles reports 64-bit value of
> microJoules consumed (or produced) since the power-up.
> 
> Some of the older boards were able to report instant power, but this
> metrics is less useful in our case.
> 
> > > Now I am getting new requests to do more with this data. In particular
> > > I'm asked how to add such information to ftrace/perf output. The second
> > > most frequent request is about providing it to a "energy aware"
> > > cpufreq governor.
> > 
> > Anything energy related would have to be along the line of "do something 
> > after a
> > certain amount of work has been performed", which at least at the surface 
> > does
> > not make much sense to me, unless you mean something along the line of a
> > process scheduler which schedules a process not based on time slices but 
> > based
> > on energy consumed, ie if you want to define a time slice not in 
> > milli-seconds
> > but in Joule.
> 
> Actually there is some research being done in this direction, but it's
> way too early to draw any conclusions...
> 
> > If so, I would argue that a similar behavior could be achieved by varying 
> > the
> > duration of time slices with the current CPU speed, or simply by using cycle
> > count instead of time as time slice parameter. Not that I am sure if such an
> > approach would really be of interest for anyone. 
> > 
> > Or do you really mean power, not energy, such as in "reduce CPU speed if its
> > power consumption is above X Watt" ?
> 
> Uh. To be completely honest I must answer: I'm not sure how the "energy
> aware" cpufreq governor is supposed to work. I have been simply asked to
> provide the data in some standard way, if possible.
> 
> > I am not sure how this would be expected to work. hwmon is, by its very 
> > nature,
> > a passive subsystem: It doesn't do anything unless data is explicitly 
> > requested
> > from it. It does not update an attribute unless that attribute is read.
> > That does not seem to fit well with the idea of tracing - which assumes
> > that some activity is happening, ultimately, all by itself, presumably
> > periodically. The idea to have a user space application read hwmon data only
> > for it to trigger trace events does not seem to be very compelling to me.
> 
> What I had in mind was similar to what adt7470 driver does. The driver
> would automatically access the device every now and then to update it's
> internal state and generate the trace event on the way. This
> auto-refresh "feature" is particularly appealing for me, as on some of
> "my" platforms can take up to 500 microseconds to actually get the data.
> So doing this in background (and providing users with the last known
> value in the meantime) seems attractive.
> 
A bad example doesn't mean it should be used elsewhere.

adt7470 needs up to two seconds for a temperature measurement cycle, and it
can not perform automatic cycles all by itself. In this context, executing
temperature measurement cycles in the background makes a lot of sense,
especially since one does not want to wait for two seconds when reading
a sysfs attribute.

But that only means that the chip is most likely not a good choice when 
selecting
a temperature sensor, not that the code necessary to get it working should be 
used
as an example for other drivers. 

Guenter

> > An exception is if a monitoring device suppports interrupts, and if its 
> > driver
> > actually implements those interrupts. This is, however, not the case for 
> > most of
> > the current drivers (if any), mostly because interrupt support for hardware
> > monitoring devices is very platform dependent and thus difficult to 
> > implement.
> 
> Interestingly enough the newest version of our platform control micro
> (doing the energy monitoring as well

Re: [PATCH v2 1/2] hwmon: add ST-Ericsson ABX500 hwmon driver

2013-02-18 Thread Guenter Roeck
On Wed, Feb 06, 2013 at 07:10:38PM +0800, Hongbo Zhang wrote:
> Each of ST-Ericsson X500 chip set series consists of both ABX500 and DBX500
> chips. This is ABX500 hwmon driver, where the abx500.c is a common layer for
> all ABX500s, and the ab8500.c is specific for AB8500 chip. Under this designed
> structure, other chip specific files can be added simply using the same common
> layer abx500.c.
> 
> Signed-off-by: Hongbo Zhang 

Better, but you did not answer the most important question: With its
functionality, I think the driver might fit much better into the thermal
subsystem. Did you explore this ?

> ---
>  Documentation/hwmon/ab8500 |  22 ++
>  Documentation/hwmon/abx500 |  26 +++
>  drivers/hwmon/Kconfig  |  13 ++
>  drivers/hwmon/Makefile |   1 +
>  drivers/hwmon/ab8500.c | 163 ++
>  drivers/hwmon/abx500.c | 544 
> +
>  drivers/hwmon/abx500.h |  84 +++
>  7 files changed, 853 insertions(+)
>  create mode 100644 Documentation/hwmon/ab8500
>  create mode 100644 Documentation/hwmon/abx500
>  create mode 100644 drivers/hwmon/ab8500.c
>  create mode 100644 drivers/hwmon/abx500.c
>  create mode 100644 drivers/hwmon/abx500.h
> 
> diff --git a/Documentation/hwmon/ab8500 b/Documentation/hwmon/ab8500
> new file mode 100644
> index 000..76c534d
> --- /dev/null
> +++ b/Documentation/hwmon/ab8500
> @@ -0,0 +1,22 @@
> +Kernel driver ab8500
> +
> +
> +Supported chips:
> +  * ST-Ericsson AB8500
> +Prefix: 'ab8500'
> +Addresses scanned: -
> +Datasheet: http://www.stericsson.com/developers/documentation.jsp
> +
> +Authors:
> +Martin Persson 
> +Hongbo Zhang 
> +
> +Description
> +---
> +
> +See also Documentation/hwmon/abx500. This is ST-Ericsson AB8500 hwmon 
> specific
> +initialization.
> +
> +Currently only the AB8500 internal sensor and one external sensor for battery
> +temperature are monitored. Other GPADC channels can also be monitored if 
> needed
> +in future.
> diff --git a/Documentation/hwmon/abx500 b/Documentation/hwmon/abx500
> new file mode 100644
> index 000..f60b73c
> --- /dev/null
> +++ b/Documentation/hwmon/abx500
> @@ -0,0 +1,26 @@
> +Kernel driver abx500
> +
> +
> +Supported chips:
> +  * ST-Ericsson ABx500 series
> +Prefix: 'abx500'
> +Addresses scanned: -
> +Datasheet: http://www.stericsson.com/developers/documentation.jsp
> +
> +Authors:
> +Martin Persson 
> +Hongbo Zhang 
> +
> +Description
> +---
> +
> +Every ST-Ericsson Ux500 SOC consists of both ABx500 and DBx500 physically,
> +this is kernel hwmon driver for ABx500.
> +
> +There are some GPADCs inside ABx500 which are designed for connecting to
> +thermal sensors, and there is also a thermal sensor inside ABx500 too, which
> +raises interrupt when critical temperature reached.
> +
> +This abx500 is a common layer which can monitor all of the sensors, every
> +specific abx500 chip has its special configurations in its own file, e.g. 
> some
> +sensors can be configured invisible if they are not available on that chip.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 32f238f..0a6fd21 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -39,6 +39,19 @@ config HWMON_DEBUG_CHIP
>  
>  comment "Native drivers"
>  
> +config SENSORS_AB8500
> + tristate "AB8500 thermal monitoring"
> + depends on AB8500_GPADC
> + default n
> + help
> +   If you say yes here you get support for the thermal sensor part
> +   of the AB8500 chip. The driver includes thermal management for
> +   AB8500 die and two GPADC channels. The GPADC channel are preferably
> +   used to access sensors outside the AB8500 chip.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called abx500-temp.
> +
>  config SENSORS_ABITUGURU
>   tristate "Abit uGuru (rev 1 & 2)"
>   depends on X86 && DMI
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 5da2874..06dfe85 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_SENSORS_W83795)+= w83795.o
>  obj-$(CONFIG_SENSORS_W83781D)+= w83781d.o
>  obj-$(CONFIG_SENSORS_W83791D)+= w83791d.o
>  
> +obj-$(CONFIG_SENSORS_AB8500) += abx500.o ab8500.o
>  obj-$(CONFIG_SENSORS_ABITUGURU)  += abituguru.o
>  obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>  obj-$(CONFIG_SENSORS_AD7314) += ad7314.o
> diff --git a/drivers/hwmon/ab8500.c b/drivers/hwmon/ab8500.c
> new file mode 100644
> index 000..ce6c4ef
> --- /dev/null
> +++ b/drivers/hwmon/ab8500.c
> @@ -0,0 +1,163 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010
> + * Author: Martin Persson 
> + * Hongbo Zhang 
> + * License Terms: GNU General Public License v2
> + *
> + * If/when the AB8500 thermal warning temperature is reached (threshold 
> cannot
> + * be changed by SW), an i

Re: [PATCH 2/2] hwmon: add ST-Ericsson ABX500 hwmon driver

2013-02-18 Thread Guenter Roeck
On Wed, Jan 30, 2013 at 06:21:28PM +0800, Hongbo Zhang wrote:
> Each of ST-Ericsson X500 chip set series consists of both ABX500 and DBX500
> chips. This is ABX500 hwmon driver, where the abx500.c is a common layer for
> all ABX500s, and the ab8500.c is specific for AB8500 chip. Under this designed
> structure, other chip specific files can be added simply using the same common
> layer abx500.c.
> 
> Signed-off-by: Hongbo Zhang 
> ---
>  drivers/hwmon/Kconfig  |  13 +
>  drivers/hwmon/Makefile |   1 +
>  drivers/hwmon/ab8500.c | 160 
>  drivers/hwmon/abx500.c | 681 
> +
>  drivers/hwmon/abx500.h |  88 +++
>  5 files changed, 943 insertions(+)
>  create mode 100644 drivers/hwmon/ab8500.c
>  create mode 100644 drivers/hwmon/abx500.c
>  create mode 100644 drivers/hwmon/abx500.h
> 
Hi,

we'll also need Documentation/hwmon/ab8500 to describe the driver, and possibly
Documentation/hwmon/ab85xx to describe the common elements.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 32f238f..0a6fd21 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -39,6 +39,19 @@ config HWMON_DEBUG_CHIP
>  
>  comment "Native drivers"
>  
> +config SENSORS_AB8500
> + tristate "AB8500 thermal monitoring"
> + depends on AB8500_GPADC
> + default n
> + help
> +   If you say yes here you get support for the thermal sensor part
> +   of the AB8500 chip. The driver includes thermal management for
> +   AB8500 die and two GPADC channels. The GPADC channel are preferably
> +   used to access sensors outside the AB8500 chip.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called abx500-temp.
> +
>  config SENSORS_ABITUGURU
>   tristate "Abit uGuru (rev 1 & 2)"
>   depends on X86 && DMI
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 5da2874..06dfe85 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_SENSORS_W83795)+= w83795.o
>  obj-$(CONFIG_SENSORS_W83781D)+= w83781d.o
>  obj-$(CONFIG_SENSORS_W83791D)+= w83791d.o
>  
> +obj-$(CONFIG_SENSORS_AB8500) += abx500.o ab8500.o
>  obj-$(CONFIG_SENSORS_ABITUGURU)  += abituguru.o
>  obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>  obj-$(CONFIG_SENSORS_AD7314) += ad7314.o
> diff --git a/drivers/hwmon/ab8500.c b/drivers/hwmon/ab8500.c
> new file mode 100644
> index 000..426872c
> --- /dev/null
> +++ b/drivers/hwmon/ab8500.c
> @@ -0,0 +1,160 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010
> + * Author: Martin Persson  for
> + * ST-Ericsson.
> + * License Terms: GNU General Public License v2
> + *
> + * If/when the AB8500 thermal warning temperature is reached (threshold 
> cannot
> + * be changed by SW), an interrupt is set and the driver notifies user space
> + * via a sysfs event. If a shut down is not triggered by user space within a
> + * certain time frame, pm_power off is called.
> + *
> + * If/when AB8500 thermal shutdown temperature is reached a hardware shutdown
> + * of the AB8500 will occur.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "abx500.h"
> +
> +#define DEFAULT_POWER_OFF_DELAY 1
> +
> +/* The driver monitors GPADC - ADC_AUX1, ADC_AUX2, BTEMP_BALL and BAT_CTRL */
> +#define NUM_MONITORED_SENSORS 4
> +
> +static int ab8500_read_sensor(struct abx500_temp *data, u8 sensor)
> +{
> + int val;
> + /*
> +  * Special treatment for the BAT_CTRL node, since this temperature
> +  * measurement is more complex than just an ADC readout
> +  */
> + if (sensor == BAT_CTRL)
> + val = ab8500_btemp_get_batctrl_temp(data->ab8500_btemp);
> + else
> + val = ab8500_gpadc_convert(data->ab8500_gpadc, sensor);
> +
> + return val;
> +}
> +
> +static void ab8500_thermal_power_off(struct work_struct *work)
> +{
> + struct abx500_temp *data = container_of(work, struct abx500_temp,
> + power_off_work.work);
> +
> + dev_warn(&data->pdev->dev,
> + "Power off due to AB8500 thermal warning.\n");
> + pm_power_off();
> +}
> +
> +static ssize_t ab8500_show_name(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + return sprintf(buf, "ab8500\n");
> +}
> +
> +static ssize_t ab8500_show_label(struct device *dev,
> + struct device_attribute *devattr,
> + char *buf)
> +{
> + char *name;
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + int index = attr->index;
> +
> + switch (index) {
> + case 1:
> + name = "ext_rtc_xtal";
> + break;
> + case 2:
> + name = "ext_db8500";
> + break;
> + cas

Re: [PATCH v3 2/3] ab8500: make res_to_temp tables public

2013-02-22 Thread Guenter Roeck
On Thu, Feb 21, 2013 at 02:24:23PM -0800, Anton Vorontsov wrote:
> On Thu, Feb 21, 2013 at 06:32:40PM +0800, Hongbo Zhang wrote:
> > These NTC resistance to temperature tables should be public, so others such 
> > as
> > ab8500 hwmon driver can look up these tables to convert NTC resistance to
> > temperature.
> > 
> > Signed-off-by: Hongbo Zhang 
> > ---
> 
> For 1/3 and 2/3 patches:
> 
> Acked-by: Anton Vorontsov 
> 
> (Do you need EXPORT_SYMBOL()? You don't use this from modules?)
> 
I would think so. Also, the variables should be exported through an include
file.

The variable names are quite generic for global variables; we need to find
something more specific/descriptive.

There is also some overlap with functionality in drivers/hwmon/ntc_thermistor.c.
Wonder if it would be possible to unify the code.

Guenter

> Thanks.
> 
> >  drivers/power/ab8500_bmdata.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/power/ab8500_bmdata.c b/drivers/power/ab8500_bmdata.c
> > index f034ae4..53f3324 100644
> > --- a/drivers/power/ab8500_bmdata.c
> > +++ b/drivers/power/ab8500_bmdata.c
> > @@ -11,7 +11,7 @@
> >   * Note that the res_to_temp table must be strictly sorted by falling 
> > resistance
> >   * values to work.
> >   */
> > -static struct abx500_res_to_temp temp_tbl_A_thermistor[] = {
> > +struct abx500_res_to_temp temp_tbl_A_thermistor[] = {
> > {-5, 53407},
> > { 0, 48594},
> > { 5, 43804},
> > @@ -29,7 +29,9 @@ static struct abx500_res_to_temp temp_tbl_A_thermistor[] 
> > = {
> > {65, 12500},
> >  };
> >  
> > -static struct abx500_res_to_temp temp_tbl_B_thermistor[] = {
> > +int temp_tbl_A_size = ARRAY_SIZE(temp_tbl_A_thermistor);
> > +
> > +struct abx500_res_to_temp temp_tbl_B_thermistor[] = {
> > {-5, 20},
> > { 0, 159024},
> > { 5, 151921},
> > @@ -47,6 +49,8 @@ static struct abx500_res_to_temp temp_tbl_B_thermistor[] 
> > = {
> > {65,  82869},
> >  };
> >  
> > +int temp_tbl_B_size = ARRAY_SIZE(temp_tbl_B_thermistor);
> > +
> >  static struct abx500_v_to_cap cap_tbl_A_thermistor[] = {
> > {4171,  100},
> > {4114,   95},
> > -- 
> > 1.8.0
> 

___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev


Re: [PATCH v3 3/3] hwmon: add ST-Ericsson ABX500 hwmon driver

2013-02-26 Thread Guenter Roeck
On Thu, Feb 21, 2013 at 06:32:41PM +0800, Hongbo Zhang wrote:
> Each of ST-Ericsson X500 chip set series consists of both ABX500 and DBX500
> chips. This is ABX500 hwmon driver, where the abx500.c is a common layer for
> all ABX500s, and the ab8500.c is specific for AB8500 chip. Under this designed
> structure, other chip specific files can be added simply using the same common
> layer abx500.c.
> 
> Signed-off-by: Hongbo Zhang 
> ---
>  Documentation/hwmon/ab8500 |  22 ++
>  Documentation/hwmon/abx500 |  26 +++
>  drivers/hwmon/Kconfig  |  13 ++
>  drivers/hwmon/Makefile |   1 +
>  drivers/hwmon/ab8500.c | 178 
>  drivers/hwmon/abx500.c | 501 
> +
>  drivers/hwmon/abx500.h |  87 
>  7 files changed, 828 insertions(+)
>  create mode 100644 Documentation/hwmon/ab8500
>  create mode 100644 Documentation/hwmon/abx500
>  create mode 100644 drivers/hwmon/ab8500.c
>  create mode 100644 drivers/hwmon/abx500.c
>  create mode 100644 drivers/hwmon/abx500.h
> 
> diff --git a/Documentation/hwmon/ab8500 b/Documentation/hwmon/ab8500
> new file mode 100644
> index 000..76c534d
> --- /dev/null
> +++ b/Documentation/hwmon/ab8500
> @@ -0,0 +1,22 @@
> +Kernel driver ab8500
> +
> +
> +Supported chips:
> +  * ST-Ericsson AB8500
> +Prefix: 'ab8500'
> +Addresses scanned: -
> +Datasheet: http://www.stericsson.com/developers/documentation.jsp
> +
> +Authors:
> +Martin Persson 
> +Hongbo Zhang 
> +
> +Description
> +---
> +
> +See also Documentation/hwmon/abx500. This is ST-Ericsson AB8500 hwmon 
> specific
> +initialization.
> +
"This is the ST-Ericsson AB8500 specific driver" or similar might be better.

> +Currently only the AB8500 internal sensor and one external sensor for battery
> +temperature are monitored. Other GPADC channels can also be monitored if 
> needed
> +in future.
> diff --git a/Documentation/hwmon/abx500 b/Documentation/hwmon/abx500
> new file mode 100644
> index 000..f60b73c
> --- /dev/null
> +++ b/Documentation/hwmon/abx500
> @@ -0,0 +1,26 @@
> +Kernel driver abx500
> +
> +
> +Supported chips:
> +  * ST-Ericsson ABx500 series
> +Prefix: 'abx500'
> +Addresses scanned: -
> +Datasheet: http://www.stericsson.com/developers/documentation.jsp
> +
> +Authors:
> +Martin Persson 
> +Hongbo Zhang 
> +
> +Description
> +---
> +
> +Every ST-Ericsson Ux500 SOC consists of both ABx500 and DBx500 physically,
> +this is kernel hwmon driver for ABx500.
> +
> +There are some GPADCs inside ABx500 which are designed for connecting to
> +thermal sensors, and there is also a thermal sensor inside ABx500 too, which
> +raises interrupt when critical temperature reached.
> +
> +This abx500 is a common layer which can monitor all of the sensors, every
> +specific abx500 chip has its special configurations in its own file, e.g. 
> some
> +sensors can be configured invisible if they are not available on that chip.

Given that limits are disabled if set to 0, you really should document that
here.

> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 32f238f..0a6fd21 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -39,6 +39,19 @@ config HWMON_DEBUG_CHIP
>  
>  comment "Native drivers"
>  
> +config SENSORS_AB8500
> + tristate "AB8500 thermal monitoring"
> + depends on AB8500_GPADC
> + default n
> + help
> +   If you say yes here you get support for the thermal sensor part
> +   of the AB8500 chip. The driver includes thermal management for
> +   AB8500 die and two GPADC channels. The GPADC channel are preferably
> +   used to access sensors outside the AB8500 chip.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called abx500-temp.
> +
>  config SENSORS_ABITUGURU
>   tristate "Abit uGuru (rev 1 & 2)"
>   depends on X86 && DMI
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 5da2874..06dfe85 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_SENSORS_W83795)+= w83795.o
>  obj-$(CONFIG_SENSORS_W83781D)+= w83781d.o
>  obj-$(CONFIG_SENSORS_W83791D)+= w83791d.o
>  
> +obj-$(CONFIG_SENSORS_AB8500) += abx500.o ab8500.o
>  obj-$(CONFIG_SENSORS_ABITUGURU)  += abituguru.o
>  obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
>  obj-$(CONFIG_SENSORS_AD7314) += ad7314.o
> diff --git a/drivers/hwmon/ab8500.c b/drivers/hwmon/ab8500.c
> new file mode 100644
> index 000..33221e7
> --- /dev/null
> +++ b/drivers/hwmon/ab8500.c
> @@ -0,0 +1,178 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010

2010 - 1013 ?

> + * Author: Martin Persson 
> + * Hongbo Zhang 
> + * License Terms: GNU General Public License v2
> + *
> + * If/when the AB8500 thermal warning temperature is reached (threshold 
> cannot
> + * be changed b