| Message ID | 1579776109-31578-1-git-send-email-pkushwaha@marvell.com |
|---|---|
| State | New |
| Headers | show |
| Series | ata: ahci: Add shutdown to freeze hardware resources of ahci | expand |
On Thu, Jan 23, 2020 at 10:41:57AM +0000, Prabhakar Kushwaha wrote: > device_shutdown() called from reboot/power_shutdown expect all > devices to be shutdown. Same is true for ahci pci driver. > As no shutdown function was implemented ata subsystem remains > always alive and DMA/interrupt still active. > > It creates problem during kexec, here "M" bit is cleared to stop > DMA usage. Maybe this is obvious to AHCI folks, but I don't know what "M" bit you are referring to, and I don't see anything in the patch itself that looks like an "M" bit. I thought maybe you meant the "Bus Master Enable" bit (PCI_COMMAND_MASTER), but I don't see an obvious connection to that either. > Any further DMA transaction may cause instability and > the hard-disk may even not get detected for second kernel. > One of possible case is periodic file system sync. I think "may cause instability" and "disk may even not get detected" is too vague and hand-wavy to really add useful information to the commit log. > So defining ahci pci driver shutdown to freeze hardware (mask > interrupt, stop DMA engine and free DMA resources). > > Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com> > --- > drivers/ata/ahci.c | 8 ++++++++ > drivers/ata/libata-core.c | 21 +++++++++++++++++++++ > include/linux/libata.h | 1 + > 3 files changed, 30 insertions(+) > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index 4bfd1b14b390..31fc934740b6 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -81,6 +81,7 @@ enum board_ids { > > static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent); > static void ahci_remove_one(struct pci_dev *dev); > +static void ahci_shutdown_one(struct pci_dev *dev); > static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class, > unsigned long deadline); > static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class, > @@ -606,6 +607,7 @@ static struct pci_driver ahci_pci_driver = { > .id_table = ahci_pci_tbl, > .probe = ahci_init_one, > .remove = ahci_remove_one, > + .shutdown = ahci_shutdown_one, > .driver = { > .pm = &ahci_pci_pm_ops, > }, > @@ -626,6 +628,7 @@ MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets"); > static void ahci_pci_save_initial_config(struct pci_dev *pdev, > struct ahci_host_priv *hpriv) > { > + Spurious whitespace change, please remove. > if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) { > dev_info(&pdev->dev, "JMB361 has only one port\n"); > hpriv->force_port_map = 1; > @@ -1877,6 +1880,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > return 0; > } > > +static void ahci_shutdown_one(struct pci_dev *pdev) > +{ > + ata_pci_shutdown_one(pdev); > +} > + > static void ahci_remove_one(struct pci_dev *pdev) > { > pm_runtime_get_noresume(&pdev->dev); > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 6f4ab5c5b52d..42c8728f6117 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -6767,6 +6767,26 @@ void ata_pci_remove_one(struct pci_dev *pdev) > ata_host_detach(host); > } > > +void ata_pci_shutdown_one(struct pci_dev *pdev) > +{ > + struct ata_host *host = pci_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < host->n_ports; i++) { > + struct ata_port *ap = host->ports[i]; > + > + ap->pflags |= ATA_PFLAG_FROZEN; > + > + /* Disable port interrupts */ > + if (ap->ops->freeze) > + ap->ops->freeze(ap); > + > + /* Stop the port DMA engines */ > + if (ap->ops->port_stop) > + ap->ops->port_stop(ap); > + } > +} > + > /* move to PCI subsystem */ > int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits) > { > @@ -7387,6 +7407,7 @@ EXPORT_SYMBOL_GPL(ata_timing_cycle2mode); > > #ifdef CONFIG_PCI > EXPORT_SYMBOL_GPL(pci_test_config_bits); > +EXPORT_SYMBOL_GPL(ata_pci_shutdown_one); > EXPORT_SYMBOL_GPL(ata_pci_remove_one); > #ifdef CONFIG_PM > EXPORT_SYMBOL_GPL(ata_pci_device_do_suspend); > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 2dbde119721d..bff539918d82 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -1221,6 +1221,7 @@ struct pci_bits { > }; > > extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits); > +extern void ata_pci_shutdown_one(struct pci_dev *pdev); > extern void ata_pci_remove_one(struct pci_dev *pdev); > > #ifdef CONFIG_PM > -- > 2.17.1 >
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 4bfd1b14b390..31fc934740b6 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -81,6 +81,7 @@ enum board_ids { static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent); static void ahci_remove_one(struct pci_dev *dev); +static void ahci_shutdown_one(struct pci_dev *dev); static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class, unsigned long deadline); static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class, @@ -606,6 +607,7 @@ static struct pci_driver ahci_pci_driver = { .id_table = ahci_pci_tbl, .probe = ahci_init_one, .remove = ahci_remove_one, + .shutdown = ahci_shutdown_one, .driver = { .pm = &ahci_pci_pm_ops, }, @@ -626,6 +628,7 @@ MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets"); static void ahci_pci_save_initial_config(struct pci_dev *pdev, struct ahci_host_priv *hpriv) { + if (pdev->vendor == PCI_VENDOR_ID_JMICRON && pdev->device == 0x2361) { dev_info(&pdev->dev, "JMB361 has only one port\n"); hpriv->force_port_map = 1; @@ -1877,6 +1880,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) return 0; } +static void ahci_shutdown_one(struct pci_dev *pdev) +{ + ata_pci_shutdown_one(pdev); +} + static void ahci_remove_one(struct pci_dev *pdev) { pm_runtime_get_noresume(&pdev->dev); diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 6f4ab5c5b52d..42c8728f6117 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6767,6 +6767,26 @@ void ata_pci_remove_one(struct pci_dev *pdev) ata_host_detach(host); } +void ata_pci_shutdown_one(struct pci_dev *pdev) +{ + struct ata_host *host = pci_get_drvdata(pdev); + int i; + + for (i = 0; i < host->n_ports; i++) { + struct ata_port *ap = host->ports[i]; + + ap->pflags |= ATA_PFLAG_FROZEN; + + /* Disable port interrupts */ + if (ap->ops->freeze) + ap->ops->freeze(ap); + + /* Stop the port DMA engines */ + if (ap->ops->port_stop) + ap->ops->port_stop(ap); + } +} + /* move to PCI subsystem */ int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits) { @@ -7387,6 +7407,7 @@ EXPORT_SYMBOL_GPL(ata_timing_cycle2mode); #ifdef CONFIG_PCI EXPORT_SYMBOL_GPL(pci_test_config_bits); +EXPORT_SYMBOL_GPL(ata_pci_shutdown_one); EXPORT_SYMBOL_GPL(ata_pci_remove_one); #ifdef CONFIG_PM EXPORT_SYMBOL_GPL(ata_pci_device_do_suspend); diff --git a/include/linux/libata.h b/include/linux/libata.h index 2dbde119721d..bff539918d82 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1221,6 +1221,7 @@ struct pci_bits { }; extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits); +extern void ata_pci_shutdown_one(struct pci_dev *pdev); extern void ata_pci_remove_one(struct pci_dev *pdev); #ifdef CONFIG_PM
device_shutdown() called from reboot/power_shutdown expect all devices to be shutdown. Same is true for ahci pci driver. As no shutdown function was implemented ata subsystem remains always alive and DMA/interrupt still active. It creates problem during kexec, here "M" bit is cleared to stop DMA usage. Any further DMA transaction may cause instability and the hard-disk may even not get detected for second kernel. One of possible case is periodic file system sync. So defining ahci pci driver shutdown to freeze hardware (mask interrupt, stop DMA engine and free DMA resources). Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com> --- drivers/ata/ahci.c | 8 ++++++++ drivers/ata/libata-core.c | 21 +++++++++++++++++++++ include/linux/libata.h | 1 + 3 files changed, 30 insertions(+)