All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Fixes for twl4030 charger
@ 2017-04-14 18:25 H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 1/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-04-14 18:25 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel, H. Nikolaus Schaller

Changes V3:
* worked in comments by Sebsatian Reichel
* clarifications of some commit messages
* rebased on v4.11rc-6

It took a while (18 months) until we propose an updated patch for upstream...

2015-11-02 12:27:39: Changes V2:
* worked in comments by Nishanth Menon <nm@ti.com>
* added another patch which solves a probing/boot stall problem (irq allocation vs. -EPROBE_DEFER)

V1:
4.3-rc1 introduced a new charger driver for the twl4030. This patch set fixes some
issues.

While making twl4030 changes from 4.3 operable we have found some issues
during testing on GTA04 and OpenPandora.

H. Nikolaus Schaller (3):
  drivers:power:twl4030-charger: don't check if battery is present
  drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and
    make it writeable
  drivers:power:twl4030-charger: remove nonstandard max_current sysfs
    attribute

 drivers/power/supply/twl4030_charger.c | 158 ++++++++++++---------------------
 1 file changed, 59 insertions(+), 99 deletions(-)

-- 
2.7.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/3] drivers:power:twl4030-charger: don't check if battery is present
  2017-04-14 18:25 [PATCH v3 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
@ 2017-04-14 18:25 ` H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute H. Nikolaus Schaller
  2 siblings, 0 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-04-14 18:25 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel, H. Nikolaus Schaller

We can't assume that the battery is or stays present after probing
on devices with replaceable battery.

On some devices (e.g. GTA04 or OpenPanodra) it can be removed
and even be hot swapped by the user while device continues to operate
through external AC or USB power (as long as system power consumption
remains below ca. 500mA as provided by USB).

So it makes no sense to check for this situation during probe and make
the charger driver (and its status reports) completely non-operational if
the battery can be inserted later.

Tested on: GTA04 and OpenPandora.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/power/supply/twl4030_charger.c | 36 ----------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
index 7bcff1c1..34e389d 100644
--- a/drivers/power/supply/twl4030_charger.c
+++ b/drivers/power/supply/twl4030_charger.c
@@ -206,35 +206,6 @@ static int twl4030bci_read_adc_val(u8 reg)
 }
 
 /*
- * Check if Battery Pack was present
- */
-static int twl4030_is_battery_present(struct twl4030_bci *bci)
-{
-	int ret;
-	u8 val = 0;
-
-	/* Battery presence in Main charge? */
-	ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE, &val, TWL4030_BCIMFSTS3);
-	if (ret)
-		return ret;
-	if (val & TWL4030_BATSTSMCHG)
-		return 0;
-
-	/*
-	 * OK, It could be that bootloader did not enable main charger,
-	 * pre-charge is h/w auto. So, Battery presence in Pre-charge?
-	 */
-	ret = twl_i2c_read_u8(TWL4030_MODULE_PRECHARGE, &val,
-			      TWL4030_BCIMFSTS1);
-	if (ret)
-		return ret;
-	if (val & TWL4030_BATSTSPCHG)
-		return 0;
-
-	return -ENODEV;
-}
-
-/*
  * TI provided formulas:
  * CGAIN == 0: ICHG = (BCIICHG * 1.7) / (2^10 - 1) - 0.85
  * CGAIN == 1: ICHG = (BCIICHG * 3.4) / (2^10 - 1) - 1.7
@@ -1011,13 +982,6 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	bci->irq_chg = platform_get_irq(pdev, 0);
 	bci->irq_bci = platform_get_irq(pdev, 1);
 
-	/* Only proceed further *IF* battery is physically present */
-	ret = twl4030_is_battery_present(bci);
-	if  (ret) {
-		dev_crit(&pdev->dev, "Battery was not detected:%d\n", ret);
-		return ret;
-	}
-
 	if (bci->dev->of_node) {
 		struct device_node *phynode;
 
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable
  2017-04-14 18:25 [PATCH v3 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 1/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
@ 2017-04-14 18:25 ` H. Nikolaus Schaller
  2017-05-01 11:15   ` Sebastian Reichel
  2017-04-14 18:25 ` [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute H. Nikolaus Schaller
  2 siblings, 1 reply; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-04-14 18:25 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel, H. Nikolaus Schaller

Currently, the twl4030 charger defines its own max_current by directly
creating sysfs nodes. It should use the input_current_limit property
which is e.g. used by the bq24257 driver.

This patch adds the input_current_property with the same semantics as
the max_current property. The code to manage the max_current property
is removed by a separate patch.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/power/supply/twl4030_charger.c | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
index 34e389d..e70c4ed 100644
--- a/drivers/power/supply/twl4030_charger.c
+++ b/drivers/power/supply/twl4030_charger.c
@@ -893,6 +893,28 @@ static int twl4030_bci_get_property(struct power_supply *psy,
 			twl4030_bci_state_to_status(state) !=
 				POWER_SUPPLY_STATUS_NOT_CHARGING;
 		break;
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		val->intval = -1;
+		if (psy->desc->type != POWER_SUPPLY_TYPE_USB) {
+			if (!bci->ac_is_active)
+				val->intval = bci->ac_cur;
+		} else {
+			if (bci->ac_is_active)
+				val->intval = bci->usb_cur_target;
+		}
+		if (val->intval < 0) {
+			u8 bcictl1;
+
+			val->intval = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
+			if (val->intval < 0)
+				return val->intval;
+			ret = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
+			if (ret < 0)
+				return ret;
+			val->intval = regval2ua(val->intval, bcictl1 &
+							TWL4030_CGAIN);
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -900,11 +922,44 @@ static int twl4030_bci_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int twl4030_bci_set_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    const union power_supply_propval *val)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(psy->dev.parent);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
+			bci->usb_cur_target = val->intval;
+		else
+			bci->ac_cur = val->intval;
+		twl4030_charger_update_current(bci);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+int twl4030_bci_property_is_writeable(struct power_supply *psy,
+				      enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static enum power_supply_property twl4030_charger_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
 };
 
 #ifdef CONFIG_OF
@@ -941,6 +996,8 @@ static const struct power_supply_desc twl4030_bci_ac_desc = {
 	.properties	= twl4030_charger_props,
 	.num_properties	= ARRAY_SIZE(twl4030_charger_props),
 	.get_property	= twl4030_bci_get_property,
+	.set_property	= twl4030_bci_set_property,
+	.property_is_writeable	= twl4030_bci_property_is_writeable,
 };
 
 static const struct power_supply_desc twl4030_bci_usb_desc = {
@@ -949,6 +1006,8 @@ static const struct power_supply_desc twl4030_bci_usb_desc = {
 	.properties	= twl4030_charger_props,
 	.num_properties	= ARRAY_SIZE(twl4030_charger_props),
 	.get_property	= twl4030_bci_get_property,
+	.set_property	= twl4030_bci_set_property,
+	.property_is_writeable	= twl4030_bci_property_is_writeable,
 };
 
 static int twl4030_bci_probe(struct platform_device *pdev)
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute
  2017-04-14 18:25 [PATCH v3 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 1/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
  2017-04-14 18:25 ` [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable H. Nikolaus Schaller
@ 2017-04-14 18:25 ` H. Nikolaus Schaller
  2017-05-01 11:26   ` Sebastian Reichel
  2017-05-11 22:35   ` Pavel Machek
  2 siblings, 2 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-04-14 18:25 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel, H. Nikolaus Schaller

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/power/supply/twl4030_charger.c | 63 ----------------------------------
 1 file changed, 63 deletions(-)

diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
index e70c4ed..e02e150 100644
--- a/drivers/power/supply/twl4030_charger.c
+++ b/drivers/power/supply/twl4030_charger.c
@@ -624,63 +624,6 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-/*
- * Provide "max_current" attribute in sysfs.
- */
-static ssize_t
-twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t n)
-{
-	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
-	int cur = 0;
-	int status = 0;
-	status = kstrtoint(buf, 10, &cur);
-	if (status)
-		return status;
-	if (cur < 0)
-		return -EINVAL;
-	if (dev == &bci->ac->dev)
-		bci->ac_cur = cur;
-	else
-		bci->usb_cur_target = cur;
-
-	twl4030_charger_update_current(bci);
-	return n;
-}
-
-/*
- * sysfs max_current show
- */
-static ssize_t twl4030_bci_max_current_show(struct device *dev,
-	struct device_attribute *attr, char *buf)
-{
-	int status = 0;
-	int cur = -1;
-	u8 bcictl1;
-	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
-
-	if (dev == &bci->ac->dev) {
-		if (!bci->ac_is_active)
-			cur = bci->ac_cur;
-	} else {
-		if (bci->ac_is_active)
-			cur = bci->usb_cur_target;
-	}
-	if (cur < 0) {
-		cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
-		if (cur < 0)
-			return cur;
-		status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
-		if (status < 0)
-			return status;
-		cur = regval2ua(cur, bcictl1 & TWL4030_CGAIN);
-	}
-	return scnprintf(buf, PAGE_SIZE, "%u\n", cur);
-}
-
-static DEVICE_ATTR(max_current, 0644, twl4030_bci_max_current_show,
-			twl4030_bci_max_current_store);
-
 static void twl4030_bci_usb_work(struct work_struct *data)
 {
 	struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
@@ -1118,14 +1061,10 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
 
 	twl4030_charger_update_current(bci);
-	if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
-		dev_warn(&pdev->dev, "could not create sysfs file\n");
 	if (device_create_file(&bci->usb->dev, &dev_attr_mode))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 	if (device_create_file(&bci->ac->dev, &dev_attr_mode))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
-	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
-		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
 	twl4030_charger_enable_ac(bci, true);
 	if (!IS_ERR_OR_NULL(bci->transceiver))
@@ -1157,9 +1096,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 
 	iio_channel_release(bci->channel_vac);
 
-	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
 	device_remove_file(&bci->usb->dev, &dev_attr_mode);
-	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
 	device_remove_file(&bci->ac->dev, &dev_attr_mode);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable
  2017-04-14 18:25 ` [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable H. Nikolaus Schaller
@ 2017-05-01 11:15   ` Sebastian Reichel
  2017-05-03 12:18     ` H. Nikolaus Schaller
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Reichel @ 2017-05-01 11:15 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel, linux-kernel

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

Hi,

On Fri, Apr 14, 2017 at 08:25:56PM +0200, H. Nikolaus Schaller wrote:
> Currently, the twl4030 charger defines its own max_current by directly
> creating sysfs nodes. It should use the input_current_limit property
> which is e.g. used by the bq24257 driver.
> 
> This patch adds the input_current_property with the same semantics as
> the max_current property. The code to manage the max_current property
> is removed by a separate patch.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Thanks, queued.

-- Sebastian

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute
  2017-04-14 18:25 ` [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute H. Nikolaus Schaller
@ 2017-05-01 11:26   ` Sebastian Reichel
  2017-05-03 12:20     ` H. Nikolaus Schaller
  2017-05-11 22:35   ` Pavel Machek
  1 sibling, 1 reply; 10+ messages in thread
From: Sebastian Reichel @ 2017-05-01 11:26 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel, linux-kernel

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

Hi,

On Fri, Apr 14, 2017 at 08:25:57PM +0200, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

I'm not queuing this. Please change the following things for the
v4 of this patch:

 - Kernel patches should always have a long description
 - Patch needs to update Documentation/ABI/testing/sysfs-class-power-twl4030
 - Make sure to use recent MAINTAINERS file for getting patch
   destination (Dmitry and David are no longer listed).
 - And for this case: please also Cc NeilBrown <neil@brown.name>. If
   the ABI is in use we cannot simply remove it. If its unlikely,
   that its used I will queue the patch and revert if any problems
   are reported.

-- Sebastian

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable
  2017-05-01 11:15   ` Sebastian Reichel
@ 2017-05-03 12:18     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-03 12:18 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel, linux-kernel

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


> Am 01.05.2017 um 13:15 schrieb Sebastian Reichel <sebastian.reichel@collabora.co.uk>:
> 
> Hi,
> 
> On Fri, Apr 14, 2017 at 08:25:56PM +0200, H. Nikolaus Schaller wrote:
>> Currently, the twl4030 charger defines its own max_current by directly
>> creating sysfs nodes. It should use the input_current_limit property
>> which is e.g. used by the bq24257 driver.
>> 
>> This patch adds the input_current_property with the same semantics as
>> the max_current property. The code to manage the max_current property
>> is removed by a separate patch.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> Thanks, queued.

TNX,
Nikolaus


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute
  2017-05-01 11:26   ` Sebastian Reichel
@ 2017-05-03 12:20     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-03 12:20 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel, linux-kernel

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

Hi,

> Am 01.05.2017 um 13:26 schrieb Sebastian Reichel <sebastian.reichel@collabora.co.uk>:
> 
> Hi,
> 
> On Fri, Apr 14, 2017 at 08:25:57PM +0200, H. Nikolaus Schaller wrote:
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> I'm not queuing this. Please change the following things for the
> v4 of this patch:
> 
> - Kernel patches should always have a long description

Ok.

> - Patch needs to update Documentation/ABI/testing/sysfs-class-power-twl4030

Ok, forgot about this.

> - Make sure to use recent MAINTAINERS file for getting patch
>   destination (Dmitry and David are no longer listed).
> - And for this case: please also Cc NeilBrown <neil@brown.name>. If
>   the ABI is in use we cannot simply remove it. If its unlikely,
>   that its used I will queue the patch and revert if any problems
>   are reported.

Well, Neil did also work for the GTA04 project, so it is the same project.
But I will add him for V4 so that he can comment himself.

> 
> -- Sebastian


BR and thanks,
Nikolaus


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute
  2017-04-14 18:25 ` [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute H. Nikolaus Schaller
  2017-05-01 11:26   ` Sebastian Reichel
@ 2017-05-11 22:35   ` Pavel Machek
  2017-05-12  8:07     ` H. Nikolaus Schaller
  1 sibling, 1 reply; 10+ messages in thread
From: Pavel Machek @ 2017-05-11 22:35 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel

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

On Fri 2017-04-14 20:25:57, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

You should explain how to obtain equivalent functionality without that
attribute.

									Pavel

> ---
>  drivers/power/supply/twl4030_charger.c | 63 ----------------------------------
>  1 file changed, 63 deletions(-)
> 
> diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
> index e70c4ed..e02e150 100644
> --- a/drivers/power/supply/twl4030_charger.c
> +++ b/drivers/power/supply/twl4030_charger.c
> @@ -624,63 +624,6 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> -/*
> - * Provide "max_current" attribute in sysfs.
> - */
> -static ssize_t
> -twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
> -	const char *buf, size_t n)
> -{
> -	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
> -	int cur = 0;
> -	int status = 0;
> -	status = kstrtoint(buf, 10, &cur);
> -	if (status)
> -		return status;
> -	if (cur < 0)
> -		return -EINVAL;
> -	if (dev == &bci->ac->dev)
> -		bci->ac_cur = cur;
> -	else
> -		bci->usb_cur_target = cur;
> -
> -	twl4030_charger_update_current(bci);
> -	return n;
> -}
> -
> -/*
> - * sysfs max_current show
> - */
> -static ssize_t twl4030_bci_max_current_show(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> -{
> -	int status = 0;
> -	int cur = -1;
> -	u8 bcictl1;
> -	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
> -
> -	if (dev == &bci->ac->dev) {
> -		if (!bci->ac_is_active)
> -			cur = bci->ac_cur;
> -	} else {
> -		if (bci->ac_is_active)
> -			cur = bci->usb_cur_target;
> -	}
> -	if (cur < 0) {
> -		cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
> -		if (cur < 0)
> -			return cur;
> -		status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
> -		if (status < 0)
> -			return status;
> -		cur = regval2ua(cur, bcictl1 & TWL4030_CGAIN);
> -	}
> -	return scnprintf(buf, PAGE_SIZE, "%u\n", cur);
> -}
> -
> -static DEVICE_ATTR(max_current, 0644, twl4030_bci_max_current_show,
> -			twl4030_bci_max_current_store);
> -
>  static void twl4030_bci_usb_work(struct work_struct *data)
>  {
>  	struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
> @@ -1118,14 +1061,10 @@ static int twl4030_bci_probe(struct platform_device *pdev)
>  		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
>  
>  	twl4030_charger_update_current(bci);
> -	if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
> -		dev_warn(&pdev->dev, "could not create sysfs file\n");
>  	if (device_create_file(&bci->usb->dev, &dev_attr_mode))
>  		dev_warn(&pdev->dev, "could not create sysfs file\n");
>  	if (device_create_file(&bci->ac->dev, &dev_attr_mode))
>  		dev_warn(&pdev->dev, "could not create sysfs file\n");
> -	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
> -		dev_warn(&pdev->dev, "could not create sysfs file\n");
>  
>  	twl4030_charger_enable_ac(bci, true);
>  	if (!IS_ERR_OR_NULL(bci->transceiver))
> @@ -1157,9 +1096,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
>  
>  	iio_channel_release(bci->channel_vac);
>  
> -	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
>  	device_remove_file(&bci->usb->dev, &dev_attr_mode);
> -	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
>  	device_remove_file(&bci->ac->dev, &dev_attr_mode);
>  	/* mask interrupts */
>  	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute
  2017-05-11 22:35   ` Pavel Machek
@ 2017-05-12  8:07     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 10+ messages in thread
From: H. Nikolaus Schaller @ 2017-05-12  8:07 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
	Grazvydas Ignotas, Andreas Kemnade, linux-pm, letux-kernel,
	linux-kernel

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

Hi Pavel,

> Am 12.05.2017 um 00:35 schrieb Pavel Machek <pavel@ucw.cz>:
> 
> On Fri 2017-04-14 20:25:57, H. Nikolaus Schaller wrote:
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> You should explain how to obtain equivalent functionality without that
> attribute.

By using the standard attribute as defined here:

http://elixir.free-electrons.com/linux/latest/source/Documentation/power/power_supply_class.txt#L125

It was introduced by 6bb1d272d7c9f.

This patch 3/3 here was intended to follow right after the one that adds the
input_current_limit property where it is (partially) described.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=3fb319c2cdcd10f775f4d4a05a7d12fa1a5679c1

But since the patches are broken apart now, we should indeed document this
explicitly in the commit message to avoid any confusion. I had already
planned that for the next patch version.

BR,
Nikolaus

> 
> 									Pavel
> 
>> ---
>> drivers/power/supply/twl4030_charger.c | 63 ----------------------------------
>> 1 file changed, 63 deletions(-)
>> 
>> diff --git a/drivers/power/supply/twl4030_charger.c b/drivers/power/supply/twl4030_charger.c
>> index e70c4ed..e02e150 100644
>> --- a/drivers/power/supply/twl4030_charger.c
>> +++ b/drivers/power/supply/twl4030_charger.c
>> @@ -624,63 +624,6 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
>> 	return IRQ_HANDLED;
>> }
>> 
>> -/*
>> - * Provide "max_current" attribute in sysfs.
>> - */
>> -static ssize_t
>> -twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
>> -	const char *buf, size_t n)
>> -{
>> -	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
>> -	int cur = 0;
>> -	int status = 0;
>> -	status = kstrtoint(buf, 10, &cur);
>> -	if (status)
>> -		return status;
>> -	if (cur < 0)
>> -		return -EINVAL;
>> -	if (dev == &bci->ac->dev)
>> -		bci->ac_cur = cur;
>> -	else
>> -		bci->usb_cur_target = cur;
>> -
>> -	twl4030_charger_update_current(bci);
>> -	return n;
>> -}
>> -
>> -/*
>> - * sysfs max_current show
>> - */
>> -static ssize_t twl4030_bci_max_current_show(struct device *dev,
>> -	struct device_attribute *attr, char *buf)
>> -{
>> -	int status = 0;
>> -	int cur = -1;
>> -	u8 bcictl1;
>> -	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
>> -
>> -	if (dev == &bci->ac->dev) {
>> -		if (!bci->ac_is_active)
>> -			cur = bci->ac_cur;
>> -	} else {
>> -		if (bci->ac_is_active)
>> -			cur = bci->usb_cur_target;
>> -	}
>> -	if (cur < 0) {
>> -		cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
>> -		if (cur < 0)
>> -			return cur;
>> -		status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
>> -		if (status < 0)
>> -			return status;
>> -		cur = regval2ua(cur, bcictl1 & TWL4030_CGAIN);
>> -	}
>> -	return scnprintf(buf, PAGE_SIZE, "%u\n", cur);
>> -}
>> -
>> -static DEVICE_ATTR(max_current, 0644, twl4030_bci_max_current_show,
>> -			twl4030_bci_max_current_store);
>> -
>> static void twl4030_bci_usb_work(struct work_struct *data)
>> {
>> 	struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
>> @@ -1118,14 +1061,10 @@ static int twl4030_bci_probe(struct platform_device *pdev)
>> 		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
>> 
>> 	twl4030_charger_update_current(bci);
>> -	if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
>> -		dev_warn(&pdev->dev, "could not create sysfs file\n");
>> 	if (device_create_file(&bci->usb->dev, &dev_attr_mode))
>> 		dev_warn(&pdev->dev, "could not create sysfs file\n");
>> 	if (device_create_file(&bci->ac->dev, &dev_attr_mode))
>> 		dev_warn(&pdev->dev, "could not create sysfs file\n");
>> -	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
>> -		dev_warn(&pdev->dev, "could not create sysfs file\n");
>> 
>> 	twl4030_charger_enable_ac(bci, true);
>> 	if (!IS_ERR_OR_NULL(bci->transceiver))
>> @@ -1157,9 +1096,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
>> 
>> 	iio_channel_release(bci->channel_vac);
>> 
>> -	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
>> 	device_remove_file(&bci->usb->dev, &dev_attr_mode);
>> -	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
>> 	device_remove_file(&bci->ac->dev, &dev_attr_mode);
>> 	/* mask interrupts */
>> 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
> 
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-12  8:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-14 18:25 [PATCH v3 0/3] Fixes for twl4030 charger H. Nikolaus Schaller
2017-04-14 18:25 ` [PATCH v3 1/3] drivers:power:twl4030-charger: don't check if battery is present H. Nikolaus Schaller
2017-04-14 18:25 ` [PATCH v3 2/3] drivers:power:twl4030-charger: add INPUT_CURRENT_LIMIT property and make it writeable H. Nikolaus Schaller
2017-05-01 11:15   ` Sebastian Reichel
2017-05-03 12:18     ` H. Nikolaus Schaller
2017-04-14 18:25 ` [PATCH v3 3/3] drivers:power:twl4030-charger: remove nonstandard max_current sysfs attribute H. Nikolaus Schaller
2017-05-01 11:26   ` Sebastian Reichel
2017-05-03 12:20     ` H. Nikolaus Schaller
2017-05-11 22:35   ` Pavel Machek
2017-05-12  8:07     ` H. Nikolaus Schaller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.