Re: [PATCH/RFC] pci: dynids.use_driver_data considered harmful

From: Greg KH <greg@kroah.com>
To: Milton Miller <miltonm@bga.com>
Cc: Greg KH <gregkh@suse.de>,
	Michael Ellerman <michael@ellerman.id.au>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-pci@vger.kernel.org, Jean Delvare <khali@linux-fr.org>
Subject: Re: [PATCH/RFC] pci: dynids.use_driver_data considered harmful
Date: Thu, 17 Jul 2008 00:07:59 -0700	[thread overview]
Message-ID: <20080717070759.GE22090@kroah.com> (raw)
In-Reply-To: <ad61f8387a737bc410e9d6dc3a8a5b63@bga.com>

On Wed, Jul 16, 2008 at 05:18:18AM -0500, Milton Miller wrote:
>
> Greg,
>
> Please respond to this email and explain why the patch
>
> pci: dynids.use_driver_data considered harmful
>
> http://www.ussg.iu.edu/hypermail/linux/kernel/0807.1/index.html#2188
>
> should not be applied.   I am not arguing the correctness of
> the removed code, rather its utility and benefit to the linux
> community.
>
> As far as I can tell, the code only succeeds in limiting the
> usefulness of the pci dynamic id addition mechanism.  We chose
> not to limit which drivers can have a table entry added, now
> let us not limit telling the driver which previous entry is
> most similar to our new entry.
>
> If a driver doesn't set this bit, and only 3 out of 419 do,
> then the facility can only be used if the driver can function
> correctly with the data zero.  In some drivers (radeonfb) a
> nonzero flag is always set, in some a list of boards or
> chipsets is listed in an arbitrary order (e1000e), and in yet
> others the field is used as a pointer without checking for NULL
> (DAC960, iwl3945).
>
>
>
> You sent your request for others to withdraw the patch
> from consideration when I resent the patch without seeing your
> comments that were less than 12 hours old, but have been silent
> for the last 60 hours since I responded to them over the next
> several hours.   If I do not hear from you with technical
> arguments for keeping the code, I will resend the patch for
> consideration.

Sorry, I'm on the road right now and will not get back until Friday.
Then I have the big merges with Linus to get through.  I'll try to get
to this by Monday, but my original point still stands, this was
implemented for a reason, saying that not enough drivers use it properly
does not make the need for it to go away.  It is required for them, so
perhaps the other 419 drivers also need to have the flag set.  That's
pretty trivial to do, right?

thanks,

greg k-h