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 48655ECAAD8 for ; Wed, 31 Aug 2022 05:57:10 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id D212384994; Wed, 31 Aug 2022 07:57:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1661925428; bh=SYu0areb7wzaDk3EaxrmBLq9l/+g5aFRTv85eT0RBGg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=G+rGfO25AHvJcdb//E4kcdBzMqQfiT0kz6e5cGjjw0Mq2sR9jSb6lGN9p1gjIx9D0 Det8oyDaDFBOG9hcw2ZUIi8nno1b09U5yZ0jUw9gG6Qd9x8MQyT0/X7f5+gN8/yMuP IHTsCBIIbQySs8iqURmu1/IC+pOB2scJYBEr9oMOLKbYYJ7HSvMhoU25vKjOS2tCLa fb+KJUVvMnfH3ALMlQRiFnB+/d4fN2EyflHI27o26vU3CCBUSHqFF8GyTCrrUIX6+5 a3uSaQuhGjMAPxODDS+VClLM2WlAS2IQcg7TMD4zPs0uXcMIqoKb6o1pXFgrVru6iX LtDQkm1itl8ZA== Received: by phobos.denx.de (Postfix, from userid 109) id 58B6184997; Wed, 31 Aug 2022 07:57:06 +0200 (CEST) Received: from mout-u-204.mailbox.org (mout-u-204.mailbox.org [IPv6:2001:67c:2050:101:465::204]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 188EE8489D for ; Wed, 31 Aug 2022 07:57:04 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-204.mailbox.org (Postfix) with ESMTPS id 4MHYNG19Z5z9sT4; Wed, 31 Aug 2022 07:57:02 +0200 (CEST) Message-ID: <65546e81-1287-3994-6e83-38fea3d1fa6c@denx.de> Date: Wed, 31 Aug 2022 07:57:01 +0200 MIME-Version: 1.0 Subject: Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support Content-Language: en-US To: Simon Glass Cc: Michael Walle , U-Boot Mailing List , =?UTF-8?Q?Pali_Roh=c3=a1r?= , Tony Dinh References: <20220830115317.410812-1-sr@denx.de> <20220830115317.410812-3-sr@denx.de> <092fe47f6dee523cb4cdbb81abc0cd40@walle.cc> <57f2671c-3841-74ec-b6c0-2c7aff3c3239@denx.de> From: Stefan Roese In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4MHYNG19Z5z9sT4 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 Hi Simon, On 30.08.22 17:56, Simon Glass wrote: > Hi Stefan, > > On Tue, 30 Aug 2022 at 06:08, Stefan Roese wrote: >> >> Adding Simon to Cc... >> >> On 30.08.22 14:00, Michael Walle wrote: >>> Am 2022-08-30 13:53, schrieb Stefan Roese: >>>> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE >>>> enabled, like pogo_v4. >>>> >>>> Signed-off-by: Stefan Roese >>>> --- >>>> drivers/timer/orion-timer.c | 22 ++++++++++++++++++++++ >>>> 1 file changed, 22 insertions(+) >>>> >>>> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c >>>> index 02ed138642b8..7e920eaeaa40 100644 >>>> --- a/drivers/timer/orion-timer.c >>>> +++ b/drivers/timer/orion-timer.c >>>> @@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void) >>>> } >>>> #endif >>>> >>>> +#if CONFIG_IS_ENABLED(BOOTSTAGE) >>>> +ulong timer_get_boot_us(void) >>>> +{ >>>> + u64 ticks = 0; >>>> + u32 rate = 1; >>>> + u64 us; >>>> + int ret; >>>> + >>>> + ret = dm_timer_init(); >>>> + if (!ret) { >>>> + /* The timer is available */ >>>> + rate = timer_get_rate(gd->timer); >>>> + timer_get_count(gd->timer, &ticks); >>>> + } else { >>>> + return 0; >>>> + } >>>> + >>>> + us = (ticks * 1000) / rate; >>>> + return us; >>>> +} >>>> +#endif >>> >>> This is duplicate code in almost all the timer drivers, shouldn't >>> this be a (weak) default implementation in timer-uclass.c? >> >> Yes. I was lazy and just copied this function and did not notice, that >> even more timer drivers have this function. Of course it makes sense >> to not duplicate code here, but have a common function for this. Frankly >> I don't even know why exactly this function is needed, as I did not >> look into BOOTSTAGE yet. >> >> Simon, do we really need this function? Can't bootstage just use the >> "normal" timer functionality instead? > > It is needed because bootstage is called before driver model is ready. > In fact it can be used to time driver model things. I see, makes sense. This brings up my next questions though, why isn't CONFIG_TIMER_EARLY enough in this case? AFAICT it's targeted exactly for this early (pre DM) bootstage. From drivers/timer/Kconfig: config TIMER_EARLY bool "Allow timer to be used early in U-Boot" depends on TIMER # initr_bootstage() requires a timer and is called before initr_dm() # so only the early timer is available default y if X86 && BOOTSTAGE help In some cases the timer must be accessible before driver model is active. Examples include when using CONFIG_TRACE to trace U-Boot's execution before driver model is set up. Enable this option to use an early timer. These functions must be supported by your timer driver: timer_early_get_count() and timer_early_get_rate(). So again, do we really need timer_get_boot_us() or isn't it enough to select TIMER_EARLY when BOOTSTAGE is enabled? Thanks, Stefan > The function here isn't that useful, since we cannot call > dm_timer_init() before driver model is set up. > > The timer_get_boot_us() function can by in a timer driver, but must > exist outside the driver infrastructure. It must init the hard and > then return the correct time. One driver model probes the timer > driver, it must then continue on in that way. > > Actually it looks like all the timers do this wrong. See > arch/arm/mach-imx/syscounter.c or arch/arm/cpu/armv8/generic_timer.c > for some ideas. Thanks, Stefan