From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A6B0C433EF for ; Mon, 20 Jun 2022 15:59:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B460881F4E; Mon, 20 Jun 2022 17:59:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=semihalf.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=semihalf.com header.i=@semihalf.com header.b="kjFEbpdS"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 334AB80166; Mon, 20 Jun 2022 17:59:55 +0200 (CEST) Received: from mail-yb1-xb35.google.com (mail-yb1-xb35.google.com [IPv6:2607:f8b0:4864:20::b35]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id EA83C81F4E for ; Mon, 20 Jun 2022 17:59:50 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=semihalf.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=pan@semihalf.com Received: by mail-yb1-xb35.google.com with SMTP id r3so19749241ybr.6 for ; Mon, 20 Jun 2022 08:59:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=EGqT31xDqUZ9Pz6uTyDjkm1tFREBt9Euc8VHeCy1VBo=; b=kjFEbpdSYAQ5lmxtYJOtvcVClw1Goju9hIk8LMUceq416xtM292B+Fp+5CAc2brsHq eE1BqhPyUeDwc1ma8CBEIJPNm0aysvddVDr4oG7+VvUYGExTAXusYwLoB/8SVX1eMj15 JeAhJtpdcOk5nG0jeVnA16uqAuNsn4qO7SiQpDO0PQa/s1RCmrmYjuBpqv0gxUSWQE3M KXPnzj/RcgvWviNHtRUDbk3RK2vg8aGmWd7rmW/4hKdee4y6lhJ0MK+EDFcX0jFTl2gH 9jIp6KjaBg5JLm+vXzioRvShbD5mk76zVsTTB+1qeozr7H5PkXoi/6LkxFBMdDO01GkI x3Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=EGqT31xDqUZ9Pz6uTyDjkm1tFREBt9Euc8VHeCy1VBo=; b=a/dViGVeyp2472bAGiPzxUarLGkd9QUJrFgE/qg/vpDnE3/1scVcuacCm/GSWQI0CU zPd7yMtZFR241xk1bwrIvVaXou2PJiS5ulPL65SCh5S7YZ7HGNo1d7HIdSdyl4CEsm87 D4n6abcHGAtGXVDQNOpXauWb72vDx7D47VS52M3Ixn9seIAZlCDTrDIwJnILsFbgmZ++ JU95ANNjPqsAvSorp0Fayp09REw/nMY5nTNXJAi66djF9ekqPfz01ClwId6udfWZhQi+ lE2eVZT2lKiJd/QJW3uFZAxkgvj3g+j3qUE3dSeav1OBFESRNNzP8LlJCFdBuMWiPuX2 ebJQ== X-Gm-Message-State: AJIora+MkzVV5gj0kRaOHoGT9iffvixkwHI/ClWi/dXGrhBfxPRhHfsM 4tL8JmIfzBjkeIP96V/MKwLXrHbl+41X1VMHBw0F7Q== X-Google-Smtp-Source: AGRyM1uLfL5JIxY4GLlODoTK9CaL7BkXwu3xLJ2aV+9/NbCUJU1v++3EGD+H9cEYzYDLHTIZMa6D9AvDKg6vfNCX998= X-Received: by 2002:a25:cb89:0:b0:668:b984:2a82 with SMTP id b131-20020a25cb89000000b00668b9842a82mr18908302ybg.573.1655740789631; Mon, 20 Jun 2022 08:59:49 -0700 (PDT) MIME-Version: 1.0 References: <20220617104726.158688-1-pan@semihalf.com> <20220617104726.158688-9-pan@semihalf.com> In-Reply-To: From: =?UTF-8?Q?Pawe=C5=82_Anikiel?= Date: Mon, 20 Jun 2022 17:59:38 +0200 Message-ID: Subject: Re: [PATCH v3 08/11] socfpga: arria10: Replace delays with busy waiting in cm_full_cfg To: "Chee, Tien Fong" Cc: "Vasut, Marek" , "simon.k.r.goldschmidt@gmail.com" , "michal.simek@xilinx.com" , "u-boot@lists.denx.de" , "sjg@chromium.org" , "festevam@denx.de" , "jagan@amarulasolutions.com" , "andre.przywara@arm.com" , "Armstrong, Neil" , "pbrobinson@gmail.com" , "tharvey@gateworks.com" , "paul.liu@linaro.org" , "christianshewitt@gmail.com" , "adrian.fiergolski@fastree3d.com" , "marek.behun@nic.cz" , "Denk, Wolfgang" , "Lim, Elly Siew Chin" , "upstream@semihalf.com" , "amstan@chromium.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Jun 20, 2022 at 2:29 PM Chee, Tien Fong wrote: > > > > > -----Original Message----- > > From: Pawe=C5=82 Anikiel > > Sent: Monday, 20 June, 2022 8:14 PM > > To: Chee, Tien Fong > > Cc: Vasut, Marek ; simon.k.r.goldschmidt@gmail.com; > > michal.simek@xilinx.com; u-boot@lists.denx.de; sjg@chromium.org; > > festevam@denx.de; jagan@amarulasolutions.com; > > andre.przywara@arm.com; Armstrong, Neil ; > > pbrobinson@gmail.com; tharvey@gateworks.com; paul.liu@linaro.org; > > christianshewitt@gmail.com; adrian.fiergolski@fastree3d.com; > > marek.behun@nic.cz; Denk, Wolfgang ; Lim, Elly Siew Chin > > ; upstream@semihalf.com; > > amstan@chromium.org > > Subject: Re: [PATCH v3 08/11] socfpga: arria10: Replace delays with bus= y > > waiting in cm_full_cfg > > > > On Mon, Jun 20, 2022 at 10:40 AM Chee, Tien Fong > > wrote: > > > > > > Hi, > > > > > > > -----Original Message----- > > > > From: Pawe=C5=82 Anikiel > > > > Sent: Friday, 17 June, 2022 6:47 PM > > > > To: Vasut, Marek ; simon.k.r.goldschmidt@gmail.com; > > > > Chee, Tien Fong ; michal.simek@xilinx.com > > > > Cc: u-boot@lists.denx.de; sjg@chromium.org; festevam@denx.de; > > > > jagan@amarulasolutions.com; andre.przywara@arm.com; Armstrong, > > Neil > > > > ; pbrobinson@gmail.com; > > > > tharvey@gateworks.com; paul.liu@linaro.org; > > > > christianshewitt@gmail.com; adrian.fiergolski@fastree3d.com; > > > > marek.behun@nic.cz; Denk, Wolfgang ; Lim, Elly Siew > > Chin > > > > ; upstream@semihalf.com; > > > > amstan@chromium.org; Pawe=C5=82 Anikiel > > > > Subject: [PATCH v3 08/11] socfpga: arria10: Replace delays with bus= y > > > > waiting in cm_full_cfg > > > > > > > > Using udelay while the clocks aren't fully configured causes the > > > > timer system to save the wrong clock rate. Use sdelay and > > > > wait_on_value instead (the values used in these functions were foun= d > > experimentally). > > > > > > > > Signed-off-by: Pawe=C5=82 Anikiel > > > > --- > > > > arch/arm/mach-socfpga/clock_manager_arria10.c | 31 > > > > +++++++++++++----- > > > > - > > > > 1 file changed, 22 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager_arria10.c > > > > b/arch/arm/mach-socfpga/clock_manager_arria10.c > > > > index 58d5d3fd8a..b48a2b47bc 100644 > > > > --- a/arch/arm/mach-socfpga/clock_manager_arria10.c > > > > +++ b/arch/arm/mach-socfpga/clock_manager_arria10.c > > > > > > Did you try to call timer_init() after cm_basic_init() in board_init_= f? If that's > > working, then no change is required to fix this clock issue. > > > > Seems like timer_init() isn't implemented on Arria 10 (it defaults to t= he > > return 0 stub). I also tried dm_timer_init(), no luck. > > > > I did some code digging, the clock rate is read by clk_get_rate(), and = the > > timer rate is set by dw_apb_timer_probe() (drivers/timer/dw-apb- > > timer.c:77), and there doesn't seem to be a good way of updating that v= alue > > later. > > > > The only other function I could find that sets the timer rate is > > timer_pre_probe() from drivers/timer/timer-uclass.c, which very much lo= oks > > like what we need, but it's static and the name suggests it shouldn't b= e called > > manually anyway. > > > > Thanks for the details finding. > > I found that both Cyclone 5 and S10 (including all AARCH64 devices) havin= g own timer_init() as solution for this issue. > Cyclone 5 : https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/m= ach-socfpga/timer.c > S10: https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-soc= fpga/timer_s10.c > > Do you think this is good idea having the same for A10 device? I don't think overriding timer_init() alone is going to help. (Re)initializing the timer after cm_basic_init() won't help the fact that xdelay() divides the clock ticks (which are correct) by gd->timer->uclass_priv_->clock_rate (https://source.denx.de/u-boot/u-boot/-/blob/master/lib/time.c#L81) (which was incorrectly set by a call to udelay() from cm_full_cfg()). I honestly don't see how Cyclone/Arria 5 solve this problem, as they don't implement a __udelay(), and their cm_basic_init() also uses timer-based delays (https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-socfpga/c= lock_manager_gen5.c#L98, eventually calls udelay(1) in include/wait_bit.h). I don't have any board on which I could test this on, but I suspect they may also save the wrong clock rate value (causing xdelay() to delay for wrong amounts of time). Stratix 10 looks okay to me, as it implements its own __udelay() and __usec_to_tick() in SPL. So a solution would be to implement a __udelay() and a __usec_to_tick(). I don't really know how to do that though, S10 uses the built-in armv8 timer for that. Regards, Pawe=C5=82