[07/13] twl4030_charger: distinguish between USB current and 'AC' current

Commit Message

NeilBrown Aug. 7, 2015, 3:45 a.m. UTC

On Thu, 6 Aug 2015 20:11:16 -0700 Tony Lindgren <[e-mail]>
wrote:

> * NeilBrown <[e-mail]> [150729 17:28]:
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> >  static int twl4030_charger_update_current(struct twl4030_bci *bci)
> >  {
> >  	int status;
> > +	int cur;
> >  	unsigned reg, cur_reg;
> >  	u8 bcictl1, oldreg, fullreg;
> >  	bool cgain = false;
> >  	u8 boot_bci;
> >  
> > +	/*
> > +	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> > +	 * and AC is enabled, set current for 'ac'
> > +	 */
> > +	if (twl4030_get_madc_conversion(11) > 4500) {
> > +		cur = bci->ac_cur;
> > +		bci->ac_is_active = true;
> > +	} else {
> > +		cur = bci->usb_cur;
> > +		bci->ac_is_active = false;
> > +	}
> > +
> >  	/* First, check thresholds and see if cgain is needed */
> >  	if (bci->ichg_eoc >= 200000)
> >  		cgain = true;
> 
> Neil, you need a stub or something for twl4030_get_madc_conversion
> if madc is not selected. Now at least omap2plus_defconfig and
> ARM allmodconfig fails in Linux next.
> 
> Regards,
> 
> Tony

Thanks, I did get notified about that by Fengguang's test robot, but
it's still on my list....

I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
acceptable?  That would pull in IIO (it didn't use to...).

If this OK?

Thanks,
NeilBrown


From: NeilBrown <[e-mail]>
Date: Fri, 7 Aug 2015 13:44:37 +1000
Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
 available.

We can only use the madc to check for 'ac' availability
if the madc has been compiled in.
If not: assume always using USB.

Reported-by: Tony Lindgren <[e-mail]>
Signed-off-by: NeilBrown <[e-mail]>

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

Comments

Tony Lindgren Aug. 7, 2015, 4:21 a.m. UTC | #1

* NeilBrown <[e-mail]> [150806 20:48]:
> 
> Thanks, I did get notified about that by Fengguang's test robot, but
> it's still on my list....
> 
> I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
> acceptable?  That would pull in IIO (it didn't use to...).
> 
> If this OK?

Looks OK to me thanks:

Acked-by: Tony Lindgren <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sebastian Reichel Aug. 7, 2015, 5:13 a.m. UTC | #2

Hi,

This actually slipped through my review. IMHO madc should be
accessed through IIO, as already done for twl4030-madc-battery
and rx51-battery. That way the custom API can be removed at
some point.

Anyway, I queued the below patch with Tony's ACK to fix the build
issue in next.

On Fri, Aug 07, 2015 at 01:45:25PM +1000, NeilBrown wrote:
> From: NeilBrown <[e-mail]>
> Date: Fri, 7 Aug 2015 13:44:37 +1000
> Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
>  available.
> 
> We can only use the madc to check for 'ac' availability
> if the madc has been compiled in.
> If not: assume always using USB.
> 
> Reported-by: Tony Lindgren <[e-mail]>
> Signed-off-by: NeilBrown <[e-mail]>
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index c7432f532a83..265fd236f4c0 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -91,6 +91,21 @@
>  #define TWL4030_MSTATEC_COMPLETE1	0x0b
>  #define TWL4030_MSTATEC_COMPLETE4	0x0e
>  
> +#if IS_ENABLED(CONFIG_TWL4030_MADC)
> +/*
> + * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> + * then AC is available.
> + */
> +static inline int ac_available(void)
> +{
> +	return twl4030_get_madc_conversion(11) > 4500;
> +}
> +#else
> +static inline int ac_available(void)
> +{
> +	return 0;
> +}
> +#endif
>  static bool allow_usb;
>  module_param(allow_usb, bool, 0644);
>  MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
> @@ -263,7 +278,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
>  	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
>  	 * and AC is enabled, set current for 'ac'
>  	 */
> -	if (twl4030_get_madc_conversion(11) > 4500) {
> +	if (ac_available()) {
>  		cur = bci->ac_cur;
>  		bci->ac_is_active = true;
>  	} else {

-- Sebastian

NeilBrown Aug. 7, 2015, 5:29 a.m. UTC | #3

On Fri, 7 Aug 2015 07:13:09 +0200 Sebastian Reichel <[e-mail]>
wrote:

> Hi,
> 
> This actually slipped through my review. IMHO madc should be
> accessed through IIO, as already done for twl4030-madc-battery
> and rx51-battery. That way the custom API can be removed at
> some point.
> 
> Anyway, I queued the below patch with Tony's ACK to fix the build
> issue in next.
> 

OK, thanks.

I'll try to figure out are more proper approach ... might be a week or
so though.

Thanks,
NeilBrown

Patch

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index c7432f532a83..265fd236f4c0 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -91,6 +91,21 @@ 
 #define TWL4030_MSTATEC_COMPLETE1	0x0b
 #define TWL4030_MSTATEC_COMPLETE4	0x0e
 
+#if IS_ENABLED(CONFIG_TWL4030_MADC)
+/*
+ * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
+ * then AC is available.
+ */
+static inline int ac_available(void)
+{
+	return twl4030_get_madc_conversion(11) > 4500;
+}
+#else
+static inline int ac_available(void)
+{
+	return 0;
+}
+#endif
 static bool allow_usb;
 module_param(allow_usb, bool, 0644);
 MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
@@ -263,7 +278,7 @@  static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
 	 * and AC is enabled, set current for 'ac'
 	 */
-	if (twl4030_get_madc_conversion(11) > 4500) {
+	if (ac_available()) {
 		cur = bci->ac_cur;
 		bci->ac_is_active = true;
 	} else {