Hi,

On 07/04/2014 08:53 PM, Henrik Nordström wrote:
> fre 2014-07-04 klockan 17:14 +0800 skrev Wills Wang:
>>>>>> +    gctrl = readl(&mmchost->reg->gctrl);
>>>>>> +    gctrl &= ~SUNXI_MMC_GCTRL_ACCESS_BY_AHB;
>>>>> The above line seems redundant / unnecessary.
>>>> These lines are based on read-modify-write principle, the mmc hardware on 
>>>> sunxi have no detail document, we don't know what happen when operate this 
>>>> register in actually.
>>> Right, but the line below this one:
>>>
>>>>>> +    gctrl |= SUNXI_MMC_GCTRL_ACCESS_BY_AHB;
>>> Undoes the clearing you're doing. And you are not working on
>>> a register here, but on a local variable. So likely the
>>> C-compiler will just optimize away the redundant line.
>>>
>>> But even if the compiler does not optimize it away, then
>>> the mmc controller still will not see it as the
>>> |= overrules the &= and the result is not written to the
>>> mmc controller until after the |= .
>> You are right, the clearing is unnecessary.
>>
>>> Can you please try with the redundant line removed ?
>> It work fine after remove the redundant line.
> 
> Should we be strict then there is the clr/setbits functions/macros for
> these bit manipulations, also taking byteorder into account.
> 
> setbits_le32(&mmchost->reg->gctrl, SUNXI_MMC_GCTRL_ACCESS_BY_AHB)

Yes that indeed is the proper way to do things, I'll push a fixup
to u-boot-sunxi, and use this for the upstream patch submission.

Regards,

Hans

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to