[PATCH v2 1/4] iio: inkern: API for reading available iio channel attribute values

[PATCH v2 1/4] iio: inkern: API for reading available iio channel attribute values

[Artur Rojek]

Extend the inkern API with a function for reading available
attribute values of iio channels.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

Changes:

v2: no change

 drivers/iio/inkern.c         | 20 ++++++++++++++++++++
 include/linux/iio/consumer.h | 14 ++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 06ca3f7fcc44..f19dbde3c945 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -733,6 +733,26 @@ static int iio_channel_read_avail(struct iio_channel *chan,
 						 vals, type, length, info);
 }
 
+int iio_read_avail_channel_attribute(struct iio_channel *chan,
+				     const int **vals, int *type, int *length,
+				     enum iio_chan_info_enum attribute)
+{
+	int ret;
+
+	mutex_lock(&chan->indio_dev->info_exist_lock);
+	if (!chan->indio_dev->info) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	ret = iio_channel_read_avail(chan, vals, type, length, attribute);
+err_unlock:
+	mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_avail_channel_attribute);
+
 int iio_read_avail_channel_raw(struct iio_channel *chan,
 			       const int **vals, int *length)
 {
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 9887f4f8e2a8..b2d34831ed7c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -290,6 +290,20 @@ int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
 int iio_read_avail_channel_raw(struct iio_channel *chan,
 			       const int **vals, int *length);
 
+/**
+ * iio_read_avail_channel_attribute() - read available channel attribute values
+ * @chan:		The channel being queried.
+ * @vals:		Available values read back.
+ * @type:		Type of values read back.
+ * @length:		Number of entries in vals.
+ * @attribute:		info attribute to be read back.
+ *
+ * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
+ */
+int iio_read_avail_channel_attribute(struct iio_channel *chan,
+				     const int **vals, int *type, int *length,
+				     enum iio_chan_info_enum attribute);
+
 /**
  * iio_get_channel_type() - get the type of a channel
  * @channel:		The channel being queried.
-- 
2.21.0

[PATCH v2 2/4] iio: inkern: Convert iio_read_avail_channel_raw into a wrapper

[Artur Rojek]

Convert "iio_read_avail_channel_raw" over to a wrapper around
"iio_read_avail_channel_attribute".

With the introduction of "iio_read_avail_channel_attribute",
the necessity of having a separate call to read raw channel values
became redundant.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

Changes:

v2: new patch

 drivers/iio/inkern.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f19dbde3c945..4a5eff3f18bc 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -759,16 +759,8 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
 	int ret;
 	int type;
 
-	mutex_lock(&chan->indio_dev->info_exist_lock);
-	if (!chan->indio_dev->info) {
-		ret = -ENODEV;
-		goto err_unlock;
-	}
-
-	ret = iio_channel_read_avail(chan,
-				     vals, &type, length, IIO_CHAN_INFO_RAW);
-err_unlock:
-	mutex_unlock(&chan->indio_dev->info_exist_lock);
+	ret = iio_read_avail_channel_attribute(chan, vals, &type, length,
+					 IIO_CHAN_INFO_RAW);
 
 	if (ret >= 0 && type != IIO_VAL_INT)
 		/* raw values are assumed to be IIO_VAL_INT */
-- 
2.21.0

[PATCH v2 3/4] dt-bindings: power: supply: Add docs for Ingenic JZ47xx SoCs battery.

[Artur Rojek]

Add documentation for the ingenic-battery driver, used on JZ47xx SoCs.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

Changes:

v2: no change

 .../bindings/power/supply/ingenic,battery.txt | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/ingenic,battery.txt

diff --git a/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
new file mode 100644
index 000000000000..66430bf73815
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
@@ -0,0 +1,31 @@
+* Ingenic JZ47xx battery bindings
+
+Required properties:
+
+- compatible: Must be "ingenic,jz4740-battery".
+- io-channels: phandle and IIO specifier pair to the IIO device.
+  Format described in iio-bindings.txt.
+- monitored-battery: phandle to a "simple-battery" compatible node.
+
+The "monitored-battery" property must be a phandle to a node using the format
+described in battery.txt, with the following properties being required:
+
+- voltage-min-design-microvolt: Drained battery voltage.
+- voltage-max-design-microvolt: Fully charged battery voltage.
+
+Example:
+
+#include <dt-bindings/iio/adc/ingenic,adc.h>
+
+simple_battery: battery {
+	compatible = "simple-battery";
+	voltage-min-design-microvolt = <3600000>;
+	voltage-max-design-microvolt = <4200000>;
+};
+
+ingenic_battery {
+	compatible = "ingenic,jz4740-battery";
+	io-channels = <&adc INGENIC_ADC_BATTERY>;
+	io-channel-names = "battery";
+	monitored-battery = <&simple_battery>;
+};
-- 
2.21.0

[PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Artur Rojek]

Add a driver for battery present on Ingenic JZ47xx SoCs.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

Changes:

v2: - rework the return logic in ingenic_battery_get_property,
    - make index offsets point forward in ingenic_battery_set_scale,
    - fix spacing around scale_raw[best_idx + 1]

 drivers/power/supply/Kconfig           |  11 ++
 drivers/power/supply/Makefile          |   1 +
 drivers/power/supply/ingenic-battery.c | 180 +++++++++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 drivers/power/supply/ingenic-battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index e901b9879e7e..9bfb1637ec28 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -169,6 +169,17 @@ config BATTERY_COLLIE
 	  Say Y to enable support for the battery on the Sharp Zaurus
 	  SL-5500 (collie) models.
 
+config BATTERY_INGENIC
+	tristate "Ingenic JZ47xx SoCs battery driver"
+	depends on MIPS || COMPILE_TEST
+	depends on INGENIC_ADC
+	help
+	  Choose this option if you want to monitor battery status on
+	  Ingenic JZ47xx SoC based devices.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ingenic-battery.
+
 config BATTERY_IPAQ_MICRO
 	tristate "iPAQ Atmel Micro ASIC battery driver"
 	depends on MFD_IPAQ_MICRO
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index b731c2a9b695..9e2c481d0187 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
 obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
 obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
 obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
+obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
 obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
diff --git a/drivers/power/supply/ingenic-battery.c b/drivers/power/supply/ingenic-battery.c
new file mode 100644
index 000000000000..822aee1c7b64
--- /dev/null
+++ b/drivers/power/supply/ingenic-battery.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Battery driver for the Ingenic JZ47xx SoCs
+ * Copyright (c) 2019 Artur Rojek <contact@artur-rojek.eu>
+ *
+ * based on drivers/power/supply/jz4740-battery.c
+ */
+
+#include <linux/iio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/property.h>
+
+struct ingenic_battery {
+	struct device *dev;
+	struct iio_channel *channel;
+	struct power_supply_desc desc;
+	struct power_supply *battery;
+	struct power_supply_battery_info info;
+};
+
+static int ingenic_battery_get_property(struct power_supply *psy,
+					enum power_supply_property psp,
+					union power_supply_propval *val)
+{
+	struct ingenic_battery *bat = power_supply_get_drvdata(psy);
+	struct power_supply_battery_info *info = &bat->info;
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = iio_read_channel_processed(bat->channel, &val->intval);
+		val->intval *= 1000;
+		if (val->intval < info->voltage_min_design_uv)
+			val->intval = POWER_SUPPLY_HEALTH_DEAD;
+		else if (val->intval > info->voltage_max_design_uv)
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		else
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		return ret;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = iio_read_channel_processed(bat->channel, &val->intval);
+		val->intval *= 1000;
+		return ret;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val->intval = info->voltage_min_design_uv;
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		val->intval = info->voltage_max_design_uv;
+		return 0;
+	default:
+		return -EINVAL;
+	};
+}
+
+/* Set the most appropriate IIO channel voltage reference scale
+ * based on the battery's max voltage.
+ */
+static int ingenic_battery_set_scale(struct ingenic_battery *bat)
+{
+	const int *scale_raw;
+	int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
+	u64 max_mV;
+
+	ret = iio_read_max_channel_raw(bat->channel, &max_raw);
+	if (ret) {
+		dev_err(bat->dev, "Unable to read max raw channel value\n");
+		return ret;
+	}
+
+	ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
+					       &scale_type, &scale_len,
+					       IIO_CHAN_INFO_SCALE);
+	if (ret < 0) {
+		dev_err(bat->dev, "Unable to read channel avail scale\n");
+		return ret;
+	}
+	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2)
+		return -EINVAL;
+
+	max_mV = bat->info.voltage_max_design_uv / 1000;
+
+	for (i = 0; i < scale_len; i += 2) {
+		u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
+
+		if (scale_mV < max_mV)
+			continue;
+
+		if (best_idx >= 0 && scale_mV > best_mV)
+			continue;
+
+		best_mV = scale_mV;
+		best_idx = i;
+	}
+
+	if (best_idx < 0) {
+		dev_err(bat->dev, "Unable to find matching voltage scale\n");
+		return -EINVAL;
+	}
+
+	return iio_write_channel_attribute(bat->channel,
+					   scale_raw[best_idx],
+					   scale_raw[best_idx + 1],
+					   IIO_CHAN_INFO_SCALE);
+}
+
+static enum power_supply_property ingenic_battery_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+};
+
+static int ingenic_battery_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ingenic_battery *bat;
+	struct power_supply_config psy_cfg = {};
+	struct power_supply_desc *desc;
+	int ret;
+
+	bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
+	if (!bat)
+		return -ENOMEM;
+
+	bat->dev = dev;
+	bat->channel = devm_iio_channel_get(dev, "battery");
+	if (IS_ERR(bat->channel))
+		return PTR_ERR(bat->channel);
+
+	desc = &bat->desc;
+	desc->name = "jz-battery";
+	desc->type = POWER_SUPPLY_TYPE_BATTERY;
+	desc->properties = ingenic_battery_properties;
+	desc->num_properties = ARRAY_SIZE(ingenic_battery_properties);
+	desc->get_property = ingenic_battery_get_property;
+	psy_cfg.drv_data = bat;
+	psy_cfg.of_node = dev->of_node;
+
+	bat->battery = devm_power_supply_register(dev, desc, &psy_cfg);
+	if (IS_ERR(bat->battery)) {
+		dev_err(dev, "Unable to register battery\n");
+		return PTR_ERR(bat->battery);
+	}
+
+	ret = power_supply_get_battery_info(bat->battery, &bat->info);
+	if (ret) {
+		dev_err(dev, "Unable to get battery info: %d\n", ret);
+		return ret;
+	}
+	if (bat->info.voltage_min_design_uv < 0) {
+		dev_err(dev, "Unable to get voltage min design\n");
+		return bat->info.voltage_min_design_uv;
+	}
+	if (bat->info.voltage_max_design_uv < 0) {
+		dev_err(dev, "Unable to get voltage max design\n");
+		return bat->info.voltage_max_design_uv;
+	}
+
+	return ingenic_battery_set_scale(bat);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ingenic_battery_of_match[] = {
+	{ .compatible = "ingenic,jz4740-battery", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ingenic_battery_of_match);
+#endif
+
+static struct platform_driver ingenic_battery_driver = {
+	.driver = {
+		.name = "ingenic-battery",
+		.of_match_table = of_match_ptr(ingenic_battery_of_match),
+	},
+	.probe = ingenic_battery_probe,
+};
+module_platform_driver(ingenic_battery_driver);
-- 
2.21.0

Re: [PATCH v2 1/4] iio: inkern: API for reading available iio channel attribute values

[Jonathan Cameron]

On Sat, 23 Mar 2019 18:28:06 +0100
Artur Rojek <contact@artur-rojek.eu> wrote:

> Extend the inkern API with a function for reading available
> attribute values of iio channels.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
If this goes through a route other than IIO (otherwise
I'll just add a signed-off-by...)

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> 
> Changes:
> 
> v2: no change
> 
>  drivers/iio/inkern.c         | 20 ++++++++++++++++++++
>  include/linux/iio/consumer.h | 14 ++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 06ca3f7fcc44..f19dbde3c945 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -733,6 +733,26 @@ static int iio_channel_read_avail(struct iio_channel *chan,
>  						 vals, type, length, info);
>  }
>  
> +int iio_read_avail_channel_attribute(struct iio_channel *chan,
> +				     const int **vals, int *type, int *length,
> +				     enum iio_chan_info_enum attribute)
> +{
> +	int ret;
> +
> +	mutex_lock(&chan->indio_dev->info_exist_lock);
> +	if (!chan->indio_dev->info) {
> +		ret = -ENODEV;
> +		goto err_unlock;
> +	}
> +
> +	ret = iio_channel_read_avail(chan, vals, type, length, attribute);
> +err_unlock:
> +	mutex_unlock(&chan->indio_dev->info_exist_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_avail_channel_attribute);
> +
>  int iio_read_avail_channel_raw(struct iio_channel *chan,
>  			       const int **vals, int *length)
>  {
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 9887f4f8e2a8..b2d34831ed7c 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -290,6 +290,20 @@ int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
>  int iio_read_avail_channel_raw(struct iio_channel *chan,
>  			       const int **vals, int *length);
>  
> +/**
> + * iio_read_avail_channel_attribute() - read available channel attribute values
> + * @chan:		The channel being queried.
> + * @vals:		Available values read back.
> + * @type:		Type of values read back.
> + * @length:		Number of entries in vals.
> + * @attribute:		info attribute to be read back.
> + *
> + * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
> + */
> +int iio_read_avail_channel_attribute(struct iio_channel *chan,
> +				     const int **vals, int *type, int *length,
> +				     enum iio_chan_info_enum attribute);
> +
>  /**
>   * iio_get_channel_type() - get the type of a channel
>   * @channel:		The channel being queried.

Re: [PATCH v2 2/4] iio: inkern: Convert iio_read_avail_channel_raw into a wrapper

[Jonathan Cameron]

On Sat, 23 Mar 2019 18:28:07 +0100
Artur Rojek <contact@artur-rojek.eu> wrote:

> Convert "iio_read_avail_channel_raw" over to a wrapper around
> "iio_read_avail_channel_attribute".
> 
> With the introduction of "iio_read_avail_channel_attribute",
> the necessity of having a separate call to read raw channel values
> became redundant.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> 
> Changes:
> 
> v2: new patch
> 
>  drivers/iio/inkern.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index f19dbde3c945..4a5eff3f18bc 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -759,16 +759,8 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
>  	int ret;
>  	int type;
>  
> -	mutex_lock(&chan->indio_dev->info_exist_lock);
> -	if (!chan->indio_dev->info) {
> -		ret = -ENODEV;
> -		goto err_unlock;
> -	}
> -
> -	ret = iio_channel_read_avail(chan,
> -				     vals, &type, length, IIO_CHAN_INFO_RAW);
> -err_unlock:
> -	mutex_unlock(&chan->indio_dev->info_exist_lock);
> +	ret = iio_read_avail_channel_attribute(chan, vals, &type, length,
> +					 IIO_CHAN_INFO_RAW);
>  
>  	if (ret >= 0 && type != IIO_VAL_INT)
>  		/* raw values are assumed to be IIO_VAL_INT */

Re: [PATCH v2 3/4] dt-bindings: power: supply: Add docs for Ingenic JZ47xx SoCs battery.

[Jonathan Cameron]

On Sat, 23 Mar 2019 18:28:08 +0100
Artur Rojek <contact@artur-rojek.eu> wrote:

> Add documentation for the ingenic-battery driver, used on JZ47xx SoCs.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Looks fine to me.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
> 
> Changes:
> 
> v2: no change
> 
>  .../bindings/power/supply/ingenic,battery.txt | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> new file mode 100644
> index 000000000000..66430bf73815
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> @@ -0,0 +1,31 @@
> +* Ingenic JZ47xx battery bindings
> +
> +Required properties:
> +
> +- compatible: Must be "ingenic,jz4740-battery".
> +- io-channels: phandle and IIO specifier pair to the IIO device.
> +  Format described in iio-bindings.txt.
> +- monitored-battery: phandle to a "simple-battery" compatible node.
> +
> +The "monitored-battery" property must be a phandle to a node using the format
> +described in battery.txt, with the following properties being required:
> +
> +- voltage-min-design-microvolt: Drained battery voltage.
> +- voltage-max-design-microvolt: Fully charged battery voltage.
> +
> +Example:
> +
> +#include <dt-bindings/iio/adc/ingenic,adc.h>
> +
> +simple_battery: battery {
> +	compatible = "simple-battery";
> +	voltage-min-design-microvolt = <3600000>;
> +	voltage-max-design-microvolt = <4200000>;
> +};
> +
> +ingenic_battery {
> +	compatible = "ingenic,jz4740-battery";
> +	io-channels = <&adc INGENIC_ADC_BATTERY>;
> +	io-channel-names = "battery";
> +	monitored-battery = <&simple_battery>;
> +};

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Jonathan Cameron]

On Sat, 23 Mar 2019 18:28:09 +0100
Artur Rojek <contact@artur-rojek.eu> wrote:

> Add a driver for battery present on Ingenic JZ47xx SoCs.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
The IIO parts look fine to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Sebastian, assuming you are happy with this version, do you have any
preference for how we handle this series?

Probably needs one of us to do an immutable branch with just this
in it.  I'm happy to do so once all the relevant Acks etc are in
place, but don't mind if you want to!

Thanks,

Jonathan

> ---
> 
> Changes:
> 
> v2: - rework the return logic in ingenic_battery_get_property,
>     - make index offsets point forward in ingenic_battery_set_scale,
>     - fix spacing around scale_raw[best_idx + 1]
> 
>  drivers/power/supply/Kconfig           |  11 ++
>  drivers/power/supply/Makefile          |   1 +
>  drivers/power/supply/ingenic-battery.c | 180 +++++++++++++++++++++++++
>  3 files changed, 192 insertions(+)
>  create mode 100644 drivers/power/supply/ingenic-battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..9bfb1637ec28 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -169,6 +169,17 @@ config BATTERY_COLLIE
>  	  Say Y to enable support for the battery on the Sharp Zaurus
>  	  SL-5500 (collie) models.
>  
> +config BATTERY_INGENIC
> +	tristate "Ingenic JZ47xx SoCs battery driver"
> +	depends on MIPS || COMPILE_TEST
> +	depends on INGENIC_ADC
> +	help
> +	  Choose this option if you want to monitor battery status on
> +	  Ingenic JZ47xx SoC based devices.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ingenic-battery.
> +
>  config BATTERY_IPAQ_MICRO
>  	tristate "iPAQ Atmel Micro ASIC battery driver"
>  	depends on MFD_IPAQ_MICRO
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..9e2c481d0187 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
>  obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
>  obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
>  obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
> +obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
>  obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
>  obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
>  obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
> diff --git a/drivers/power/supply/ingenic-battery.c b/drivers/power/supply/ingenic-battery.c
> new file mode 100644
> index 000000000000..822aee1c7b64
> --- /dev/null
> +++ b/drivers/power/supply/ingenic-battery.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Battery driver for the Ingenic JZ47xx SoCs
> + * Copyright (c) 2019 Artur Rojek <contact@artur-rojek.eu>
> + *
> + * based on drivers/power/supply/jz4740-battery.c
> + */
> +
> +#include <linux/iio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/property.h>
> +
> +struct ingenic_battery {
> +	struct device *dev;
> +	struct iio_channel *channel;
> +	struct power_supply_desc desc;
> +	struct power_supply *battery;
> +	struct power_supply_battery_info info;
> +};
> +
> +static int ingenic_battery_get_property(struct power_supply *psy,
> +					enum power_supply_property psp,
> +					union power_supply_propval *val)
> +{
> +	struct ingenic_battery *bat = power_supply_get_drvdata(psy);
> +	struct power_supply_battery_info *info = &bat->info;
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> +		val->intval *= 1000;
> +		if (val->intval < info->voltage_min_design_uv)
> +			val->intval = POWER_SUPPLY_HEALTH_DEAD;
> +		else if (val->intval > info->voltage_max_design_uv)
> +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +		else
> +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		return ret;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> +		val->intval *= 1000;
> +		return ret;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		val->intval = info->voltage_min_design_uv;
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		val->intval = info->voltage_max_design_uv;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	};
> +}
> +
> +/* Set the most appropriate IIO channel voltage reference scale
> + * based on the battery's max voltage.
> + */
> +static int ingenic_battery_set_scale(struct ingenic_battery *bat)
> +{
> +	const int *scale_raw;
> +	int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
> +	u64 max_mV;
> +
> +	ret = iio_read_max_channel_raw(bat->channel, &max_raw);
> +	if (ret) {
> +		dev_err(bat->dev, "Unable to read max raw channel value\n");
> +		return ret;
> +	}
> +
> +	ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
> +					       &scale_type, &scale_len,
> +					       IIO_CHAN_INFO_SCALE);
> +	if (ret < 0) {
> +		dev_err(bat->dev, "Unable to read channel avail scale\n");
> +		return ret;
> +	}
> +	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2)
> +		return -EINVAL;
> +
> +	max_mV = bat->info.voltage_max_design_uv / 1000;
> +
> +	for (i = 0; i < scale_len; i += 2) {
> +		u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
> +
> +		if (scale_mV < max_mV)
> +			continue;
> +
> +		if (best_idx >= 0 && scale_mV > best_mV)
> +			continue;
> +
> +		best_mV = scale_mV;
> +		best_idx = i;
> +	}
> +
> +	if (best_idx < 0) {
> +		dev_err(bat->dev, "Unable to find matching voltage scale\n");
> +		return -EINVAL;
> +	}
> +
> +	return iio_write_channel_attribute(bat->channel,
> +					   scale_raw[best_idx],
> +					   scale_raw[best_idx + 1],
> +					   IIO_CHAN_INFO_SCALE);
> +}
> +
> +static enum power_supply_property ingenic_battery_properties[] = {
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> +};
> +
> +static int ingenic_battery_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ingenic_battery *bat;
> +	struct power_supply_config psy_cfg = {};
> +	struct power_supply_desc *desc;
> +	int ret;
> +
> +	bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
> +	if (!bat)
> +		return -ENOMEM;
> +
> +	bat->dev = dev;
> +	bat->channel = devm_iio_channel_get(dev, "battery");
> +	if (IS_ERR(bat->channel))
> +		return PTR_ERR(bat->channel);
> +
> +	desc = &bat->desc;
> +	desc->name = "jz-battery";
> +	desc->type = POWER_SUPPLY_TYPE_BATTERY;
> +	desc->properties = ingenic_battery_properties;
> +	desc->num_properties = ARRAY_SIZE(ingenic_battery_properties);
> +	desc->get_property = ingenic_battery_get_property;
> +	psy_cfg.drv_data = bat;
> +	psy_cfg.of_node = dev->of_node;
> +
> +	bat->battery = devm_power_supply_register(dev, desc, &psy_cfg);
> +	if (IS_ERR(bat->battery)) {
> +		dev_err(dev, "Unable to register battery\n");
> +		return PTR_ERR(bat->battery);
> +	}
> +
> +	ret = power_supply_get_battery_info(bat->battery, &bat->info);
> +	if (ret) {
> +		dev_err(dev, "Unable to get battery info: %d\n", ret);
> +		return ret;
> +	}
> +	if (bat->info.voltage_min_design_uv < 0) {
> +		dev_err(dev, "Unable to get voltage min design\n");
> +		return bat->info.voltage_min_design_uv;
> +	}
> +	if (bat->info.voltage_max_design_uv < 0) {
> +		dev_err(dev, "Unable to get voltage max design\n");
> +		return bat->info.voltage_max_design_uv;
> +	}
> +
> +	return ingenic_battery_set_scale(bat);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id ingenic_battery_of_match[] = {
> +	{ .compatible = "ingenic,jz4740-battery", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_battery_of_match);
> +#endif
> +
> +static struct platform_driver ingenic_battery_driver = {
> +	.driver = {
> +		.name = "ingenic-battery",
> +		.of_match_table = of_match_ptr(ingenic_battery_of_match),

I guess we can be fairly sure no one will even user this with ACPI
(magic DT) bindings?  That would make this a rare case where the
of_match_pr protection and ifdef fun is still fine.


> +	},
> +	.probe = ingenic_battery_probe,
> +};
> +module_platform_driver(ingenic_battery_driver);

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 9737 bytes --]

Hi,

On Sun, Mar 24, 2019 at 03:31:37PM +0000, Jonathan Cameron wrote:
> On Sat, 23 Mar 2019 18:28:09 +0100
> Artur Rojek <contact@artur-rojek.eu> wrote:
> 
> > Add a driver for battery present on Ingenic JZ47xx SoCs.
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> The IIO parts look fine to me.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Sebastian, assuming you are happy with this version,

The driver itself looks ok. I'm a bit unhappy, that we already have
jz4740-battery. This driver is much cleaner, but does not yet seem
to be ready to replace it. Artur Rojek what are your plans regarding
to the existing driver? Is there currently work going on migrating
JZ47xx to DT?

> do you have any preference for how we handle this series?
> 
> Probably needs one of us to do an immutable branch with just this
> in it.  I'm happy to do so once all the relevant Acks etc are in
> place, but don't mind if you want to!

I suppose you could provide an immutable branch with just the first
two patches in it. Then I can pull it and apply the power-supply
patches on top.

-- Sebastian

> Thanks,
> 
> Jonathan
> 
> > ---
> > 
> > Changes:
> > 
> > v2: - rework the return logic in ingenic_battery_get_property,
> >     - make index offsets point forward in ingenic_battery_set_scale,
> >     - fix spacing around scale_raw[best_idx + 1]
> > 
> >  drivers/power/supply/Kconfig           |  11 ++
> >  drivers/power/supply/Makefile          |   1 +
> >  drivers/power/supply/ingenic-battery.c | 180 +++++++++++++++++++++++++
> >  3 files changed, 192 insertions(+)
> >  create mode 100644 drivers/power/supply/ingenic-battery.c
> > 
> > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> > index e901b9879e7e..9bfb1637ec28 100644
> > --- a/drivers/power/supply/Kconfig
> > +++ b/drivers/power/supply/Kconfig
> > @@ -169,6 +169,17 @@ config BATTERY_COLLIE
> >  	  Say Y to enable support for the battery on the Sharp Zaurus
> >  	  SL-5500 (collie) models.
> >  
> > +config BATTERY_INGENIC
> > +	tristate "Ingenic JZ47xx SoCs battery driver"
> > +	depends on MIPS || COMPILE_TEST
> > +	depends on INGENIC_ADC
> > +	help
> > +	  Choose this option if you want to monitor battery status on
> > +	  Ingenic JZ47xx SoC based devices.
> > +
> > +	  This driver can also be built as a module. If so, the module will be
> > +	  called ingenic-battery.
> > +
> >  config BATTERY_IPAQ_MICRO
> >  	tristate "iPAQ Atmel Micro ASIC battery driver"
> >  	depends on MFD_IPAQ_MICRO
> > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> > index b731c2a9b695..9e2c481d0187 100644
> > --- a/drivers/power/supply/Makefile
> > +++ b/drivers/power/supply/Makefile
> > @@ -34,6 +34,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
> >  obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
> >  obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
> >  obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
> > +obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
> >  obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
> >  obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
> >  obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
> > diff --git a/drivers/power/supply/ingenic-battery.c b/drivers/power/supply/ingenic-battery.c
> > new file mode 100644
> > index 000000000000..822aee1c7b64
> > --- /dev/null
> > +++ b/drivers/power/supply/ingenic-battery.c
> > @@ -0,0 +1,180 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Battery driver for the Ingenic JZ47xx SoCs
> > + * Copyright (c) 2019 Artur Rojek <contact@artur-rojek.eu>
> > + *
> > + * based on drivers/power/supply/jz4740-battery.c
> > + */
> > +
> > +#include <linux/iio/consumer.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/power_supply.h>
> > +#include <linux/property.h>
> > +
> > +struct ingenic_battery {
> > +	struct device *dev;
> > +	struct iio_channel *channel;
> > +	struct power_supply_desc desc;
> > +	struct power_supply *battery;
> > +	struct power_supply_battery_info info;
> > +};
> > +
> > +static int ingenic_battery_get_property(struct power_supply *psy,
> > +					enum power_supply_property psp,
> > +					union power_supply_propval *val)
> > +{
> > +	struct ingenic_battery *bat = power_supply_get_drvdata(psy);
> > +	struct power_supply_battery_info *info = &bat->info;
> > +	int ret;
> > +
> > +	switch (psp) {
> > +	case POWER_SUPPLY_PROP_HEALTH:
> > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> > +		val->intval *= 1000;
> > +		if (val->intval < info->voltage_min_design_uv)
> > +			val->intval = POWER_SUPPLY_HEALTH_DEAD;
> > +		else if (val->intval > info->voltage_max_design_uv)
> > +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > +		else
> > +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> > +		return ret;
> > +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> > +		val->intval *= 1000;
> > +		return ret;
> > +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> > +		val->intval = info->voltage_min_design_uv;
> > +		return 0;
> > +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> > +		val->intval = info->voltage_max_design_uv;
> > +		return 0;
> > +	default:
> > +		return -EINVAL;
> > +	};
> > +}
> > +
> > +/* Set the most appropriate IIO channel voltage reference scale
> > + * based on the battery's max voltage.
> > + */
> > +static int ingenic_battery_set_scale(struct ingenic_battery *bat)
> > +{
> > +	const int *scale_raw;
> > +	int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
> > +	u64 max_mV;
> > +
> > +	ret = iio_read_max_channel_raw(bat->channel, &max_raw);
> > +	if (ret) {
> > +		dev_err(bat->dev, "Unable to read max raw channel value\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
> > +					       &scale_type, &scale_len,
> > +					       IIO_CHAN_INFO_SCALE);
> > +	if (ret < 0) {
> > +		dev_err(bat->dev, "Unable to read channel avail scale\n");
> > +		return ret;
> > +	}
> > +	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2)
> > +		return -EINVAL;
> > +
> > +	max_mV = bat->info.voltage_max_design_uv / 1000;
> > +
> > +	for (i = 0; i < scale_len; i += 2) {
> > +		u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
> > +
> > +		if (scale_mV < max_mV)
> > +			continue;
> > +
> > +		if (best_idx >= 0 && scale_mV > best_mV)
> > +			continue;
> > +
> > +		best_mV = scale_mV;
> > +		best_idx = i;
> > +	}
> > +
> > +	if (best_idx < 0) {
> > +		dev_err(bat->dev, "Unable to find matching voltage scale\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return iio_write_channel_attribute(bat->channel,
> > +					   scale_raw[best_idx],
> > +					   scale_raw[best_idx + 1],
> > +					   IIO_CHAN_INFO_SCALE);
> > +}
> > +
> > +static enum power_supply_property ingenic_battery_properties[] = {
> > +	POWER_SUPPLY_PROP_HEALTH,
> > +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> > +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> > +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> > +};
> > +
> > +static int ingenic_battery_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct ingenic_battery *bat;
> > +	struct power_supply_config psy_cfg = {};
> > +	struct power_supply_desc *desc;
> > +	int ret;
> > +
> > +	bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
> > +	if (!bat)
> > +		return -ENOMEM;
> > +
> > +	bat->dev = dev;
> > +	bat->channel = devm_iio_channel_get(dev, "battery");
> > +	if (IS_ERR(bat->channel))
> > +		return PTR_ERR(bat->channel);
> > +
> > +	desc = &bat->desc;
> > +	desc->name = "jz-battery";
> > +	desc->type = POWER_SUPPLY_TYPE_BATTERY;
> > +	desc->properties = ingenic_battery_properties;
> > +	desc->num_properties = ARRAY_SIZE(ingenic_battery_properties);
> > +	desc->get_property = ingenic_battery_get_property;
> > +	psy_cfg.drv_data = bat;
> > +	psy_cfg.of_node = dev->of_node;
> > +
> > +	bat->battery = devm_power_supply_register(dev, desc, &psy_cfg);
> > +	if (IS_ERR(bat->battery)) {
> > +		dev_err(dev, "Unable to register battery\n");
> > +		return PTR_ERR(bat->battery);
> > +	}
> > +
> > +	ret = power_supply_get_battery_info(bat->battery, &bat->info);
> > +	if (ret) {
> > +		dev_err(dev, "Unable to get battery info: %d\n", ret);
> > +		return ret;
> > +	}
> > +	if (bat->info.voltage_min_design_uv < 0) {
> > +		dev_err(dev, "Unable to get voltage min design\n");
> > +		return bat->info.voltage_min_design_uv;
> > +	}
> > +	if (bat->info.voltage_max_design_uv < 0) {
> > +		dev_err(dev, "Unable to get voltage max design\n");
> > +		return bat->info.voltage_max_design_uv;
> > +	}
> > +
> > +	return ingenic_battery_set_scale(bat);
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id ingenic_battery_of_match[] = {
> > +	{ .compatible = "ingenic,jz4740-battery", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ingenic_battery_of_match);
> > +#endif
> > +
> > +static struct platform_driver ingenic_battery_driver = {
> > +	.driver = {
> > +		.name = "ingenic-battery",
> > +		.of_match_table = of_match_ptr(ingenic_battery_of_match),
> 
> I guess we can be fairly sure no one will even user this with ACPI
> (magic DT) bindings?  That would make this a rare case where the
> of_match_pr protection and ifdef fun is still fine.
> 
> 
> > +	},
> > +	.probe = ingenic_battery_probe,
> > +};
> > +module_platform_driver(ingenic_battery_driver);
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Paul Cercueil]

Hi Sebastian,

Le dim. 7 avril 2019 à 18:52, Sebastian Reichel <sre@kernel.org> a 
écrit :
> Hi,
> 
> On Sun, Mar 24, 2019 at 03:31:37PM +0000, Jonathan Cameron wrote:
>>  On Sat, 23 Mar 2019 18:28:09 +0100
>>  Artur Rojek <contact@artur-rojek.eu> wrote:
>> 
>>  > Add a driver for battery present on Ingenic JZ47xx SoCs.
>>  >
>>  > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
>>  The IIO parts look fine to me.
>>  Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> 
>>  Sebastian, assuming you are happy with this version,
> 
> The driver itself looks ok. I'm a bit unhappy, that we already have
> jz4740-battery. This driver is much cleaner, but does not yet seem
> to be ready to replace it. Artur Rojek what are your plans regarding
> to the existing driver? Is there currently work going on migrating
> JZ47xx to DT?

Why do you think it's not ready? Feature-wise, it has everything we
need to replace jz4740-battery, which will be trashed as soon as the
LB60 board code is updated to use the new driver.

We are working on porting the JZ47xx code to devicetree, yes. The
ultimate goal is to completely get rid of
arch/mips/jz4740/board-qi_lb60.c and arch/mips/jz4740/platform.c.

>>  do you have any preference for how we handle this series?
>> 
>>  Probably needs one of us to do an immutable branch with just this
>>  in it.  I'm happy to do so once all the relevant Acks etc are in
>>  place, but don't mind if you want to!
> 
> I suppose you could provide an immutable branch with just the first
> two patches in it. Then I can pull it and apply the power-supply
> patches on top.
> 
> -- Sebastian
> 
>>  Thanks,
>> 
>>  Jonathan
>> 
>>  > ---
>>  >
>>  > Changes:
>>  >
>>  > v2: - rework the return logic in ingenic_battery_get_property,
>>  >     - make index offsets point forward in 
>> ingenic_battery_set_scale,
>>  >     - fix spacing around scale_raw[best_idx + 1]
>>  >
>>  >  drivers/power/supply/Kconfig           |  11 ++
>>  >  drivers/power/supply/Makefile          |   1 +
>>  >  drivers/power/supply/ingenic-battery.c | 180 
>> +++++++++++++++++++++++++
>>  >  3 files changed, 192 insertions(+)
>>  >  create mode 100644 drivers/power/supply/ingenic-battery.c
>>  >
>>  > diff --git a/drivers/power/supply/Kconfig 
>> b/drivers/power/supply/Kconfig
>>  > index e901b9879e7e..9bfb1637ec28 100644
>>  > --- a/drivers/power/supply/Kconfig
>>  > +++ b/drivers/power/supply/Kconfig
>>  > @@ -169,6 +169,17 @@ config BATTERY_COLLIE
>>  >  	  Say Y to enable support for the battery on the Sharp Zaurus
>>  >  	  SL-5500 (collie) models.
>>  >
>>  > +config BATTERY_INGENIC
>>  > +	tristate "Ingenic JZ47xx SoCs battery driver"
>>  > +	depends on MIPS || COMPILE_TEST
>>  > +	depends on INGENIC_ADC
>>  > +	help
>>  > +	  Choose this option if you want to monitor battery status on
>>  > +	  Ingenic JZ47xx SoC based devices.
>>  > +
>>  > +	  This driver can also be built as a module. If so, the module 
>> will be
>>  > +	  called ingenic-battery.
>>  > +
>>  >  config BATTERY_IPAQ_MICRO
>>  >  	tristate "iPAQ Atmel Micro ASIC battery driver"
>>  >  	depends on MFD_IPAQ_MICRO
>>  > diff --git a/drivers/power/supply/Makefile 
>> b/drivers/power/supply/Makefile
>>  > index b731c2a9b695..9e2c481d0187 100644
>>  > --- a/drivers/power/supply/Makefile
>>  > +++ b/drivers/power/supply/Makefile
>>  > @@ -34,6 +34,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
>>  >  obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
>>  >  obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
>>  >  obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
>>  > +obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
>>  >  obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
>>  >  obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
>>  >  obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
>>  > diff --git a/drivers/power/supply/ingenic-battery.c 
>> b/drivers/power/supply/ingenic-battery.c
>>  > new file mode 100644
>>  > index 000000000000..822aee1c7b64
>>  > --- /dev/null
>>  > +++ b/drivers/power/supply/ingenic-battery.c
>>  > @@ -0,0 +1,180 @@
>>  > +// SPDX-License-Identifier: GPL-2.0
>>  > +/*
>>  > + * Battery driver for the Ingenic JZ47xx SoCs
>>  > + * Copyright (c) 2019 Artur Rojek <contact@artur-rojek.eu>
>>  > + *
>>  > + * based on drivers/power/supply/jz4740-battery.c
>>  > + */
>>  > +
>>  > +#include <linux/iio/consumer.h>
>>  > +#include <linux/module.h>
>>  > +#include <linux/of.h>
>>  > +#include <linux/platform_device.h>
>>  > +#include <linux/power_supply.h>
>>  > +#include <linux/property.h>
>>  > +
>>  > +struct ingenic_battery {
>>  > +	struct device *dev;
>>  > +	struct iio_channel *channel;
>>  > +	struct power_supply_desc desc;
>>  > +	struct power_supply *battery;
>>  > +	struct power_supply_battery_info info;
>>  > +};
>>  > +
>>  > +static int ingenic_battery_get_property(struct power_supply *psy,
>>  > +					enum power_supply_property psp,
>>  > +					union power_supply_propval *val)
>>  > +{
>>  > +	struct ingenic_battery *bat = power_supply_get_drvdata(psy);
>>  > +	struct power_supply_battery_info *info = &bat->info;
>>  > +	int ret;
>>  > +
>>  > +	switch (psp) {
>>  > +	case POWER_SUPPLY_PROP_HEALTH:
>>  > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
>>  > +		val->intval *= 1000;
>>  > +		if (val->intval < info->voltage_min_design_uv)
>>  > +			val->intval = POWER_SUPPLY_HEALTH_DEAD;
>>  > +		else if (val->intval > info->voltage_max_design_uv)
>>  > +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
>>  > +		else
>>  > +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
>>  > +		return ret;
>>  > +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>>  > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
>>  > +		val->intval *= 1000;
>>  > +		return ret;
>>  > +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
>>  > +		val->intval = info->voltage_min_design_uv;
>>  > +		return 0;
>>  > +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
>>  > +		val->intval = info->voltage_max_design_uv;
>>  > +		return 0;
>>  > +	default:
>>  > +		return -EINVAL;
>>  > +	};
>>  > +}
>>  > +
>>  > +/* Set the most appropriate IIO channel voltage reference scale
>>  > + * based on the battery's max voltage.
>>  > + */
>>  > +static int ingenic_battery_set_scale(struct ingenic_battery *bat)
>>  > +{
>>  > +	const int *scale_raw;
>>  > +	int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, 
>> ret;
>>  > +	u64 max_mV;
>>  > +
>>  > +	ret = iio_read_max_channel_raw(bat->channel, &max_raw);
>>  > +	if (ret) {
>>  > +		dev_err(bat->dev, "Unable to read max raw channel value\n");
>>  > +		return ret;
>>  > +	}
>>  > +
>>  > +	ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
>>  > +					       &scale_type, &scale_len,
>>  > +					       IIO_CHAN_INFO_SCALE);
>>  > +	if (ret < 0) {
>>  > +		dev_err(bat->dev, "Unable to read channel avail scale\n");
>>  > +		return ret;
>>  > +	}
>>  > +	if (ret != IIO_AVAIL_LIST || scale_type != 
>> IIO_VAL_FRACTIONAL_LOG2)
>>  > +		return -EINVAL;
>>  > +
>>  > +	max_mV = bat->info.voltage_max_design_uv / 1000;
>>  > +
>>  > +	for (i = 0; i < scale_len; i += 2) {
>>  > +		u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
>>  > +
>>  > +		if (scale_mV < max_mV)
>>  > +			continue;
>>  > +
>>  > +		if (best_idx >= 0 && scale_mV > best_mV)
>>  > +			continue;
>>  > +
>>  > +		best_mV = scale_mV;
>>  > +		best_idx = i;
>>  > +	}
>>  > +
>>  > +	if (best_idx < 0) {
>>  > +		dev_err(bat->dev, "Unable to find matching voltage scale\n");
>>  > +		return -EINVAL;
>>  > +	}
>>  > +
>>  > +	return iio_write_channel_attribute(bat->channel,
>>  > +					   scale_raw[best_idx],
>>  > +					   scale_raw[best_idx + 1],
>>  > +					   IIO_CHAN_INFO_SCALE);
>>  > +}
>>  > +
>>  > +static enum power_supply_property ingenic_battery_properties[] = 
>> {
>>  > +	POWER_SUPPLY_PROP_HEALTH,
>>  > +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
>>  > +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
>>  > +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
>>  > +};
>>  > +
>>  > +static int ingenic_battery_probe(struct platform_device *pdev)
>>  > +{
>>  > +	struct device *dev = &pdev->dev;
>>  > +	struct ingenic_battery *bat;
>>  > +	struct power_supply_config psy_cfg = {};
>>  > +	struct power_supply_desc *desc;
>>  > +	int ret;
>>  > +
>>  > +	bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
>>  > +	if (!bat)
>>  > +		return -ENOMEM;
>>  > +
>>  > +	bat->dev = dev;
>>  > +	bat->channel = devm_iio_channel_get(dev, "battery");
>>  > +	if (IS_ERR(bat->channel))
>>  > +		return PTR_ERR(bat->channel);
>>  > +
>>  > +	desc = &bat->desc;
>>  > +	desc->name = "jz-battery";
>>  > +	desc->type = POWER_SUPPLY_TYPE_BATTERY;
>>  > +	desc->properties = ingenic_battery_properties;
>>  > +	desc->num_properties = ARRAY_SIZE(ingenic_battery_properties);
>>  > +	desc->get_property = ingenic_battery_get_property;
>>  > +	psy_cfg.drv_data = bat;
>>  > +	psy_cfg.of_node = dev->of_node;
>>  > +
>>  > +	bat->battery = devm_power_supply_register(dev, desc, &psy_cfg);
>>  > +	if (IS_ERR(bat->battery)) {
>>  > +		dev_err(dev, "Unable to register battery\n");
>>  > +		return PTR_ERR(bat->battery);
>>  > +	}
>>  > +
>>  > +	ret = power_supply_get_battery_info(bat->battery, &bat->info);
>>  > +	if (ret) {
>>  > +		dev_err(dev, "Unable to get battery info: %d\n", ret);
>>  > +		return ret;
>>  > +	}
>>  > +	if (bat->info.voltage_min_design_uv < 0) {
>>  > +		dev_err(dev, "Unable to get voltage min design\n");
>>  > +		return bat->info.voltage_min_design_uv;
>>  > +	}
>>  > +	if (bat->info.voltage_max_design_uv < 0) {
>>  > +		dev_err(dev, "Unable to get voltage max design\n");
>>  > +		return bat->info.voltage_max_design_uv;
>>  > +	}
>>  > +
>>  > +	return ingenic_battery_set_scale(bat);
>>  > +}
>>  > +
>>  > +#ifdef CONFIG_OF
>>  > +static const struct of_device_id ingenic_battery_of_match[] = {
>>  > +	{ .compatible = "ingenic,jz4740-battery", },
>>  > +	{ },
>>  > +};
>>  > +MODULE_DEVICE_TABLE(of, ingenic_battery_of_match);
>>  > +#endif
>>  > +
>>  > +static struct platform_driver ingenic_battery_driver = {
>>  > +	.driver = {
>>  > +		.name = "ingenic-battery",
>>  > +		.of_match_table = of_match_ptr(ingenic_battery_of_match),
>> 
>>  I guess we can be fairly sure no one will even user this with ACPI
>>  (magic DT) bindings?  That would make this a rare case where the
>>  of_match_pr protection and ifdef fun is still fine.
>> 
>> 
>>  > +	},
>>  > +	.probe = ingenic_battery_probe,
>>  > +};
>>  > +module_platform_driver(ingenic_battery_driver);
>> 

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

Hi Paul,

On Sun, Apr 07, 2019 at 09:07:57PM +0200, Paul Cercueil wrote:
> Hi Sebastian,
> 
> Le dim. 7 avril 2019 à 18:52, Sebastian Reichel <sre@kernel.org> a écrit :
> > Hi,
> > 
> > On Sun, Mar 24, 2019 at 03:31:37PM +0000, Jonathan Cameron wrote:
> > >  On Sat, 23 Mar 2019 18:28:09 +0100
> > >  Artur Rojek <contact@artur-rojek.eu> wrote:
> > > 
> > >  > Add a driver for battery present on Ingenic JZ47xx SoCs.
> > >  >
> > >  > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> > >  The IIO parts look fine to me.
> > >  Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > >  Sebastian, assuming you are happy with this version,
> > 
> > The driver itself looks ok. I'm a bit unhappy, that we already have
> > jz4740-battery. This driver is much cleaner, but does not yet seem
> > to be ready to replace it. Artur Rojek what are your plans regarding
> > to the existing driver? Is there currently work going on migrating
> > JZ47xx to DT?
> 
> Why do you think it's not ready? Feature-wise, it has everything we
> need to replace jz4740-battery, which will be trashed as soon as the
> LB60 board code is updated to use the new driver.

jz4740-battery has a few features not provided by your driver.
For example the gpio providing the charging status. Assuming
you plan to trash the old driver soon:

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

> We are working on porting the JZ47xx code to devicetree, yes. The
> ultimate goal is to completely get rid of
> arch/mips/jz4740/board-qi_lb60.c and arch/mips/jz4740/platform.c.

Ok, sounds good.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Paul Cercueil]

Le lun. 8 avril 2019 à 11:38, Sebastian Reichel <sre@kernel.org> a 
écrit :
> Hi Paul,
> 
> On Sun, Apr 07, 2019 at 09:07:57PM +0200, Paul Cercueil wrote:
>>  Hi Sebastian,
>> 
>>  Le dim. 7 avril 2019 à 18:52, Sebastian Reichel <sre@kernel.org> a 
>> écrit :
>>  > Hi,
>>  >
>>  > On Sun, Mar 24, 2019 at 03:31:37PM +0000, Jonathan Cameron wrote:
>>  > >  On Sat, 23 Mar 2019 18:28:09 +0100
>>  > >  Artur Rojek <contact@artur-rojek.eu> wrote:
>>  > >
>>  > >  > Add a driver for battery present on Ingenic JZ47xx SoCs.
>>  > >  >
>>  > >  > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
>>  > >  The IIO parts look fine to me.
>>  > >  Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>  > >
>>  > >  Sebastian, assuming you are happy with this version,
>>  >
>>  > The driver itself looks ok. I'm a bit unhappy, that we already 
>> have
>>  > jz4740-battery. This driver is much cleaner, but does not yet seem
>>  > to be ready to replace it. Artur Rojek what are your plans 
>> regarding
>>  > to the existing driver? Is there currently work going on migrating
>>  > JZ47xx to DT?
>> 
>>  Why do you think it's not ready? Feature-wise, it has everything we
>>  need to replace jz4740-battery, which will be trashed as soon as the
>>  LB60 board code is updated to use the new driver.
> 
> jz4740-battery has a few features not provided by your driver.
> For example the gpio providing the charging status.

Artur sumitted a separate patchset for this functionality:
https://lkml.org/lkml/2019/2/26/501

> Assuming you plan to trash the old driver soon:
> 
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
>>  We are working on porting the JZ47xx code to devicetree, yes. The
>>  ultimate goal is to completely get rid of
>>  arch/mips/jz4740/board-qi_lb60.c and arch/mips/jz4740/platform.c.
> 
> Ok, sounds good.
> 
> -- Sebastian

Re: [PATCH v2 1/4] iio: inkern: API for reading available iio channel attribute values

[Jonathan Cameron]

On Sun, 24 Mar 2019 15:27:25 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 23 Mar 2019 18:28:06 +0100
> Artur Rojek <contact@artur-rojek.eu> wrote:
> 
> > Extend the inkern API with a function for reading available
> > attribute values of iio channels.
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>  
> If this goes through a route other than IIO (otherwise
> I'll just add a signed-off-by...)
> 
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Applied to the ib-jz47xx-prereq branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

I'll merge that into the togreg branch as well shortly.

Thanks,

Jonathan
> 
> > ---
> > 
> > Changes:
> > 
> > v2: no change
> > 
> >  drivers/iio/inkern.c         | 20 ++++++++++++++++++++
> >  include/linux/iio/consumer.h | 14 ++++++++++++++
> >  2 files changed, 34 insertions(+)
> > 
> > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> > index 06ca3f7fcc44..f19dbde3c945 100644
> > --- a/drivers/iio/inkern.c
> > +++ b/drivers/iio/inkern.c
> > @@ -733,6 +733,26 @@ static int iio_channel_read_avail(struct iio_channel *chan,
> >  						 vals, type, length, info);
> >  }
> >  
> > +int iio_read_avail_channel_attribute(struct iio_channel *chan,
> > +				     const int **vals, int *type, int *length,
> > +				     enum iio_chan_info_enum attribute)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&chan->indio_dev->info_exist_lock);
> > +	if (!chan->indio_dev->info) {
> > +		ret = -ENODEV;
> > +		goto err_unlock;
> > +	}
> > +
> > +	ret = iio_channel_read_avail(chan, vals, type, length, attribute);
> > +err_unlock:
> > +	mutex_unlock(&chan->indio_dev->info_exist_lock);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iio_read_avail_channel_attribute);
> > +
> >  int iio_read_avail_channel_raw(struct iio_channel *chan,
> >  			       const int **vals, int *length)
> >  {
> > diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> > index 9887f4f8e2a8..b2d34831ed7c 100644
> > --- a/include/linux/iio/consumer.h
> > +++ b/include/linux/iio/consumer.h
> > @@ -290,6 +290,20 @@ int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
> >  int iio_read_avail_channel_raw(struct iio_channel *chan,
> >  			       const int **vals, int *length);
> >  
> > +/**
> > + * iio_read_avail_channel_attribute() - read available channel attribute values
> > + * @chan:		The channel being queried.
> > + * @vals:		Available values read back.
> > + * @type:		Type of values read back.
> > + * @length:		Number of entries in vals.
> > + * @attribute:		info attribute to be read back.
> > + *
> > + * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
> > + */
> > +int iio_read_avail_channel_attribute(struct iio_channel *chan,
> > +				     const int **vals, int *type, int *length,
> > +				     enum iio_chan_info_enum attribute);
> > +
> >  /**
> >   * iio_get_channel_type() - get the type of a channel
> >   * @channel:		The channel being queried.  
> 

Re: [PATCH v2 2/4] iio: inkern: Convert iio_read_avail_channel_raw into a wrapper

[Jonathan Cameron]

On Sun, 24 Mar 2019 15:27:50 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat, 23 Mar 2019 18:28:07 +0100
> Artur Rojek <contact@artur-rojek.eu> wrote:
> 
> > Convert "iio_read_avail_channel_raw" over to a wrapper around
> > "iio_read_avail_channel_attribute".
> > 
> > With the introduction of "iio_read_avail_channel_attribute",
> > the necessity of having a separate call to read raw channel values
> > became redundant.
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>  
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied to the ib-jz47xx-battery-prereq branch of iio.git

Thanks,

Jonathan

> 
> > ---
> > 
> > Changes:
> > 
> > v2: new patch
> > 
> >  drivers/iio/inkern.c | 12 ++----------
> >  1 file changed, 2 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> > index f19dbde3c945..4a5eff3f18bc 100644
> > --- a/drivers/iio/inkern.c
> > +++ b/drivers/iio/inkern.c
> > @@ -759,16 +759,8 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
> >  	int ret;
> >  	int type;
> >  
> > -	mutex_lock(&chan->indio_dev->info_exist_lock);
> > -	if (!chan->indio_dev->info) {
> > -		ret = -ENODEV;
> > -		goto err_unlock;
> > -	}
> > -
> > -	ret = iio_channel_read_avail(chan,
> > -				     vals, &type, length, IIO_CHAN_INFO_RAW);
> > -err_unlock:
> > -	mutex_unlock(&chan->indio_dev->info_exist_lock);
> > +	ret = iio_read_avail_channel_attribute(chan, vals, &type, length,
> > +					 IIO_CHAN_INFO_RAW);
> >  
> >  	if (ret >= 0 && type != IIO_VAL_INT)
> >  		/* raw values are assumed to be IIO_VAL_INT */  
> 

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Jonathan Cameron]

On Sun, 7 Apr 2019 18:52:34 +0200
Sebastian Reichel <sre@kernel.org> wrote:

> Hi,
> 
> On Sun, Mar 24, 2019 at 03:31:37PM +0000, Jonathan Cameron wrote:
> > On Sat, 23 Mar 2019 18:28:09 +0100
> > Artur Rojek <contact@artur-rojek.eu> wrote:
> >   
> > > Add a driver for battery present on Ingenic JZ47xx SoCs.
> > > 
> > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>  
> > The IIO parts look fine to me.
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Sebastian, assuming you are happy with this version,  
> 
> The driver itself looks ok. I'm a bit unhappy, that we already have
> jz4740-battery. This driver is much cleaner, but does not yet seem
> to be ready to replace it. Artur Rojek what are your plans regarding
> to the existing driver? Is there currently work going on migrating
> JZ47xx to DT?
> 
> > do you have any preference for how we handle this series?
> > 
> > Probably needs one of us to do an immutable branch with just this
> > in it.  I'm happy to do so once all the relevant Acks etc are in
> > place, but don't mind if you want to!  
> 
> I suppose you could provide an immutable branch with just the first
> two patches in it. Then I can pull it and apply the power-supply
> patches on top.
Done. ib-jz47xx-battery-prereq branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

which is based on v5.0

I'll pull that into the togreg branch of iio.git shortly.

Thanks,

Jonathan

> 
> -- Sebastian
> 
> > Thanks,
> > 
> > Jonathan
> >   
> > > ---
> > > 
> > > Changes:
> > > 
> > > v2: - rework the return logic in ingenic_battery_get_property,
> > >     - make index offsets point forward in ingenic_battery_set_scale,
> > >     - fix spacing around scale_raw[best_idx + 1]
> > > 
> > >  drivers/power/supply/Kconfig           |  11 ++
> > >  drivers/power/supply/Makefile          |   1 +
> > >  drivers/power/supply/ingenic-battery.c | 180 +++++++++++++++++++++++++
> > >  3 files changed, 192 insertions(+)
> > >  create mode 100644 drivers/power/supply/ingenic-battery.c
> > > 
> > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> > > index e901b9879e7e..9bfb1637ec28 100644
> > > --- a/drivers/power/supply/Kconfig
> > > +++ b/drivers/power/supply/Kconfig
> > > @@ -169,6 +169,17 @@ config BATTERY_COLLIE
> > >  	  Say Y to enable support for the battery on the Sharp Zaurus
> > >  	  SL-5500 (collie) models.
> > >  
> > > +config BATTERY_INGENIC
> > > +	tristate "Ingenic JZ47xx SoCs battery driver"
> > > +	depends on MIPS || COMPILE_TEST
> > > +	depends on INGENIC_ADC
> > > +	help
> > > +	  Choose this option if you want to monitor battery status on
> > > +	  Ingenic JZ47xx SoC based devices.
> > > +
> > > +	  This driver can also be built as a module. If so, the module will be
> > > +	  called ingenic-battery.
> > > +
> > >  config BATTERY_IPAQ_MICRO
> > >  	tristate "iPAQ Atmel Micro ASIC battery driver"
> > >  	depends on MFD_IPAQ_MICRO
> > > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> > > index b731c2a9b695..9e2c481d0187 100644
> > > --- a/drivers/power/supply/Makefile
> > > +++ b/drivers/power/supply/Makefile
> > > @@ -34,6 +34,7 @@ obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
> > >  obj-$(CONFIG_BATTERY_OLPC)	+= olpc_battery.o
> > >  obj-$(CONFIG_BATTERY_TOSA)	+= tosa_battery.o
> > >  obj-$(CONFIG_BATTERY_COLLIE)	+= collie_battery.o
> > > +obj-$(CONFIG_BATTERY_INGENIC)	+= ingenic-battery.o
> > >  obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
> > >  obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
> > >  obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
> > > diff --git a/drivers/power/supply/ingenic-battery.c b/drivers/power/supply/ingenic-battery.c
> > > new file mode 100644
> > > index 000000000000..822aee1c7b64
> > > --- /dev/null
> > > +++ b/drivers/power/supply/ingenic-battery.c
> > > @@ -0,0 +1,180 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Battery driver for the Ingenic JZ47xx SoCs
> > > + * Copyright (c) 2019 Artur Rojek <contact@artur-rojek.eu>
> > > + *
> > > + * based on drivers/power/supply/jz4740-battery.c
> > > + */
> > > +
> > > +#include <linux/iio/consumer.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/power_supply.h>
> > > +#include <linux/property.h>
> > > +
> > > +struct ingenic_battery {
> > > +	struct device *dev;
> > > +	struct iio_channel *channel;
> > > +	struct power_supply_desc desc;
> > > +	struct power_supply *battery;
> > > +	struct power_supply_battery_info info;
> > > +};
> > > +
> > > +static int ingenic_battery_get_property(struct power_supply *psy,
> > > +					enum power_supply_property psp,
> > > +					union power_supply_propval *val)
> > > +{
> > > +	struct ingenic_battery *bat = power_supply_get_drvdata(psy);
> > > +	struct power_supply_battery_info *info = &bat->info;
> > > +	int ret;
> > > +
> > > +	switch (psp) {
> > > +	case POWER_SUPPLY_PROP_HEALTH:
> > > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> > > +		val->intval *= 1000;
> > > +		if (val->intval < info->voltage_min_design_uv)
> > > +			val->intval = POWER_SUPPLY_HEALTH_DEAD;
> > > +		else if (val->intval > info->voltage_max_design_uv)
> > > +			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > > +		else
> > > +			val->intval = POWER_SUPPLY_HEALTH_GOOD;
> > > +		return ret;
> > > +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> > > +		ret = iio_read_channel_processed(bat->channel, &val->intval);
> > > +		val->intval *= 1000;
> > > +		return ret;
> > > +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> > > +		val->intval = info->voltage_min_design_uv;
> > > +		return 0;
> > > +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> > > +		val->intval = info->voltage_max_design_uv;
> > > +		return 0;
> > > +	default:
> > > +		return -EINVAL;
> > > +	};
> > > +}
> > > +
> > > +/* Set the most appropriate IIO channel voltage reference scale
> > > + * based on the battery's max voltage.
> > > + */
> > > +static int ingenic_battery_set_scale(struct ingenic_battery *bat)
> > > +{
> > > +	const int *scale_raw;
> > > +	int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
> > > +	u64 max_mV;
> > > +
> > > +	ret = iio_read_max_channel_raw(bat->channel, &max_raw);
> > > +	if (ret) {
> > > +		dev_err(bat->dev, "Unable to read max raw channel value\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
> > > +					       &scale_type, &scale_len,
> > > +					       IIO_CHAN_INFO_SCALE);
> > > +	if (ret < 0) {
> > > +		dev_err(bat->dev, "Unable to read channel avail scale\n");
> > > +		return ret;
> > > +	}
> > > +	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2)
> > > +		return -EINVAL;
> > > +
> > > +	max_mV = bat->info.voltage_max_design_uv / 1000;
> > > +
> > > +	for (i = 0; i < scale_len; i += 2) {
> > > +		u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
> > > +
> > > +		if (scale_mV < max_mV)
> > > +			continue;
> > > +
> > > +		if (best_idx >= 0 && scale_mV > best_mV)
> > > +			continue;
> > > +
> > > +		best_mV = scale_mV;
> > > +		best_idx = i;
> > > +	}
> > > +
> > > +	if (best_idx < 0) {
> > > +		dev_err(bat->dev, "Unable to find matching voltage scale\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return iio_write_channel_attribute(bat->channel,
> > > +					   scale_raw[best_idx],
> > > +					   scale_raw[best_idx + 1],
> > > +					   IIO_CHAN_INFO_SCALE);
> > > +}
> > > +
> > > +static enum power_supply_property ingenic_battery_properties[] = {
> > > +	POWER_SUPPLY_PROP_HEALTH,
> > > +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> > > +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> > > +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> > > +};
> > > +
> > > +static int ingenic_battery_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *dev = &pdev->dev;
> > > +	struct ingenic_battery *bat;
> > > +	struct power_supply_config psy_cfg = {};
> > > +	struct power_supply_desc *desc;
> > > +	int ret;
> > > +
> > > +	bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
> > > +	if (!bat)
> > > +		return -ENOMEM;
> > > +
> > > +	bat->dev = dev;
> > > +	bat->channel = devm_iio_channel_get(dev, "battery");
> > > +	if (IS_ERR(bat->channel))
> > > +		return PTR_ERR(bat->channel);
> > > +
> > > +	desc = &bat->desc;
> > > +	desc->name = "jz-battery";
> > > +	desc->type = POWER_SUPPLY_TYPE_BATTERY;
> > > +	desc->properties = ingenic_battery_properties;
> > > +	desc->num_properties = ARRAY_SIZE(ingenic_battery_properties);
> > > +	desc->get_property = ingenic_battery_get_property;
> > > +	psy_cfg.drv_data = bat;
> > > +	psy_cfg.of_node = dev->of_node;
> > > +
> > > +	bat->battery = devm_power_supply_register(dev, desc, &psy_cfg);
> > > +	if (IS_ERR(bat->battery)) {
> > > +		dev_err(dev, "Unable to register battery\n");
> > > +		return PTR_ERR(bat->battery);
> > > +	}
> > > +
> > > +	ret = power_supply_get_battery_info(bat->battery, &bat->info);
> > > +	if (ret) {
> > > +		dev_err(dev, "Unable to get battery info: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +	if (bat->info.voltage_min_design_uv < 0) {
> > > +		dev_err(dev, "Unable to get voltage min design\n");
> > > +		return bat->info.voltage_min_design_uv;
> > > +	}
> > > +	if (bat->info.voltage_max_design_uv < 0) {
> > > +		dev_err(dev, "Unable to get voltage max design\n");
> > > +		return bat->info.voltage_max_design_uv;
> > > +	}
> > > +
> > > +	return ingenic_battery_set_scale(bat);
> > > +}
> > > +
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id ingenic_battery_of_match[] = {
> > > +	{ .compatible = "ingenic,jz4740-battery", },
> > > +	{ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ingenic_battery_of_match);
> > > +#endif
> > > +
> > > +static struct platform_driver ingenic_battery_driver = {
> > > +	.driver = {
> > > +		.name = "ingenic-battery",
> > > +		.of_match_table = of_match_ptr(ingenic_battery_of_match),  
> > 
> > I guess we can be fairly sure no one will even user this with ACPI
> > (magic DT) bindings?  That would make this a rare case where the
> > of_match_pr protection and ifdef fun is still fine.
> > 
> >   
> > > +	},
> > > +	.probe = ingenic_battery_probe,
> > > +};
> > > +module_platform_driver(ingenic_battery_driver);  
> >   

Re: [PATCH v2 4/4] power: supply: add Ingenic JZ47xx battery driver.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Hi,

On Sat, Mar 23, 2019 at 06:28:09PM +0100, Artur Rojek wrote:
> Add a driver for battery present on Ingenic JZ47xx SoCs.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

I just applied the patch, but it results in the following warning:

WARNING: modpost: missing MODULE_LICENSE() in drivers/power/supply/ingenic-battery.o

Please add MODULE_LICENSE & Co and send a v4 of patch 4/4.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 3/4] dt-bindings: power: supply: Add docs for Ingenic JZ47xx SoCs battery.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

Hi,

On Sun, Mar 24, 2019 at 03:30:18PM +0000, Jonathan Cameron wrote:
> On Sat, 23 Mar 2019 18:28:08 +0100
> Artur Rojek <contact@artur-rojek.eu> wrote:
> 
> > Add documentation for the ingenic-battery driver, used on JZ47xx SoCs.
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> Looks fine to me.
> 
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks, queued.

-- Sebastian

> 
> > ---
> > 
> > Changes:
> > 
> > v2: no change
> > 
> >  .../bindings/power/supply/ingenic,battery.txt | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> > new file mode 100644
> > index 000000000000..66430bf73815
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/supply/ingenic,battery.txt
> > @@ -0,0 +1,31 @@
> > +* Ingenic JZ47xx battery bindings
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "ingenic,jz4740-battery".
> > +- io-channels: phandle and IIO specifier pair to the IIO device.
> > +  Format described in iio-bindings.txt.
> > +- monitored-battery: phandle to a "simple-battery" compatible node.
> > +
> > +The "monitored-battery" property must be a phandle to a node using the format
> > +described in battery.txt, with the following properties being required:
> > +
> > +- voltage-min-design-microvolt: Drained battery voltage.
> > +- voltage-max-design-microvolt: Fully charged battery voltage.
> > +
> > +Example:
> > +
> > +#include <dt-bindings/iio/adc/ingenic,adc.h>
> > +
> > +simple_battery: battery {
> > +	compatible = "simple-battery";
> > +	voltage-min-design-microvolt = <3600000>;
> > +	voltage-max-design-microvolt = <4200000>;
> > +};
> > +
> > +ingenic_battery {
> > +	compatible = "ingenic,jz4740-battery";
> > +	io-channels = <&adc INGENIC_ADC_BATTERY>;
> > +	io-channel-names = "battery";
> > +	monitored-battery = <&simple_battery>;
> > +};
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/4] iio: inkern: API for reading available iio channel attribute values

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

Hi,

On Sun, Apr 14, 2019 at 11:34:54AM +0100, Jonathan Cameron wrote:
> On Sun, 24 Mar 2019 15:27:25 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Sat, 23 Mar 2019 18:28:06 +0100
> > Artur Rojek <contact@artur-rojek.eu> wrote:
> > 
> > > Extend the inkern API with a function for reading available
> > > attribute values of iio channels.
> > > 
> > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>  
> > If this goes through a route other than IIO (otherwise
> > I'll just add a signed-off-by...)
> > 
> > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Applied to the ib-jz47xx-prereq branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> 
> I'll merge that into the togreg branch as well shortly.

Thanks, I pulled that branch into power-supply next for the
other two patches.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]