Message-ID: <1259192401.15916.48.camel@maxim-laptop>
Date:	Thu, 26 Nov 2009 01:40:01 +0200
From:	Maxim Levitsky <maximlevitsky@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Philip Langdale <philipl@...rt.org>,
	Pierre Ossman <pierre@...man.eu>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Port ricoh_mmc from driver to pci quirk.

On Wed, 2009-11-25 at 11:35 -0800, Andrew Morton wrote: 
> On Wed, 25 Nov 2009 08:53:24 -0800
> Philip Langdale <philipl@...rt.org> wrote:
> 
> > On Wed, 25 Nov 2009 16:58:41 +0200
> > Maxim Levitsky <maximlevitsky@...il.com> wrote:
> > 
> > > >From 5c5e6f5ab1a5a09a430f410cab4b160a5e65501c Mon Sep 17 00:00:00
> > > >2001
> > > From: Maxim Levitsky <maximlevitsky@...il.com>
> > > Date: Wed, 25 Nov 2009 16:37:46 +0200
> > > Subject: [PATCH] Port ricoh_mmc from driver to pci quirk.
> > >  This is much cleaner and correct solution
> 
> This patch actually fixes a bug but the changelog forgot to tell us
> this important fact.
> 
> > I'm fine with the concept, but when I originally started work on
> > Ricoh support, Pierre specifically didn't want a pci quirk.
> > 
> > Pierre wrote:
> > > I'd rather we didn't. The current style of quirks are bad enough,
> > > making them even more vendor or device specific is a bit more than I'm
> > > willing to accept right now (seriously, how hard can it be to follow
> > > the damn spec?).
> 
> Can the bug be fixed by other means, within ricoh_mmc.c?
> 
> It's a bit sad to add a lump of code to everyone's kernel like this -
> what percentage of those machines actually have a ricoh mmc controller?

You have valid point here.
However let me explain the situation:

We have a device with 5 functions,
One of the functions is propertary MMC contoller that writing driver for
will be a coolosal waste of time, because it can be disabled.

However, when it is disabled, it actually vanishes, and all pci
functions that belong to controller decrease by one.

Example: 
07:00.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller (rev 05)
07:00.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 22)
07:00.2 System peripheral: Ricoh Co Ltd R5C843 MMC Host Controller (rev 12)
07:00.3 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 12)
07:00.4 System peripheral: Ricoh Co Ltd xD-Picture Card Controller (rev ff)

However the correct listing is:
07:00.0 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller (rev 05)
07:00.1 SD Host controller: Ricoh Co Ltd R5C822 SD/SDIO/MMC/MS/MSPro Host Adapter (rev 22)
07:00.2 System peripheral: Ricoh Co Ltd R5C592 Memory Stick Bus Host Adapter (rev 12)
07:00.3 System peripheral: Ricoh Co Ltd xD-Picture Card Controller (rev 12)


PCI core can handle hotplug, but it sure can't handle the sudden shift
in function numbers.

Thus the disable step should be done before it enumerates the device, as
my patch does, and unfortunately this can't be module.
Up to now this driver did work, because both functions whose numbers are
affected didn't have a driver, but I am going soon to implement one
driver, and maybe another too.

Of course, I can put that under config condition, that users that are
sure that have no such device could save few hundreds of bytes.
However a distro probably will enable this option.

I also agree that I need to print some notice to user about device being
disabled.

Aside from being almost an 1:1 copy of original driver, this patch was
sort of RFC, so I update it soon.

Best regards,
Maxim Levitsky

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

Powered by blists - more mailing lists