| Message ID | 1442200140-30808-4-git-send-email-jiang.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 09/14/2015 05:08 AM, Jiang Liu wrote: > Due to having no hardware for testing, this is just a sample code > to support PCI device hot-removal. It just passing compilation, > no any tests. > > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> > --- > drivers/scsi/eata.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c > index b92e6856f909..f3bd7cbf260e 100644 > --- a/drivers/scsi/eata.c > +++ b/drivers/scsi/eata.c > @@ -1474,6 +1474,21 @@ static unsigned int port_probe(unsigned long port_base, > #ifdef CONFIG_PCI > static int eata2x_pci_device_count; > > +/* TODO: need help here to shutdown the scsi host and release resources */ > +static void port_remove(unsigned int id, resource_size_t port_base, > + struct pci_dev *pdev) > +{ > + struct Scsi_Host *shost = sh[id]; > + > + /* TODO: stop scsi device */ > + scsi_unregister(shost); > + /* TODO: clean up resources allocated by port_detect() */ > + clear_bit(id, eata_board_bitmap); > + free_irq(shost->irq, &sha[id]); > + release_region(port_base, REGION_SIZE); > + ida_simple_remove(&eata_ida, id); > +} > + > static int eata2x_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > { > int i, ret = -ENXIO; > @@ -1521,6 +1536,16 @@ out_error: > return ret; > } > > +static void eata2x_pci_remove(struct pci_dev *pdev) > +{ > + int id = (int)(long)dev_get_drvdata(&pdev->dev); > + resource_size_t port_base; > + > + port_base = pci_resource_start(pdev, 0) + PCI_BASE_ADDRESS_0; > + port_remove(id, port_base, pdev); > + pci_disable_device(pdev); > +} > + > static struct pci_device_id eata2x_tbl[] = { > { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_SCSI << 8, PCI_ANY_ID) }, > { }, > @@ -1531,6 +1556,7 @@ static struct pci_driver eata2x_pci_driver = { > .name = "eata", > .id_table = eata2x_tbl, > .probe = eata2x_pci_probe, > + .remove = eata2x_pci_remove, > }; > > static int eata2x_probe_pci_devices(struct scsi_host_template *tpnt) > Welll ... if you don't have hardware (and I strongly hope you refer to 'hardware able to do hotplugging', not 'hardware for the eata driver' ...) why add the code at all? Chances are no-one will ever need eata PCI hotplug; SCSI parallel typically isn't very good at hotplugging, so throwing in PCI hotplug will only confuse matters more. Plus due to the sheer mechanics involved here I find it very unlikely anyone will be using it in real life. Cheers, Hannes
Agreed, It does not make sense to have this driver converted to a hot plug api. Cheers, *************************************** Ph.D. Dario Ballabio Principal Field Support Specialist, EMC EMEA Mobile phone: +393487978851 -----Original Message----- From: Hannes Reinecke [mailto:hare@suse.de] Sent: Monday, September 14, 2015 10:21 AM To: Jiang Liu; Thomas Gleixner; Bjorn Helgaas; Arthur Marsh; Ballabio, Dario; James E.J. Bottomley Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; linux-scsi@vger.kernel.org; x86@kernel.org Subject: Re: [Bugfix 3/3] eata: Enhance eata driver to support PCI device hot-removal On 09/14/2015 05:08 AM, Jiang Liu wrote: > Due to having no hardware for testing, this is just a sample code to > support PCI device hot-removal. It just passing compilation, no any > tests. > > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> > --- > drivers/scsi/eata.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c index > b92e6856f909..f3bd7cbf260e 100644 > --- a/drivers/scsi/eata.c > +++ b/drivers/scsi/eata.c > @@ -1474,6 +1474,21 @@ static unsigned int port_probe(unsigned long > port_base, #ifdef CONFIG_PCI static int eata2x_pci_device_count; > > +/* TODO: need help here to shutdown the scsi host and release > +resources */ static void port_remove(unsigned int id, resource_size_t port_base, > + struct pci_dev *pdev) > +{ > + struct Scsi_Host *shost = sh[id]; > + > + /* TODO: stop scsi device */ > + scsi_unregister(shost); > + /* TODO: clean up resources allocated by port_detect() */ > + clear_bit(id, eata_board_bitmap); > + free_irq(shost->irq, &sha[id]); > + release_region(port_base, REGION_SIZE); > + ida_simple_remove(&eata_ida, id); > +} > + > static int eata2x_pci_probe(struct pci_dev *dev, const struct > pci_device_id *id) { > int i, ret = -ENXIO; > @@ -1521,6 +1536,16 @@ out_error: > return ret; > } > > +static void eata2x_pci_remove(struct pci_dev *pdev) { > + int id = (int)(long)dev_get_drvdata(&pdev->dev); > + resource_size_t port_base; > + > + port_base = pci_resource_start(pdev, 0) + PCI_BASE_ADDRESS_0; > + port_remove(id, port_base, pdev); > + pci_disable_device(pdev); > +} > + > static struct pci_device_id eata2x_tbl[] = { > { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_SCSI << 8, PCI_ANY_ID) }, > { }, > @@ -1531,6 +1556,7 @@ static struct pci_driver eata2x_pci_driver = { > .name = "eata", > .id_table = eata2x_tbl, > .probe = eata2x_pci_probe, > + .remove = eata2x_pci_remove, > }; > > static int eata2x_probe_pci_devices(struct scsi_host_template *tpnt) > Welll ... if you don't have hardware (and I strongly hope you refer to 'hardware able to do hotplugging', not 'hardware for the eata driver' ...) why add the code at all? Chances are no-one will ever need eata PCI hotplug; SCSI parallel typically isn't very good at hotplugging, so throwing in PCI hotplug will only confuse matters more. Plus due to the sheer mechanics involved here I find it very unlikely anyone will be using it in real life. Cheers, Hannes
On 2015/9/14 16:31, Ballabio, Dario wrote: > Agreed, It does not make sense to have this driver converted to a hot plug api. Thanks, got the point:) Originally i thought user may trigger PCI device hot-removal by sysfs interfaces and there's comment mentioning that eata driver breaks PCI hotplug, so I tried to solve it. If it's no real use, we may just drop the third patch. Thanks! Gerry > > Cheers, > > > > *************************************** > Ph.D. Dario Ballabio > Principal Field Support Specialist, EMC EMEA > Mobile phone: +393487978851 > > > > -----Original Message----- > From: Hannes Reinecke [mailto:hare@suse.de] > Sent: Monday, September 14, 2015 10:21 AM > To: Jiang Liu; Thomas Gleixner; Bjorn Helgaas; Arthur Marsh; Ballabio, Dario; James E.J. Bottomley > Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; linux-scsi@vger.kernel.org; x86@kernel.org > Subject: Re: [Bugfix 3/3] eata: Enhance eata driver to support PCI device hot-removal > > On 09/14/2015 05:08 AM, Jiang Liu wrote: >> Due to having no hardware for testing, this is just a sample code to >> support PCI device hot-removal. It just passing compilation, no any >> tests. >> >> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> >> --- >> drivers/scsi/eata.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c index >> b92e6856f909..f3bd7cbf260e 100644 >> --- a/drivers/scsi/eata.c >> +++ b/drivers/scsi/eata.c >> @@ -1474,6 +1474,21 @@ static unsigned int port_probe(unsigned long >> port_base, #ifdef CONFIG_PCI static int eata2x_pci_device_count; >> >> +/* TODO: need help here to shutdown the scsi host and release >> +resources */ static void port_remove(unsigned int id, resource_size_t port_base, >> + struct pci_dev *pdev) >> +{ >> + struct Scsi_Host *shost = sh[id]; >> + >> + /* TODO: stop scsi device */ >> + scsi_unregister(shost); >> + /* TODO: clean up resources allocated by port_detect() */ >> + clear_bit(id, eata_board_bitmap); >> + free_irq(shost->irq, &sha[id]); >> + release_region(port_base, REGION_SIZE); >> + ida_simple_remove(&eata_ida, id); >> +} >> + >> static int eata2x_pci_probe(struct pci_dev *dev, const struct >> pci_device_id *id) { >> int i, ret = -ENXIO; >> @@ -1521,6 +1536,16 @@ out_error: >> return ret; >> } >> >> +static void eata2x_pci_remove(struct pci_dev *pdev) { >> + int id = (int)(long)dev_get_drvdata(&pdev->dev); >> + resource_size_t port_base; >> + >> + port_base = pci_resource_start(pdev, 0) + PCI_BASE_ADDRESS_0; >> + port_remove(id, port_base, pdev); >> + pci_disable_device(pdev); >> +} >> + >> static struct pci_device_id eata2x_tbl[] = { >> { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_SCSI << 8, PCI_ANY_ID) }, >> { }, >> @@ -1531,6 +1556,7 @@ static struct pci_driver eata2x_pci_driver = { >> .name = "eata", >> .id_table = eata2x_tbl, >> .probe = eata2x_pci_probe, >> + .remove = eata2x_pci_remove, >> }; >> >> static int eata2x_probe_pci_devices(struct scsi_host_template *tpnt) >> > Welll ... if you don't have hardware (and I strongly hope you refer to 'hardware able to do hotplugging', not 'hardware for the eata driver' > ...) why add the code at all? > Chances are no-one will ever need eata PCI hotplug; SCSI parallel typically isn't very good at hotplugging, so throwing in PCI hotplug will only confuse matters more. > Plus due to the sheer mechanics involved here I find it very unlikely anyone will be using it in real life. > > Cheers, > > Hannes > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 14, 2015 at 10:21:14AM +0200, Hannes Reinecke wrote: > Welll ... if you don't have hardware (and I strongly hope you refer to > 'hardware able to do hotplugging', not 'hardware for the eata driver' > ...) why add the code at all? > Chances are no-one will ever need eata PCI hotplug; SCSI parallel > typically isn't very good at hotplugging, so throwing in PCI hotplug > will only confuse matters more. > Plus due to the sheer mechanics involved here I find it very unlikely > anyone will be using it in real life. Because it's used for module removal and we want every driver to use the standard interface that. Jiang, you also need to convert the driver to scsi_add_host/scsi_remove_host from the legacy scsi_register interface, otherwise the SCSI layer will be very unhappy. Take a look at commit 0d31f8759109cbc1e6fc196d08e6b0e8a9e93b3f for example, the change should be straight forward. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/9/16 21:42, Christoph Hellwig wrote: > On Mon, Sep 14, 2015 at 10:21:14AM +0200, Hannes Reinecke wrote: >> Welll ... if you don't have hardware (and I strongly hope you refer to >> 'hardware able to do hotplugging', not 'hardware for the eata driver' >> ...) why add the code at all? >> Chances are no-one will ever need eata PCI hotplug; SCSI parallel >> typically isn't very good at hotplugging, so throwing in PCI hotplug >> will only confuse matters more. >> Plus due to the sheer mechanics involved here I find it very unlikely >> anyone will be using it in real life. > > Because it's used for module removal and we want every driver to use > the standard interface that. > > Jiang, you also need to convert the driver to > scsi_add_host/scsi_remove_host from the legacy scsi_register interface, > otherwise the SCSI layer will be very unhappy. > > Take a look at commit 0d31f8759109cbc1e6fc196d08e6b0e8a9e93b3f for > example, the change should be straight forward. Hi Christoph, I have taken a look at the commit 0d31f8759109cb and it seems that the conversion is not so big. But I have no hardware for testing, so could only ask for help from community. Thanks! Gerry -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig wrote on 16/09/15 23:12: > Jiang, you also need to convert the driver to > scsi_add_host/scsi_remove_host from the legacy scsi_register interface, > otherwise the SCSI layer will be very unhappy. > > Take a look at commit 0d31f8759109cbc1e6fc196d08e6b0e8a9e93b3f for > example, the change should be straight forward. > I am pleased to note that when I tried a Linus git head kernel from the last 24 hours, the IRQ routing for my DPT2044W SCSI card using eata module worked again, although the shut-down/kexec issue remains. Arthur. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c index b92e6856f909..f3bd7cbf260e 100644 --- a/drivers/scsi/eata.c +++ b/drivers/scsi/eata.c @@ -1474,6 +1474,21 @@ static unsigned int port_probe(unsigned long port_base, #ifdef CONFIG_PCI static int eata2x_pci_device_count; +/* TODO: need help here to shutdown the scsi host and release resources */ +static void port_remove(unsigned int id, resource_size_t port_base, + struct pci_dev *pdev) +{ + struct Scsi_Host *shost = sh[id]; + + /* TODO: stop scsi device */ + scsi_unregister(shost); + /* TODO: clean up resources allocated by port_detect() */ + clear_bit(id, eata_board_bitmap); + free_irq(shost->irq, &sha[id]); + release_region(port_base, REGION_SIZE); + ida_simple_remove(&eata_ida, id); +} + static int eata2x_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) { int i, ret = -ENXIO; @@ -1521,6 +1536,16 @@ out_error: return ret; } +static void eata2x_pci_remove(struct pci_dev *pdev) +{ + int id = (int)(long)dev_get_drvdata(&pdev->dev); + resource_size_t port_base; + + port_base = pci_resource_start(pdev, 0) + PCI_BASE_ADDRESS_0; + port_remove(id, port_base, pdev); + pci_disable_device(pdev); +} + static struct pci_device_id eata2x_tbl[] = { { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_SCSI << 8, PCI_ANY_ID) }, { }, @@ -1531,6 +1556,7 @@ static struct pci_driver eata2x_pci_driver = { .name = "eata", .id_table = eata2x_tbl, .probe = eata2x_pci_probe, + .remove = eata2x_pci_remove, }; static int eata2x_probe_pci_devices(struct scsi_host_template *tpnt)
Due to having no hardware for testing, this is just a sample code to support PCI device hot-removal. It just passing compilation, no any tests. Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com> --- drivers/scsi/eata.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)