Re: [NEW DRIVER V4 6/7] DA9058 HWMON driver

2013-04-12 Thread Guenter Roeck
On Fri, Apr 12, 2013 at 05:53:48PM +0200, Lars-Peter Clausen wrote:
> On 04/12/2013 03:05 PM, Anthony Olech wrote:
> > This is the HWMON component driver of the Dialog DA9058 PMIC.
> > This driver is just one component of the whole DA9058 PMIC driver.
> > It depends on the CORE and ADC component drivers of the DA9058 MFD.
> > 
> > Signed-off-by: Anthony Olech 
> > Signed-off-by: David Dajun Chen 
> 
> Hi,
> 
> can't you use the generic IIO to HWMON bridge driver? And if not it's probably
> better to extent the bridge driver than writing this custom driver.
> 
I like that idea.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [NEW DRIVER V4 6/7] DA9058 HWMON driver

2013-04-12 Thread Lars-Peter Clausen
On 04/12/2013 03:05 PM, Anthony Olech wrote:
> This is the HWMON component driver of the Dialog DA9058 PMIC.
> This driver is just one component of the whole DA9058 PMIC driver.
> It depends on the CORE and ADC component drivers of the DA9058 MFD.
> 
> Signed-off-by: Anthony Olech 
> Signed-off-by: David Dajun Chen 

Hi,

can't you use the generic IIO to HWMON bridge driver? And if not it's probably
better to extent the bridge driver than writing this custom driver.

- Lars


> ---
>  Documentation/hwmon/da9058   |   38 +
>  drivers/hwmon/Kconfig|   10 ++
>  drivers/hwmon/Makefile   |3 +-
>  drivers/hwmon/da9058-hwmon.c |  341 
> ++
>  4 files changed, 391 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/hwmon/da9058
>  create mode 100644 drivers/hwmon/da9058-hwmon.c
> 
> diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058
> new file mode 100644
> index 000..841148f
> --- /dev/null
> +++ b/Documentation/hwmon/da9058
> @@ -0,0 +1,38 @@
> +Kernel driver da9058-hwmon
> +==
> +
> +Supported chips:
> +  * Dialog Semiconductor DA9058 PMIC
> +Prefix: 'da9058'
> +Datasheet:
> + http://www.dialog-semiconductor.com/products/power-management/da9058
> +
> +Authors: Opensource [Anthony Olech] 
> +
> +Description
> +---
> +
> +The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a
> +range of system operating parameters, including the battery voltage and
> +temperature.  The ADC measures voltage, but two of the ADC channels can
> +be configured to supply a current, so that if an NTC termister is connected
> +then the voltage reading can be converted to a temperature. Currently the
> +driver provides reporting of all the input values but does not provide any
> +alarms.
> +
> +Voltage Monitoring
> +--
> +
> +Voltages are sampled in either 'automatic' or 'manual' mode, which is an
> +initialization parameter set in the platform data by the machine driver.
> +In manual mode the ADC conversion is 12 bit and in automatic mode it is
> +10 bit. However all the raw readings are reported as 12 bit numbers.
> +
> +Physical Limits
> +---
> +
> +vbat   2500 - 4500 milliVolts
> +tbat   0- 2500 milliVolts
> +adc0- 2500 milliVolts
> +vfpin  0- 4095 milliVolts
> +tjunc  there is a correction factor programmed during manufacturing
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 89ac1cb..9a3f226 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -304,6 +304,16 @@ config SENSORS_ATXP1
> This driver can also be built as a module.  If so, the module
> will be called atxp1.
>  
> +config SENSORS_DA9058
> + tristate "Dialog Semiconductor DA9058 ADC"
> + depends on MFD_DA9058 && DA9058_ADC
> + help
> +   If you say yes here you get support for the hardware monitoring
> +   functionality of the Dialog Semiconductor DA9058 PMIC.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called da9058-hwmon.
> +
>  config SENSORS_DS620
>   tristate "Dallas Semiconductor DS620"
>   depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8d6d97e..c3b103b 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -44,7 +44,8 @@ obj-$(CONFIG_SENSORS_ASC7621)   += asc7621.o
>  obj-$(CONFIG_SENSORS_ATXP1)  += atxp1.o
>  obj-$(CONFIG_SENSORS_CORETEMP)   += coretemp.o
>  obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> -obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> +obj-$(CONFIG_SENSORS_DA9055) += da9055-hwmon.o
> +obj-$(CONFIG_SENSORS_DA9058) += da9058-hwmon.o
>  obj-$(CONFIG_SENSORS_DME1737)+= dme1737.o
>  obj-$(CONFIG_SENSORS_DS620)  += ds620.o
>  obj-$(CONFIG_SENSORS_DS1621) += ds1621.o
> diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c
> new file mode 100644
> index 000..3859b16
> --- /dev/null
> +++ b/drivers/hwmon/da9058-hwmon.c
> @@ -0,0 +1,341 @@
> +/*
> + *  Copyright (C) 2012 Dialog Semiconductor Ltd.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static ssize_t da9058_vbat_show_adc(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
> + int voltage; /* x000 .. xFFF = 2500 .. 4500 mV */
> + int ret;
> +
> + ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VBAT,
> + hwmon->use_automatic_adc, &vol

Re: [NEW DRIVER V4 6/7] DA9058 HWMON driver

2013-04-12 Thread Guenter Roeck
On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> This is the HWMON component driver of the Dialog DA9058 PMIC.
> This driver is just one component of the whole DA9058 PMIC driver.
> It depends on the CORE and ADC component drivers of the DA9058 MFD.
> 
> Signed-off-by: Anthony Olech 
> Signed-off-by: David Dajun Chen 
> ---

No changelog ? That makes it unnecessarily difficult to review the driver,
especially since a cursory glance suggests that you introduced new problems
since V3. What is the point of calling devm_kfree() ? Why bother with
devm_kzalloc() if you ignore its benefits anyway ?

Anyway, I don't have time right now to re-review the entire thing as I'll have
to pull my comments from the previous version, compare and figure out what you
changed and what you didn't change. So this will have to wait a bit.

Guenter

>  Documentation/hwmon/da9058   |   38 +
>  drivers/hwmon/Kconfig|   10 ++
>  drivers/hwmon/Makefile   |3 +-
>  drivers/hwmon/da9058-hwmon.c |  341 
> ++
>  4 files changed, 391 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/hwmon/da9058
>  create mode 100644 drivers/hwmon/da9058-hwmon.c
> 
> diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058
> new file mode 100644
> index 000..841148f
> --- /dev/null
> +++ b/Documentation/hwmon/da9058
> @@ -0,0 +1,38 @@
> +Kernel driver da9058-hwmon
> +==
> +
> +Supported chips:
> +  * Dialog Semiconductor DA9058 PMIC
> +Prefix: 'da9058'
> +Datasheet:
> + http://www.dialog-semiconductor.com/products/power-management/da9058
> +
> +Authors: Opensource [Anthony Olech] 
> +
> +Description
> +---
> +
> +The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a
> +range of system operating parameters, including the battery voltage and
> +temperature.  The ADC measures voltage, but two of the ADC channels can
> +be configured to supply a current, so that if an NTC termister is connected
> +then the voltage reading can be converted to a temperature. Currently the
> +driver provides reporting of all the input values but does not provide any
> +alarms.
> +
> +Voltage Monitoring
> +--
> +
> +Voltages are sampled in either 'automatic' or 'manual' mode, which is an
> +initialization parameter set in the platform data by the machine driver.
> +In manual mode the ADC conversion is 12 bit and in automatic mode it is
> +10 bit. However all the raw readings are reported as 12 bit numbers.
> +
> +Physical Limits
> +---
> +
> +vbat   2500 - 4500 milliVolts
> +tbat   0- 2500 milliVolts
> +adc0- 2500 milliVolts
> +vfpin  0- 4095 milliVolts
> +tjunc  there is a correction factor programmed during manufacturing
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 89ac1cb..9a3f226 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -304,6 +304,16 @@ config SENSORS_ATXP1
> This driver can also be built as a module.  If so, the module
> will be called atxp1.
>  
> +config SENSORS_DA9058
> + tristate "Dialog Semiconductor DA9058 ADC"
> + depends on MFD_DA9058 && DA9058_ADC
> + help
> +   If you say yes here you get support for the hardware monitoring
> +   functionality of the Dialog Semiconductor DA9058 PMIC.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called da9058-hwmon.
> +
>  config SENSORS_DS620
>   tristate "Dallas Semiconductor DS620"
>   depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8d6d97e..c3b103b 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -44,7 +44,8 @@ obj-$(CONFIG_SENSORS_ASC7621)   += asc7621.o
>  obj-$(CONFIG_SENSORS_ATXP1)  += atxp1.o
>  obj-$(CONFIG_SENSORS_CORETEMP)   += coretemp.o
>  obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> -obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> +obj-$(CONFIG_SENSORS_DA9055) += da9055-hwmon.o
> +obj-$(CONFIG_SENSORS_DA9058) += da9058-hwmon.o
>  obj-$(CONFIG_SENSORS_DME1737)+= dme1737.o
>  obj-$(CONFIG_SENSORS_DS620)  += ds620.o
>  obj-$(CONFIG_SENSORS_DS1621) += ds1621.o
> diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c
> new file mode 100644
> index 000..3859b16
> --- /dev/null
> +++ b/drivers/hwmon/da9058-hwmon.c
> @@ -0,0 +1,341 @@
> +/*
> + *  Copyright (C) 2012 Dialog Semiconductor Ltd.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static ssize_t da9058_vbat_show_adc(struct device *d

[NEW DRIVER V4 6/7] DA9058 HWMON driver

2013-04-12 Thread Anthony Olech
This is the HWMON component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the CORE and ADC component drivers of the DA9058 MFD.

Signed-off-by: Anthony Olech 
Signed-off-by: David Dajun Chen 
---
 Documentation/hwmon/da9058   |   38 +
 drivers/hwmon/Kconfig|   10 ++
 drivers/hwmon/Makefile   |3 +-
 drivers/hwmon/da9058-hwmon.c |  341 ++
 4 files changed, 391 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/hwmon/da9058
 create mode 100644 drivers/hwmon/da9058-hwmon.c

diff --git a/Documentation/hwmon/da9058 b/Documentation/hwmon/da9058
new file mode 100644
index 000..841148f
--- /dev/null
+++ b/Documentation/hwmon/da9058
@@ -0,0 +1,38 @@
+Kernel driver da9058-hwmon
+==
+
+Supported chips:
+  * Dialog Semiconductor DA9058 PMIC
+Prefix: 'da9058'
+Datasheet:
+   http://www.dialog-semiconductor.com/products/power-management/da9058
+
+Authors: Opensource [Anthony Olech] 
+
+Description
+---
+
+The DA9058 PMIC contains a 5 channel ADC which can be used to monitor a
+range of system operating parameters, including the battery voltage and
+temperature.  The ADC measures voltage, but two of the ADC channels can
+be configured to supply a current, so that if an NTC termister is connected
+then the voltage reading can be converted to a temperature. Currently the
+driver provides reporting of all the input values but does not provide any
+alarms.
+
+Voltage Monitoring
+--
+
+Voltages are sampled in either 'automatic' or 'manual' mode, which is an
+initialization parameter set in the platform data by the machine driver.
+In manual mode the ADC conversion is 12 bit and in automatic mode it is
+10 bit. However all the raw readings are reported as 12 bit numbers.
+
+Physical Limits
+---
+
+vbat   2500 - 4500 milliVolts
+tbat   0- 2500 milliVolts
+adc0- 2500 milliVolts
+vfpin  0- 4095 milliVolts
+tjunc  there is a correction factor programmed during manufacturing
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 89ac1cb..9a3f226 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -304,6 +304,16 @@ config SENSORS_ATXP1
  This driver can also be built as a module.  If so, the module
  will be called atxp1.
 
+config SENSORS_DA9058
+   tristate "Dialog Semiconductor DA9058 ADC"
+   depends on MFD_DA9058 && DA9058_ADC
+   help
+ If you say yes here you get support for the hardware monitoring
+ functionality of the Dialog Semiconductor DA9058 PMIC.
+
+ This driver can also be built as a module.  If so, the module
+ will be called da9058-hwmon.
+
 config SENSORS_DS620
tristate "Dallas Semiconductor DS620"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 8d6d97e..c3b103b 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -44,7 +44,8 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
 obj-$(CONFIG_SENSORS_ATXP1)+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
 obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
-obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DA9055)   += da9055-hwmon.o
+obj-$(CONFIG_SENSORS_DA9058)   += da9058-hwmon.o
 obj-$(CONFIG_SENSORS_DME1737)  += dme1737.o
 obj-$(CONFIG_SENSORS_DS620)+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)   += ds1621.o
diff --git a/drivers/hwmon/da9058-hwmon.c b/drivers/hwmon/da9058-hwmon.c
new file mode 100644
index 000..3859b16
--- /dev/null
+++ b/drivers/hwmon/da9058-hwmon.c
@@ -0,0 +1,341 @@
+/*
+ *  Copyright (C) 2012 Dialog Semiconductor Ltd.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+static ssize_t da9058_vbat_show_adc(struct device *dev,
+   struct device_attribute *devattr, char *buf)
+{
+   struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+   int voltage; /* x000 .. xFFF = 2500 .. 4500 mV */
+   int ret;
+
+   ret = da9058_adc_read(hwmon->da9058, DA9058_ADCMAN_MUXSEL_VBAT,
+   hwmon->use_automatic_adc, &voltage);
+   if (ret)
+   return ret;
+
+   return sprintf(buf, "%d\n", 2500 + voltage * 2000 / 0xFFF);
+}
+
+static ssize_t da9058_tbat_show_type(struct device *dev,
+   struct device_attribute *devattr, char *buf)
+{
+   struct da9058_hwmon *hwmon = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", hwmon->battery_sensor_type);
+}
+
+static ssize_t da9058_tbat_show_adc(struct devic