Hi,
this series is a resend of a previous submission with some
contentious patches removed and extra maintainers added.
They allow the twl4030 to charge the backup battery (voltage and
current configured in board file) and allow the charger to continue
charging while the USB PHY is otherwise idle (no gadget module
loaded, or host in suspend-to-RAM).
Anton: could you take the first three? Do I need to split the third
patch into an 'MFD' part for Samuel and a power_supply part for you?
Felipe: could you take the usb/otg patch? It can be applied quite
independently of the others, but won't be effective until they are
also present.
Thanks,
NeilBrown
---
NeilBrown (4):
usb: otg: twl4030-usb: Don't power down phy when it is in-use by charger.
power_supply/MFD: twl4030_charger: Allow charger to control the regulator that feeds it.
power_supply: twl4030_charger: add backup-battery charging.
power_supply: twl4030_charger: Fix some typos
drivers/mfd/twl-core.c | 9 ++--
drivers/power/twl4030_charger.c | 80 ++++++++++++++++++++++++++++++++++++++-
drivers/usb/otg/twl4030-usb.c | 8 +++-
include/linux/i2c/twl.h | 2 +
4 files changed, 91 insertions(+), 8 deletions(-)
--
Signature
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index fdad850..3e6e991 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -103,7 +103,7 @@ static int twl4030_bci_read(u8 reg, u8 *val)
static int twl4030_clear_set_boot_bci(u8 clear, u8 set)
{
- return twl4030_clear_set(TWL4030_MODULE_PM_MASTER, 0,
+ return twl4030_clear_set(TWL4030_MODULE_PM_MASTER, clear,
TWL4030_CONFIG_DONE | TWL4030_BCIAUTOWEN | set,
TWL4030_PM_MASTER_BOOT_BCI);
}
@@ -151,14 +151,14 @@ static int twl4030_bci_have_vbus(struct twl4030_bci *bci)
}
/*
- * Enable/Disable USB Charge funtionality.
+ * Enable/Disable USB Charge functionality.
*/
static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
{
int ret;
if (enable) {
- /* Check for USB charger conneted */
+ /* Check for USB charger connected */
if (!twl4030_bci_have_vbus(bci))
return -ENODEV;
This allows a voltage and current (bb_uvolts and bb_uamps)
to be specified in the platform_data, and charging of the backup
battery will be enabled with those specification.
As it is not possible to monitor the backup battery at all
there is no new device created to represent it.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/power/twl4030_charger.c | 59 +++++++++++++++++++++++++++++++++++++++
include/linux/i2c/twl.h | 2 +
2 files changed, 61 insertions(+)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 3e6e991..0511610 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -28,6 +28,7 @@
#define TWL4030_BCIVBUS 0x0c
#define TWL4030_BCIMFSTS4 0x10
#define TWL4030_BCICTL1 0x23
+#define TWL4030_BB_CFG 0x12
#define TWL4030_BCIAUTOWEN BIT(5)
#define TWL4030_CONFIG_DONE BIT(4)
@@ -37,6 +38,17 @@
#define TWL4030_USBFASTMCHG BIT(2)
#define TWL4030_STS_VBUS BIT(7)
#define TWL4030_STS_USB_ID BIT(2)
+#define TWL4030_BBCHEN BIT(4)
+#define TWL4030_BBSEL_MASK 0b1100
+#define TWL4030_BBSEL_2V5 0b0000
+#define TWL4030_BBSEL_3V0 0b0100
+#define TWL4030_BBSEL_3V1 0b1000
+#define TWL4030_BBSEL_3V2 0b1100
+#define TWL4030_BBISEL_MASK 0b11
+#define TWL4030_BBISEL_25uA 0b00
+#define TWL4030_BBISEL_150uA 0b01
+#define TWL4030_BBISEL_500uA 0b10
+#define TWL4030_BBISEL_1000uA 0b11
/* BCI interrupts */
#define TWL4030_WOVF BIT(0) /* Watchdog overflow */
@@ -202,6 +214,49 @@ static int twl4030_charger_enable_ac(bool enable)
}
/*
+ * Enable/Disable charging of Backup Battery.
+ */
+static int twl4030_charger_enable_backup(int uvolt, int uamp)
+{
+ int ret;
+ u8 flags;
+
+ if (uvolt < 2500000 ||
+ uamp < 25) {
+ /* disable charging of backup battery */
+ ret = twl4030_clear_set(TWL4030_MODULE_PM_RECEIVER,
+ TWL4030_BBCHEN, 0, TWL4030_BB_CFG);
+ return ret;
+ }
+
+ flags = TWL4030_BBCHEN;
+ if (uvolt >= 3200000)
+ flags |= TWL4030_BBSEL_3V2;
+ else if (uvolt >= 3100000)
+ flags |= TWL4030_BBSEL_3V1;
+ else if (uvolt >= 3000000)
+ flags |= TWL4030_BBSEL_3V0;
+ else
+ flags |= TWL4030_BBSEL_2V5;
+
+ if (uamp >= 1000)
+ flags |= TWL4030_BBISEL_1000uA;
+ else if (uamp >= 500)
+ flags |= TWL4030_BBISEL_500uA;
+ else if (uamp >= 150)
+ flags |= TWL4030_BBISEL_150uA;
+ else
+ flags |= TWL4030_BBISEL_25uA;
+
+ ret = twl4030_clear_set(TWL4030_MODULE_PM_RECEIVER,
+ TWL4030_BBSEL_MASK | TWL4030_BBISEL_MASK,
+ flags,
+ TWL4030_BB_CFG);
+
+ return ret;
+}
+
+/*
* TWL4030 CHG_PRES (AC charger presence) events
*/
static irqreturn_t twl4030_charger_interrupt(int irq, void *arg)
@@ -424,6 +479,7 @@ static enum power_supply_property twl4030_charger_props[] = {
static int __init twl4030_bci_probe(struct platform_device *pdev)
{
struct twl4030_bci *bci;
+ struct twl4030_bci_platform_data *pdata = pdev->dev.platform_data;
int ret;
u32 reg;
@@ -503,6 +559,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
twl4030_charger_enable_ac(true);
twl4030_charger_enable_usb(bci, true);
+ twl4030_charger_enable_backup(pdata->bb_uvolt,
+ pdata->bb_uamp);
return 0;
@@ -531,6 +589,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
twl4030_charger_enable_ac(false);
twl4030_charger_enable_usb(bci, false);
+ twl4030_charger_enable_backup(0, 0);
/* mask interrupts */
twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 1f90de0..b526031 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -557,6 +557,8 @@ struct twl4030_clock_init_data {
struct twl4030_bci_platform_data {
int *battery_tmp_tbl;
unsigned int tblsize;
+ int bb_uvolt; /* voltage to charge backup battery */
+ int bb_uamp; /* current for backup battery charging */
};
/* TWL4030_GPIO_MAX (18) GPIOs, with interrupts */
The charger needs usb3v1 to be running, so add a new consumer to
keep it running.
This allows the charger to draw current even when the USB driver has
powered down.
Acked-by: Tero Kristo <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
---
drivers/mfd/twl-core.c | 9 +++++----
drivers/power/twl4030_charger.c | 15 +++++++++++++++
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 7c2267e..4cbf285 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -723,8 +723,9 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
static struct regulator_consumer_supply usb1v8 = {
.supply = "usb1v8",
};
- static struct regulator_consumer_supply usb3v1 = {
- .supply = "usb3v1",
+ static struct regulator_consumer_supply usb3v1[] = {
+ { .supply = "usb3v1" },
+ { .supply = "bci3v1" },
};
/* First add the regulators so that they can be used by transceiver */
@@ -752,7 +753,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
return PTR_ERR(child);
child = add_regulator_linked(TWL4030_REG_VUSB3V1,
- &usb_fixed, &usb3v1, 1,
+ &usb_fixed, usb3v1, 2,
features);
if (IS_ERR(child))
return PTR_ERR(child);
@@ -773,7 +774,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
if (twl_has_regulator() && child) {
usb1v5.dev_name = dev_name(child);
usb1v8.dev_name = dev_name(child);
- usb3v1.dev_name = dev_name(child);
+ usb3v1[0].dev_name = dev_name(child);
}
}
if (twl_has_usb() && pdata->usb && twl_class_is_6030()) {
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 0511610..bbda083 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -21,6 +21,7 @@
#include <linux/power_supply.h>
#include <linux/notifier.h>
#include <linux/usb/otg.h>
+#include <linux/regulator/machine.h>
#define TWL4030_BCIMSTATEC 0x02
#define TWL4030_BCIICHG 0x08
@@ -86,6 +87,8 @@ struct twl4030_bci {
struct work_struct work;
int irq_chg;
int irq_bci;
+ struct regulator *usb_reg;
+ int usb_enabled;
unsigned long event;
};
@@ -183,6 +186,12 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
return -EACCES;
}
+ /* Need to keep regulator on */
+ if (!bci->usb_enabled) {
+ regulator_enable(bci->usb_reg);
+ bci->usb_enabled = 1;
+ }
+
/* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
if (ret < 0)
@@ -193,6 +202,10 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
} else {
ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
+ if (bci->usb_enabled) {
+ regulator_disable(bci->usb_reg);
+ bci->usb_enabled = 0;
+ }
}
return ret;
@@ -511,6 +524,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
bci->usb.get_property = twl4030_bci_get_property;
+ bci->usb_reg = regulator_get(bci->dev, "bci3v1");
+
ret = power_supply_register(&pdev->dev, &bci->usb);
if (ret) {
dev_err(&pdev->dev, "failed to register usb: %d\n", ret);
The USB phy is used both for data transfer and to charge the battery.
If the charger is active it will hold a reference to
usb3v1. In that case we don't want to power-down the phy.
This allows charging to continue while the device is suspended.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/usb/otg/twl4030-usb.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index c4a86da..bd8fe9b 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -388,10 +388,16 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
(PHY_CLK_CTRL_CLOCKGATING_EN |
PHY_CLK_CTRL_CLK32K_EN));
} else {
- __twl4030_phy_power(twl, 0);
regulator_disable(twl->usb1v5);
regulator_disable(twl->usb1v8);
regulator_disable(twl->usb3v1);
+ if (!regulator_is_enabled(twl->usb3v1))
+ /* no-one else is requesting this
+ * so it is OK to power-down the
+ * phy. Sometimes a charger might
+ * hold the regulator active.
+ */
+ __twl4030_phy_power(twl, 0);
}
}
Hi Neil,
On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> The charger needs usb3v1 to be running, so add a new consumer to
> keep it running.
>
> This allows the charger to draw current even when the USB driver has
> powered down.
>
> Acked-by: Tero Kristo <[email protected]>
> Signed-off-by: NeilBrown <[email protected]>
Acked-by: Samuel Ortiz <[email protected]>
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
Hi Neil,
On Wed, May 09, 2012 at 07:40:39AM +1000, NeilBrown wrote:
> Hi,
> this series is a resend of a previous submission with some
> contentious patches removed and extra maintainers added.
>
> They allow the twl4030 to charge the backup battery (voltage and
> current configured in board file) and allow the charger to continue
> charging while the USB PHY is otherwise idle (no gadget module
> loaded, or host in suspend-to-RAM).
>
> Anton: could you take the first three? Do I need to split the third
> patch into an 'MFD' part for Samuel and a power_supply part for you?
I'm fine with Anton taking patch #3 as it is.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
Hi Neil,
On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> @@ -183,6 +186,12 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> return -EACCES;
> }
>
> + /* Need to keep regulator on */
> + if (!bci->usb_enabled) {
> + regulator_enable(bci->usb_reg);
Should you consider here a failure case?
> + bci->usb_enabled = 1;
> + }
> +
> /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
> ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
> if (ret < 0)
> @@ -511,6 +524,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
> bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
> bci->usb.get_property = twl4030_bci_get_property;
>
> + bci->usb_reg = regulator_get(bci->dev, "bci3v1");
... and here.
Andi
Hi,
On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> @@ -388,10 +388,16 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
> (PHY_CLK_CTRL_CLOCKGATING_EN |
> PHY_CLK_CTRL_CLK32K_EN));
> } else {
> - __twl4030_phy_power(twl, 0);
> regulator_disable(twl->usb1v5);
> regulator_disable(twl->usb1v8);
> regulator_disable(twl->usb3v1);
> + if (!regulator_is_enabled(twl->usb3v1))
> + /* no-one else is requesting this
> + * so it is OK to power-down the
> + * phy. Sometimes a charger might
> + * hold the regulator active.
> + */
> + __twl4030_phy_power(twl, 0);
> }
Usually a regulator line is shared by more than one device and
regulator_is_enable() returns true if at least one of these
devices is holding the regulator. This means that here the check
will not work if this is your case.
Andi
On Sun, 13 May 2012 20:12:08 +0200 Andi Shyti <[email protected]> wrote:
> Hi Neil,
>
> On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> > @@ -183,6 +186,12 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> > return -EACCES;
> > }
> >
> > + /* Need to keep regulator on */
> > + if (!bci->usb_enabled) {
> > + regulator_enable(bci->usb_reg);
>
> Should you consider here a failure case?
Probably. Something like this?
+ /* Need to keep regulator on */
+ if (!bci->usb_enabled &&
+ bci->usb_reg &&
+ regulator_enable(bci->usb_reg) == 0)
+ bci->usb_enabled = 1;
+
>
> > + bci->usb_enabled = 1;
> > + }
> > +
> > /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
> > ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
> > if (ret < 0)
> > @@ -511,6 +524,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
> > bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
> > bci->usb.get_property = twl4030_bci_get_property;
> >
> > + bci->usb_reg = regulator_get(bci->dev, "bci3v1");
>
> ... and here.
In what circumstances can that fail, I wonder.
Still, maybe this is best:
+ bci->usb_reg = regulator_get(bci->dev, "bci3v1");
+ if (IS_ERR(bci->usb_reg)) {
+ dev_warn(&bci->dev, "regulator get bci3v1 failed\n");
+ bci->usb_reg = NULL;
+ }
+
Thanks,
NeilBrown
On Sun, 13 May 2012 20:14:09 +0200 Andi Shyti <[email protected]> wrote:
> Hi,
>
> On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> > @@ -388,10 +388,16 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
> > (PHY_CLK_CTRL_CLOCKGATING_EN |
> > PHY_CLK_CTRL_CLK32K_EN));
> > } else {
> > - __twl4030_phy_power(twl, 0);
> > regulator_disable(twl->usb1v5);
> > regulator_disable(twl->usb1v8);
> > regulator_disable(twl->usb3v1);
> > + if (!regulator_is_enabled(twl->usb3v1))
> > + /* no-one else is requesting this
> > + * so it is OK to power-down the
> > + * phy. Sometimes a charger might
> > + * hold the regulator active.
> > + */
> > + __twl4030_phy_power(twl, 0);
> > }
>
> Usually a regulator line is shared by more than one device and
> regulator_is_enable() returns true if at least one of these
> devices is holding the regulator. This means that here the check
> will not work if this is your case.
>
> Andi
This regulator is inside an MFD and it only feeds a very limited number of
devices within that MFD. So I don't think there is much room for confusion.
However is it a somewhat indirect method of signalling. I want the charger
to be able to tell the USB controller that it is using the PHY so please
don't turn it off. Doing that through the regulator seems simple and
effective.
Maybe there is a better way, but it isn't immediately clear what that would
be.
Suggestions welcome.
Thanks,
NeilBrown
Hi Neil,
On Fri, May 18, 2012 at 12:46:51PM +1000, NeilBrown wrote:
> On Sun, 13 May 2012 20:12:08 +0200 Andi Shyti <[email protected]> wrote:
>
> > Hi Neil,
> >
> > On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> > > @@ -183,6 +186,12 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> > > return -EACCES;
> > > }
> > >
> > > + /* Need to keep regulator on */
> > > + if (!bci->usb_enabled) {
> > > + regulator_enable(bci->usb_reg);
> >
> > Should you consider here a failure case?
>
> Probably. Something like this?
>
> + /* Need to keep regulator on */
> + if (!bci->usb_enabled &&
> + bci->usb_reg &&
> + regulator_enable(bci->usb_reg) == 0)
> + bci->usb_enabled = 1;
> +
maybe you could write it a bit nicer :)
>
> >
> > > + bci->usb_enabled = 1;
> > > + }
> > > +
> > > /* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
> > > ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
> > > if (ret < 0)
> > > @@ -511,6 +524,8 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
> > > bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
> > > bci->usb.get_property = twl4030_bci_get_property;
> > >
> > > + bci->usb_reg = regulator_get(bci->dev, "bci3v1");
> >
> > ... and here.
>
> In what circumstances can that fail, I wonder.
> Still, maybe this is best:
>
>
> + bci->usb_reg = regulator_get(bci->dev, "bci3v1");
> + if (IS_ERR(bci->usb_reg)) {
> + dev_warn(&bci->dev, "regulator get bci3v1 failed\n");
> + bci->usb_reg = NULL;
> + }
> +
Yep!
Andi
Hi,
On Fri, May 18, 2012 at 12:50:10PM +1000, NeilBrown wrote:
> On Sun, 13 May 2012 20:14:09 +0200 Andi Shyti <[email protected]> wrote:
> > On Wed, May 09, 2012 at 07:40:40AM +1000, NeilBrown wrote:
> > > @@ -388,10 +388,16 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
> > > (PHY_CLK_CTRL_CLOCKGATING_EN |
> > > PHY_CLK_CTRL_CLK32K_EN));
> > > } else {
> > > - __twl4030_phy_power(twl, 0);
> > > regulator_disable(twl->usb1v5);
> > > regulator_disable(twl->usb1v8);
> > > regulator_disable(twl->usb3v1);
> > > + if (!regulator_is_enabled(twl->usb3v1))
> > > + /* no-one else is requesting this
> > > + * so it is OK to power-down the
> > > + * phy. Sometimes a charger might
> > > + * hold the regulator active.
> > > + */
> > > + __twl4030_phy_power(twl, 0);
> > > }
> >
> > Usually a regulator line is shared by more than one device and
> > regulator_is_enable() returns true if at least one of these
> > devices is holding the regulator. This means that here the check
> > will not work if this is your case.
> >
> > Andi
>
> This regulator is inside an MFD and it only feeds a very limited number of
> devices within that MFD. So I don't think there is much room for confusion.
>
> However is it a somewhat indirect method of signalling. I want the charger
> to be able to tell the USB controller that it is using the PHY so please
> don't turn it off. Doing that through the regulator seems simple and
> effective.
> Maybe there is a better way, but it isn't immediately clear what that would
> be.
> Suggestions welcome.
When using regulators I would keep track of his status internally
in the driver... 'true' if on or 'false' if of, in this way there
is no room for confusion.
But the mine is just a suggestion, you know the environment
better
Andi
On Wed, May 09, 2012 at 07:40:39AM +1000, NeilBrown wrote:
> Hi,
> this series is a resend of a previous submission with some
> contentious patches removed and extra maintainers added.
>
> They allow the twl4030 to charge the backup battery (voltage and
> current configured in board file) and allow the charger to continue
> charging while the USB PHY is otherwise idle (no gadget module
> loaded, or host in suspend-to-RAM).
>
> Anton: could you take the first three? Do I need to split the third
> patch into an 'MFD' part for Samuel and a power_supply part for you?
Applied the three patches (w/ Samuel's Ack).
Though, please consider addressing Andi's comments (as follow-up
patches).
Thank you guys!
--
Anton Vorontsov
Email: [email protected]