linux-kernel - RE: [PATCH v3 2/5] mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <004001ce8154$36753510$a35f9f30$%jun@samsung.com>
Date:	Mon, 15 Jul 2013 21:09:53 +0900
From:	Seungwon Jeon <tgih.jun@...sung.com>
To:	'Doug Anderson' <dianders@...omium.org>,
	'Chris Ball' <cjb@...top.org>
Cc:	'Olof Johansson' <olof@...om.net>,
	'Jaehoon Chung' <jh80.chung@...sung.com>,
	'James Hogan' <james.hogan@...tec.com>,
	'Grant Grundler' <grundler@...omium.org>,
	'Alim Akhtar' <alim.akhtar@...sung.com>,
	'Abhilash Kesavan' <a.kesavan@...sung.com>,
	'Tomasz Figa' <tomasz.figa@...il.com>,
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: RE: [PATCH v3 2/5] mmc: dw_mmc: Add suspend_noirq/resume_noirq
 callbacks for dw_mmc-pltfm

On Thu, July 11, 2013, Doug Anderson wrote:
> On some devices (like exynos5420) the dw_mmc controller may be in a
> strange state after we wake up from sleep.  Add callbacks to allow for
> dealing with these quirks.  We use the "_noirq" versions of the
> callbacks since in the case of exynos5420 the strange state caused
> interrupts to fire so we need to deal with it while interrupts are
> still off.
> 
> At the moment this support is only added to dw_mmc-pltfm which calls
> straight to the callback, since nobody but exynos needs it.  We can
> add some levels of indirection (a call into the generic dw_mmc code)
> when someone finds a need.
I think It would be better to add _noirq only to dw_mmc-exynos.
That is we can add dev_pm_ops for dw_mmc-exynos's own.
As you recognize, there is no common routine which is not introduced for dw_mmc xxx_noirq now.
I feel like it is for handling quirk.
If we meet use case for that in some day, it could be added commonly.
How do you think?

Thanks,
Seungwon Jeon
> 
> Signed-off-by: Doug Anderson <dianders@...omium.org>
> Reviewed-by: James Hogan <james.hogan@...tec.com>
> ---
> Changes in v3:
> - Add freeze/thaw and poweroff/restore noirq entries.
> 
> Changes in v2:
> - Use suspend_noirq as per James Hogan.
> 
>  drivers/mmc/host/dw_mmc-pltfm.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  drivers/mmc/host/dw_mmc.h       |  4 ++++
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index 41c27b7..742ef76 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -105,12 +105,47 @@ static int dw_mci_pltfm_resume(struct device *dev)
> 
>  	return 0;
>  }
> +
> +static int dw_mci_pltfm_suspend_noirq(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +	const struct dw_mci_drv_data *drv_data = host->drv_data;
> +
> +	if (drv_data && drv_data->suspend_noirq)
> +		return drv_data->suspend_noirq(host);
> +
> +	return 0;
> +}
> +
> +static int dw_mci_pltfm_resume_noirq(struct device *dev)
> +{
> +	struct dw_mci *host = dev_get_drvdata(dev);
> +	const struct dw_mci_drv_data *drv_data = host->drv_data;
> +
> +	if (drv_data && drv_data->resume_noirq)
> +		return drv_data->resume_noirq(host);
> +
> +	return 0;
> +}
> +
> +
>  #else
> -#define dw_mci_pltfm_suspend	NULL
> -#define dw_mci_pltfm_resume	NULL
> +#define dw_mci_pltfm_suspend		NULL
> +#define dw_mci_pltfm_resume		NULL
> +#define dw_mci_pltfm_suspend_noirq	NULL
> +#define dw_mci_pltfm_resume_noirq	NULL
>  #endif /* CONFIG_PM_SLEEP */
> 
> -SIMPLE_DEV_PM_OPS(dw_mci_pltfm_pmops, dw_mci_pltfm_suspend, dw_mci_pltfm_resume);
> +const struct dev_pm_ops dw_mci_pltfm_pmops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(dw_mci_pltfm_suspend, dw_mci_pltfm_resume)
> +	.suspend_noirq = dw_mci_pltfm_suspend_noirq,
> +	.resume_noirq = dw_mci_pltfm_resume_noirq,
> +	.freeze_noirq = dw_mci_pltfm_suspend_noirq,
> +	.thaw_noirq = dw_mci_pltfm_resume_noirq,
> +	.poweroff_noirq = dw_mci_pltfm_suspend_noirq,
> +	.restore_noirq = dw_mci_pltfm_resume_noirq,
> +};
> +
>  EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
> 
>  static const struct of_device_id dw_mci_pltfm_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 0b74189..5d0398f 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -190,6 +190,8 @@ extern int dw_mci_resume(struct dw_mci *host);
>   * @prepare_command: handle CMD register extensions.
>   * @set_ios: handle bus specific extensions.
>   * @parse_dt: parse implementation specific device tree properties.
> + * @suspend_noirq: called late in the suspend process
> + * @resume_noirq: called early in the resume process
>   *
>   * Provide controller implementation specific extensions. The usage of this
>   * data structure is fully optional and usage of each member in this structure
> @@ -202,5 +204,7 @@ struct dw_mci_drv_data {
>  	void		(*prepare_command)(struct dw_mci *host, u32 *cmdr);
>  	void		(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
>  	int		(*parse_dt)(struct dw_mci *host);
> +	int		(*suspend_noirq)(struct dw_mci *host);
> +	int		(*resume_noirq)(struct dw_mci *host);
>  };
>  #endif /* _DW_MMC_H_ */
> --
> 1.8.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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