On 02/17/2017 10:10 AM, Ley Foon Tan wrote:
On Fri, Feb 17, 2017 at 3:54 PM, Marek Vasut marex@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 marex@denx.de wrote:
On 01/10/2017 06:20 AM, Chee Tien Fong wrote:
From: Tien Fong Chee tien.fong.chee@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 dinguyen@opensource.altera.com Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com Cc: Marek Vasut marex@denx.de Cc: Dinh Nguyen dinguyen@kernel.org Cc: Chin Liang See chin.liang.see@intel.com Cc: Tien Fong skywindctf@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