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.
Regards Ley Foon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
