On 02/17/2017 10:10 AM, Ley Foon Tan wrote: > On Fri, Feb 17, 2017 at 3:54 PM, Marek Vasut <[email protected]> wrote: >> On 02/16/2017 04:34 AM, Ley Foon Tan wrote: >>> Hi Marek >>> >>> On Mon, Jan 23, 2017 at 11:58 AM, Marek Vasut <[email protected]> wrote: >>>> >>>> On 01/10/2017 06:20 AM, Chee Tien Fong wrote: >>>>> From: Tien Fong Chee <[email protected]> >>>>> >>>>> There is no dependency on doing a separate clrbits first in the >>>>> dwmac_deassert_reset function. Combine them into a single >>>>> clrsetbits call. >>>>> >>>>> Signed-off-by: Dinh Nguyen <[email protected]> >>>>> Signed-off-by: Tien Fong Chee <[email protected]> >>>>> Cc: Marek Vasut <[email protected]> >>>>> Cc: Dinh Nguyen <[email protected]> >>>>> Cc: Chin Liang See <[email protected]> >>>>> Cc: Tien Fong <[email protected]> >>>>> --- >>>>> arch/arm/mach-socfpga/misc.c | 9 +++------ >>>>> 1 file changed, 3 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c >>>>> index 2645129..c97caea 100644 >>>>> --- a/arch/arm/mach-socfpga/misc.c >>>>> +++ b/arch/arm/mach-socfpga/misc.c >>>>> @@ -100,13 +100,10 @@ static void dwmac_deassert_reset(const unsigned int >>>>> of_reset_id, >>>>> return; >>>>> } >>>>> >>>>> - /* Clearing emac0 PHY interface select to 0 */ >>>>> - clrbits_le32(&sysmgr_regs->emacgrp_ctrl, >>>>> - SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << physhift); >>>>> - >>>>> /* configure to PHY interface select choosed */ >>>>> - setbits_le32(&sysmgr_regs->emacgrp_ctrl, >>>>> - phymode << physhift); >>>> >>>> I don't think this patch is correct. The purpose of using these calls >>>> separately is so that the write with cleared physel mask actually >>>> reaches hardware, followed by read and another write with the physel >>>> mask set. clrsetbits will do only one read-modify-write cycle. >>> >>> The cleared physel mask is OR with the phymode set and then write to >>> hardware. So, I think the >>> result is the same. >> >> The result isn't the same, look at how setbits_le32() is implemented. >> The previous code will perform two read-modify-writes, while >> clrsetbits_le32() will perform one read-modify-write. >> >> I dunno how the hardware is implemented internally, but there might be a >> reason why the original code contained two writes. > I checked the code in linux driver, it only need one read-modify-write. > So, the new code should be fine.
OK, thanks -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
