Re: [PATCH 1/1] intel_ips: blacklist ASUSTek G60JX laptops

[Joseph Salisbury <joseph.salisbury@canonical.com>]

On 08/14/2013 05:11 PM, Linus Torvalds wrote:
> On Wed, Aug 14, 2013 at 9:38 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> Linus, you may want to pick this up directly, as I'm not sure if
>> Matthew is still looking after the x86 drivers these days.
> Can't we make the simpler patch be to just not spam the logs? Do it
> once, and forget about it. Maybe make the timeouts long enough to make
> sure that there are no other downsides from having the driver
> occasionally testing if it's back?
>
> I *detest* hardware-specific blacklists. They are impossible to
> maintain, and we've had the situation more than once that we fixed the
> real bug that caused the blacklist in the first place, but the entry
> stays around because nobody knows/cares/tests it.
>
>                   Linus
Hi Jesse,

What are your thoughts on alternatives to using a blacklist?  Is there a
bit we can read to determine if IPS is supported on a platform?

If not, can we do something like the following?  Basically increase the
timeout to 30 seconds then return an error from the monitor if there was
no update in that time:

diff --git a/drivers/platform/x86/intel_ips.c
b/drivers/platform/x86/intel_ips.c
index 18dcb58..a2391f7 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -1095,8 +1095,15 @@ static int ips_monitor(void *data)
                cur_seqno = (thm_readl(THM_ITV) & ITV_ME_SEQNO_MASK) >>
                        ITV_ME_SEQNO_SHIFT;
                if (cur_seqno == last_seqno &&
-                   time_after(jiffies, seqno_timestamp + HZ)) {
-                       dev_warn(&ips->dev->dev, "ME failed to update
for more than 1s, likely hung\n");
+                   time_after(jiffies, seqno_timestamp + (HZ*30))) {
+                       /* It's been 30s.  ME is likely hung.  Print
message and return. */
+                       dev_warn(&ips->dev->dev, "ME failed to update
for more than 30s, likely hung so exiting\n");
+                       del_timer_sync(&timer);
+                       destroy_timer_on_stack(&timer);
+
+                       dev_dbg(&ips->dev->dev, "ips-monitor thread
stopped\n");
+                       return -1;
+
                } else {
                        seqno_timestamp = get_jiffies_64();
                        last_seqno = cur_seqno;

There may be more to include such as restarting the monitor later to see
if the ME ever came back, just looking for other ideas and/or suggestions.

Thanks,

Joe