[PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Luciano Coelho]

The order in which the MMC cards are defined in the the 4430sdp board
file seems to have been mistakenly reorderded as part of an unrelated
patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
the dev_name was supposed to be changed, the mmc order was changed as
well.  This caused the external SD card reader not to be recognized,
at least on Blaze.

This patch reverts this change so that the external SD card is
recognized again.

Cc: Kishore Kadiyala <kishore.kadiyala@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Signed-off-by: Luciano Coelho <coelho@ti.com>
---

I have started investigating the cause for this problem, because it
seems to me that the value in the mmc element is what should matter,
but it doesn't seem to be the case.  I believe there is a bug
elsewhere, that causes the order of the array to matter, but I'm not
very familiar with hsmmc and I don't have much time right now to delve
into the problem, so I leave this to the omap people. ;) I can always
help testing if necessary.

The change indeed seems to have been a mistake, because it was
introduced silently in v6:
https://patchwork.kernel.org/patch/595861/

In v5, the change was not there:
https://patchwork.kernel.org/patch/590441/

 arch/arm/mach-omap2/board-4430sdp.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 56702c5..8991d56 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -351,6 +351,11 @@ static struct twl4030_usb_data omap4_usbphy_data = {
 
 static struct omap2_hsmmc_info mmc[] = {
 	{
+		.mmc		= 1,
+		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpio_wp	= -EINVAL,
+	},
+	{
 		.mmc		= 2,
 		.caps		=  MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
 		.gpio_cd	= -EINVAL,
@@ -358,11 +363,6 @@ static struct omap2_hsmmc_info mmc[] = {
 		.nonremovable   = true,
 		.ocr_mask	= MMC_VDD_29_30,
 	},
-	{
-		.mmc		= 1,
-		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
-		.gpio_wp	= -EINVAL,
-	},
 	{}	/* Terminator */
 };
 
-- 
1.7.1

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Kishore Kadiyala]

On Fri, Apr 1, 2011 at 12:22 PM, Luciano Coelho <coelho@ti.com> wrote:
> The order in which the MMC cards are defined in the the 4430sdp board
> file seems to have been mistakenly reorderded as part of an unrelated
> patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
> the dev_name was supposed to be changed, the mmc order was changed as
> well.  This caused the external SD card reader not to be recognized,
> at least on Blaze.

Both OMAP4 SDP & Blaze boards have internal eMMC as storage.

Just think of some scenario as below with FS in  eMMC
[with external card recognized as mmcblk0 and eMMC as mmcblk1]:
Booting with both external card on MMC1 and eMMC on MMC2 and having
bootargs set to root=/dev/mmcblk1px [x= parition number].
Removing the external card from slot makes eMMC recognized as mmcblk0 and
in this case kernel can't pick the file system as passed above in the bootargs.

So, making the permanent storage on the boards registered as first
block device gets
rid of the problem.

Regards,
Kishore
>
> This patch reverts this change so that the external SD card is
> recognized again.
>
> Cc: Kishore Kadiyala <kishore.kadiyala@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Signed-off-by: Luciano Coelho <coelho@ti.com>
> ---
>
> I have started investigating the cause for this problem, because it
> seems to me that the value in the mmc element is what should matter,
> but it doesn't seem to be the case.  I believe there is a bug
> elsewhere, that causes the order of the array to matter, but I'm not
> very familiar with hsmmc and I don't have much time right now to delve
> into the problem, so I leave this to the omap people. ;) I can always
> help testing if necessary.
>
> The change indeed seems to have been a mistake, because it was
> introduced silently in v6:
> https://patchwork.kernel.org/patch/595861/
>
> In v5, the change was not there:
> https://patchwork.kernel.org/patch/590441/
>
>  arch/arm/mach-omap2/board-4430sdp.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 56702c5..8991d56 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -351,6 +351,11 @@ static struct twl4030_usb_data omap4_usbphy_data = {
>
>  static struct omap2_hsmmc_info mmc[] = {
>        {
> +               .mmc            = 1,
> +               .caps           = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
> +               .gpio_wp        = -EINVAL,
> +       },
> +       {
>                .mmc            = 2,
>                .caps           =  MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
>                .gpio_cd        = -EINVAL,
> @@ -358,11 +363,6 @@ static struct omap2_hsmmc_info mmc[] = {
>                .nonremovable   = true,
>                .ocr_mask       = MMC_VDD_29_30,
>        },
> -       {
> -               .mmc            = 1,
> -               .caps           = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
> -               .gpio_wp        = -EINVAL,
> -       },
>        {}      /* Terminator */
>  };
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Luciano Coelho]

On Fri, 2011-04-01 at 12:46 +0530, Kishore Kadiyala wrote:
> On Fri, Apr 1, 2011 at 12:22 PM, Luciano Coelho <coelho@ti.com> wrote:
> > The order in which the MMC cards are defined in the the 4430sdp board
> > file seems to have been mistakenly reorderded as part of an unrelated
> > patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
> > the dev_name was supposed to be changed, the mmc order was changed as
> > well.  This caused the external SD card reader not to be recognized,
> > at least on Blaze.
> 
> Both OMAP4 SDP & Blaze boards have internal eMMC as storage.
> 
> Just think of some scenario as below with FS in  eMMC
> [with external card recognized as mmcblk0 and eMMC as mmcblk1]:
> Booting with both external card on MMC1 and eMMC on MMC2 and having
> bootargs set to root=/dev/mmcblk1px [x= parition number].
> Removing the external card from slot makes eMMC recognized as mmcblk0 and
> in this case kernel can't pick the file system as passed above in the bootargs.

Yes, this makes sense.  The internal eMMC should be mapped first, I
agree.


> So, making the permanent storage on the boards registered as first
> block device gets
> rid of the problem.

Yes, but there is something wrong that causes the external MMC not to be
recognized at all if you map the internal MMC first.  There is a bug
elsewhere that needs to be fixed before this change can be made.

As I said, I don't know much about the OMAP MMC subsystem and I don't
have the time right now to investigate what really is wrong.  That's why
I sent this patch, as quick fix (or rather getting rid of a regression).

Another thing is that this is a crosspatch change.  It shouldn't be part
of the same patch that changes the name of the driver, since this is
totally unrelated.  If you really think this needs to be done, it should
have been done in a separate patch.

-- 
Cheers,
Luca.

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Kishore Kadiyala]

On Fri, Apr 1, 2011 at 1:50 PM, Luciano Coelho <coelho@ti.com> wrote:
> On Fri, 2011-04-01 at 12:46 +0530, Kishore Kadiyala wrote:
>> On Fri, Apr 1, 2011 at 12:22 PM, Luciano Coelho <coelho@ti.com> wrote:
>> > The order in which the MMC cards are defined in the the 4430sdp board
>> > file seems to have been mistakenly reorderded as part of an unrelated
>> > patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
>> > the dev_name was supposed to be changed, the mmc order was changed as
>> > well.  This caused the external SD card reader not to be recognized,
>> > at least on Blaze.
>>
>> Both OMAP4 SDP & Blaze boards have internal eMMC as storage.
>>
>> Just think of some scenario as below with FS in  eMMC
>> [with external card recognized as mmcblk0 and eMMC as mmcblk1]:
>> Booting with both external card on MMC1 and eMMC on MMC2 and having
>> bootargs set to root=/dev/mmcblk1px [x= parition number].
>> Removing the external card from slot makes eMMC recognized as mmcblk0 and
>> in this case kernel can't pick the file system as passed above in the bootargs.
>
> Yes, this makes sense.  The internal eMMC should be mapped first, I
> agree.
>
>
>> So, making the permanent storage on the boards registered as first
>> block device gets
>> rid of the problem.
>
> Yes, but there is something wrong that causes the external MMC not to be
> recognized at all if you map the internal MMC first.  There is a bug
> elsewhere that needs to be fixed before this change can be made.
>
> As I said, I don't know much about the OMAP MMC subsystem and I don't
> have the time right now to investigate what really is wrong.  That's why
> I sent this patch, as quick fix (or rather getting rid of a regression).

I don't see any regression with 2.6.39-rc1 on either OMAP4430SDP or Blaze,
logs below:

Blaze log : http://pastebin.com/mGB5uD7P
SDP log : http://pastebin.com/A5uAFfsr

>
> Another thing is that this is a crosspatch change.  It shouldn't be part
> of the same patch that changes the name of the driver, since this is
> totally unrelated.  If you really think this needs to be done, it should
> have been done in a separate patch.

Agree , it went accidentally in V6.

>
> --
> Cheers,
> Luca.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Luciano Coelho]

On Fri, 2011-04-01 at 15:56 +0530, Kishore Kadiyala wrote:
> > As I said, I don't know much about the OMAP MMC subsystem and I
> don't
> > have the time right now to investigate what really is wrong.  That's
> why
> > I sent this patch, as quick fix (or rather getting rid of a
> regression).
> 
> I don't see any regression with 2.6.39-rc1 on either OMAP4430SDP or
> Blaze,
> logs below:
> 
> Blaze log : http://pastebin.com/mGB5uD7P
> SDP log : http://pastebin.com/A5uAFfsr

Hmmm... right.  Actually I have now changed rootwait for rootdelay in my
kernel parameters and it worked fine with my setup too.  This is weird,
shouldn't rootwait cause the kernel to wait indefinitely until the root
device shows up?

-- 
Cheers,
Luca.

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Cousson, Benoit]

On 4/1/2011 8:52 AM, Coelho, Luciano wrote:
> The order in which the MMC cards are defined in the the 4430sdp board
> file seems to have been mistakenly reorderded as part of an unrelated
> patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
> the dev_name was supposed to be changed, the mmc order was changed as
> well.  This caused the external SD card reader not to be recognized,
> at least on Blaze.
>
> This patch reverts this change so that the external SD card is
> recognized again.
>
> Cc: Kishore Kadiyala<kishore.kadiyala@ti.com>
> Cc: Benoit Cousson<b-cousson@ti.com>
> Signed-off-by: Luciano Coelho<coelho@ti.com>
> ---
>
> I have started investigating the cause for this problem, because it
> seems to me that the value in the mmc element is what should matter,
> but it doesn't seem to be the case.  I believe there is a bug
> elsewhere, that causes the order of the array to matter, but I'm not
> very familiar with hsmmc and I don't have much time right now to delve
> into the problem, so I leave this to the omap people. ;) I can always
> help testing if necessary.

I was about to make the same comment. Why does the order matter since we 
have a .mmc field with that information? There is probably something 
broken behind that.

Regards,
Benoit

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Luciano Coelho]

On Fri, 2011-04-01 at 09:18 +0200, Cousson, Benoit wrote:
> On 4/1/2011 8:52 AM, Coelho, Luciano wrote:
> > The order in which the MMC cards are defined in the the 4430sdp board
> > file seems to have been mistakenly reorderded as part of an unrelated
> > patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
> > the dev_name was supposed to be changed, the mmc order was changed as
> > well.  This caused the external SD card reader not to be recognized,
> > at least on Blaze.
> >
> > This patch reverts this change so that the external SD card is
> > recognized again.
> >
> > Cc: Kishore Kadiyala<kishore.kadiyala@ti.com>
> > Cc: Benoit Cousson<b-cousson@ti.com>
> > Signed-off-by: Luciano Coelho<coelho@ti.com>
> > ---
> >
> > I have started investigating the cause for this problem, because it
> > seems to me that the value in the mmc element is what should matter,
> > but it doesn't seem to be the case.  I believe there is a bug
> > elsewhere, that causes the order of the array to matter, but I'm not
> > very familiar with hsmmc and I don't have much time right now to delve
> > into the problem, so I leave this to the omap people. ;) I can always
> > help testing if necessary.
> 
> I was about to make the same comment. Why does the order matter since we 
> have a .mmc field with that information? There is probably something 
> broken behind that.

That's my impression too.  From a bird's eye view, it seems that this
change should have worked.  But there's a bug somewhere, so it
doesn't. :(

-- 
Cheers,
Luca.

Re: [PATCH 2.6.39] omap: board-4430sdp: revert hsmmc_info reordering

[Luciano Coelho]

On Fri, 2011-04-01 at 09:52 +0300, Luciano Coelho wrote:
> The order in which the MMC cards are defined in the the 4430sdp board
> file seems to have been mistakenly reorderded as part of an unrelated
> patch.  In commit 0005ae73cfe44ee42d0be12a12cc82bf982f518e, where only
> the dev_name was supposed to be changed, the mmc order was changed as
> well.  This caused the external SD card reader not to be recognized,
> at least on Blaze.
> 
> This patch reverts this change so that the external SD card is
> recognized again.
> 
> Cc: Kishore Kadiyala <kishore.kadiyala@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Signed-off-by: Luciano Coelho <coelho@ti.com>
> ---

Please drop this patch.  The problem was not in the order the MMCs were
mounted, but something wrong with my bootloader settings where the
rootwait was not been passed to the kernel properly.

Now it works fine for me too.  Sorry for the trouble!

-- 
Cheers,
Luca.