Re: blink driver power saving

[Linus Torvalds]

On Mon, 2 Jul 2007, Andi Kleen wrote:
>
> It's not intended for normal kernels. It's a debugging feature.
> It's not intended for normal kernels. It's a debugging feature.
> It's not intended for normal kernels. It's a debugging feature.
> 
> Got it now?

You seem to have some reading comprehension problems.

The email you replied to had this in it:

  "No, its main problem is that PEOPLE SHOULD NOT USE IT, but it sounds cool,
   so people end up configuring the damn thing even though they shouldn't."

but you seem to not have understood.

Got it now?

> > It hangs machines when it tries to blink.
> 
> Yes, there seem to be more buggy keyboard controllers
> around than I anticipated. Very sad that IBM couldn't even
> get such a simple thing right.

Well, I would say that the driver itself is buggy. It calls 
"panic_blink()", which doesn't do the proper locking (i8042_lock is 
required to protect the accesses, otherwise you can have different 
entities in the system writing to the command ports concurrently, and get 
random stuff happening!).

So blaming "buggy keyboard controllers" is pretty damn silly of you, when 
the real problem is that the driver is broken. That interface is for 
panic, and panic *only*, and avoids the lock exactly because it's meant to 
be called when the system is basically dead.

Why did you think that function is called "panic_blink()"?

Yes, it could be hidden by making it do the buggy calls less often. That 
makes some machines work, but it doesn't change the fact that it would 
still be buggy.

> Anyways, Stephen's patch just doesn't make sense: 
> he clearly didn't understand the code at all. Before you
> apply it and cripple it better drop the driver completely.

I think I will have to.

		Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/