[U-Boot] Possible bug in sdhci.c

[U-Boot] Possible bug in sdhci.c

[Matt Reimer]

In sdhci_init() a writel() is used on a byte-sized register, which would clobber the adjacent registers. Below is my suggested fix. If it looks correct, I can submit a proper patch.

Matt


diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 82d7984..1f8917b 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
        if (host->quirks & SDHCI_QUIRK_NO_CD) {
                unsigned int status;
 
-               sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
+               sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
                        SDHCI_HOST_CONTROL);
 
                status = sdhci_readl(host, SDHCI_PRESENT_STATE);

[U-Boot] Possible bug in sdhci.c

[Pantelis Antoniou]

Hi Matt,

> On Feb 19, 2015, at 20:33 , Matt Reimer <mreimer@sdgsystems.com> wrote:
> 
> In sdhci_init() a writel() is used on a byte-sized register, which would clobber the adjacent registers. Below is my suggested fix. If it looks correct, I can submit a proper patch.
> 
> Matt
> 
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 82d7984..1f8917b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
>        if (host->quirks & SDHCI_QUIRK_NO_CD) {
>                unsigned int status;
> 
> -               sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
> +               sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
>                        SDHCI_HOST_CONTROL);
> 
>                status = sdhci_readl(host, SDHCI_PRESENT_STATE);
> 

Correct again. Please send a proper patch.

Regards

? Pantelis

[U-Boot] [PATCH] mmc: sdhci: don't clobber adjacent registers

[Matt Reimer]

SDHCI_HOST_CONTROL is a byte-sized register, so don't write to it
as if it were a long, as that would result in clobbering the three
registers following.

Signed-off-by: Matt Reimer <mreimer@sdgsystems.com>
---
 drivers/mmc/sdhci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 82d7984..1f8917b 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
 	if (host->quirks & SDHCI_QUIRK_NO_CD) {
 		unsigned int status;
 
-		sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
+		sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
 			SDHCI_HOST_CONTROL);
 
 		status = sdhci_readl(host, SDHCI_PRESENT_STATE);
-- 
1.7.9.5

[U-Boot] [PATCH] mmc: sdhci: don't clobber adjacent registers

[Matt Reimer]

On Mon, Feb 23, 2015 at 2:56 PM, Matt Reimer <mreimer@sdgsystems.com> wrote:

> SDHCI_HOST_CONTROL is a byte-sized register, so don't write to it
> as if it were a long, as that would result in clobbering the three
> registers following.
>
> Signed-off-by: Matt Reimer <mreimer@sdgsystems.com>
> ---
>  drivers/mmc/sdhci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 82d7984..1f8917b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
>         if (host->quirks & SDHCI_QUIRK_NO_CD) {
>                 unsigned int status;
>
> -               sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS |
> SDHCI_CTRL_CD_TEST,
> +               sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS |
> SDHCI_CTRL_CD_TEST,
>                         SDHCI_HOST_CONTROL);
>
>                 status = sdhci_readl(host, SDHCI_PRESENT_STATE);
> --
> 1.7.9.5
>
>
ping

[U-Boot] [PATCH] mmc: sdhci: don't clobber adjacent registers

[Pantelis Antoniou]

Hi Matt,

> On Feb 23, 2015, at 23:56 , Matt Reimer <mreimer@sdgsystems.com> wrote:
> 
> SDHCI_HOST_CONTROL is a byte-sized register, so don't write to it
> as if it were a long, as that would result in clobbering the three
> registers following.
> 
> Signed-off-by: Matt Reimer <mreimer@sdgsystems.com>
> ---
> drivers/mmc/sdhci.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 82d7984..1f8917b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
> 	if (host->quirks & SDHCI_QUIRK_NO_CD) {
> 		unsigned int status;
> 
> -		sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
> +		sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
> 			SDHCI_HOST_CONTROL);
> 
> 		status = sdhci_readl(host, SDHCI_PRESENT_STATE);
> -- 
> 1.7.9.5
> 

The patch is obviously correct and I?ll pick it up for my next pull-req.

Relax :)

Regards

? Pantelis

[U-Boot] [PATCH] mmc: sdhci: don't clobber adjacent registers

[Pantelis Antoniou]

Hi Matt,

> On Feb 23, 2015, at 23:56 , Matt Reimer <mreimer@sdgsystems.com> wrote:
> 
> SDHCI_HOST_CONTROL is a byte-sized register, so don't write to it
> as if it were a long, as that would result in clobbering the three
> registers following.
> 
> Signed-off-by: Matt Reimer <mreimer@sdgsystems.com>
> ---
> drivers/mmc/sdhci.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 82d7984..1f8917b 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -412,7 +412,7 @@ static int sdhci_init(struct mmc *mmc)
> 	if (host->quirks & SDHCI_QUIRK_NO_CD) {
> 		unsigned int status;
> 
> -		sdhci_writel(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
> +		sdhci_writeb(host, SDHCI_CTRL_CD_TEST_INS | SDHCI_CTRL_CD_TEST,
> 			SDHCI_HOST_CONTROL);
> 
> 		status = sdhci_readl(host, SDHCI_PRESENT_STATE);
> -- 
> 1.7.9.5
> 

Thanks, applied.

? Pantelis