[U-Boot] Possible bug in s5p_sdhci.c
The use of SDHCI_CTRL2_SELBASECLK_SHIFT in s5p_sdhci_set_control_reg() seems wrong, because a shifting offset is being used as a mask, not to generate a mask.
It's unclear what the original intent was. Below is my suggested fix.
What think ye?
Matt
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2); - val &= SDHCI_CTRL2_SELBASECLK_SHIFT; + val &= SDHCI_CTRL2_SELBASECLK_MASK(3);
val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |
Hi Matt,
On Feb 19, 2015, at 20:31 , Matt Reimer mreimer@sdgsystems.com wrote:
The use of SDHCI_CTRL2_SELBASECLK_SHIFT in s5p_sdhci_set_control_reg() seems wrong, because a shifting offset is being used as a mask, not to generate a mask.
It's unclear what the original intent was. Below is my suggested fix.
What think ye?
Matt
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2);
val &= SDHCI_CTRL2_SELBASECLK_SHIFT;
val &= SDHCI_CTRL2_SELBASECLK_MASK(3); val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |
At first glance it seems you’re right. Please prepare a proper patch. CCing Jaehoon…
Regards
— Pantelis
Properly mask SELBASECLK by using an actual mask rather than the number of bits to shift in order to create the mask.
Signed-off-by: Matt Reimer mreimer@sdgsystems.com --- drivers/mmc/s5p_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2); - val &= SDHCI_CTRL2_SELBASECLK_SHIFT; + val &= SDHCI_CTRL2_SELBASECLK_MASK(3);
val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |
Hi, Matt.
You're right, it's wrong masking.
Acked-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
On 02/24/2015 06:52 AM, Matt Reimer wrote:
Properly mask SELBASECLK by using an actual mask rather than the number of bits to shift in order to create the mask.
Signed-off-by: Matt Reimer mreimer@sdgsystems.com
drivers/mmc/s5p_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2);
- val &= SDHCI_CTRL2_SELBASECLK_SHIFT;
val &= SDHCI_CTRL2_SELBASECLK_MASK(3);
val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |
On Mon, Feb 23, 2015 at 2:52 PM, Matt Reimer mreimer@sdgsystems.com wrote:
Properly mask SELBASECLK by using an actual mask rather than the number of bits to shift in order to create the mask.
Signed-off-by: Matt Reimer mreimer@sdgsystems.com
drivers/mmc/s5p_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2);
val &= SDHCI_CTRL2_SELBASECLK_SHIFT;
val &= SDHCI_CTRL2_SELBASECLK_MASK(3); val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |-- 1.7.9.5
ping
Hi Matt,
On Feb 23, 2015, at 23:52 , Matt Reimer mreimer@sdgsystems.com wrote:
Properly mask SELBASECLK by using an actual mask rather than the number of bits to shift in order to create the mask.
Signed-off-by: Matt Reimer mreimer@sdgsystems.com
drivers/mmc/s5p_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 3899372..0eec731 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -30,7 +30,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host) sdhci_writel(host, SDHCI_CTRL4_DRIVE_MASK(0x3), SDHCI_CONTROL4);
val = sdhci_readl(host, SDHCI_CONTROL2);
- val &= SDHCI_CTRL2_SELBASECLK_SHIFT;
val &= SDHCI_CTRL2_SELBASECLK_MASK(3);
val |= SDHCI_CTRL2_ENSTAASYNCCLR | SDHCI_CTRL2_ENCMDCNFMSK |
-- 1.7.9.5
Thanks, applied.
— Pantelis
participants (3)
-
Jaehoon Chung -
Matt Reimer -
Pantelis Antoniou