Regression for non-eMMC cards with commit fa550189?

Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

Hi Ulf & Chris,

Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
cut from late init) causes MMC card to stop working at least with
n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.

Reverting fa550189 makes things work again. Any ideas what could
be causing this?

No debug output from MMC_DEBUG, looks like commands won't even
get started?

Regards,

Tony

Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

Hi Ulf & Chris,

Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
cut from late init) causes MMC card to stop working at least with
n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.

Reverting fa550189 makes things work again. Any ideas what could
be causing this?

No debug output from MMC_DEBUG, looks like commands won't even
get started?

Regards,

Tony

Re: Regression for non-eMMC cards with commit fa550189?

[Ulf Hansson]

Hi Tony,

This patch changes the sequence of how the host drivers set_ios
function gets called during probe.

Before:
1. mmc_power_off
2. mmc_power_up

Now:
1. mmc_power_up

My guess is then; the omap driver set_ios function requires that
mmc_power_off is called before a mmc_power_up.
Hopefully this requirement can be removed and fixed in the host driver somehow.

Please get back to me if you need some more assistance around this matter.

Kind regards
Ulf Hansson


On 28 May 2012 21:21, Tony Lindgren <tony@atomide.com> wrote:
> Hi Ulf & Chris,
>
> Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
> cut from late init) causes MMC card to stop working at least with
> n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.
>
> Reverting fa550189 makes things work again. Any ideas what could
> be causing this?
>
> No debug output from MMC_DEBUG, looks like commands won't even
> get started?
>
> Regards,
>
> Tony

Regression for non-eMMC cards with commit fa550189?

[Ulf Hansson]

Hi Tony,

This patch changes the sequence of how the host drivers set_ios
function gets called during probe.

Before:
1. mmc_power_off
2. mmc_power_up

Now:
1. mmc_power_up

My guess is then; the omap driver set_ios function requires that
mmc_power_off is called before a mmc_power_up.
Hopefully this requirement can be removed and fixed in the host driver somehow.

Please get back to me if you need some more assistance around this matter.

Kind regards
Ulf Hansson


On 28 May 2012 21:21, Tony Lindgren <tony@atomide.com> wrote:
> Hi Ulf & Chris,
>
> Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
> cut from late init) causes MMC card to stop working at least with
> n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.
>
> Reverting fa550189 makes things work again. Any ideas what could
> be causing this?
>
> No debug output from MMC_DEBUG, looks like commands won't even
> get started?
>
> Regards,
>
> Tony

Re: Regression for non-eMMC cards with commit fa550189?

[Felipe Balbi]

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

Hi,

On Tue, May 29, 2012 at 02:52:34PM +0800, Ulf Hansson wrote:
> Hi Tony,
> 
> This patch changes the sequence of how the host drivers set_ios
> function gets called during probe.
> 
> Before:
> 1. mmc_power_off
> 2. mmc_power_up
> 
> Now:
> 1. mmc_power_up
> 
> My guess is then; the omap driver set_ios function requires that
> mmc_power_off is called before a mmc_power_up.
> Hopefully this requirement can be removed and fixed in the host driver somehow.
> 
> Please get back to me if you need some more assistance around this matter.
> 
> Kind regards
> Ulf Hansson
> 
> 
> On 28 May 2012 21:21, Tony Lindgren <tony@atomide.com> wrote:
> > Hi Ulf & Chris,
> >
> > Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
> > cut from late init) causes MMC card to stop working at least with
> > n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.
> >
> > Reverting fa550189 makes things work again. Any ideas what could
> > be causing this?
> >
> > No debug output from MMC_DEBUG, looks like commands won't even
> > get started?

Can you enable debugging on menelaus so we see what the driver is doing ?

I believe that to change the VMMC's supply voltage you need to turn off
the regulator, meaning LDO_CTRL7[1:0] = 0b00.

That could be one thing. It would be cool what menelaus is writing to
the registers on working and failing case. Also what are the contents of
LDO_CTRL1 and LDO_CTRL7 before any operation. Something like this will
help:

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index cb4910a..8cab234 100644
--- a/drivers/mfd/menelaus.c
+++ b/drivers/mfd/menelaus.c
@@ -31,6 +31,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 
+#define DEBUG
+
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -469,6 +471,10 @@ static int menelaus_set_voltage(const struct menelaus_vtg *vtg, int mV,
 	if (vtg == 0)
 		goto set_voltage;
 
+	dev_dbg(&c->dev, "VTG_REG %02x MODE_REG %02x\n",
+			menelaus_read_reg(vtg->vtg_reg),
+			menelaus_read_reg(mode_reg));
+
 	ret = menelaus_read_reg(vtg->vtg_reg);
 	if (ret < 0)
 		goto out;

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Regression for non-eMMC cards with commit fa550189?

[Felipe Balbi]

Hi,

On Tue, May 29, 2012 at 02:52:34PM +0800, Ulf Hansson wrote:
> Hi Tony,
> 
> This patch changes the sequence of how the host drivers set_ios
> function gets called during probe.
> 
> Before:
> 1. mmc_power_off
> 2. mmc_power_up
> 
> Now:
> 1. mmc_power_up
> 
> My guess is then; the omap driver set_ios function requires that
> mmc_power_off is called before a mmc_power_up.
> Hopefully this requirement can be removed and fixed in the host driver somehow.
> 
> Please get back to me if you need some more assistance around this matter.
> 
> Kind regards
> Ulf Hansson
> 
> 
> On 28 May 2012 21:21, Tony Lindgren <tony@atomide.com> wrote:
> > Hi Ulf & Chris,
> >
> > Looks like commit fa550189 (mmc: core: Prevent eMMC VCC supply to be
> > cut from late init) causes MMC card to stop working at least with
> > n8x0 using drivers/mmc/host/omap.c. The card(s) on it are not eMMC.
> >
> > Reverting fa550189 makes things work again. Any ideas what could
> > be causing this?
> >
> > No debug output from MMC_DEBUG, looks like commands won't even
> > get started?

Can you enable debugging on menelaus so we see what the driver is doing ?

I believe that to change the VMMC's supply voltage you need to turn off
the regulator, meaning LDO_CTRL7[1:0] = 0b00.

That could be one thing. It would be cool what menelaus is writing to
the registers on working and failing case. Also what are the contents of
LDO_CTRL1 and LDO_CTRL7 before any operation. Something like this will
help:

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index cb4910a..8cab234 100644
--- a/drivers/mfd/menelaus.c
+++ b/drivers/mfd/menelaus.c
@@ -31,6 +31,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 
+#define DEBUG
+
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -469,6 +471,10 @@ static int menelaus_set_voltage(const struct menelaus_vtg *vtg, int mV,
 	if (vtg == 0)
 		goto set_voltage;
 
+	dev_dbg(&c->dev, "VTG_REG %02x MODE_REG %02x\n",
+			menelaus_read_reg(vtg->vtg_reg),
+			menelaus_read_reg(mode_reg));
+
 	ret = menelaus_read_reg(vtg->vtg_reg);
 	if (ret < 0)
 		goto out;

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120529/cd8579c6/attachment.sig>

Re: Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120529 00:46]:
> 
> Can you enable debugging on menelaus so we see what the driver is doing ?
> 
> I believe that to change the VMMC's supply voltage you need to turn off
> the regulator, meaning LDO_CTRL7[1:0] = 0b00.

Looks like it's some kind of race between the slots that now gets triggered
in the omap driver.

Commenting out the second slot in the board file fixes things. I tried adding
a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.

Also adding printk statements here and there makes it work again.

Regards,

Tony

Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120529 00:46]:
> 
> Can you enable debugging on menelaus so we see what the driver is doing ?
> 
> I believe that to change the VMMC's supply voltage you need to turn off
> the regulator, meaning LDO_CTRL7[1:0] = 0b00.

Looks like it's some kind of race between the slots that now gets triggered
in the omap driver.

Commenting out the second slot in the board file fixes things. I tried adding
a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.

Also adding printk statements here and there makes it work again.

Regards,

Tony

Re: Regression for non-eMMC cards with commit fa550189?

[Felipe Balbi]

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

Hi,

On Tue, May 29, 2012 at 05:47:32AM -0700, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [120529 00:46]:
> > 
> > Can you enable debugging on menelaus so we see what the driver is doing ?
> > 
> > I believe that to change the VMMC's supply voltage you need to turn off
> > the regulator, meaning LDO_CTRL7[1:0] = 0b00.
> 
> Looks like it's some kind of race between the slots that now gets triggered
> in the omap driver.
> 
> Commenting out the second slot in the board file fixes things. I tried adding
> a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.
> 
> Also adding printk statements here and there makes it work again.

does this help ?

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 518091c..5a224fc 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -397,8 +397,8 @@ static int n8x0_mmc_late_init(struct device *dev)
 	if (r < 0)
 		return r;
 
-	n8x0_mmc_set_power(dev, 0, MMC_POWER_ON, 16); /* MMC_VDD_28_29 */
-	n8x0_mmc_set_power(dev, 1, MMC_POWER_ON, 16);
+//	n8x0_mmc_set_power(dev, 0, MMC_POWER_ON, 16); /* MMC_VDD_28_29 */
+//	n8x0_mmc_set_power(dev, 1, MMC_POWER_ON, 16);
 
 	r = menelaus_set_mmc_slot(1, 1, 0, 1);
 	if (r < 0)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Regression for non-eMMC cards with commit fa550189?

[Felipe Balbi]

Hi,

On Tue, May 29, 2012 at 05:47:32AM -0700, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [120529 00:46]:
> > 
> > Can you enable debugging on menelaus so we see what the driver is doing ?
> > 
> > I believe that to change the VMMC's supply voltage you need to turn off
> > the regulator, meaning LDO_CTRL7[1:0] = 0b00.
> 
> Looks like it's some kind of race between the slots that now gets triggered
> in the omap driver.
> 
> Commenting out the second slot in the board file fixes things. I tried adding
> a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.
> 
> Also adding printk statements here and there makes it work again.

does this help ?

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 518091c..5a224fc 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -397,8 +397,8 @@ static int n8x0_mmc_late_init(struct device *dev)
 	if (r < 0)
 		return r;
 
-	n8x0_mmc_set_power(dev, 0, MMC_POWER_ON, 16); /* MMC_VDD_28_29 */
-	n8x0_mmc_set_power(dev, 1, MMC_POWER_ON, 16);
+//	n8x0_mmc_set_power(dev, 0, MMC_POWER_ON, 16); /* MMC_VDD_28_29 */
+//	n8x0_mmc_set_power(dev, 1, MMC_POWER_ON, 16);
 
 	r = menelaus_set_mmc_slot(1, 1, 0, 1);
 	if (r < 0)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120529/3cde1dc1/attachment.sig>

Re: Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120529 05:56]:
> Hi,
> 
> On Tue, May 29, 2012 at 05:47:32AM -0700, Tony Lindgren wrote:
> > * Felipe Balbi <balbi@ti.com> [120529 00:46]:
> > > 
> > > Can you enable debugging on menelaus so we see what the driver is doing ?
> > > 
> > > I believe that to change the VMMC's supply voltage you need to turn off
> > > the regulator, meaning LDO_CTRL7[1:0] = 0b00.
> > 
> > Looks like it's some kind of race between the slots that now gets triggered
> > in the omap driver.
> > 
> > Commenting out the second slot in the board file fixes things. I tried adding
> > a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.
> > 
> > Also adding printk statements here and there makes it work again.
> 
> does this help ?

Thanks does not seem to help.

Tony

Regression for non-eMMC cards with commit fa550189?

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120529 05:56]:
> Hi,
> 
> On Tue, May 29, 2012 at 05:47:32AM -0700, Tony Lindgren wrote:
> > * Felipe Balbi <balbi@ti.com> [120529 00:46]:
> > > 
> > > Can you enable debugging on menelaus so we see what the driver is doing ?
> > > 
> > > I believe that to change the VMMC's supply voltage you need to turn off
> > > the regulator, meaning LDO_CTRL7[1:0] = 0b00.
> > 
> > Looks like it's some kind of race between the slots that now gets triggered
> > in the omap driver.
> > 
> > Commenting out the second slot in the board file fixes things. I tried adding
> > a mutex to protect the sequence between POWER_UP and POWER_ON, but no luck yet.
> > 
> > Also adding printk statements here and there makes it work again.
> 
> does this help ?

Thanks does not seem to help.

Tony

[PATCH] mmc: omap: Fix broken reg_shift initialization

[Tony Lindgren]

Commit fa550189 (mmc: core: Prevent eMMC VCC supply to be cut from late init)
slightly affected timings for how things are done for the omap MMC driver
causing the MMC cards not getting detected any longer.

Turns out this was caused by buggy reg_shift initialization in the omap
MMC driver that was happening after mmc_add_host() was being called.

Fix this by initializing reg_shift before mmc_add_host() is called.

Signed-off-by: Tony Lindgren <tony@atomide.com>

---

Heh found it! Chris, maybe Cc stable on this one?

--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1485,6 +1485,7 @@ static int __devinit mmc_omap_probe(struct platform_device *pdev)
 	}
 
 	host->nr_slots = pdata->nr_slots;
+	host->reg_shift = (cpu_is_omap7xx() ? 1 : 2);
 
 	host->mmc_omap_wq = alloc_workqueue("mmc_omap", 0, 0);
 	if (!host->mmc_omap_wq)
@@ -1500,8 +1501,6 @@ static int __devinit mmc_omap_probe(struct platform_device *pdev)
 		}
 	}
 
-	host->reg_shift = (cpu_is_omap7xx() ? 1 : 2);
-
 	return 0;
 
 err_destroy_wq:

[PATCH] mmc: omap: Fix broken reg_shift initialization

[Tony Lindgren]

Commit fa550189 (mmc: core: Prevent eMMC VCC supply to be cut from late init)
slightly affected timings for how things are done for the omap MMC driver
causing the MMC cards not getting detected any longer.

Turns out this was caused by buggy reg_shift initialization in the omap
MMC driver that was happening after mmc_add_host() was being called.

Fix this by initializing reg_shift before mmc_add_host() is called.

Signed-off-by: Tony Lindgren <tony@atomide.com>

---

Heh found it! Chris, maybe Cc stable on this one?

--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1485,6 +1485,7 @@ static int __devinit mmc_omap_probe(struct platform_device *pdev)
 	}
 
 	host->nr_slots = pdata->nr_slots;
+	host->reg_shift = (cpu_is_omap7xx() ? 1 : 2);
 
 	host->mmc_omap_wq = alloc_workqueue("mmc_omap", 0, 0);
 	if (!host->mmc_omap_wq)
@@ -1500,8 +1501,6 @@ static int __devinit mmc_omap_probe(struct platform_device *pdev)
 		}
 	}
 
-	host->reg_shift = (cpu_is_omap7xx() ? 1 : 2);
-
 	return 0;
 
 err_destroy_wq:

Re: [PATCH] mmc: omap: Fix broken reg_shift initialization

[Chris Ball]

Hi,

On Wed, May 30 2012, Tony Lindgren wrote:
> Commit fa550189 (mmc: core: Prevent eMMC VCC supply to be cut from late init)
> slightly affected timings for how things are done for the omap MMC driver
> causing the MMC cards not getting detected any longer.
>
> Turns out this was caused by buggy reg_shift initialization in the omap
> MMC driver that was happening after mmc_add_host() was being called.
>
> Fix this by initializing reg_shift before mmc_add_host() is called.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Heh found it! Chris, maybe Cc stable on this one?

Thanks, pushed to mmc-next for 3.5 with a stable@ tag added.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH] mmc: omap: Fix broken reg_shift initialization

[Chris Ball]

Hi,

On Wed, May 30 2012, Tony Lindgren wrote:
> Commit fa550189 (mmc: core: Prevent eMMC VCC supply to be cut from late init)
> slightly affected timings for how things are done for the omap MMC driver
> causing the MMC cards not getting detected any longer.
>
> Turns out this was caused by buggy reg_shift initialization in the omap
> MMC driver that was happening after mmc_add_host() was being called.
>
> Fix this by initializing reg_shift before mmc_add_host() is called.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Heh found it! Chris, maybe Cc stable on this one?

Thanks, pushed to mmc-next for 3.5 with a stable@ tag added.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child