[U-Boot] [PATCH] mmc: fsl_esdhc fix register offset
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com --- drivers/mmc/fsl_esdhc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c5e270d..db4d251 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -56,7 +56,7 @@ struct fsl_esdhc { uint adsaddr; /* ADMA system address register */ char reserved2[100]; /* reserved */ uint vendorspec; /* Vendor Specific register */ - char reserved3[59]; /* reserved */ + char reserved3[56]; /* reserved */ uint hostver; /* Host controller version register */ char reserved4[4]; /* reserved */ uint dmaerraddr; /* DMA error address register */
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
Thanks for spotting this!
Best regards, Marek Vasut
Hi, Marek
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test. From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is used, so the offset of scr is wrong if using `char reserved3[59]`
Thanks for spotting this!
Best regards, Marek Vasut
Regards, Peng.
On Wednesday, March 11, 2015 at 01:58:37 AM, Peng Fan wrote:
Hi, Marek
Hi!
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test. From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is used, so the offset of scr is wrong if using `char reserved3[59]`
Uh, is the patch tested at all on real hardware ?
Best regards, Marek Vasut
Hi, Marek
On 3/11/2015 10:03 AM, Marek Vasut wrote:
On Wednesday, March 11, 2015 at 01:58:37 AM, Peng Fan wrote:
Hi, Marek
Hi!
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test. From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is used, so the offset of scr is wrong if using `char reserved3[59]`
Uh, is the patch tested at all on real hardware ?
Still not test on real hardware. From commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1, " uint adsaddr; /* ADMA system address register */ - char reserved2[160]; /* reserved */ + char reserved2[100]; /* reserved */ + uint vendorspec; /* Vendor Specific register */ + char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */ " It's clear that 160 bytes does not equal with (100 + 4 + 59)bytes.
Best regards, Marek Vasut
Regards, Peng.
On Wednesday, March 11, 2015 at 03:17:00 AM, Peng Fan wrote:
Hi, Marek
On 3/11/2015 10:03 AM, Marek Vasut wrote:
On Wednesday, March 11, 2015 at 01:58:37 AM, Peng Fan wrote:
Hi, Marek
Hi!
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test.
From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is
used, so the offset of scr is wrong if using `char reserved3[59]`
Uh, is the patch tested at all on real hardware ?
Still not test on real hardware. From commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1, " uint adsaddr; /* ADMA system address register */
char reserved2[160]; /* reserved */
char reserved2[100]; /* reserved */uint vendorspec; /* Vendor Specific register */char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */" It's clear that 160 bytes does not equal with (100 + 4 + 59)bytes.
Hi!
I agree with the change, I'm just not very impressed by patches that are completely untested. If you could do a quick boot on some machine before sending, that'd be nice.
Best regards, Marek Vasut
Hi Peng,
On Mar 11, 2015, at 04:17 , Peng Fan Peng.Fan@freescale.com wrote:
Hi, Marek
On 3/11/2015 10:03 AM, Marek Vasut wrote:
On Wednesday, March 11, 2015 at 01:58:37 AM, Peng Fan wrote:
Hi, Marek
Hi!
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test. From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is used, so the offset of scr is wrong if using `char reserved3[59]`
Uh, is the patch tested at all on real hardware ?
Still not test on real hardware. From commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1, " uint adsaddr; /* ADMA system address register */
char reserved2[160]; /* reserved */
char reserved2[100]; /* reserved */uint vendorspec; /* Vendor Specific register */char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */" It's clear that 160 bytes does not equal with (100 + 4 + 59)bytes.
Best regards, Marek Vasut
Regards, Peng.
Although I agree with fixing this, I’m kinda scared about how fragile structs for describing hardware registers are.
But we’re stuck with it I guess.
Regards
— Pantelis
On Wed, 2015-03-11 at 15:55 +0200, Pantelis Antoniou wrote:
Hi Peng,
On Mar 11, 2015, at 04:17 , Peng Fan Peng.Fan@freescale.com wrote:
Hi, Marek
On 3/11/2015 10:03 AM, Marek Vasut wrote:
On Wednesday, March 11, 2015 at 01:58:37 AM, Peng Fan wrote:
Hi, Marek
Hi!
On 3/10/2015 9:45 PM, Marek Vasut wrote:
On Tuesday, March 10, 2015 at 08:35:46 AM, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
This should probably be applied to 2015.04 .
What are the symptoms of this bug please ?
I just found the reserved3 size is wrong, did not do test. From the driver, only the entry 'scr' of fsl_esdhc below reserved3 is used, so the offset of scr is wrong if using `char reserved3[59]`
Uh, is the patch tested at all on real hardware ?
Still not test on real hardware. From commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1, " uint adsaddr; /* ADMA system address register */
char reserved2[160]; /* reserved */
char reserved2[100]; /* reserved */uint vendorspec; /* Vendor Specific register */char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */" It's clear that 160 bytes does not equal with (100 + 4 + 59)bytes.
Best regards, Marek Vasut
Regards, Peng.
Although I agree with fixing this, I’m kinda scared about how fragile structs for describing hardware registers are.
But we’re stuck with it I guess.
Without this patch my emmc (T1042)is broken beyond repair, please commit.
On Wed, Mar 11, 2015 at 10:55 AM, Pantelis Antoniou panto@antoniou-consulting.com wrote:
Although I agree with fixing this, I’m kinda scared about how fragile structs for describing hardware registers are.
Agreed!
But we’re stuck with it I guess.
Yes, it seems this is mandatory in U-boot.
Kernel does not have such requirement and the standard way there is to use (base + offset) for register accesses.
Regards,
Fabio Estevam
On Wed, Mar 11, 2015 at 01:03:16PM -0300, Fabio Estevam wrote:
On Wed, Mar 11, 2015 at 10:55 AM, Pantelis Antoniou panto@antoniou-consulting.com wrote:
Although I agree with fixing this, I’m kinda scared about how fragile structs for describing hardware registers are.
Agreed!
But we’re stuck with it I guess.
Yes, it seems this is mandatory in U-boot.
Kernel does not have such requirement and the standard way there is to use (base + offset) for register accesses.
So this is one of those topics that long term, I'd like to change U-Boot for but it's both a giant change and something we need to do a lot of prep-work for still. The long ago argument for why U-Boot does things the way it does boils down to type checking. The kernel gets this I think with a combination of sparse and other preprocessor magic / checks.
We'll also need to migrate once device model work is farther along and people want more seriously to look at splitting out a runs-many-places U-Boot from a "must be board-centric, pretty much" SPL.
On Tue, Mar 10, 2015 at 4:35 AM, Peng Fan Peng.Fan@freescale.com wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
On a imx6sl-warp:
Tested-by: Fabio Estevam fabio.estevam@freescale.com
Hi,
On 3/11/2015 9:00 PM, Fabio Estevam wrote:
On Tue, Mar 10, 2015 at 4:35 AM, Peng Fan Peng.Fan@freescale.com wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
On a imx6sl-warp:
Tested-by: Fabio Estevam fabio.estevam@freescale.com
Just kindly ask, will this patch be applied?
Regards, Peng.
On Sunday, March 15, 2015 at 06:54:49 AM, Peng Fan wrote:
Hi,
On 3/11/2015 9:00 PM, Fabio Estevam wrote:
On Tue, Mar 10, 2015 at 4:35 AM, Peng Fan Peng.Fan@freescale.com wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
On a imx6sl-warp:
Tested-by: Fabio Estevam fabio.estevam@freescale.com
Just kindly ask, will this patch be applied?
Tom ?
Best regards, Marek Vasut
On Sun, 2015-03-15 at 13:54 +0800, Peng Fan wrote:
Hi,
On 3/11/2015 9:00 PM, Fabio Estevam wrote:
On Tue, Mar 10, 2015 at 4:35 AM, Peng Fan Peng.Fan@freescale.com wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com
On a imx6sl-warp:
Tested-by: Fabio Estevam fabio.estevam@freescale.com
Just kindly ask, will this patch be applied?
Please do, my fsl emmc(T1042) is broken without this patch.
Jocke
On Tue, Mar 10, 2015 at 03:35:46PM +0800, Peng Fan wrote:
Commit f022d36e8a4517b2a9d25ff2d75bd2459d0c68b1 introduces error register offset.
Change the "char reserved3[59]" to "char reserved3[56]".
Signed-off-by: Peng Fan Peng.Fan@freescale.com Tested-by: Fabio Estevam fabio.estevam@freescale.com
Applied to u-boot/master, thanks!
participants (6)
-
Fabio Estevam -
Joakim Tjernlund -
Marek Vasut -
Pantelis Antoniou -
Peng Fan -
Tom Rini