Subject: Re: [patch] e1000=y && e1000e=m regression fix

From: "Kok, Auke" <[email-masked]>
Date: Thu, 10 Apr 2008 11:29:29 -0700

Kok, Auke wrote:
> Ingo Molnar wrote:
>> * Jeff Garzik <[email-masked]> wrote:
>>
>>>> We've got three thousand Kconfig options - it is clearly not 
>>>> realistic for users to keep such details in mind to avoid pitfalls.
>>> Agreed -- hence the multiple announcements, including in this thread, 
>>> to put said details into mind.
>> which part of "it took a kernel developer more than an hour to figure 
>> out why his laptop had a dead network interface" did you not understand? 
>> Whatever you did, it was not apparent to me. I dont follow every tiny 
>> detail of the e1000 driver family, nor do 99%+ [*] of our users.
>>
>> find the fix below, against current -git.
>>
>> the current upstream behavior is the worst possible one and is just a 
>> plain bug, and the solution is dead-simple.
> 
> 
> If this makes people happy then I am happy to ack this.
> 
> Acked-by: Auke Kok <[email-masked]>
> 
>> 	Ingo
>>
>> [*] guesstimate
>>
>> --------------->
>> Subject: e1000=y && e1000e=m regression fix
>> From: Ingo Molnar <[email-masked]>
>> Date: Wed Apr 09 21:09:35 CEST 2008
>>
>> fix a regression from v2.6.24: do not transfer the e1000e PCI IDs from 
>> e1000 to e1000e if e1000 is built-in and e1000e is a module.
>>
>> Built-in drivers take precedence over modules in many ways - and in this 
>> case it's clear that the user intended the e1000 driver to be the 
>> primary one. "Silently change behavior and break existing configs" is 
>> never a good migration strategy. Most users will use distro kernels that 
>> are not affected by this problem at all - nor are they affected by this 
>> patch - but this problem can hit users and developers who build their 
>> kernels themselves and migrate from v2.6.24 to v2.6.25.
>>
>> this fixes: http://bugzilla.kernel.org/show_bug.cgi?id=10427
>>
>> Signed-off-by: Ingo Molnar <[email-masked]>
>> ---
>>  drivers/net/Kconfig |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: linux-x86.q/drivers/net/Kconfig
>> ===================================================================
>> --- linux-x86.q.orig/drivers/net/Kconfig
>> +++ linux-x86.q/drivers/net/Kconfig
>> @@ -2022,7 +2022,7 @@ config E1000E
>>  	  will be called e1000e.
>>  
>>  config E1000E_ENABLED
>> -	def_bool E1000E != n
>> +	def_bool E1000E = y || ((E1000E != n) && (E1000 = E1000E))

Matthew Wilcox wrote:
> On Thu, Apr 10, 2008 at 11:55:03AM -0600, Grant Grundler wrote:
>> Agreed. I like Ingo's Kconfig patch which forces both drivers
>> (e1000 and e1000e) to be built the same way (ie both modules or both
>> builtin).
>
> Uh, that's /not/ what Ingo's patch does.  His patch makes e1000 claim
> the e1000e IDs if e1000 is built-in and e1000e is a module.

so that's definately _not_ what I would like to see at all. Matthew points out
that this will just prolong users to use e1000 instead of e1000e (which is what
they should be encouraged to switch to in those cases).

so I'm dropping my ACK

this patch doesn't solve anything and we'll have the same issue back when 2.6.26
ships which will have no PCI Express device IDs in e1000.

Auke