Hi,
This is PATCHv1 for converting rx51-battery to the IIO API
and adding DT support. The patchset compiles and has been
tested on my Nokia N900. It depends on another patchset
converting twl4030-madc to the IIO API:
https://lkml.org/lkml/2014/2/25/627
-- Sebastian
Sebastian Reichel (2):
rx51_battery: convert to iio consumer
Documentation: DT: Document rx51-battery binding
.../devicetree/bindings/power/rx51-battery.txt | 25 ++++++++
drivers/power/rx51_battery.c | 68 ++++++++++++++--------
2 files changed, 70 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt
--
1.8.5.3
Update rx51-battery driver to use the new IIO API of
twl4030-madc and add DT support.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
index 1bc5857..f7cb58e 100644
--- a/drivers/power/rx51_battery.c
+++ b/drivers/power/rx51_battery.c
@@ -24,34 +24,27 @@
#include <linux/power_supply.h>
#include <linux/slab.h>
#include <linux/i2c/twl4030-madc.h>
-
-/* RX51 specific channels */
-#define TWL4030_MADC_BTEMP_RX51 TWL4030_MADC_ADCIN0
-#define TWL4030_MADC_BCI_RX51 TWL4030_MADC_ADCIN4
+#include <linux/iio/consumer.h>
+#include <linux/of.h>
struct rx51_device_info {
struct device *dev;
struct power_supply bat;
+ struct iio_channel *channel_temp;
+ struct iio_channel *channel_bsi;
+ struct iio_channel *channel_vbat;
};
/*
* Read ADCIN channel value, code copied from maemo kernel
*/
-static int rx51_battery_read_adc(int channel)
+static int rx51_battery_read_adc(struct iio_channel *channel)
{
- struct twl4030_madc_request req;
-
- req.channels = channel;
- req.do_avg = 1;
- req.method = TWL4030_MADC_SW1;
- req.func_cb = NULL;
- req.type = TWL4030_MADC_WAIT;
- req.raw = true;
-
- if (twl4030_madc_conversion(&req) <= 0)
- return -ENODATA;
-
- return req.rbuf[ffs(channel) - 1];
+ int val, err;
+ err = iio_read_channel_average_raw(channel, &val);
+ if (err < 0)
+ return err;
+ return val;
}
/*
@@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
*/
static int rx51_battery_read_voltage(struct rx51_device_info *di)
{
- int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
+ int voltage = rx51_battery_read_adc(di->channel_vbat);
- if (voltage < 0)
+ if (voltage < 0) {
+ dev_err(di->dev, "Could not read ADC: %d\n", voltage);
return voltage;
+ }
return 1000 * (10000 * voltage / 1705);
}
@@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
{
int min = 0;
int max = ARRAY_SIZE(rx51_temp_table2) - 1;
- int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
+ int raw = rx51_battery_read_adc(di->channel_temp);
+
+ if (raw < 0)
+ dev_err(di->dev, "Could not read ADC: %d\n", raw);
/* Zero and negative values are undefined */
if (raw <= 0)
@@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
*/
static int rx51_battery_read_capacity(struct rx51_device_info *di)
{
- int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
+ int capacity = rx51_battery_read_adc(di->channel_bsi);
- if (capacity < 0)
+ if (capacity < 0) {
+ dev_err(di->dev, "Could not read ADC: %d\n", capacity);
return capacity;
+ }
return 1280 * (1200 * capacity)/(1024 - capacity);
}
@@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, di);
+ di->dev = &pdev->dev;
di->bat.name = dev_name(&pdev->dev);
di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
di->bat.properties = rx51_battery_props;
di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
di->bat.get_property = rx51_battery_get_property;
+ di->channel_temp = iio_channel_get(di->dev, "temp");
+ if (IS_ERR(di->channel_temp))
+ return PTR_ERR(di->channel_temp);
+
+ di->channel_bsi = iio_channel_get(di->dev, "bsi");
+ if (IS_ERR(di->channel_bsi))
+ return PTR_ERR(di->channel_bsi);
+
+ di->channel_vbat = iio_channel_get(di->dev, "vbat");
+ if (IS_ERR(di->channel_vbat))
+ return PTR_ERR(di->channel_vbat);
+
ret = power_supply_register(di->dev, &di->bat);
if (ret)
return ret;
@@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_OF
+static const struct of_device_id n900_battery_of_match[] = {
+ {.compatible = "nokia,n900-battery", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, n900_battery_of_match);
+#endif
+
static struct platform_driver rx51_battery_driver = {
.probe = rx51_battery_probe,
.remove = rx51_battery_remove,
.driver = {
.name = "rx51-battery",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(n900_battery_of_match),
},
};
module_platform_driver(rx51_battery_driver);
--
1.8.5.3
Add devicetree binding documentation for rx51-battery,
which is a simple A/D converter consumer.
Signed-off-by: Sebastian Reichel <[email protected]>
---
.../devicetree/bindings/power/rx51-battery.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt
diff --git a/Documentation/devicetree/bindings/power/rx51-battery.txt b/Documentation/devicetree/bindings/power/rx51-battery.txt
new file mode 100644
index 0000000..9043845
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/rx51-battery.txt
@@ -0,0 +1,25 @@
+Binding for Nokia N900 battery
+
+The Nokia N900 battery status can be read via the TWL4030's A/D converter.
+
+Required properties:
+- compatible: Should contain one of the following:
+ * "nokia,n900-battery"
+- io-channels: Should contain IIO channel specifiers
+ for each element in io-channel-names.
+- io-channel-names: Should contain the following values:
+ * "temp" - The ADC channel for temperature reading
+ * "bsi" - The ADC channel for battery size identification
+ * "vbat" - The ADC channel to measure the battery voltage
+
+Example from Nokia N900:
+
+battery: n900-battery {
+ compatible = "nokia,n900-battery";
+ io-channels = <&twl4030_madc 0>,
+ <&twl4030_madc 4>,
+ <&twl4030_madc 12>;
+ io-channel-names = "temp",
+ "bsi",
+ "vbat";
+};
--
1.8.5.3
Hi!
2014-02-26 1:46 GMT+01:00 Sebastian Reichel <[email protected]>:
> Hi,
>
> This is PATCHv1 for converting rx51-battery to the IIO API
> and adding DT support. The patchset compiles and has been
> tested on my Nokia N900. It depends on another patchset
> converting twl4030-madc to the IIO API:
>
> https://lkml.org/lkml/2014/2/25/627
>
> -- Sebastian
>
> Sebastian Reichel (2):
> rx51_battery: convert to iio consumer
> Documentation: DT: Document rx51-battery binding
>
> .../devicetree/bindings/power/rx51-battery.txt | 25 ++++++++
> drivers/power/rx51_battery.c | 68 ++++++++++++++--------
> 2 files changed, 70 insertions(+), 23 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/power/rx51-battery.txt
>
> --
> 1.8.5.3
>
Thanks for patch!
I would like to ask other kernel developers what do you think about
moving ADC channel numbers from rx51_battery.ko driver code to DT.
Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
it is usefull only for this one device.
Before this patch all driver data (look-up tables, adc channel
numbers, etc...) were in driver code. Now after this patch adc channel
numbers were moved to DT. What do you think? It is better to have all
data in one place (driver code) or some in DT and some in driver code?
For me it does not make sense to move these numbers to DT, because
driver is rx51 device specific and chaning it in DT does not make
sense. And I think it is better to have add driver data in one place
and not in two...
Sebastian already wrote me that this is normal to have numbers in DT
and other code in driver. But I think that driver which can be used
only in one device (so specified only in one DT file) does not need to
have configuration (via DT or board files).
Or do you think that driver specified only for one device needs to
have ADC numbers configuration via DT?
--
Pali Rohár
[email protected]
Hi Pali,
On Wed, Feb 26, 2014 at 08:40:54AM +0100, Pali Roh?r wrote:
> I would like to ask other kernel developers what do you think about
> moving ADC channel numbers from rx51_battery.ko driver code to DT.
> Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
> it is usefull only for this one device.
>
> Before this patch all driver data (look-up tables, adc channel
> numbers, etc...) were in driver code. Now after this patch adc channel
> numbers were moved to DT. What do you think? It is better to have all
> data in one place (driver code) or some in DT and some in driver code?
>
> For me it does not make sense to move these numbers to DT, because
> driver is rx51 device specific and chaning it in DT does not make
> sense. And I think it is better to have add driver data in one place
> and not in two...
>
> Sebastian already wrote me that this is normal to have numbers in DT
> and other code in driver. But I think that driver which can be used
> only in one device (so specified only in one DT file) does not need to
> have configuration (via DT or board files).
>
> Or do you think that driver specified only for one device needs to
> have ADC numbers configuration via DT?
I think the problem is, that you think of ADC channel numbers as
configuration data. This means you think of rx51-battery as an
alternative driver for the twl4030-madc.
I think of rx51-battery as its own platform device, which makes use
of the ADC similar to a button making use of a GPIO chip.
For me the ADC channel numbers are not configuration data, but an
inter-device resource reference, like e.g. GPIO references.
This is exactly the data you would put into the device tree.
Now let's take the rx51-audio device as another example for an n900
specific device (not yet in mainline kernel). It does not need ADC
channels, but GPIO lines:
sound: n900-audio {
/* ... some more references ... */
nokia,tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
nokia,jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
nokia,eci-switch-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
nokia,speaker-amplifier-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
};
Since GPIO numbers are not guaranteed to be consistent in DT boot
mode at least the GPIO chip must be referenced. Do you really think
it's a good idea to leave out the exact GPIO number just because
the driver knows it needs the 8th GPIO from the second GPIO chip?
IMHO this is really ugly, since it splits the information, which
GPIO is used into two parts - one living in the DT and one living
in the driver with no advantage at all. So it does make sense to
specify inter-device resources via DT even for platform specific
devices.
-- Sebastian
Hi Sebastian,
On Wed, Feb 26, 2014 at 1:46 AM, Sebastian Reichel <[email protected]> wrote:
> Update rx51-battery driver to use the new IIO API of
> twl4030-madc and add DT support.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 1bc5857..f7cb58e 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -24,34 +24,27 @@
> #include <linux/power_supply.h>
> #include <linux/slab.h>
> #include <linux/i2c/twl4030-madc.h>
> -
> -/* RX51 specific channels */
> -#define TWL4030_MADC_BTEMP_RX51 TWL4030_MADC_ADCIN0
> -#define TWL4030_MADC_BCI_RX51 TWL4030_MADC_ADCIN4
> +#include <linux/iio/consumer.h>
> +#include <linux/of.h>
>
> struct rx51_device_info {
> struct device *dev;
> struct power_supply bat;
> + struct iio_channel *channel_temp;
> + struct iio_channel *channel_bsi;
> + struct iio_channel *channel_vbat;
> };
>
> /*
> * Read ADCIN channel value, code copied from maemo kernel
> */
> -static int rx51_battery_read_adc(int channel)
> +static int rx51_battery_read_adc(struct iio_channel *channel)
> {
> - struct twl4030_madc_request req;
> -
> - req.channels = channel;
> - req.do_avg = 1;
> - req.method = TWL4030_MADC_SW1;
> - req.func_cb = NULL;
> - req.type = TWL4030_MADC_WAIT;
> - req.raw = true;
> -
> - if (twl4030_madc_conversion(&req) <= 0)
> - return -ENODATA;
> -
> - return req.rbuf[ffs(channel) - 1];
> + int val, err;
> + err = iio_read_channel_average_raw(channel, &val);
Where this function comes from? I cannot find it in current linux-next
(only iio_read_channel_raw()).
Am I missing some patches? Thx. BTW when I convert to iio consumer and
use DT some of values work
but some of them just report 0 :( (I don't have latest twl4030-madc
iio patches).
> + if (err < 0)
> + return err;
> + return val;
> }
>
> /*
> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
> */
> static int rx51_battery_read_voltage(struct rx51_device_info *di)
> {
> - int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
> + int voltage = rx51_battery_read_adc(di->channel_vbat);
>
> - if (voltage < 0)
> + if (voltage < 0) {
> + dev_err(di->dev, "Could not read ADC: %d\n", voltage);
> return voltage;
> + }
>
> return 1000 * (10000 * voltage / 1705);
> }
> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
> {
> int min = 0;
> int max = ARRAY_SIZE(rx51_temp_table2) - 1;
> - int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
> + int raw = rx51_battery_read_adc(di->channel_temp);
> +
> + if (raw < 0)
> + dev_err(di->dev, "Could not read ADC: %d\n", raw);
>
> /* Zero and negative values are undefined */
> if (raw <= 0)
> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
> */
> static int rx51_battery_read_capacity(struct rx51_device_info *di)
> {
> - int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
> + int capacity = rx51_battery_read_adc(di->channel_bsi);
>
> - if (capacity < 0)
> + if (capacity < 0) {
> + dev_err(di->dev, "Could not read ADC: %d\n", capacity);
> return capacity;
> + }
>
> return 1280 * (1200 * capacity)/(1024 - capacity);
> }
> @@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, di);
>
> + di->dev = &pdev->dev;
> di->bat.name = dev_name(&pdev->dev);
> di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
> di->bat.properties = rx51_battery_props;
> di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
> di->bat.get_property = rx51_battery_get_property;
>
> + di->channel_temp = iio_channel_get(di->dev, "temp");
> + if (IS_ERR(di->channel_temp))
> + return PTR_ERR(di->channel_temp);
> +
> + di->channel_bsi = iio_channel_get(di->dev, "bsi");
> + if (IS_ERR(di->channel_bsi))
> + return PTR_ERR(di->channel_bsi);
> +
> + di->channel_vbat = iio_channel_get(di->dev, "vbat");
> + if (IS_ERR(di->channel_vbat))
> + return PTR_ERR(di->channel_vbat);
> +
> ret = power_supply_register(di->dev, &di->bat);
> if (ret)
> return ret;
> @@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id n900_battery_of_match[] = {
> + {.compatible = "nokia,n900-battery", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
> +#endif
> +
> static struct platform_driver rx51_battery_driver = {
> .probe = rx51_battery_probe,
> .remove = rx51_battery_remove,
> .driver = {
> .name = "rx51-battery",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(n900_battery_of_match),
> },
> };
> module_platform_driver(rx51_battery_driver);
> --
> 1.8.5.3
>
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
Hi,
On Wed, Feb 26, 2014 at 10:43:40PM +0100, Belisko Marek wrote:
> [...]
> > + int val, err;
> > + err = iio_read_channel_average_raw(channel, &val);
> Where this function comes from? I cannot find it in current linux-next
> (only iio_read_channel_raw()). Am I missing some patches? Thx.
Ah right. I planned to send a patch adding this function together
with the rx51-battery patchset, but it seems I forgot to include it.
Sorry for the inconvenience. I will send it out as a separate patch
now.
> BTW when I convert to iio consumer and use DT some of values work
> but some of them just report 0 :( (I don't have latest twl4030-madc
> iio patches).
Can you retry with the twl4030-madc iio patch from today? The
older patchsets, which do not contain the "tested on real hw"
note are slightly broken.
-- Sebastian
Hi Sebastian,
On Wed, Feb 26, 2014 at 10:54 PM, Sebastian Reichel <[email protected]> wrote:
> Hi,
>
> On Wed, Feb 26, 2014 at 10:43:40PM +0100, Belisko Marek wrote:
>> [...]
>> > + int val, err;
>> > + err = iio_read_channel_average_raw(channel, &val);
>> Where this function comes from? I cannot find it in current linux-next
>> (only iio_read_channel_raw()). Am I missing some patches? Thx.
>
> Ah right. I planned to send a patch adding this function together
> with the rx51-battery patchset, but it seems I forgot to include it.
>
> Sorry for the inconvenience. I will send it out as a separate patch
> now.
>
>> BTW when I convert to iio consumer and use DT some of values work
>> but some of them just report 0 :( (I don't have latest twl4030-madc
>> iio patches).
>
> Can you retry with the twl4030-madc iio patch from today? The
> older patchsets, which do not contain the "tested on real hw"
> note are slightly broken.
Well I've tried and it's worse :). I got during booting:
[ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
[ 2.224639] platform battery.4: Driver twl4030_madc_battery
requests probe deferral
Not sure if it's just error or warning but temp is always reported as
0 (and also other values in sysfs).
My patches ca be found here:
https://patchwork.kernel.org/patch/3735981/
https://patchwork.kernel.org/patch/3735961/
https://patchwork.kernel.org/patch/3735941/
>
> -- Sebastian
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
> Well I've tried and it's worse :). I got during booting:
> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
> requests probe deferral
> Not sure if it's just error or warning but temp is always reported as
> 0 (and also other values in sysfs).
This is an error, which basically means, that twl4030-madc has not
yet been loaded. Do you get proper values when you use the old madc
API with the patchset applied?
-- Sebastian
Hi Sebastian,
On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <[email protected]> wrote:
> On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>> Well I've tried and it's worse :). I got during booting:
>> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
>> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
>> requests probe deferral
>> Not sure if it's just error or warning but temp is always reported as
>> 0 (and also other values in sysfs).
>
> This is an error, which basically means, that twl4030-madc has not
> yet been loaded. Do you get proper values when you use the old madc
> API with the patchset applied?
It works without converting to iio consumer (at least I get some
reasonable values).
With conversion it fails with above error. I recheck (add printk to
iio twl4030-madc) that
madc driver is loaded. Could this be that twl4030_madc_battery is
loaded earlier then
twl4030_madc and than it fails to get iio channels?
>
> -- Sebastian
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
On Fri, Feb 28, 2014 at 9:32 PM, Belisko Marek <[email protected]> wrote:
> Hi Sebastian,
>
> On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <[email protected]> wrote:
>> On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>>> Well I've tried and it's worse :). I got during booting:
>>> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
>>> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
>>> requests probe deferral
>>> Not sure if it's just error or warning but temp is always reported as
>>> 0 (and also other values in sysfs).
>>
>> This is an error, which basically means, that twl4030-madc has not
>> yet been loaded. Do you get proper values when you use the old madc
>> API with the patchset applied?
> It works without converting to iio consumer (at least I get some
> reasonable values).
> With conversion it fails with above error. I recheck (add printk to
> iio twl4030-madc) that
> madc driver is loaded. Could this be that twl4030_madc_battery is
> loaded earlier then
> twl4030_madc and than it fails to get iio channels?
Hmm I wasn't far away from truth in previous email. I think that channel doesn't
exists because twl4030_madc driver isn't loaded yet and that is the
reason for deferral probe
message in log. So it seems after some time it is loaded correctly but
not working. No ideas.
>>
>> -- Sebastian
>
> BR,
>
> marek
>
> --
> as simple and primitive as possible
> -------------------------------------------------
> Marek Belisko - OPEN-NANDRA
> Freelance Developer
>
> Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
> Tel: +421 915 052 184
> skype: marekwhite
> twitter: #opennandra
> web: http://open-nandra.com
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
On Fri, Feb 28, 2014 at 09:32:12PM +0100, Belisko Marek wrote:
> On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <[email protected]> wrote:
> > On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
> >> Well I've tried and it's worse :). I got during booting:
> >> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
> >> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
> >> requests probe deferral
> >> Not sure if it's just error or warning but temp is always reported as
> >> 0 (and also other values in sysfs).
> >
> > This is an error, which basically means, that twl4030-madc has not
> > yet been loaded. Do you get proper values when you use the old madc
> > API with the patchset applied?
>
> It works without converting to iio consumer (at least I get some
> reasonable values). With conversion it fails with above error. I
> recheck (add printk to iio twl4030-madc) that madc driver is
> loaded. Could this be that twl4030_madc_battery is loaded earlier
> then twl4030_madc and than it fails to get iio channels?
The error above implies, that twl4030-madc has not been loaded when
twl4030-madc-battery was loaded. This iio_channel_get() fails and
returns -EPROBE_DEFER. This results in twl4030-madc-battery probe
function returning -EPROBE_DEFER. Thus you can simply ignore the
error if the twl4030-madc-battery driver is loaded later.
I guess the easiest way to debug the problem is adding some
dev_dbg() at the start of twl4030_madc_conversion(), which
prints out the entries of twl4030_madc_request. Currently
the IIO API simply calls twl4030_madc_request(), so you
should be able to find out the difference.
Also: Can you post you DTS? I use the following for Nokia N900:
/ {
battery: n900-battery {
compatible = "nokia,n900-battery";
io-channels = <&twl_madc 0>, <&twl_madc 4>, <&twl_madc 12>;
io-channel-names = "temp", "bsi", "vbat";
};
};
&twl {
twl_madc: madc {
compatible = "ti,twl4030-madc";
interrupts = <3>;
#io-channel-cells = <1>;
};
};
-- Sebastian
On Fri, Feb 28, 2014 at 10:08 PM, Sebastian Reichel <[email protected]> wrote:
> On Fri, Feb 28, 2014 at 09:32:12PM +0100, Belisko Marek wrote:
>> On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <[email protected]> wrote:
>> > On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>> >> Well I've tried and it's worse :). I got during booting:
>> >> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
>> >> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
>> >> requests probe deferral
>> >> Not sure if it's just error or warning but temp is always reported as
>> >> 0 (and also other values in sysfs).
>> >
>> > This is an error, which basically means, that twl4030-madc has not
>> > yet been loaded. Do you get proper values when you use the old madc
>> > API with the patchset applied?
>>
>> It works without converting to iio consumer (at least I get some
>> reasonable values). With conversion it fails with above error. I
>> recheck (add printk to iio twl4030-madc) that madc driver is
>> loaded. Could this be that twl4030_madc_battery is loaded earlier
>> then twl4030_madc and than it fails to get iio channels?
>
> The error above implies, that twl4030-madc has not been loaded when
> twl4030-madc-battery was loaded. This iio_channel_get() fails and
> returns -EPROBE_DEFER. This results in twl4030-madc-battery probe
> function returning -EPROBE_DEFER. Thus you can simply ignore the
> error if the twl4030-madc-battery driver is loaded later.
>
> I guess the easiest way to debug the problem is adding some
> dev_dbg() at the start of twl4030_madc_conversion(), which
> prints out the entries of twl4030_madc_request. Currently
> the IIO API simply calls twl4030_madc_request(), so you
> should be able to find out the difference.
OK I'll try to debug it deeper.
>
> Also: Can you post you DTS? I use the following for Nokia N900:
>
> / {
> battery: n900-battery {
> compatible = "nokia,n900-battery";
> io-channels = <&twl_madc 0>, <&twl_madc 4>, <&twl_madc 12>;
> io-channel-names = "temp", "bsi", "vbat";
> };
> };
>
> &twl {
> twl_madc: madc {
> compatible = "ti,twl4030-madc";
> interrupts = <3>;
> #io-channel-cells = <1>;
> };
> };
I put twl_madc property to twl4030.dtsi as following:
twl_madc: madc {
compatible = "ti,twl4030-madc";
interrupts = <3>;
#io-channel-cells = <1>;
};
and node for battery is:
battery {
compatible = "ti,twl4030-madc-battery";
capacity = <1200000>;
charging-calibration-data = <4200 100
4100 75
4000 55
3900 25
3800 5
3700 2
3600 1
3300 0>;
discharging-calibration-data = <4200 100
4100 95
4000 70
3800 50
3700 10
3600 5
3300 0>;
io-channels = <&twl_madc 1>,
<&twl_madc 10>,
<&twl_madc 12>;
io-channel-names = "temp",
"ichg",
"vbat";
};
>
> -- Sebastian
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
On Fri, Feb 28, 2014 at 10:13 PM, Belisko Marek <[email protected]> wrote:
> On Fri, Feb 28, 2014 at 10:08 PM, Sebastian Reichel <[email protected]> wrote:
>> On Fri, Feb 28, 2014 at 09:32:12PM +0100, Belisko Marek wrote:
>>> On Fri, Feb 28, 2014 at 3:05 AM, Sebastian Reichel <[email protected]> wrote:
>>> > On Thu, Feb 27, 2014 at 10:34:35PM +0100, Belisko Marek wrote:
>>> >> Well I've tried and it's worse :). I got during booting:
>>> >> [ 2.218383] ERROR: could not get IIO channel /battery:temp(0)
>>> >> [ 2.224639] platform battery.4: Driver twl4030_madc_battery
>>> >> requests probe deferral
>>> >> Not sure if it's just error or warning but temp is always reported as
>>> >> 0 (and also other values in sysfs).
>>> >
>>> > This is an error, which basically means, that twl4030-madc has not
>>> > yet been loaded. Do you get proper values when you use the old madc
>>> > API with the patchset applied?
>>>
>>> It works without converting to iio consumer (at least I get some
>>> reasonable values). With conversion it fails with above error. I
>>> recheck (add printk to iio twl4030-madc) that madc driver is
>>> loaded. Could this be that twl4030_madc_battery is loaded earlier
>>> then twl4030_madc and than it fails to get iio channels?
>>
>> The error above implies, that twl4030-madc has not been loaded when
>> twl4030-madc-battery was loaded. This iio_channel_get() fails and
>> returns -EPROBE_DEFER. This results in twl4030-madc-battery probe
>> function returning -EPROBE_DEFER. Thus you can simply ignore the
>> error if the twl4030-madc-battery driver is loaded later.
>>
>> I guess the easiest way to debug the problem is adding some
>> dev_dbg() at the start of twl4030_madc_conversion(), which
>> prints out the entries of twl4030_madc_request. Currently
>> the IIO API simply calls twl4030_madc_request(), so you
>> should be able to find out the difference.
> OK I'll try to debug it deeper.
Seems I found issue. I have missing property ti,system-uses-second-madc-irq
as original twl4030_madc_battery used SW2 but this change doesn't fix completely
the problem. What it fixed completely is change:
- req.raw = !(mask == IIO_CHAN_INFO_PROCESSED);
+ req.raw = 0;//!(mask == IIO_CHAN_INFO_PROCESSED);
>>
>> Also: Can you post you DTS? I use the following for Nokia N900:
>>
>> / {
>> battery: n900-battery {
>> compatible = "nokia,n900-battery";
>> io-channels = <&twl_madc 0>, <&twl_madc 4>, <&twl_madc 12>;
>> io-channel-names = "temp", "bsi", "vbat";
>> };
>> };
>>
>> &twl {
>> twl_madc: madc {
>> compatible = "ti,twl4030-madc";
>> interrupts = <3>;
>> #io-channel-cells = <1>;
>> };
>> };
> I put twl_madc property to twl4030.dtsi as following:
> twl_madc: madc {
> compatible = "ti,twl4030-madc";
> interrupts = <3>;
> #io-channel-cells = <1>;
> };
>
> and node for battery is:
> battery {
> compatible = "ti,twl4030-madc-battery";
> capacity = <1200000>;
> charging-calibration-data = <4200 100
> 4100 75
> 4000 55
> 3900 25
> 3800 5
> 3700 2
> 3600 1
> 3300 0>;
> discharging-calibration-data = <4200 100
> 4100 95
> 4000 70
> 3800 50
> 3700 10
> 3600 5
> 3300 0>;
> io-channels = <&twl_madc 1>,
> <&twl_madc 10>,
> <&twl_madc 12>;
> io-channel-names = "temp",
> "ichg",
> "vbat";
> };
>>
>> -- Sebastian
>
> BR,
>
> marek
>
> --
> as simple and primitive as possible
> -------------------------------------------------
> Marek Belisko - OPEN-NANDRA
> Freelance Developer
>
> Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
> Tel: +421 915 052 184
> skype: marekwhite
> twitter: #opennandra
> web: http://open-nandra.com
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
Hi Marek,
On Fri, Feb 28, 2014 at 11:32:22PM +0100, Belisko Marek wrote:
> Seems I found issue. I have missing property ti,system-uses-second-madc-irq
> as original twl4030_madc_battery used SW2 but this change doesn't fix completely
> the problem.
I remember adding this property because you requested it :) Good to
know, that its really needed.
> What it fixed completely is change:
> - req.raw = !(mask == IIO_CHAN_INFO_PROCESSED);
> + req.raw = 0;//!(mask == IIO_CHAN_INFO_PROCESSED);
ok, that figures it. Instead of changing the twl4030-madc driver
you should change the twl4030-madc-battery driver. You currently
call iio_read_channel_raw(), which gives you raw values. Your
driver wants processed data from twl4030-madc, so you should use
iio_read_channel_processed() instead.
-- Sebastian