lkml.org 
[lkml]   [2015]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] drivers/memory: Add deep sleep support for IFC
From
Date
On Tue, 2015-10-27 at 16:34 +0530, Raghav Dogra wrote:
> Add support of suspend, resume function to support deep sleep.
> Also make sure of SRAM initialization during resume.
>
> Signed-off-by: Raghav Dogra <raghav@freescale.com>
> ---
> drivers/memory/fsl_ifc.c | 56
> ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fsl_ifc.h | 6 ++++++
> 2 files changed, 62 insertions(+)

Who are you asking to apply this? If me, please send to
linuxppc-dev@lists.ozlabs.orgso it gets on patchwork.

Also keep Prabhakar CCed on IFC patches.

> diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> index e87459f..163ccf2 100644
> --- a/drivers/memory/fsl_ifc.c
> +++ b/drivers/memory/fsl_ifc.c
> @@ -23,6 +23,7 @@
> #include <linux/kernel.h>
> #include <linux/compiler.h>
> #include <linux/spinlock.h>
> +#include <linux/delay.h>
> #include <linux/types.h>
> #include <linux/slab.h>
> #include <linux/io.h>
> @@ -35,6 +36,9 @@
> struct fsl_ifc_ctrl *fsl_ifc_ctrl_dev;
> EXPORT_SYMBOL(fsl_ifc_ctrl_dev);
>
> +#define FSL_IFC_V1_3_0 0x01030000
> +#define IFC_TIMEOUT_MSECS 100000 /* 100ms */
> +
> /*
> * convert_ifc_address - convert the base address
> * @addr_base: base address of the memory bank
> @@ -308,6 +312,53 @@ err:
> return ret;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +/* save ifc registers */
> +static int fsl_ifc_suspend(struct device *dev)
> +{
> + struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(dev);
> + struct fsl_ifc_regs __iomem *ifc = ctrl->regs;
> +
> + ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs), GFP_KERNEL);
> + if (!ctrl->saved_regs)
> + return -ENOMEM;
> +
> + _memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct fsl_ifc_regs));
> +
> + return 0;
> +}

Why _memcpy_fromio() rather than memcpy_fromio()?

> +/* restore ifc registers */
> +static int fsl_ifc_resume(struct device *dev)
> +{
> + struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(dev);
> + struct fsl_ifc_regs __iomem *ifc = ctrl->regs;
> + uint32_t ver = 0, ncfgr, status;
> +
> + if (ctrl->saved_regs) {
> + _memcpy_toio(ifc, ctrl->saved_regs,
> + sizeof(struct fsl_ifc_regs));

This is too simplistic, and doesn't account for things like read-to-clear
bits, or the possibility of reserved fields that expose internal state that
shouldn't be messed with. Registers that need state saving or
reinitialization should be handled individually.

You should also make sure that interrupts are disabled at the device.

> + kfree(ctrl->saved_regs);
> + ctrl->saved_regs = NULL;
> + }
> +
> + ver = in_be32(&ctrl->regs->ifc_rev);
> + ncfgr = in_be32(&ifc->ifc_nand.ncfgr);
> + if (ver >= FSL_IFC_V1_3_0) {
> + out_be32(&ifc->ifc_nand.ncfgr, ncfgr | IFC_NAND_SRAM_INIT_EN);
> +
> + /* wait for SRAM_INIT bit to be clear or timeout */
> + status = spin_event_timeout(!(in_be32(&ifc->ifc_nand.ncfgr)
> + & IFC_NAND_SRAM_INIT_EN),
> + IFC_TIMEOUT_MSECS, 0);
> + if (!status)
> + dev_err(ctrl->dev, "Timeout waiting for IFC SRAM INIT");
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */

Normally we do NAND SRAM init in the NAND driver. Why are you doing it here?
How is SRAM going to be reset on older IFCs?

BTW, I see that Linux is missing SRAM init for IFCs newer than v1.1 -- we're
relying on U-Boot to do so, which we shouldn't be.

-Scott



\
 
 \ /
  Last update: 2015-10-29 17:41    [W:0.063 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site