Re: [PATCH 13/15] twl4030_charger: add ac/mode to match usb/mode

From: NeilBrown <neilb@suse.de>
To: Grazvydas Ignotas <notasas@gmail.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>,
	Tony Lindgren <tony@atomide.com>,
	Lee Jones <lee.jones@linaro.org>,
	Sebastian Reichel <sre@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	GTA04 owners <gta04-owner@goldelico.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH 13/15] twl4030_charger: add ac/mode to match usb/mode
Date: Mon, 23 Mar 2015 10:13:19 +1100	[thread overview]
Message-ID: <20150323101319.642c6638@notabene.brown> (raw)
In-Reply-To: <CANOLnOOJhz3JSevuz7T+XZDquEvJ-UiVM4KVHxxa4mSqb-yTpg@mail.gmail.com>

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

On Fri, 6 Mar 2015 23:59:04 +0200 Grazvydas Ignotas <notasas@gmail.com> wrote:

> On Tue, Feb 24, 2015 at 6:33 AM, NeilBrown <neilb@suse.de> wrote:
> > This allows AC charging to be turned off, much like usb charging.
> >
> > "continuous" (aka "linear") mode maps to the CVENAC (constant voltage)
> > feature of the twl4030.
> 
> Are you sure? Before your patches CVENAC was set at all times and and
> charger still worked in automatic mode.
> 
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/power/twl4030_charger.c |   40 +++++++++++++++++++++++++++++----------
> >  1 file changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 6c53f0b601a4..e5a0225ea87e 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -112,7 +112,7 @@ 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 */
> > +       int                     usb_mode, ac_mode; /* charging mode requested */
> >  #define        CHARGE_OFF      0
> >  #define        CHARGE_AUTO     1
> >  #define        CHARGE_LINEAR   2
> > @@ -449,12 +449,18 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> >  /*
> >   * Enable/Disable AC Charge funtionality.
> >   */
> > -static int twl4030_charger_enable_ac(bool enable)
> > +static int twl4030_charger_enable_ac(struct twl4030_bci *bci, bool enable)
> >  {
> >         int ret;
> >
> > -       if (enable)
> > -               ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
> > +       if (bci->ac_mode == CHARGE_OFF)
> > +               enable = false;
> > +
> > +       if (enable && bci->ac_mode == CHARGE_LINEAR)
> > +               ret = twl4030_clear_set_boot_bci(0, (TWL4030_CVENAC |
> > +                                                    TWL4030_BCIAUTOAC));
> > +       else if (enable)
> > +               ret = twl4030_clear_set_boot_bci(TWL4030_CVENAC, TWL4030_BCIAUTOAC);
> >         else
> >                 ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC, 0);
> 
> CVENAC is required to be set for operation on AC without battery
> (which works fine on most pandora boards). After this patch, when
> booted without battery,  the board will reset before there is a chance
> to set the linear mode by userspace, because this is called on
> probe...
> 

Yes, it looks like I misunderstood CVENAC a bit - thanks.

I've removed 'continuous' mode for AC and no longer clear or set that bit.

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]