block: mtip32xx: remove HOTPLUG_PCI_PCIE dependancy
PATCH 1/1 block: mtip32xx: remove HOTPLUG_PCI_PCIE dependancy
This removes the HOTPLUG_PCI_PCIE dependency on the driver and makes it depend on PCI.
---
Note, the driver builds just fine without this dependency, and I have a system that does not have HOTPLUG PCIE, yet it has PCIE slots, which should be all that is required for this driver.
What am I missing that required this dependency in the first place?
drivers/block/mtip32xx/Kconfig +1 -1
@@ -4,6 +4,6 @@ config BLK_DEV_PCIESSD_MTIP32XX tristate "Block Device Driver for Micron PCIe SSDs" - depends on HOTPLUG_PCI_PCIE + depends on PCI help This enables the block driver for Micron PCIe SSDs.
Jens Axboe
maintainerQuoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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?
Signature · 1 line
Jens Axboe
Quoted 9 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
--
Regards,
Asai Thambi
Jens Axboe
maintainerQuoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
That goes for all devices, though. So I think we can safely remove this dependency. Nobody should expect hotplug/removal to work, without actually including that :-)
If you or Sam would formally ack the patch, then I'll add it to the pending mtip32xx series.
Signature · 1 line
Jens Axboe
Asai Thambi S P
Acked-byQuoted 18 lines · Jens Axboe
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 7 lines
That goes for all devices, though. So I think we can safely remove this dependency. Nobody should expect hotplug/removal to work, without actually including that :-)
If you or Sam would formally ack the patch, then I'll add it to the pending mtip32xx series.
--
Regards,
Asai Thambi
Quoted 24 lines · Asai Thambi S P
Quoted 22 lines · Jens Axboe
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
That goes for all devices, though. So I think we can safely remove this dependency. Nobody should expect hotplug/removal to work, without actually including that :-)
Quoted 5 lines
Quoted 3 lines
If you or Sam would formally ack the patch, then I'll add it to the pending mtip32xx series.
Acked-by Asai Thambi S P <[email protected]>
Thanks applied Gregs patch - I see the two of you battled it out while I was sound asleep ;-)
Signature · 1 line
Jens Axboe
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
thanks,
greg k-h
Quoted 18 lines · Greg KH
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 5 lines
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.
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. While there aren’t many systems today that have PCIe hotplug controllers, you will begin to see such systems very soon.
The mtip32xx driver depends on remove() being called for graceful handling of surprise removal events. 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.
--
Regards,
Asai Thambi
Quoted 24 lines · Asai Thambi S P
Quoted 22 lines · Greg KH
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.)
Quoted 8 lines
Quoted 1 line
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.
Quoted 5 lines
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_PCIEdependency 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.
Quoted 2 lines
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 :)
Quoted 2 lines
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.
Quoted 6 lines
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 callremove()for cleanup, hence theHOTPLUG_PCI_PCIEdependency.
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
Quoted 34 lines · Greg KH
Quoted 24 lines · Asai Thambi S P
Quoted 22 lines · Greg KH
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.)
Quoted 8 lines
Quoted 1 line
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.
Quoted 2 lines
Huh? No, hotplug on Linux happens on the PCI level, and has done so for over 10 years.
Just to clarify, I was referring to hotplug of SAS/SATA/FC storage devices, not HBA.
Quoted 40 lines
Quoted 6 lines
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_PCIEdependency 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_PCIEenabled in my kernel because I do not have that type of PCI Hotplug controller.Quoted 2 lines
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 :)
Quoted 2 lines
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.
Quoted 6 lines
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 callremove()for cleanup, hence theHOTPLUG_PCI_PCIEdependency.No, that's not a valid dependency at all, sorry.
All you should depend on is
CONFIG_PCIandCONFIG_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.
Quoted 3 lines
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 :)
Agree to remove the dependency. I will reply to Jens email too.
--
Regards,
Asai Thambi
Quoted 36 lines · Asai Thambi S P
Quoted 34 lines · Greg KH
Quoted 32 lines · Asai Thambi S P
Quoted 18 lines · Greg KH
Quoted 9 lines · Asai Thambi S P
Quoted 7 lines · Jens Axboe
Quoted 2 lines · Greg KH
This removes the
HOTPLUG_PCI_PCIEdependency 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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 5 lines
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.
Quoted 5 lines
Quoted 2 lines
Huh? No, hotplug on Linux happens on the PCI level, and has done so for over 10 years.
Just to clarify, I was referring to hotplug of SAS/SATA/FC storage devices, not HBA.
Ah, ok, yes, you are right, but as you aren't using the SCSI layer here (odd, but ok), you can only handle hotplug at the PCI layer.
thanks,
greg k-h