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

2015-03-04 Thread Pavel Machek
On Wed 2015-03-04 17:15:09, NeilBrown wrote:
 On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek pa...@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 pa...@ucw.cz
 
 Thanks a lot!

You are welcome :-).
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-03 Thread NeilBrown
On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek pa...@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 ne...@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;
  boolac_is_active;
  +   int usb_mode; /* charging mode requested */
  +#defineCHARGE_OFF  0
  +#defineCHARGE_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 = 50;  /* 500mA */
  else
  bci-usb_cur = 10;  /* 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 pa...@ucw.cz

Thanks a lot!

NeilBrown


pgpcgY61j9ce8.pgp
Description: OpenPGP digital signature


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

2015-03-02 Thread Pavel Machek
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 ne...@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;
   boolac_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 = 50;  /* 500mA */
   else
   bci-usb_cur = 10;  /* 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 pa...@ucw.cz
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html