Re: mmc: sdhci-pci: why no .shutdown() implemented

From: Adrian Hunter
Date: Mon Mar 12 2012 - 09:25:29 EST


On 12/03/12 10:15, Mansoor, Illyas wrote:
> Hi All,
>
> I'm not sure if this is the right mailing list to discuss this question, but
> anyways I'll ask:
>
> On our platform we implemented a BUG in pci_set_power_state callback when the
> shutdown
> Is in progress, and we caught sdci-pci doing pci_set_power_state(D0) when
> shutdown was
> Already in progress.
>
> My question, why doesn't sdhci-pci.c implement a .shutdown() callback and close
> the device
> After doing sys_sync()?
>
> Is there some reason behind it not doing a graceful shutdown.

In general, the kernel does not know how to do a graceful shutdown. There
may be any number of housekeeping activities that userspace wishes to do
before shutting down.

If file systems are not getting sync'd that is a userspace problem, not a
kernel problem.

>
> I think it could cause corruption, since there is no guarantee when the system
> will power-off/reboot/halt
>
> I just implemented a patch to fix this, if it's a good fix I'll submit.
>
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index f5fe05c..e0818c9 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -23,6 +23,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/pm_runtime.h>
> #include <linux/async.h>
> +#include <linux/syscalls.h>
>
> #include <asm/scatterlist.h>
> #include <asm/io.h>
> @@ -1342,6 +1343,34 @@ err:
> return ret;
> }
>
> +static void sdhci_pci_shutdown(struct pci_dev *pdev)
> +{
> + int i;
> + struct sdhci_pci_chip *chip;
> +
> + printk(KERN_INFO "%s: Syncing filesystems ... ", __func__);
> + sys_sync();
> + printk("done.\n");
> +
> + pm_runtime_get_sync(&pdev->dev);
> +
> + chip = pci_get_drvdata(pdev);
> +
> + if (chip) {
> + for (i = 0;i < chip->num_slots; i++)
> + sdhci_pci_remove_slot(chip->slots[i]);
> +
> + pci_set_drvdata(pdev, NULL);
> + kfree(chip);
> + }
> +
> + pci_disable_device(pdev);
> +
> + pm_runtime_put_noidle(&pdev->dev);
> + pm_runtime_forbid(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> +}
> +
> static void __devexit sdhci_pci_remove(struct pci_dev *pdev)
> {
> int i;
> @@ -1473,6 +1502,7 @@ static struct pci_driver sdhci_driver = {
> .id_table = pci_ids,
> .probe = sdhci_pci_probe,
> .remove = __devexit_p(sdhci_pci_remove),
> + .shutdown = sdhci_pci_shutdown,
>
>
> -Illyas

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/