* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-08-19 15:24 Tony Lindgren
2014-08-19 21:33 ` Aaro Koskinen
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Tony Lindgren @ 2014-08-19 15:24 UTC (permalink / raw)
To: linux-arm-kernel
Commit 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn
off oscillator during off-idle) added support for configuring the PMIC
to cut off resources during deeper idle states to save power.
This however caused regression for n900 display power that needed the
PMIC configuration to be disabled with commit d937678ab625 (ARM: dts:
Revert enabling of twl configuration for n900).
Turns out the root cause of the problem is that we must use
TWL4030_RESCONFIG_UNDEF instead of DEV_GRP_NULL to avoid disabling
regulators that may have been enabled before the init function
for twl4030-power.c runs. With TWL4030_RESCONFIG_UNDEF we let the
regulator framework control the regulators like it should. Here we
need to only configure the sys_clken and sys_off_mode triggers for
the regulators that cannot be done by the regulator framework as
it's not running at that point.
This allows us to enable the PMIC configuration for n900.
Fixes: 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn off oscillator during off-idle)
Cc: stable at vger.kernel.org # v3.16
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Lee, can you please pick this one for the v3.17-rc series?
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -361,7 +361,7 @@
};
twl_power: power {
- compatible = "ti,twl4030-power-n900";
+ compatible = "ti,twl4030-power-n900", "ti,twl4030-power-idle-osc-off";
ti,use_poweroff;
};
};
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -724,24 +724,24 @@ static struct twl4030_script *omap3_idle_scripts[] = {
* above.
*/
static struct twl4030_resconfig omap3_idle_rconfig[] = {
- TWL_REMAP_SLEEP(RES_VAUX1, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VAUX2, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VAUX3, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VAUX4, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VMMC1, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VMMC2, DEV_GRP_NULL, 0, 0),
+ TWL_REMAP_SLEEP(RES_VAUX1, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VAUX2, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VAUX3, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VAUX4, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VMMC1, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VMMC2, TWL4030_RESCONFIG_UNDEF, 0, 0),
TWL_REMAP_OFF(RES_VPLL1, DEV_GRP_P1, 3, 1),
TWL_REMAP_SLEEP(RES_VPLL2, DEV_GRP_P1, 0, 0),
- TWL_REMAP_SLEEP(RES_VSIM, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VDAC, DEV_GRP_NULL, 0, 0),
+ TWL_REMAP_SLEEP(RES_VSIM, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VDAC, TWL4030_RESCONFIG_UNDEF, 0, 0),
TWL_REMAP_SLEEP(RES_VINTANA1, TWL_DEV_GRP_P123, 1, 2),
TWL_REMAP_SLEEP(RES_VINTANA2, TWL_DEV_GRP_P123, 0, 2),
TWL_REMAP_SLEEP(RES_VINTDIG, TWL_DEV_GRP_P123, 1, 2),
TWL_REMAP_SLEEP(RES_VIO, TWL_DEV_GRP_P123, 2, 2),
TWL_REMAP_OFF(RES_VDD1, DEV_GRP_P1, 4, 1),
TWL_REMAP_OFF(RES_VDD2, DEV_GRP_P1, 3, 1),
- TWL_REMAP_SLEEP(RES_VUSB_1V5, DEV_GRP_NULL, 0, 0),
- TWL_REMAP_SLEEP(RES_VUSB_1V8, DEV_GRP_NULL, 0, 0),
+ TWL_REMAP_SLEEP(RES_VUSB_1V5, TWL4030_RESCONFIG_UNDEF, 0, 0),
+ TWL_REMAP_SLEEP(RES_VUSB_1V8, TWL4030_RESCONFIG_UNDEF, 0, 0),
TWL_REMAP_SLEEP(RES_VUSB_3V1, TWL_DEV_GRP_P123, 0, 0),
/* Resource #20 USB charge pump skipped */
TWL_REMAP_SLEEP(RES_REGEN, TWL_DEV_GRP_P123, 2, 1),
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-08-19 15:24 [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators Tony Lindgren
@ 2014-08-19 21:33 ` Aaro Koskinen
2014-08-20 12:30 ` Lee Jones
2014-09-02 8:29 ` Sebastian Andrzej Siewior
2 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2014-08-19 21:33 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Aug 19, 2014 at 08:24:05AM -0700, Tony Lindgren wrote:
> Commit 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn
> off oscillator during off-idle) added support for configuring the PMIC
> to cut off resources during deeper idle states to save power.
[...]
> Fixes: 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn off oscillator during off-idle)
> Cc: stable at vger.kernel.org # v3.16
> Signed-off-by: Tony Lindgren <tony@atomide.com>
Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
A.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-08-19 15:24 [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators Tony Lindgren
2014-08-19 21:33 ` Aaro Koskinen
@ 2014-08-20 12:30 ` Lee Jones
2014-09-02 8:29 ` Sebastian Andrzej Siewior
2 siblings, 0 replies; 9+ messages in thread
From: Lee Jones @ 2014-08-20 12:30 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 19 Aug 2014, Tony Lindgren wrote:
> Commit 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn
> off oscillator during off-idle) added support for configuring the PMIC
> to cut off resources during deeper idle states to save power.
>
> This however caused regression for n900 display power that needed the
> PMIC configuration to be disabled with commit d937678ab625 (ARM: dts:
> Revert enabling of twl configuration for n900).
>
> Turns out the root cause of the problem is that we must use
> TWL4030_RESCONFIG_UNDEF instead of DEV_GRP_NULL to avoid disabling
> regulators that may have been enabled before the init function
> for twl4030-power.c runs. With TWL4030_RESCONFIG_UNDEF we let the
> regulator framework control the regulators like it should. Here we
> need to only configure the sys_clken and sys_off_mode triggers for
> the regulators that cannot be done by the regulator framework as
> it's not running at that point.
>
> This allows us to enable the PMIC configuration for n900.
>
> Fixes: 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn off oscillator during off-idle)
> Cc: stable at vger.kernel.org # v3.16
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Lee, can you please pick this one for the v3.17-rc series?
Applied with Aaro's Tested-by.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-08-19 15:24 [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators Tony Lindgren
2014-08-19 21:33 ` Aaro Koskinen
2014-08-20 12:30 ` Lee Jones
@ 2014-09-02 8:29 ` Sebastian Andrzej Siewior
2014-09-03 0:24 ` Tony Lindgren
2 siblings, 1 reply; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-09-02 8:29 UTC (permalink / raw)
To: linux-arm-kernel
On 2014-08-19 08:24:05 [-0700], Tony Lindgren wrote:
>
> This allows us to enable the PMIC configuration for n900.
>
> Fixes: 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn off oscillator during off-idle)
My beaglebone-ab does not like this. With this patch applied I end up
with:
[ 2.437316] Waiting for root device /dev/mmcblk0p2...
[ 4.428192] mmc0: card never left busy state
[ 4.432647] mmc0: error -110 whilst initialising SD card
I noticed this after I disabled lock debugging and tracing (syscall tracing
was enabled). Enabling both again makes the error go away.
With debugging + tracing disabled (so the error pops up) and git bisect
I get this commit (daebabd57) reported.
A diff between a good/bad bootlog (with lock-debug + tracing switched
off):
| smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized
| smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized
| hsusb2_vbus: disabling
| VPLL2: disabling
| VUSB3V1: disabling
|+VDAC: disabling
|+VAUX3: disabling
| Waiting for root device /dev/mmcblk0p2...
|-mmc0: host does not support reading read-only switch. assuming write-enable.
|-mmc0: new high speed SDHC card at address 1234
|-mmcblk0: mmc0:1234 SA04G 3.63 GiB
|- mmcblk0: p1 p2 p3
|-BTRFS: device fsid 2e050bbb-1520-425e-a215-1e5bf865c7dc devid 1 transid 496 /dev/root
|-BTRFS info (device mmcblk0p2): disk space caching is enabled
|-BTRFS: detected SSD devices, enabling SSD mode
|-VFS: Mounted root (btrfs filesystem) readonly on device 0:13.
|-devtmpfs: mounted
|-Freeing unused kernel memory: 216K (c051a000 - c0550000)
|-BTRFS info (device mmcblk0p2): disk space caching is enabled
|-
|-Please press Enter to activate this console.
|+mmc0: card never left busy state
|+mmc0: error -110 whilst initialising SD card
reverting this commit on top of -rc3 makes mmc0 work again.
Sebastian
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-09-02 8:29 ` Sebastian Andrzej Siewior
@ 2014-09-03 0:24 ` Tony Lindgren
2014-09-03 8:38 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 9+ messages in thread
From: Tony Lindgren @ 2014-09-03 0:24 UTC (permalink / raw)
To: linux-arm-kernel
* Sebastian Andrzej Siewior <sebastian@breakpoint.cc> [140902 01:29]:
> On 2014-08-19 08:24:05 [-0700], Tony Lindgren wrote:
> >
> > This allows us to enable the PMIC configuration for n900.
> >
> > Fixes: 43fef47f94a1 (mfd: twl4030-power: Add a configuration to turn off oscillator during off-idle)
>
> My beaglebone-ab does not like this. With this patch applied I end up
> with:
> [ 2.437316] Waiting for root device /dev/mmcblk0p2...
> [ 4.428192] mmc0: card never left busy state
> [ 4.432647] mmc0: error -110 whilst initialising SD card
I assume you mean beagleboard-ab, not beaglebone-ab :)
> I noticed this after I disabled lock debugging and tracing (syscall tracing
> was enabled). Enabling both again makes the error go away.
> With debugging + tracing disabled (so the error pops up) and git bisect
> I get this commit (daebabd57) reported.
OK. Sounds like we still have a race between the regulator code
and twl4030-power.c for accessing some twl registers.
> A diff between a good/bad bootlog (with lock-debug + tracing switched
> off):
>
> | smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized
> | smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized
> | hsusb2_vbus: disabling
> | VPLL2: disabling
> | VUSB3V1: disabling
> |+VDAC: disabling
> |+VAUX3: disabling
This seems to be the reason. Seems you're on v3.17-rc3, but with
commit daebabd57 we are not even touching the group registers that
twl-regulator.c accesses for VDAC and VAUX3 as they are set to
TWL4030_RESCONFIG_UNDEF. So I'm a bit baffled what's going on.
> reverting this commit on top of -rc3 makes mmc0 work again.
Again, I assume you're talking about reverting daebabd57,
not 43fef47f94a1. Anyways, here's a debug hack I used earlier to
dump out the twl configuration in late_initcall and via sysfs
so maybe try that and see what the values are with working
and non-working case?
Regards,
Tony
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -358,6 +358,146 @@ out:
return err;
}
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+static void twl4030_dump_resource(unsigned index, struct seq_file *s)
+{
+ int addr;
+ int err;
+ u8 type;
+ u8 grp;
+ u8 remap;
+
+ addr = res_config_addrs[index];
+
+ err = twl_i2c_read_u8(TWL_MODULE_PM_RECEIVER, &grp,
+ addr + DEV_GRP_OFFSET);
+ if (err) {
+ pr_err("TWL4030 Resource group 0x%02x could not be read\n",
+ addr);
+ return;
+ }
+
+ err = twl_i2c_read_u8(TWL_MODULE_PM_RECEIVER, &type,
+ addr + TYPE_OFFSET);
+ if (err < 0) {
+ pr_err("TWL4030 Resource type 0x%02x could not be read\n",
+ addr);
+ return;
+ }
+
+ err = twl_i2c_read_u8(TWL_MODULE_PM_RECEIVER, &remap,
+ addr + REMAP_OFFSET);
+ if (err < 0) {
+ pr_err("TWL4030 Resource 0x%02x remap could not be read\n",
+ addr);
+ return;
+ }
+
+ if (s)
+ seq_printf(s, "%i: addr: 0x%04x grp: 0x%04x type: 0x%04x remap: 0x%04x\n",
+ index, addr, grp, type, remap);
+ else
+ printk("%i: addr: 0x%04x grp: 0x%04x type: 0x%04x remap: 0x%04x\n",
+ index, addr, grp, type, remap);
+}
+
+static void twl4030_dump_resources(void)
+{
+ int i;
+
+ for (i = 1; i <= RES_MAIN_REF; i++)
+ twl4030_dump_resource(i, NULL);
+}
+
+static struct dentry *dbg_root;
+
+static int dbg_show(struct seq_file *s, void *unused)
+{
+ unsigned long offset = (unsigned long)s->private;
+
+ twl4030_dump_resource(offset, s);
+
+ return 0;
+}
+
+static ssize_t dbg_write(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct seq_file *seqf;
+ unsigned long offset;
+ u8 val;
+ int res;
+
+ res = kstrtou8_from_user(user_buf, count, 0x10, &val);
+ if (res < 0)
+ return res;
+
+ seqf = file->private_data;
+ offset = (unsigned long)seqf->private;
+ offset = res_config_addrs[offset];
+
+ res = twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER,
+ val, offset + DEV_GRP_OFFSET);
+ if (res < 0) {
+ pr_err("TWL4030 failed to program devgroup\n");
+ return res;
+ }
+
+ *ppos += count;
+
+ return count;
+}
+
+static int dbg_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, dbg_show, inode->i_private);
+}
+
+static const struct file_operations dbg_fops = {
+ .open = dbg_open,
+ .read = seq_read,
+ .write = dbg_write,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static void __init dbg_init(struct twl4030_resconfig *rconfig)
+{
+ int i;
+
+ dbg_root = debugfs_create_dir("twl4030-power", NULL);
+ if (IS_ERR(dbg_root) || !dbg_root)
+ return;
+
+ for (i = 1; i <= RES_MAIN_REF; i++) {
+ u8 name[16];
+
+ sprintf(name, "0x%02x", res_config_addrs[i]);
+ (void)debugfs_create_file(name, S_IRUGO,
+ dbg_root,
+ (void *)i,
+ &dbg_fops);
+ }
+}
+
+static int __init dbg_initcall(void)
+{
+ twl4030_dump_resources();
+
+ return 0;
+}
+late_initcall(dbg_initcall);
+
+#else
+static inline void dbg_init(struct twl4030_resconfig *rconfig)
+{
+}
+#endif
+
static int twl4030_configure_resource(struct twl4030_resconfig *rconfig)
{
int rconfig_addr;
@@ -864,6 +1004,8 @@ relock:
return err2;
}
+ dbg_init(pdata->resource_config);
+
return err;
}
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-09-03 0:24 ` Tony Lindgren
@ 2014-09-03 8:38 ` Sebastian Andrzej Siewior
2014-09-03 18:39 ` Tony Lindgren
0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-09-03 8:38 UTC (permalink / raw)
To: linux-arm-kernel
On 09/03/2014 02:24 AM, Tony Lindgren wrote:
> I assume you mean beagleboard-ab, not beaglebone-ab :)
you assume correct.
>> reverting this commit on top of -rc3 makes mmc0 work again.
>
> Again, I assume you're talking about reverting daebabd57,
> not 43fef47f94a1. Anyways, here's a debug hack I used earlier to
Yes, I reverted daebabd57 which was pointed to me by git bisect.
> dump out the twl configuration in late_initcall and via sysfs
> so maybe try that and see what the values are with working
> and non-working case?
good.txt and bad.txt are from the late_initcall.
$ diff -u good.txt bad.txt
--- good.txt 2014-09-03 10:29:58.920317368 +0200
+++ bad.txt 2014-09-03 10:28:57.064313222 +0200
@@ -1,13 +1,13 @@
1: addr: 0x0017 grp: 0x0000 type: 0x0000 remap: 0x0008
- 2: addr: 0x001b grp: 0x0000 type: 0x0000 remap: 0x0008
- 3: addr: 0x001f grp: 0x0000 type: 0x0000 remap: 0x0008
+ 2: addr: 0x001b grp: 0x002e type: 0x0000 remap: 0x0008
+ 3: addr: 0x001f grp: 0x002e type: 0x0000 remap: 0x0008
4: addr: 0x0023 grp: 0x0000 type: 0x0000 remap: 0x0008
5: addr: 0x0027 grp: 0x002e type: 0x0000 remap: 0x0008
6: addr: 0x002b grp: 0x0000 type: 0x0000 remap: 0x0008
7: addr: 0x002f grp: 0x002e type: 0x000b remap: 0x0000
8: addr: 0x0033 grp: 0x002e type: 0x0000 remap: 0x0008
9: addr: 0x0037 grp: 0x002e type: 0x0000 remap: 0x0008
-10: addr: 0x003b grp: 0x0000 type: 0x0000 remap: 0x0008
+10: addr: 0x003b grp: 0x002e type: 0x0000 remap: 0x0008
11: addr: 0x003f grp: 0x00ef type: 0x0011 remap: 0x0008
12: addr: 0x0043 grp: 0x00ef type: 0x0010 remap: 0x0008
13: addr: 0x0047 grp: 0x00ef type: 0x0011 remap: 0x0008
@@ -17,12 +17,8 @@
17: addr: 0x0071 grp: 0x0000 type: 0x0000 remap: 0x0008
18: addr: 0x0074 grp: 0x0000 type: 0x0000 remap: 0x0008
19: addr: 0x0077 grp: 0x00ef type: 0x0000 remap: 0x0008
-mmc0: host does not support reading read-only switch. assuming
write-enable.
20: addr: 0x007a grp: 0x0000 type: 0x0000 remap: 0x0000
-mmc0: new high speed SDHC card at address 1234
-mmcblk0: mmc0:1234 SA04G 3.63 GiB
21: addr: 0x007f grp: 0x00ef type: 0x000a remap: 0x0008
- mmcblk0: p1 p2 p3
22: addr: 0x0082 grp: 0x00ee type: 0x0008 remap: 0x0008
23: addr: 0x0085 grp: 0x00af type: 0x0013 remap: 0x0000
24: addr: 0x0088 grp: 0x00ef type: 0x000e remap: 0x0008
>
> Regards,
>
> Tony
Sebastian
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-09-03 8:38 ` Sebastian Andrzej Siewior
@ 2014-09-03 18:39 ` Tony Lindgren
2014-09-04 8:45 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 9+ messages in thread
From: Tony Lindgren @ 2014-09-03 18:39 UTC (permalink / raw)
To: linux-arm-kernel
* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140903 01:39]:
> On 09/03/2014 02:24 AM, Tony Lindgren wrote:
>
> > dump out the twl configuration in late_initcall and via sysfs
> > so maybe try that and see what the values are with working
> > and non-working case?
>
> good.txt and bad.txt are from the late_initcall.
>
> $ diff -u good.txt bad.txt
> --- good.txt 2014-09-03 10:29:58.920317368 +0200
> +++ bad.txt 2014-09-03 10:28:57.064313222 +0200
Hmm can you check that you have good.txt and bad.txt the
right way? I'd assume you need VAUX2 or VAUX3 enabled
not disabled for the MMC to work?
> @@ -1,13 +1,13 @@
> 1: addr: 0x0017 grp: 0x0000 type: 0x0000 remap: 0x0008
> - 2: addr: 0x001b grp: 0x0000 type: 0x0000 remap: 0x0008
> - 3: addr: 0x001f grp: 0x0000 type: 0x0000 remap: 0x0008
> + 2: addr: 0x001b grp: 0x002e type: 0x0000 remap: 0x0008
> + 3: addr: 0x001f grp: 0x002e type: 0x0000 remap: 0x0008
OK so looking at res_config_addrs[], we have RES_VAUX2 at
0x1b and RES_VAUX3 at 0x1f. The value for the group register
0x2e means that bit5 is set and it's used by the p1 device
group. So when the group register is 0x2e, the resource is
enabled. Those got most likely enabled by twl-regulator.c
as twl4030-power.c has TWL4030_RESCONFIG_UNDEF for VAUX2 and
VAUX3.
> 4: addr: 0x0023 grp: 0x0000 type: 0x0000 remap: 0x0008
> 5: addr: 0x0027 grp: 0x002e type: 0x0000 remap: 0x0008
> 6: addr: 0x002b grp: 0x0000 type: 0x0000 remap: 0x0008
> 7: addr: 0x002f grp: 0x002e type: 0x000b remap: 0x0000
> 8: addr: 0x0033 grp: 0x002e type: 0x0000 remap: 0x0008
> 9: addr: 0x0037 grp: 0x002e type: 0x0000 remap: 0x0008
> -10: addr: 0x003b grp: 0x0000 type: 0x0000 remap: 0x0008
> +10: addr: 0x003b grp: 0x002e type: 0x0000 remap: 0x0008
And that's RES_VDAC at 0x3b with the same story, it's also
marked TWL4030_RESCONFIG_UNDEF and only modified by the
twl4030-power.c.
I think the use on beaglboard for these are:
VAUX2 USB_1V8
VAUX3 CAM_CORE
VDAC VDAC_1V8
Not quite seeing the connection.. But I'm assuming you have
good.txt and bad.txt the wrong way around, and you need
VAUX2, VAUX3 or VDAC _enabled_ to get MMC working :)
So this seems to hint we have issue in one of these:
1. We are not claiming VAUX2, VAUX3 or VDAC for beagleboard,
and there's now a timing issue where the regulator
framework disables the unused regulators before MMC.
2. We may have a bug that causes register access to
DEV_GRP_OFFSET in twl4030-power.c even with those
resources marked as TWL4030_RESCONFIG_UNDEF.
3. There's a I2C race somewhere accessing twl registers.
Guessing #1 above, maybe give the following patch a try
and see if that helps? If that works, try commenting out
vaux3 or vdac and see which one is needed. I'm guessing
it's the vdac.
Regards,
Tony
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -331,6 +331,18 @@
regulator-always-on;
};
+&vaux3 {
+ regulator-name = "cam_core";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+};
+
+&vdac {
+ regulator-name = "vdac_1v8";
+ regulator-always-on;
+};
+
&mcbsp2 {
status = "okay";
};
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-09-03 18:39 ` Tony Lindgren
@ 2014-09-04 8:45 ` Sebastian Andrzej Siewior
2014-09-04 16:39 ` Tony Lindgren
0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-09-04 8:45 UTC (permalink / raw)
To: linux-arm-kernel
On 09/03/2014 08:39 PM, Tony Lindgren wrote:
>> good.txt and bad.txt are from the late_initcall.
>>
>> $ diff -u good.txt bad.txt
>> --- good.txt 2014-09-03 10:29:58.920317368 +0200
>> +++ bad.txt 2014-09-03 10:28:57.064313222 +0200
>
> Hmm can you check that you have good.txt and bad.txt the
> right way? I'd assume you need VAUX2 or VAUX3 enabled
> not disabled for the MMC to work?
No, it was correct. If you look at the complete file you will notice
the - which removes the mmc detect/mount in the bad case and + which
adds the -110 error
>> @@ -1,13 +1,13 @@
>> 1: addr: 0x0017 grp: 0x0000 type: 0x0000 remap: 0x0008
>> - 2: addr: 0x001b grp: 0x0000 type: 0x0000 remap: 0x0008
>> - 3: addr: 0x001f grp: 0x0000 type: 0x0000 remap: 0x0008
>> + 2: addr: 0x001b grp: 0x002e type: 0x0000 remap: 0x0008
>> + 3: addr: 0x001f grp: 0x002e type: 0x0000 remap: 0x0008
>
> OK so looking at res_config_addrs[], we have RES_VAUX2 at
> 0x1b and RES_VAUX3 at 0x1f. The value for the group register
> 0x2e means that bit5 is set and it's used by the p1 device
> group. So when the group register is 0x2e, the resource is
> enabled. Those got most likely enabled by twl-regulator.c
> as twl4030-power.c has TWL4030_RESCONFIG_UNDEF for VAUX2 and
> VAUX3.
>
>> 4: addr: 0x0023 grp: 0x0000 type: 0x0000 remap: 0x0008
>> 5: addr: 0x0027 grp: 0x002e type: 0x0000 remap: 0x0008
>> 6: addr: 0x002b grp: 0x0000 type: 0x0000 remap: 0x0008
>> 7: addr: 0x002f grp: 0x002e type: 0x000b remap: 0x0000
>> 8: addr: 0x0033 grp: 0x002e type: 0x0000 remap: 0x0008
>> 9: addr: 0x0037 grp: 0x002e type: 0x0000 remap: 0x0008
>> -10: addr: 0x003b grp: 0x0000 type: 0x0000 remap: 0x0008
>> +10: addr: 0x003b grp: 0x002e type: 0x0000 remap: 0x0008
>
> And that's RES_VDAC at 0x3b with the same story, it's also
> marked TWL4030_RESCONFIG_UNDEF and only modified by the
> twl4030-power.c.
>
> I think the use on beaglboard for these are:
>
> VAUX2 USB_1V8
> VAUX3 CAM_CORE
> VDAC VDAC_1V8
>
> Not quite seeing the connection.. But I'm assuming you have
> good.txt and bad.txt the wrong way around, and you need
> VAUX2, VAUX3 or VDAC _enabled_ to get MMC working :)
>
> So this seems to hint we have issue in one of these:
>
> 1. We are not claiming VAUX2, VAUX3 or VDAC for beagleboard,
> and there's now a timing issue where the regulator
> framework disables the unused regulators before MMC.
>
> 2. We may have a bug that causes register access to
> DEV_GRP_OFFSET in twl4030-power.c even with those
> resources marked as TWL4030_RESCONFIG_UNDEF.
>
> 3. There's a I2C race somewhere accessing twl registers.
>
> Guessing #1 above, maybe give the following patch a try
> and see if that helps? If that works, try commenting out
> vaux3 or vdac and see which one is needed. I'm guessing
> it's the vdac.
With that patch it seems it is a little harder to trigger. It is
usually every other boot that fails. Here a diff between two that
worked (say good-v1 vs good-v2):
@@ -151,19 +151,15 @@
Btrfs loaded
hsusb2_vbus: 3300 mV
1: addr: 0x0017 grp: 0x0000 type: 0x0000 remap: 0x0008
-2: addr: 0x001b grp: 0x002e type: 0x0000 remap: 0x0008
-3: addr: 0x001f grp: 0x002e type: 0x0000 remap: 0x0008
+2: addr: 0x001b grp: 0x0000 type: 0x0000 remap: 0x0008
+3: addr: 0x001f grp: 0x0000 type: 0x0000 remap: 0x0008
4: addr: 0x0023 grp: 0x0000 type: 0x0000 remap: 0x0008
5: addr: 0x0027 grp: 0x002e type: 0x0000 remap: 0x0008
6: addr: 0x002b grp: 0x0000 type: 0x0000 remap: 0x0008
7: addr: 0x002f grp: 0x002e type: 0x000b remap: 0x0000
8: addr: 0x0033 grp: 0x002e type: 0x0000 remap: 0x0008
-mmc0: host does not support reading read-only switch. assuming
write-enable.
9: addr: 0x0037 grp: 0x002e type: 0x0000 remap: 0x0008
-mmc0: new high speed SDHC card at address 1234
-mmcblk0: mmc0:1234 SA04G 3.63 GiB
-10: addr: 0x003b grp: 0x002e type: 0x0000 remap: 0x0008
- mmcblk0: p1 p2 p3
+10: addr: 0x003b grp: 0x0000 type: 0x0000 remap: 0x0008
11: addr: 0x003f grp: 0x00ef type: 0x0011 remap: 0x0008
12: addr: 0x0043 grp: 0x00ef type: 0x0010 remap: 0x0008
13: addr: 0x0047 grp: 0x00ef type: 0x0011 remap: 0x0008
@@ -173,8 +169,12 @@
17: addr: 0x0071 grp: 0x0000 type: 0x0000 remap: 0x0008
18: addr: 0x0074 grp: 0x0000 type: 0x0000 remap: 0x0008
19: addr: 0x0077 grp: 0x00ef type: 0x0000 remap: 0x0008
+mmc0: host does not support reading read-only switch. assuming
write-enable.
20: addr: 0x007a grp: 0x0000 type: 0x0000 remap: 0x0000
+mmc0: new high speed SDHC card at address 1234
+mmcblk0: mmc0:1234 SA04G 3.63 GiB
21: addr: 0x007f grp: 0x00ef type: 0x000a remap: 0x0008
+ mmcblk0: p1 p2 p3
22: addr: 0x0082 grp: 0x00ee type: 0x0008 remap: 0x0008
23: addr: 0x0085 grp: 0x00af type: 0x0013 remap: 0x0000
24: addr: 0x0088 grp: 0x00ef type: 0x000e remap: 0x0008
It took mmc a little longer to detect but it worked. And the content of
the three registers seem not to matter _or_ it was dumped before MMC
got active.
Now a diff of good v1 vs bad:
@@ -158,12 +158,8 @@
6: addr: 0x002b grp: 0x0000 type: 0x0000 remap: 0x0008
7: addr: 0x002f grp: 0x002e type: 0x000b remap: 0x0000
8: addr: 0x0033 grp: 0x002e type: 0x0000 remap: 0x0008
-mmc0: host does not support reading read-only switch. assuming
write-enable.
9: addr: 0x0037 grp: 0x002e type: 0x0000 remap: 0x0008
-mmc0: new high speed SDHC card at address 1234
-mmcblk0: mmc0:1234 SA04G 3.63 GiB
10: addr: 0x003b grp: 0x002e type: 0x0000 remap: 0x0008
- mmcblk0: p1 p2 p3
11: addr: 0x003f grp: 0x00ef type: 0x0011 remap: 0x0008
12: addr: 0x0043 grp: 0x00ef type: 0x0010 remap: 0x0008
13: addr: 0x0047 grp: 0x00ef type: 0x0011 remap: 0x0008
@@ -183,19 +179,13 @@
27: addr: 0x0091 grp: 0x00ee type: 0x0006 remap: 0x0008
28: addr: 0x0094 grp: 0x00ef type: 0x0000 remap: 0x0008
input: gpio_keys as /devices/gpio_keys/input/input2
-twl_rtc 48070000.i2c:twl at 48:rtc: setting system clock to 2000-01-01
00:02:39 UTC (946684959)
+twl_rtc 48070000.i2c:twl at 48:rtc: setting system clock to 2000-01-01
00:02:47 UTC (946684967)
sr_init: No PMIC hook to init smartreflex
smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized
smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized
hsusb2_vbus: disabling
VPLL2: disabling
VUSB3V1: disabling
-BTRFS: device fsid 2e050bbb-1520-425e-a215-1e5bf865c7dc devid 1 transid
1189 /dev/root
-BTRFS info (device mmcblk0p2): disk space caching is enabled
-BTRFS: detected SSD devices, enabling SSD mode
-VFS: Mounted root (btrfs filesystem) readonly on device 0:13.
-devtmpfs: mounted
-Freeing unused kernel memory: 316K (c057a000 - c05c9000)
-BTRFS info (device mmcblk0p2): disk space caching is enabled
-
-(none) login: [ 5.252380] SysRq : Resetting
+Waiting for root device /dev/mmcblk0p2...
+mmc0: card never left busy state
+mmc0: error -110 whilst initialising SD card
I didn't change a thing, I just pressed reset. As you see the content
of the TWL registers don't seem to be that important since it was the
same at the time of the dump. And the mmc didn't came back.
>
> Regards,
>
> Tony
>
Sebastian
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
2014-09-04 8:45 ` Sebastian Andrzej Siewior
@ 2014-09-04 16:39 ` Tony Lindgren
0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2014-09-04 16:39 UTC (permalink / raw)
To: linux-arm-kernel
* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140904 01:46]:
> On 09/03/2014 08:39 PM, Tony Lindgren wrote:
> >> good.txt and bad.txt are from the late_initcall.
> >>
> >> $ diff -u good.txt bad.txt
> >> --- good.txt 2014-09-03 10:29:58.920317368 +0200
> >> +++ bad.txt 2014-09-03 10:28:57.064313222 +0200
> >
> > Hmm can you check that you have good.txt and bad.txt the
> > right way? I'd assume you need VAUX2 or VAUX3 enabled
> > not disabled for the MMC to work?
>
> No, it was correct. If you look at the complete file you will notice
> the - which removes the mmc detect/mount in the bad case and + which
> adds the -110 error
...
> With that patch it seems it is a little harder to trigger. It is
> usually every other boot that fails. Here a diff between two that
> worked (say good-v1 vs good-v2):
...
> It took mmc a little longer to detect but it worked. And the content of
> the three registers seem not to matter _or_ it was dumped before MMC
> got active.
...
> I didn't change a thing, I just pressed reset. As you see the content
> of the TWL registers don't seem to be that important since it was the
> same at the time of the dump. And the mmc didn't came back.
OK. I guess that means that it's most likely some timing related issue,
or an issue with some regulator not being ready by the time the MMC
probes. Or it's a bug somewhere that I'm not figuring out.
Looking at the twl-regulator.c, twl4030reg_enable just sets the group
bit and we're not checking any status and don't have startup-delay-us
for them.
At least phy-twl4030-usb.c has twl4030_i2c_write_u8_verify(), I wonder
if adding a read back of the register to twl-regulator.c would help?
Regards,
Tony
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-04 16:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-19 15:24 [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators Tony Lindgren
2014-08-19 21:33 ` Aaro Koskinen
2014-08-20 12:30 ` Lee Jones
2014-09-02 8:29 ` Sebastian Andrzej Siewior
2014-09-03 0:24 ` Tony Lindgren
2014-09-03 8:38 ` Sebastian Andrzej Siewior
2014-09-03 18:39 ` Tony Lindgren
2014-09-04 8:45 ` Sebastian Andrzej Siewior
2014-09-04 16:39 ` Tony Lindgren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).