From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751292AbdAYF2j (ORCPT ); Wed, 25 Jan 2017 00:28:39 -0500 Received: from mga06.intel.com ([134.134.136.31]:1358 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751137AbdAYF2i (ORCPT ); Wed, 25 Jan 2017 00:28:38 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,282,1477983600"; d="scan'208";a="1087006810" Subject: Re: [PATCH v5 1/4] usb: dbc: early driver for xhci debug capability To: Ingo Molnar References: <1479189731-2728-1-git-send-email-baolu.lu@linux.intel.com> <1479189731-2728-2-git-send-email-baolu.lu@linux.intel.com> <20170119093743.GC22865@gmail.com> <58817A25.6080305@linux.intel.com> <20170122090423.GA15061@gmail.com> <5886DBB7.4070501@linux.intel.com> <20170124082039.GB8667@gmail.com> Cc: Greg Kroah-Hartman , Mathias Nyman , Ingo Molnar , tglx@linutronix.de, peterz@infradead.org, linux-usb@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, Jiri Slaby From: Lu Baolu Message-ID: <5888377F.8090709@linux.intel.com> Date: Wed, 25 Jan 2017 13:28:31 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20170124082039.GB8667@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ingo, On 01/24/2017 04:20 PM, Ingo Molnar wrote: > * Lu Baolu wrote: > >> Hi Ingo, >> >> On 01/22/2017 05:04 PM, Ingo Molnar wrote: >>> * Lu Baolu wrote: >>> >>>>>> +static void xdbc_runtime_delay(unsigned long count) >>>>>> +{ >>>>>> + udelay(count); >>>>>> +} >>>>>> +static void (*xdbc_delay)(unsigned long) = xdbc_early_delay; >>>>> Is this udelay() complication really necessary? udelay() should work fine even in >>>>> early code. It might not be precisely calibrated, but should be good enough. >>>> I tried udelay() in the early code. It's not precise enough for the >>>> hardware handshaking. >>> Possibly because on x86 early udelay() did not work at all - i.e. there's no delay >>> whatsoever. >> Yes. >> >>> Could you try it on top of this commit in tip:timers/core: >>> >>> 4c45c5167c95 x86/timer: Make delay() work during early bootup >>> >>> ? >> I tried tip:timers/core. It's not precise enough for my context either. >> >> __const_udelay(). >> >> 157 inline void __const_udelay(unsigned long xloops) >> 158 { >> 159 unsigned long lpj = this_cpu_read(cpu_info.loops_per_jiffy) ? : loops_per_jiffy; >> 160 int d0; >> 161 >> 162 xloops *= 4; >> 163 asm("mull %%edx" >> 164 :"=d" (xloops), "=&a" (d0) >> 165 :"1" (xloops), "0" (lpj * (HZ / 4))); >> 166 >> 167 __delay(++xloops); >> 168 } >> >> >> In my early code, loops_per_jiffy is not initialized yet. Hence "lpj" for the asm line >> is 4096 (default value). >> >> The cpu_info.loops_per_jiffy actually reads 8832000 after initialization. They are >> about 2000 times different. >> >> I did a hacky test in kernel to check the difference between these two different >> "lpj" values. (The hacky patch is attached.) Below is the output for 100ms delay. >> >> [ 2.494751] udelay_test uninitialized ---->start >> [ 2.494820] udelay_test uninitialized ---->end >> [ 2.494828] udelay_test initialized ---->start >> [ 2.595234] udelay_test initialized ---->end >> >> For 100ms delay, udelay() with uninitialized loops_per_jiffy only gives a delay of >> only 69us. > Ok, then could we add some simple calibration to make udelay work much better - or > perhaps move the udelay calibration up earlier? > > Hiding essentially an early udelay() implementation in an early-printk driver is > ugly and counterproductive. Sure. How about below change? diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c index d3f0c84..940989e 100644 --- a/drivers/usb/early/xhci-dbc.c +++ b/drivers/usb/early/xhci-dbc.c @@ -587,6 +587,35 @@ static int xdbc_bulk_transfer(void *data, int size, bool read) return size; } +static void __init xdbc_udelay_calibration(void) +{ + unsigned long lpj = 0; + unsigned int tsc_khz, cpu_khz; + + if (!boot_cpu_has(X86_FEATURE_TSC)) + goto calibration_out; + + cpu_khz = x86_platform.calibrate_cpu(); + tsc_khz = x86_platform.calibrate_tsc(); + + if (tsc_khz == 0) + tsc_khz = cpu_khz; + else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz) + cpu_khz = tsc_khz; + + if (!tsc_khz) + goto calibration_out; + + lpj = tsc_khz * 1000; + do_div(lpj, HZ); + +calibration_out: + if (!lpj) + lpj = 1 << 22; + + loops_per_jiffy = lpj; +} + static int __init xdbc_early_setup(void) { int ret; @@ -686,6 +715,8 @@ int __init early_xdbc_parse_parameter(char *s) } xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset); + xdbc_udelay_calibration(); + return 0; } Best regards, Lu Baolu