Re: [PATCH V4] Decouple SandyBridge quirk from VTd timeout

[Don Dugger <n0ano@n0ano.com>]

On Fri, Dec 05, 2014 at 05:30:52AM +0000, Tian, Kevin wrote:
> > From: Donald D. Dugger
> > Sent: Friday, November 21, 2014 1:12 PM
> > 
> > Currently the quirk code for SandyBridge uses the VTd timeout value when
> > writing to an IGD register.  This is the wrong timeout to use and, at
> > 1000 msec., is also much too large.  This patch changes the quirk code
> > to use a timeout that is specific to the IGD device and allows the user
> > control of the timeout.
> > 
> > Boolean settings for the boot parameter `snb_igd_quirk' keep their current
> > meaning, enabling or disabling the quirk code with a timeout of 1000 msec.
> > 
> > In addition specifying `snb_igd_quirk=default' will enable the code and
> > set the timeout to the theoretical maximum of 670 msec.  For finer control,
> > specifying `snb_igd_quirk=n', where `n' is a decimal number, will enable
> > the code and set the timeout to `n' msec.
> 
> I'm OK with the patch except one comment. User usually thinks bool option
> means default, but here an explicit 'default' actually means a different value.
> It's a bit confused to me. How about changing 'default' to another more
> meaningful word, e.g. 'max'? and you may still keep a 'snb_igd_quirk=
> legacy' option, so all the possibilities are included in the assigned case, with
> bool option alias to 'legacy'.

I don't see a need for a `legacy' option, merely enabling the option (which is
what current users are doing) will select the current 1 sec. timeout.  If you
don't like `default' (and I don't like `max' since that would select a value
less that the current default) how about the string `cap' (the cap on the
theoretical timeout) to select the 670 msec value?

> 
> Thanks
> Kevin

...

-- 
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0ano@n0ano.com
Ph: 303/443-3786