diff mbox

[11/15] twl4030_charger: enable manual enable/disable of usb charging.

Message ID 20150224043352.4243.543.stgit@notabene.brown (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

NeilBrown Feb. 24, 2015, 4:33 a.m. UTC
'off' or 'auto' to

 /sys/class/power/twl4030_usb/mode

will now enable or disable charging from USB port.  Normally this is
enabled on 'plug' and disabled on 'unplug'.
Unplug will still disable charging.  'plug' will only enable it if
'auto' if selected.

Signed-off-by: NeilBrown <neilb@suse.de>

Conflicts:
	drivers/power/twl4030_charger.c
---
 drivers/power/twl4030_charger.c |   57 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)



--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pavel Machek March 2, 2015, 9:03 p.m. UTC | #1
On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> 'off' or 'auto' to
> 
>  /sys/class/power/twl4030_usb/mode
> 
> will now enable or disable charging from USB port.  Normally this is
> enabled on 'plug' and disabled on 'unplug'.
> Unplug will still disable charging.  'plug' will only enable it if
> 'auto' if selected.

This should go to Documentation/

> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Conflicts:
> 	drivers/power/twl4030_charger.c

Is it supposed to be here?

> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index 01090a440583..19e8dbb1303e 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -107,6 +107,9 @@ struct twl4030_bci {
>  	int			ichg_eoc, ichg_lo, ichg_hi;
>  	int			usb_cur, ac_cur;
>  	bool			ac_is_active;
> +	int			usb_mode; /* charging mode requested */
> +#define	CHARGE_OFF	0
> +#define	CHARGE_AUTO	1
>  
>  	unsigned long		event;
>  };
> @@ -377,6 +380,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
>  {
>  	int ret;
>  
n> +	if (bci->usb_mode == CHARGE_OFF)
> +		enable = false;

Sometimes, you use = false and sometimes = 0 when assigning to bools...

> @@ -629,6 +634,54 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
>  	return NOTIFY_OK;
>  }
>  
> +/*
> + * sysfs charger enabled store
> + */
> +static char *modes[] = { "off", "auto" };

I'd move this before the comment. Or better near struct twl4030_bci.

> +static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
> +		   twl4030_bci_mode_store);
> +
>  static int twl4030_charger_get_current(void)
>  {
>  	int curr;
> @@ -799,6 +852,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
>  		bci->usb_cur = 500000;  /* 500mA */
>  	else
>  		bci->usb_cur = 100000;  /* 100mA */
> +	bci->usb_mode = CHARGE_AUTO;
>  
>  	bci->dev = &pdev->dev;
>  	bci->irq_chg = platform_get_irq(pdev, 0);
> @@ -885,6 +939,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
>  	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_max_current))
>  		dev_warn(&pdev->dev, "could not create sysfs file\n");
>  
> @@ -917,6 +973,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
>  
>  	device_remove_file(bci->usb.dev, &dev_attr_max_current);
>  	device_remove_file(bci->ac.dev, &dev_attr_max_current);
> +	device_remove_file(bci->usb.dev, &dev_attr_mode);

move the line above for consistency with create?

Acked-by: Pavel Machek <pavel@ucw.cz>
NeilBrown March 4, 2015, 6:15 a.m. UTC | #2
On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > 'off' or 'auto' to
> > 
> >  /sys/class/power/twl4030_usb/mode
> > 
> > will now enable or disable charging from USB port.  Normally this is
> > enabled on 'plug' and disabled on 'unplug'.
> > Unplug will still disable charging.  'plug' will only enable it if
> > 'auto' if selected.
> 
> This should go to Documentation/

You mean in Documentation/ABI/stable I guess??

That strikes me as a failed experiment - there is hardly anything there.

I'd be very happy to put the documentation with the code if there was some
mechanism to automatically extract it.  But I really see little value in
Documentation/ABI.

Or did you mean somewhere else?


> 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > 
> > Conflicts:
> > 	drivers/power/twl4030_charger.c
> 
> Is it supposed to be here?

ah, no.  Gone now.  Thanks.


> 
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 01090a440583..19e8dbb1303e 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -107,6 +107,9 @@ struct twl4030_bci {
> >  	int			ichg_eoc, ichg_lo, ichg_hi;
> >  	int			usb_cur, ac_cur;
> >  	bool			ac_is_active;
> > +	int			usb_mode; /* charging mode requested */
> > +#define	CHARGE_OFF	0
> > +#define	CHARGE_AUTO	1
> >  
> >  	unsigned long		event;
> >  };
> > @@ -377,6 +380,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> >  {
> >  	int ret;
> >  
> n> +	if (bci->usb_mode == CHARGE_OFF)
> > +		enable = false;
> 
> Sometimes, you use = false and sometimes = 0 when assigning to bools...

I found a fixed a few - thanks.


> 
> > @@ -629,6 +634,54 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
> >  	return NOTIFY_OK;
> >  }
> >  
> > +/*
> > + * sysfs charger enabled store
> > + */
> > +static char *modes[] = { "off", "auto" };
> 
> I'd move this before the comment. Or better near struct twl4030_bci.

Makes sense.  Done.  Thanks.


> 
> > +static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
> > +		   twl4030_bci_mode_store);
> > +
> >  static int twl4030_charger_get_current(void)
> >  {
> >  	int curr;
> > @@ -799,6 +852,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
> >  		bci->usb_cur = 500000;  /* 500mA */
> >  	else
> >  		bci->usb_cur = 100000;  /* 100mA */
> > +	bci->usb_mode = CHARGE_AUTO;
> >  
> >  	bci->dev = &pdev->dev;
> >  	bci->irq_chg = platform_get_irq(pdev, 0);
> > @@ -885,6 +939,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
> >  	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_max_current))
> >  		dev_warn(&pdev->dev, "could not create sysfs file\n");
> >  
> > @@ -917,6 +973,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
> >  
> >  	device_remove_file(bci->usb.dev, &dev_attr_max_current);
> >  	device_remove_file(bci->ac.dev, &dev_attr_max_current);
> > +	device_remove_file(bci->usb.dev, &dev_attr_mode);
> 
> move the line above for consistency with create?

Yep.

> 
> Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks a lot!

NeilBrown
Pavel Machek March 4, 2015, 10:19 a.m. UTC | #3
On Wed 2015-03-04 17:15:09, NeilBrown wrote:
> On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > > 'off' or 'auto' to
> > > 
> > >  /sys/class/power/twl4030_usb/mode
> > > 
> > > will now enable or disable charging from USB port.  Normally this is
> > > enabled on 'plug' and disabled on 'unplug'.
> > > Unplug will still disable charging.  'plug' will only enable it if
> > > 'auto' if selected.
> > 
> > This should go to Documentation/
> 
> You mean in Documentation/ABI/stable I guess??

Yes.

> That strikes me as a failed experiment - there is hardly anything there.
> 
> I'd be very happy to put the documentation with the code if there was some
> mechanism to automatically extract it.  But I really see little value in
> Documentation/ABI.

It was useful in past, and rules say it is required. Feel free to talk
to greg about better place, but this is the place we have now and
documentation is useful. And since you already document it in the
changelogs... it should not be that much work.

> > Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> Thanks a lot!

You are welcome :-).
									Pavel
diff mbox

Patch

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 01090a440583..19e8dbb1303e 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -107,6 +107,9 @@  struct twl4030_bci {
 	int			ichg_eoc, ichg_lo, ichg_hi;
 	int			usb_cur, ac_cur;
 	bool			ac_is_active;
+	int			usb_mode; /* charging mode requested */
+#define	CHARGE_OFF	0
+#define	CHARGE_AUTO	1
 
 	unsigned long		event;
 };
@@ -377,6 +380,8 @@  static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 {
 	int ret;
 
+	if (bci->usb_mode == CHARGE_OFF)
+		enable = false;
 	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
 
 		twl4030_charger_update_current(bci);
@@ -629,6 +634,54 @@  static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 	return NOTIFY_OK;
 }
 
+/*
+ * sysfs charger enabled store
+ */
+static char *modes[] = { "off", "auto" };
+static ssize_t
+twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t n)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+	int mode;
+	int status;
+
+	if (sysfs_streq(buf, modes[0]))
+		mode = 0;
+	else if (sysfs_streq(buf, modes[1]))
+		mode = 1;
+	else
+		return -EINVAL;
+	twl4030_charger_enable_usb(bci, false);
+	bci->usb_mode = mode;
+	status = twl4030_charger_enable_usb(bci, true);
+	return (status == 0) ? n : status;
+}
+
+/*
+ * sysfs charger enabled show
+ */
+static ssize_t
+twl4030_bci_mode_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+	int len = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(modes); i++)
+		if (bci->usb_mode == i)
+			len += snprintf(buf+len, PAGE_SIZE-len,
+					"[%s] ", modes[i]);
+		else
+			len += snprintf(buf+len, PAGE_SIZE-len,
+					"%s ", modes[i]);
+	buf[len-1] = '\n';
+	return len;
+}
+static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
+		   twl4030_bci_mode_store);
+
 static int twl4030_charger_get_current(void)
 {
 	int curr;
@@ -799,6 +852,7 @@  static int __init twl4030_bci_probe(struct platform_device *pdev)
 		bci->usb_cur = 500000;  /* 500mA */
 	else
 		bci->usb_cur = 100000;  /* 100mA */
+	bci->usb_mode = CHARGE_AUTO;
 
 	bci->dev = &pdev->dev;
 	bci->irq_chg = platform_get_irq(pdev, 0);
@@ -885,6 +939,8 @@  static int __init twl4030_bci_probe(struct platform_device *pdev)
 	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_max_current))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
@@ -917,6 +973,7 @@  static int __exit twl4030_bci_remove(struct platform_device *pdev)
 
 	device_remove_file(bci->usb.dev, &dev_attr_max_current);
 	device_remove_file(bci->ac.dev, &dev_attr_max_current);
+	device_remove_file(bci->usb.dev, &dev_attr_mode);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR1A);