All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-08-19 15:24 ` Tony Lindgren
  0 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2014-08-19 15:24 UTC (permalink / raw)
  To: Lee Jones
  Cc: Aaro Koskinen, Sebastian Reichel, Pali Rohár, Pavel Machek,
	linux-omap, 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@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] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-08-19 15:24 ` Tony Lindgren
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2014-08-19 21:33 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Lee Jones, Sebastian Reichel, Pali Rohár, Pavel Machek,
	linux-omap, 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@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] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-08-19 21:33   ` Aaro Koskinen
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
  2014-08-19 15:24 ` Tony Lindgren
@ 2014-08-20 12:30   ` Lee Jones
  -1 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2014-08-20 12:30 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Aaro Koskinen, Sebastian Reichel, Pavel Machek, Pali Rohár,
	linux-omap, 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@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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-08-20 12:30   ` Lee Jones
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
  2014-08-19 15:24 ` Tony Lindgren
@ 2014-09-02  8:29   ` Sebastian Andrzej Siewior
  -1 siblings, 0 replies; 17+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-09-02  8:29 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Lee Jones, Aaro Koskinen, Sebastian Reichel, Pavel Machek,
	Pali Rohár, linux-omap, linux-arm-kernel, bigeasy

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] 17+ 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
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2014-09-03  0:24 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Lee Jones, Aaro Koskinen, Sebastian Reichel, Pavel Machek,
	Pali Rohár, linux-omap, linux-arm-kernel, bigeasy

* 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] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-09-03  0:24     ` Tony Lindgren
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 17+ messages in thread
From: Sebastian Andrzej Siewior @ 2014-09-03  8:38 UTC (permalink / raw)
  To: Tony Lindgren, Sebastian Andrzej Siewior
  Cc: Lee Jones, Aaro Koskinen, Sebastian Reichel, Pavel Machek,
	Pali Rohár, linux-omap, 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] 17+ 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
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 17+ messages in thread
From: Tony Lindgren @ 2014-09-03 18:39 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Sebastian Andrzej Siewior, Lee Jones, Aaro Koskinen,
	Sebastian Reichel, Pavel Machek, Pali Rohár, linux-omap,
	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] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-09-03 18:39         ` Tony Lindgren
  0 siblings, 0 replies; 17+ 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] 17+ 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
  -1 siblings, 1 reply; 17+ 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] 17+ messages in thread

* Re: [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; 17+ messages in thread
From: Tony Lindgren @ 2014-09-04 16:39 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Sebastian Andrzej Siewior, Lee Jones, Aaro Koskinen,
	Sebastian Reichel, Pavel Machek, Pali Rohár, linux-omap,
	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] 17+ messages in thread

* [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators
@ 2014-09-04 16:39             ` Tony Lindgren
  0 siblings, 0 replies; 17+ 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] 17+ messages in thread

end of thread, other threads:[~2014-09-04 16:39 UTC | newest]

Thread overview: 17+ 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 15:24 ` Tony Lindgren
2014-08-19 21:33 ` Aaro Koskinen
2014-08-19 21:33   ` Aaro Koskinen
2014-08-20 12:30 ` Lee Jones
2014-08-20 12:30   ` Lee Jones
2014-09-02  8:29 ` Sebastian Andrzej Siewior
2014-09-02  8:29   ` Sebastian Andrzej Siewior
2014-09-03  0:24   ` Tony Lindgren
2014-09-03  0:24     ` Tony Lindgren
2014-09-03  8:38     ` Sebastian Andrzej Siewior
2014-09-03  8:38       ` Sebastian Andrzej Siewior
2014-09-03 18:39       ` Tony Lindgren
2014-09-03 18:39         ` Tony Lindgren
2014-09-04  8:45         ` Sebastian Andrzej Siewior
2014-09-04 16:39           ` Tony Lindgren
2014-09-04 16:39             ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.