Re: [PATCH] block: mtip32xx: remove HOTPLUG_PCI_PCIE dependancy

[Greg KH]

On Wed, Apr 11, 2012 at 03:22:56PM -0700, Asai Thambi S P wrote:
> On 4/11/2012 1:40 PM, Greg KH wrote:
> 
> > On Wed, Apr 11, 2012 at 01:33:39PM -0700, Asai Thambi S P wrote:
> >> On 4/11/2012 12:57 PM, Jens Axboe wrote:
> >>
> >>> On 2012-04-11 20:34, Greg KH wrote:
> >>>> This removes the HOTPLUG_PCI_PCIE dependency on the driver and makes it
> >>>> depend on PCI.
> >>>
> >>> I think it's an old dependency. I've built and run it here without as
> >>> well, and no functional issues either.
> >>>
> >>> Sam/Asai?
> >>>
> >>
> >>
> >> Both driver and device will work fine without PCIe hotplug dependency. This
> >> dependency is required for supporting surprise removal and surprise insertion
> >> of the device on systems with PCIe hotplug controller.
> > 
> > But that's not a driver-specific thing at all.  All PCI drivers need to
> > be able to handle this (I like how you constantly check the pci id,
> > that's cute.)
> > 
> > So I think a basic dependancy on PCI should be fine here.
> 
> The P320 is different from existing PCIe devices supporting surprise removal
> and surprise insertion (SRSI) capability (aka hotplug). We equate the hotplug
> functionality enabled by PCIe hotplug controllers to that of any other storage
> endpoint (SAS, SATA, FC, etc). For those devices, hotplug functionality is
> enabled by the transport layer and propagated up to host storage stack for
> handling.

Huh?  No, hotplug on Linux happens on the PCI level, and has done so for
over 10 years.

> There are two types of users for P320 device â 1) those with systems that have
> PCIe hotplug controllers and intend to use hotplug and 2) those who do not
> need or intend to use hotplug. The HOTPLUG_PCI_PCIE dependency enables users
> in the first bucket to use the deviceâs capability without affecting those in
> the second bucket.

No, it prevents the users in the second group from ever using this
driver as it will not be built at all.

I have such a system right here, and I want to use the device, yet could
not as I do not have HOTPLUG_PCI_PCIE enabled in my kernel because I do
not have that type of PCI Hotplug controller.

> While there arenât many systems today that have PCIe
> hotplug controllers, you will begin to see such systems very soon.

Manufacturers have been telling me that for over 10 years ago, when I
got the first PCI Hotplug driver merged into the kernel tree :)

> The mtip32xx driver depends on remove() being called for graceful handling of
> surprise removal events.

As it should, nothing new there, that's how PCI hotplug works.

> Such cleanup is necessary to enable clean surprise
> insertion of the same/different device in the same slot. We do check the PCI
> id in non-fast path code to detect the surprise removal and fail any
> outstanding I/Os with -ENODEV, but the driver still depends on the pci core
> with help from the pcie hotplug module to call remove() for cleanup, hence the
> HOTPLUG_PCI_PCIE dependency.

No, that's not a valid dependency at all, sorry.

All you should depend on is CONFIG_PCI and CONFIG_BLOCK.  Your driver
doesn't know, or care, if the system is a PCI hotplug one, as that is
how we architected the PCI hotplug layer.  Otherwise all PCI drivers
would have to have this type of logic in it, and that would be a lot of
extra work for no good reason.

So, I still think this dependency should be removed, the fact that I'm
typing this from a system running on this card at the moment, without
PCIE hotplug capabilities, kind of proves it :)

thanks,

greg k-h
--
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/