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@xxxxxxxxxxx>
> ---
> 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
>
