From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Donald D. Dugger" Subject: Re: [PATCH V3] Decouple SnadyBridge quirk form VTd timeout Date: Thu, 20 Nov 2014 22:03:42 -0700 Message-ID: <20141121050342.GA22258@n0ano.com> References: <20141119194611.GA2600@tlaloc.n0ano.com> <546DDF34020000780004950D@smtp.nue.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <546DDF34020000780004950D@smtp.nue.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: eddie.dong@intel.com, will.auld@intel.com, allen.m.kay@intel.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, Nov 20, 2014 at 12:31:48PM +0100, Jan Beulich wrote: > >>> On 19.11.14 at 20:46, wrote: > > @@ -237,6 +248,42 @@ > > } > > } > > > > +static void __init parse_snb_timeout(const char *s) > > +{ > > + int not; > > + > > + switch (*s) { > > + > > + case '\0': > > + snb_igd_timeout = SNB_IGD_TIMEOUT_LEGACY; > > + break; > > + > > + case '0': case '1': case '2': > > + case '3': case '4': case '5': > > + case '6': case '7': case '8': > > + case '9': > > + snb_igd_timeout = MILLISECS(simple_strtoul(s, &s, 0)); > > + if ( snb_igd_timeout == MILLISECS(1) ) > > + snb_igd_timeout = SNB_IGD_TIMEOUT_LEGACY; > > + break; > > Overly complicated. Just parse_bool() first, if that returns negative > check for "default" or "", and (if not matched) invoke strtoul(). No > need for this switch statement. Personally, I prefer switch statements whenever possible, they're linear (if statements are evil) and the compiler optimizes them well. Anyway, NP, I'll follow your suggestion. > > > + > > + default: > > + if ( strncmp("default", s, 7) == 0 ) { > > + snb_igd_timeout = SNB_IGD_TIMEOUT; > > + break; > > + } > > + not = !strncmp("no-", s, 3); > > This makes no sense - you're looking for e.g. "snb_igd_quirk=no-no" > here. If the use specified "no-snb_igd_quirk", you'll end up seeing > "=no" when this function gets entered. I was confused about the `no-' prefix, I thought it applied to the value when it is really applied to the key (which makes a lot more sense). Fixed in next version. > > Also the whole function is white space damaged (using hard tabs) > and has misplaced opening braces. Forgot Xen doesn't like tabs, my bad. > > Jan > > And, as Ian pointed out, dyslexia in the subject line, also fixed in the next version. -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786