diff mbox

[U-Boot,RFC,1/1] omap3: incorrect logical check in do_emif4_init

Message ID 20170415141112.27145-1-xypron.glpk@gmx.de
State Accepted
Commit b730ff3fd65c8d1c33f1c05ca3fbab579a86abb4
Delegated to: Tom Rini
Headers show

Commit Message

Heinrich Schuchardt April 15, 2017, 2:11 p.m. UTC
((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
is always false.
This does not match the comment
/*Wait till that bit clears*/

The problem was indicated by cppcheck.

I do not have the hardware to test if the code change below
leads to a correct system behavior.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/arm/mach-omap2/omap3/emif4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Glass April 16, 2017, 7:34 p.m. UTC | #1
On 15 April 2017 at 08:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
> is always false.
> This does not match the comment
> /*Wait till that bit clears*/
>
> The problem was indicated by cppcheck.
>
> I do not have the hardware to test if the code change below
> leads to a correct system behavior.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/arm/mach-omap2/omap3/emif4.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Heinrich Schuchardt June 25, 2017, 6:13 a.m. UTC | #2
Hello Tom,

could you, please, pull the patch
https://patchwork.ozlabs.org/patch/751043/
which has been reviewed in April.

Best regards

Heinrich Schuchardt

On 04/16/2017 09:34 PM, Simon Glass wrote:
> On 15 April 2017 at 08:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
>> is always false.
>> This does not match the comment
>> /*Wait till that bit clears*/
>>
>> The problem was indicated by cppcheck.
>>
>> I do not have the hardware to test if the code change below
>> leads to a correct system behavior.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  arch/arm/mach-omap2/omap3/emif4.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
Tom Rini June 25, 2017, 1:54 p.m. UTC | #3
On Sun, Jun 25, 2017 at 08:13:19AM +0200, Heinrich Schuchardt wrote:

> Hello Tom,
> 
> could you, please, pull the patch
> https://patchwork.ozlabs.org/patch/751043/
> which has been reviewed in April.

I actually want to run-test this patch.  I happen to have a platform,
and I even set it up the other day, but forgot about this patch.  I'll
set it back up and test it, then apply.  Thanks!

> 
> Best regards
> 
> Heinrich Schuchardt
> 
> On 04/16/2017 09:34 PM, Simon Glass wrote:
> > On 15 April 2017 at 08:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
> >> is always false.
> >> This does not match the comment
> >> /*Wait till that bit clears*/
> >>
> >> The problem was indicated by cppcheck.
> >>
> >> I do not have the hardware to test if the code change below
> >> leads to a correct system behavior.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  arch/arm/mach-omap2/omap3/emif4.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
>
Heinrich Schuchardt July 24, 2017, 6:37 p.m. UTC | #4
On 06/25/2017 03:54 PM, Tom Rini wrote:
> On Sun, Jun 25, 2017 at 08:13:19AM +0200, Heinrich Schuchardt wrote:
> 
>> Hello Tom,
>>
>> could you, please, pull the patch
>> https://patchwork.ozlabs.org/patch/751043/
>> which has been reviewed in April.
> 
> I actually want to run-test this patch.  I happen to have a platform,
> and I even set it up the other day, but forgot about this patch.  I'll
> set it back up and test it, then apply.  Thanks!
> 

Hello Tom,

any update?

Regards

Heinrich


>>
>> Best regards
>>
>> Heinrich Schuchardt
>>
>> On 04/16/2017 09:34 PM, Simon Glass wrote:
>>> On 15 April 2017 at 08:11, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>> ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
>>>> is always false.
>>>> This does not match the comment
>>>> /*Wait till that bit clears*/
>>>>
>>>> The problem was indicated by cppcheck.
>>>>
>>>> I do not have the hardware to test if the code change below
>>>> leads to a correct system behavior.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> ---
>>>>  arch/arm/mach-omap2/omap3/emif4.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>
> 
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
Tom Rini Aug. 14, 2017, 12:06 a.m. UTC | #5
On Sat, Apr 15, 2017 at 04:11:12PM +0200, xypron.glpk@gmx.de wrote:

> ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x01)
> is always false.
> This does not match the comment
> /*Wait till that bit clears*/
> 
> The problem was indicated by cppcheck.
> 
> I do not have the hardware to test if the code change below
> leads to a correct system behavior.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap3/emif4.c b/arch/arm/mach-omap2/omap3/emif4.c
index d540cf08d2..8197e7b032 100644
--- a/arch/arm/mach-omap2/omap3/emif4.c
+++ b/arch/arm/mach-omap2/omap3/emif4.c
@@ -76,7 +76,7 @@  static void do_emif4_init(void)
 	regval |= (1<<10);
 	writel(regval, &emif4_base->sdram_iodft_tlgc);
 	/*Wait till that bit clears*/
-	while ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) == 0x1);
+	while ((readl(&emif4_base->sdram_iodft_tlgc) & (1<<10)) != 0x0);
 	/*Re-verify the DDR PHY status*/
 	while ((readl(&emif4_base->sdram_sts) & (1<<2)) == 0x0);