On 02/17/2017 10:10 AM, Ley Foon Tan wrote: > On Fri, Feb 17, 2017 at 3:54 PM, Marek Vasut <ma...@denx.de> 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 <ma...@denx.de> wrote: >>>> >>>> On 01/10/2017 06:20 AM, Chee Tien Fong wrote: >>>>> From: Tien Fong Chee <tien.fong.c...@intel.com> >>>>> >>>>> 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 <dingu...@opensource.altera.com> >>>>> Signed-off-by: Tien Fong Chee <tien.fong.c...@intel.com> >>>>> Cc: Marek Vasut <ma...@denx.de> >>>>> Cc: Dinh Nguyen <dingu...@kernel.org> >>>>> Cc: Chin Liang See <chin.liang....@intel.com> >>>>> Cc: Tien Fong <skywind...@gmail.com> >>>>> --- >>>>> 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 U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
