From: Bjorn Helgaas <helgaas@kernel.org>
To: Prabhakar Kushwaha <pkushwaha@marvell.com>
Cc: "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"kexec@lists.infradead.org" <kexec@lists.infradead.org>,
Ganapatrao Prabhakerrao Kulkarni <gkulkarni@marvell.com>,
Kamlakant Patel <kamlakantp@marvell.com>,
"prabhakar.pkin@gmail.com" <prabhakar.pkin@gmail.com>
Subject: Re: [PATCH] ata: ahci: Add shutdown to freeze hardware resources of ahci
Date: Thu, 23 Jan 2020 07:54:13 -0600 [thread overview]
Message-ID: <20200123135413.GA90883@google.com> (raw)
In-Reply-To: <1579776109-31578-1-git-send-email-pkushwaha@marvell.com>
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
>
next prev parent reply other threads:[~2020-01-23 13:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-23 10:41 [PATCH] ata: ahci: Add shutdown to freeze hardware resources of ahci Prabhakar Kushwaha
2020-01-23 13:54 ` Bjorn Helgaas [this message]
[not found] <CAJ2QiJKLJCa7QKs3MFn1_EgsjD7-Qp7ab1DLab87hhqTr-Cnzg@mail.gmail.com>
2020-01-24 20:35 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200123135413.GA90883@google.com \
--to=helgaas@kernel.org \
--cc=axboe@kernel.dk \
--cc=gkulkarni@marvell.com \
--cc=kamlakantp@marvell.com \
--cc=kexec@lists.infradead.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=pkushwaha@marvell.com \
--cc=prabhakar.pkin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).