* [PATCH][v3] drivers/memory: Add deep sleep support for IFC
@ 2016-02-15 6:14 Raghav Dogra
2016-02-16 8:34 ` Scott Wood
0 siblings, 1 reply; 7+ messages in thread
From: Raghav Dogra @ 2016-02-15 6:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: oss, prabhakar.kushwaha, Raghav Dogra
Add support of suspend, resume function to support deep sleep.
Also make sure of SRAM initialization during resume.
Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
Signed-off-by: Raghav Dogra <raghav.dogra@nxp.com>
---
Changes for v3: Replace spin_event_timeout() with arch independent macro
Based on git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
branch "master"
drivers/memory/fsl_ifc.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fsl_ifc.h | 6 ++
2 files changed, 171 insertions(+)
diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
index acd1460..fa028bd 100644
--- a/drivers/memory/fsl_ifc.c
+++ b/drivers/memory/fsl_ifc.c
@@ -24,6 +24,7 @@
#include <linux/compiler.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
+#include <linux/delay.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/io.h>
@@ -35,6 +36,8 @@
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
@@ -309,6 +312,163 @@ 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;
+ __be32 nand_evter_intr_en, cm_evter_intr_en, nor_evter_intr_en,
+ gpcm_evter_intr_en;
+
+ ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs), GFP_KERNEL);
+ if (!ctrl->saved_regs)
+ return -ENOMEM;
+
+ cm_evter_intr_en = ifc_in32(&ifc->cm_evter_intr_en);
+ nand_evter_intr_en = ifc_in32(&ifc->ifc_nand.nand_evter_intr_en);
+ nor_evter_intr_en = ifc_in32(&ifc->ifc_nor.nor_evter_intr_en);
+ gpcm_evter_intr_en = ifc_in32(&ifc->ifc_gpcm.gpcm_evter_intr_en);
+
+/* IFC interrupts disabled */
+
+ ifc_out32(0x0, &ifc->cm_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
+
+ memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct fsl_ifc_regs));
+
+/* save the interrupt values */
+ ctrl->saved_regs->cm_evter_intr_en = cm_evter_intr_en;
+ ctrl->saved_regs->ifc_nand.nand_evter_intr_en = nand_evter_intr_en;
+ ctrl->saved_regs->ifc_nor.nor_evter_intr_en = nor_evter_intr_en;
+ ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en = gpcm_evter_intr_en;
+
+ return 0;
+}
+
+/* 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;
+ struct fsl_ifc_regs *savd_regs = ctrl->saved_regs;
+ uint32_t ver = 0, ncfgr, timeout, ifc_bank, i;
+
+/*
+ * IFC interrupts disabled
+ */
+ ifc_out32(0x0, &ifc->cm_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
+ ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
+
+
+ if (ctrl->saved_regs) {
+ for (ifc_bank = 0; ifc_bank < FSL_IFC_BANK_COUNT; ifc_bank++) {
+ ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr_ext,
+ &ifc->cspr_cs[ifc_bank].cspr_ext);
+ ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr,
+ &ifc->cspr_cs[ifc_bank].cspr);
+ ifc_out32(savd_regs->amask_cs[ifc_bank].amask,
+ &ifc->amask_cs[ifc_bank].amask);
+ ifc_out32(savd_regs->csor_cs[ifc_bank].csor_ext,
+ &ifc->csor_cs[ifc_bank].csor_ext);
+ ifc_out32(savd_regs->csor_cs[ifc_bank].csor,
+ &ifc->csor_cs[ifc_bank].csor);
+ for (i = 0; i < 4; i++) {
+ ifc_out32(savd_regs->ftim_cs[ifc_bank].ftim[i],
+ &ifc->ftim_cs[ifc_bank].ftim[i]);
+ }
+ }
+ ifc_out32(savd_regs->ifc_gcr, &ifc->ifc_gcr);
+ ifc_out32(savd_regs->cm_evter_en, &ifc->cm_evter_en);
+
+/*
+* IFC controller NAND machine registers
+*/
+ ifc_out32(savd_regs->ifc_nand.ncfgr, &ifc->ifc_nand.ncfgr);
+ ifc_out32(savd_regs->ifc_nand.nand_fcr0,
+ &ifc->ifc_nand.nand_fcr0);
+ ifc_out32(savd_regs->ifc_nand.nand_fcr1,
+ &ifc->ifc_nand.nand_fcr1);
+ ifc_out32(savd_regs->ifc_nand.row0, &ifc->ifc_nand.row0);
+ ifc_out32(savd_regs->ifc_nand.row1, &ifc->ifc_nand.row1);
+ ifc_out32(savd_regs->ifc_nand.col0, &ifc->ifc_nand.col0);
+ ifc_out32(savd_regs->ifc_nand.col1, &ifc->ifc_nand.col1);
+ ifc_out32(savd_regs->ifc_nand.row2, &ifc->ifc_nand.row2);
+ ifc_out32(savd_regs->ifc_nand.col2, &ifc->ifc_nand.col2);
+ ifc_out32(savd_regs->ifc_nand.row3, &ifc->ifc_nand.row3);
+ ifc_out32(savd_regs->ifc_nand.col3, &ifc->ifc_nand.col3);
+ ifc_out32(savd_regs->ifc_nand.nand_fbcr,
+ &ifc->ifc_nand.nand_fbcr);
+ ifc_out32(savd_regs->ifc_nand.nand_fir0,
+ &ifc->ifc_nand.nand_fir0);
+ ifc_out32(savd_regs->ifc_nand.nand_fir1,
+ &ifc->ifc_nand.nand_fir1);
+ ifc_out32(savd_regs->ifc_nand.nand_fir2,
+ &ifc->ifc_nand.nand_fir2);
+ ifc_out32(savd_regs->ifc_nand.nand_csel,
+ &ifc->ifc_nand.nand_csel);
+ ifc_out32(savd_regs->ifc_nand.nandseq_strt,
+ &ifc->ifc_nand.nandseq_strt);
+ ifc_out32(savd_regs->ifc_nand.nand_evter_en,
+ &ifc->ifc_nand.nand_evter_en);
+ ifc_out32(savd_regs->ifc_nand.nanndcr, &ifc->ifc_nand.nanndcr);
+
+/*
+* IFC controller NOR machine registers
+*/
+ ifc_out32(savd_regs->ifc_nor.nor_evter_en,
+ &ifc->ifc_nor.nor_evter_en);
+ ifc_out32(savd_regs->ifc_nor.norcr, &ifc->ifc_nor.norcr);
+
+/*
+ * IFC controller GPCM Machine registers
+ */
+ ifc_out32(savd_regs->ifc_gpcm.gpcm_evter_en,
+ &ifc->ifc_gpcm.gpcm_evter_en);
+
+
+
+/*
+ * IFC interrupts enabled
+ */
+ ifc_out32(ctrl->saved_regs->cm_evter_intr_en, &ifc->cm_evter_intr_en);
+ ifc_out32(ctrl->saved_regs->ifc_nand.nand_evter_intr_en,
+ &ifc->ifc_nand.nand_evter_intr_en);
+ ifc_out32(ctrl->saved_regs->ifc_nor.nor_evter_intr_en,
+ &ifc->ifc_nor.nor_evter_intr_en);
+ ifc_out32(ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en,
+ &ifc->ifc_gpcm.gpcm_evter_intr_en);
+
+ kfree(ctrl->saved_regs);
+ ctrl->saved_regs = NULL;
+ }
+
+ ver = ifc_in32(&ctrl->regs->ifc_rev);
+ ncfgr = ifc_in32(&ifc->ifc_nand.ncfgr);
+ if (ver >= FSL_IFC_V1_3_0) {
+
+ ifc_out32(ncfgr | IFC_NAND_SRAM_INIT_EN,
+ &ifc->ifc_nand.ncfgr);
+ /* wait for SRAM_INIT bit to be clear or timeout */
+ timeout = IFC_TIMEOUT_MSECS;
+ while ((ifc_in32(&ifc->ifc_nand.ncfgr) &
+ IFC_NAND_SRAM_INIT_EN) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout)
+ dev_err(ctrl->dev, "Timeout waiting for IFC SRAM INIT");
+ }
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
static const struct of_device_id fsl_ifc_match[] = {
{
.compatible = "fsl,ifc",
@@ -316,10 +476,15 @@ static const struct of_device_id fsl_ifc_match[] = {
{},
};
+static const struct dev_pm_ops ifc_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(fsl_ifc_suspend, fsl_ifc_resume)
+};
+
static struct platform_driver fsl_ifc_ctrl_driver = {
.driver = {
.name = "fsl-ifc",
.of_match_table = fsl_ifc_match,
+ .pm = &ifc_pm_ops,
},
.probe = fsl_ifc_ctrl_probe,
.remove = fsl_ifc_ctrl_remove,
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 0023088..850481a 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -270,6 +270,8 @@
*/
/* Auto Boot Mode */
#define IFC_NAND_NCFGR_BOOT 0x80000000
+/* SRAM INIT EN */
+#define IFC_NAND_SRAM_INIT_EN 0x20000000
/* Addressing Mode-ROW0+n/COL0 */
#define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x00000000
/* Addressing Mode-ROW0+n/COL0+n */
@@ -842,6 +844,10 @@ struct fsl_ifc_ctrl {
u32 nand_stat;
wait_queue_head_t nand_wait;
bool little_endian;
+#ifdef CONFIG_PM_SLEEP
+ /* save regs when system go to deep-sleep */
+ struct fsl_ifc_regs *saved_regs;
+#endif
};
extern struct fsl_ifc_ctrl *fsl_ifc_ctrl_dev;
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-15 6:14 [PATCH][v3] drivers/memory: Add deep sleep support for IFC Raghav Dogra
@ 2016-02-16 8:34 ` Scott Wood
2016-02-17 14:40 ` Raghav Dogra
0 siblings, 1 reply; 7+ messages in thread
From: Scott Wood @ 2016-02-16 8:34 UTC (permalink / raw)
To: Raghav Dogra, linuxppc-dev; +Cc: prabhakar.kushwaha
On Mon, 2016-02-15 at 11:44 +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: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Signed-off-by: Raghav Dogra <raghav.dogra@nxp.com>
> ---
> Changes for v3: Replace spin_event_timeout() with arch independent macro
>
> Based on git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> branch "master"
>
> drivers/memory/fsl_ifc.c | 165
> +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fsl_ifc.h | 6 ++
> 2 files changed, 171 insertions(+)
>
> diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> index acd1460..fa028bd 100644
> --- a/drivers/memory/fsl_ifc.c
> +++ b/drivers/memory/fsl_ifc.c
> @@ -24,6 +24,7 @@
> #include <linux/compiler.h>
> #include <linux/sched.h>
> #include <linux/spinlock.h>
> +#include <linux/delay.h>
> #include <linux/types.h>
> #include <linux/slab.h>
> #include <linux/io.h>
> @@ -35,6 +36,8 @@
>
> 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 */
What does the "MSECS" mean in IFC_TIMEOUT_MSECS? It's a unit without a
quantity.
>
> /*
> * convert_ifc_address - convert the base address
> @@ -309,6 +312,163 @@ 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;
> + __be32 nand_evter_intr_en, cm_evter_intr_en, nor_evter_intr_en,
> + gpcm_evter_intr_en
> ;
s/__be32/u32/ as they've already been converted to host endianness.
Also please repeat the type on a new line rather than use continuation lines
to declare more variables (and don't indent continuation lines so far).
> +
> + ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs),
> GFP_KERNEL);
> + if (!ctrl->saved_regs)
> + return -ENOMEM;
Allocate memory at probe time, not here.
> + cm_evter_intr_en = ifc_in32(&ifc->cm_evter_intr_en);
> + nand_evter_intr_en = ifc_in32(&ifc->ifc_nand.nand_evter_intr_en);
> + nor_evter_intr_en = ifc_in32(&ifc->ifc_nor.nor_evter_intr_en);
> + gpcm_evter_intr_en = ifc_in32(&ifc->ifc_gpcm.gpcm_evter_intr_en);
> +
> +/* IFC interrupts disabled */
> +
> + ifc_out32(0x0, &ifc->cm_evter_intr_en);
Indent the comments the same as the code.
> + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
> + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
> + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
> +
> + memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct fsl_ifc_regs));
> +
> +/* save the interrupt values */
> + ctrl->saved_regs->cm_evter_intr_en = cm_evter_intr_en;
> + ctrl->saved_regs->ifc_nand.nand_evter_intr_en = nand_evter_intr_en;
> + ctrl->saved_regs->ifc_nor.nor_evter_intr_en = nor_evter_intr_en;
> + ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en = gpcm_evter_intr_en;
Why didn't you use the memcpy_fromio() to save these, and clear intr_en later?
That said, I still don't like this approach. I'd rather see the nand driver
save the registers it cares about, and this driver wouldn't have to do much
other than quiesce the rest of the interrupts.
> +
> + return 0;
> +}
> +
> +/* 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;
> + struct fsl_ifc_regs *savd_regs = ctrl->saved_regs;
> + uint32_t ver = 0, ncfgr, timeout, ifc_bank, i;
s/savd/saved/
> +
> +/*
> + * IFC interrupts disabled
> + */
> + ifc_out32(0x0, &ifc->cm_evter_intr_en);
> + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
> + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
> + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
> +
> +
> + if (ctrl->saved_regs) {
> + for (ifc_bank = 0; ifc_bank < FSL_IFC_BANK_COUNT;
> ifc_bank++) {
> + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr_ext,
> + &ifc->cspr_cs[ifc_bank].cspr_ext);
> + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr,
> + &ifc->cspr_cs[ifc_bank].cspr);
> + ifc_out32(savd_regs->amask_cs[ifc_bank].amask,
> + &ifc->amask_cs[ifc_bank].amask);
> + ifc_out32(savd_regs->csor_cs[ifc_bank].csor_ext,
> + &ifc->csor_cs[ifc_bank].csor_ext);
> + ifc_out32(savd_regs->csor_cs[ifc_bank].csor,
> + &ifc->csor_cs[ifc_bank].csor);
Align continuation lines the way patchwork suggests ("&ifc" aligned with
"savd").
Does resume from deep sleep go via U-Boot (which would initialize these
registers) on these chips?
> + for (i = 0; i < 4; i++) {
> + ifc_out32(savd_regs
> ->ftim_cs[ifc_bank].ftim[i],
> + &ifc->ftim_cs[ifc_bank].ftim[i]);
> + }
> + }
> + ifc_out32(savd_regs->ifc_gcr, &ifc->ifc_gcr);
> + ifc_out32(savd_regs->cm_evter_en, &ifc->cm_evter_en);
> +
> +/*
> +* IFC controller NAND machine registers
> +*/
> + ifc_out32(savd_regs->ifc_nand.ncfgr, &ifc->ifc_nand.ncfgr);
> + ifc_out32(savd_regs->ifc_nand.nand_fcr0,
> + &ifc->ifc_nand.nand_fcr0);
> + ifc_out32(savd_regs->ifc_nand.nand_fcr1,
> + &ifc->ifc_nand.nand_fcr1);
> + ifc_out32(savd_regs->ifc_nand.row0, &ifc->ifc_nand.row0);
> + ifc_out32(savd_regs->ifc_nand.row1, &ifc->ifc_nand.row1);
> + ifc_out32(savd_regs->ifc_nand.col0, &ifc->ifc_nand.col0);
> + ifc_out32(savd_regs->ifc_nand.col1, &ifc->ifc_nand.col1);
> + ifc_out32(savd_regs->ifc_nand.row2, &ifc->ifc_nand.row2);
> + ifc_out32(savd_regs->ifc_nand.col2, &ifc->ifc_nand.col2);
> + ifc_out32(savd_regs->ifc_nand.row3, &ifc->ifc_nand.row3);
> + ifc_out32(savd_regs->ifc_nand.col3, &ifc->ifc_nand.col3);
> + ifc_out32(savd_regs->ifc_nand.nand_fbcr,
> + &ifc->ifc_nand.nand_fbcr);
> + ifc_out32(savd_regs->ifc_nand.nand_fir0,
> + &ifc->ifc_nand.nand_fir0);
> + ifc_out32(savd_regs->ifc_nand.nand_fir1,
> + &ifc->ifc_nand.nand_fir1);
> + ifc_out32(savd_regs->ifc_nand.nand_fir2,
> + &ifc->ifc_nand.nand_fir2);
> + ifc_out32(savd_regs->ifc_nand.nand_csel,
> + &ifc->ifc_nand.nand_csel);
> + ifc_out32(savd_regs->ifc_nand.nandseq_strt,
> + &ifc
> ->ifc_nand.nandseq_strt);
> + ifc_out32(savd_regs->ifc_nand.nand_evter_en,
> + &ifc
> ->ifc_nand.nand_evter_en);
> + ifc_out32(savd_regs->ifc_nand.nanndcr, &ifc
> ->ifc_nand.nanndcr);
Many of these are either initialized by the NAND driver for each transaction,
or are not used at all.
> +
> +/*
> +* IFC controller NOR machine registers
> +*/
> + ifc_out32(savd_regs->ifc_nor.nor_evter_en,
> + &ifc->ifc_nor.nor_evter_en);
> + ifc_out32(savd_regs->ifc_nor.norcr, &ifc->ifc_nor.norcr);
What uses these?
> +
> +/*
> + * IFC controller GPCM Machine registers
> + */
> + ifc_out32(savd_regs->ifc_gpcm.gpcm_evter_en,
> + &ifc->ifc_gpcm.gpcm_evter_en);
> +
> +
> +
> +/*
> + * IFC interrupts enabled
> + */
> + ifc_out32(ctrl->saved_regs->cm_evter_intr_en, &ifc
> ->cm_evter_intr_en);
> + ifc_out32(ctrl->saved_regs->ifc_nand.nand_evter_intr_en,
> + &ifc->ifc_nand.nand_evter_intr_en);
> + ifc_out32(ctrl->saved_regs->ifc_nor.nor_evter_intr_en,
> + &ifc->ifc_nor.nor_evter_intr_en);
> + ifc_out32(ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en,
> + &ifc->ifc_gpcm.gpcm_evter_intr_en);
> +
> + kfree(ctrl->saved_regs);
> + ctrl->saved_regs = NULL;
> + }
Bad indentation
> +
> + ver = ifc_in32(&ctrl->regs->ifc_rev);
> + ncfgr = ifc_in32(&ifc->ifc_nand.ncfgr);
> + if (ver >= FSL_IFC_V1_3_0) {
> +
> + ifc_out32(ncfgr | IFC_NAND_SRAM_INIT_EN,
> + &ifc->ifc_nand.ncfgr);
> + /* wait for SRAM_INIT bit to be clear or timeout */
> + timeout = IFC_TIMEOUT_MSECS;
> + while ((ifc_in32(&ifc->ifc_nand.ncfgr) &
> + IFC_NAND_SRAM_INIT_EN) && timeout)
> {
> + cpu_relax();
> + timeout--;
> + }
How can this timeout be in milliseconds or any other real unit of time, if
it's actually measuring loop iterations with no udelay() or similar?
Is it really necessary to spin here rather than waiting for an interrupt like
normal?
> +
> + if (!timeout)
> + dev_err(ctrl->dev, "Timeout waiting for IFC SRAM
> INIT");
> + }
U-boot does this when the version is > 1.1.0 -- why >= 1.3.0 here? Are there
any versions in between 1.1.0 and 1.3.0?
Also, how did Linux and U-Boot end up having opposite argument ordering for
ifc_out32()? :-(
-Scott
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-16 8:34 ` Scott Wood
@ 2016-02-17 14:40 ` Raghav Dogra
2016-02-17 23:19 ` Leo Li
2016-02-18 1:19 ` Scott Wood
0 siblings, 2 replies; 7+ messages in thread
From: Raghav Dogra @ 2016-02-17 14:40 UTC (permalink / raw)
To: Scott Wood, linuxppc-dev@lists.ozlabs.org; +Cc: Prabhakar Kushwaha
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU2NvdHQgV29vZCBbbWFp
bHRvOm9zc0BidXNlcnJvci5uZXRdDQo+IFNlbnQ6IFR1ZXNkYXksIEZlYnJ1YXJ5IDE2LCAyMDE2
IDI6MDUgUE0NCj4gVG86IFJhZ2hhdiBEb2dyYSA8cmFnaGF2LmRvZ3JhQG54cC5jb20+OyBsaW51
eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiBDYzogUHJhYmhha2FyIEt1c2h3YWhhIDxwcmFi
aGFrYXIua3VzaHdhaGFAbnhwLmNvbT4NCj4gU3ViamVjdDogUmU6IFtQQVRDSF1bdjNdIGRyaXZl
cnMvbWVtb3J5OiBBZGQgZGVlcCBzbGVlcCBzdXBwb3J0IGZvciBJRkMNCj4gDQo+IE9uIE1vbiwg
MjAxNi0wMi0xNSBhdCAxMTo0NCArMDUzMCwgUmFnaGF2IERvZ3JhIHdyb3RlOg0KPiA+IEFkZCBz
dXBwb3J0IG9mIHN1c3BlbmQsIHJlc3VtZSBmdW5jdGlvbiB0byBzdXBwb3J0IGRlZXAgc2xlZXAu
DQo+ID4gQWxzbyBtYWtlIHN1cmUgb2YgU1JBTSBpbml0aWFsaXphdGlvbiAgZHVyaW5nIHJlc3Vt
ZS4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFByYWJoYWthciBLdXNod2FoYSA8cHJhYmhha2Fy
Lmt1c2h3YWhhQG54cC5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTogUmFnaGF2IERvZ3JhIDxyYWdo
YXYuZG9ncmFAbnhwLmNvbT4NCj4gPiAtLS0NCj4gPiBDaGFuZ2VzIGZvciB2MzogUmVwbGFjZSBz
cGluX2V2ZW50X3RpbWVvdXQoKSB3aXRoIGFyY2ggaW5kZXBlbmRlbnQNCj4gPiBtYWNybw0KPiA+
DQo+ID4gQmFzZWQgb24NCj4gPiBnaXQ6Ly9naXQua2VybmVsLm9yZy9wdWIvc2NtL2xpbnV4L2tl
cm5lbC9naXQvdG9ydmFsZHMvbGludXguZ2l0DQo+ID4gYnJhbmNoICJtYXN0ZXIiDQo+ID4NCj4g
PiAgZHJpdmVycy9tZW1vcnkvZnNsX2lmYy5jIHwgMTY1DQo+ID4gKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysNCj4gPiAgaW5jbHVkZS9saW51eC9mc2xfaWZj
LmggIHwgICA2ICsrDQo+ID4gIDIgZmlsZXMgY2hhbmdlZCwgMTcxIGluc2VydGlvbnMoKykNCj4g
Pg0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL21lbW9yeS9mc2xfaWZjLmMgYi9kcml2ZXJzL21l
bW9yeS9mc2xfaWZjLmMgaW5kZXgNCj4gPiBhY2QxNDYwLi5mYTAyOGJkIDEwMDY0NA0KPiA+IC0t
LSBhL2RyaXZlcnMvbWVtb3J5L2ZzbF9pZmMuYw0KPiA+ICsrKyBiL2RyaXZlcnMvbWVtb3J5L2Zz
bF9pZmMuYw0KPiA+IEBAIC0yNCw2ICsyNCw3IEBADQo+ID4gICNpbmNsdWRlIDxsaW51eC9jb21w
aWxlci5oPg0KPiA+ICAjaW5jbHVkZSA8bGludXgvc2NoZWQuaD4NCj4gPiAgI2luY2x1ZGUgPGxp
bnV4L3NwaW5sb2NrLmg+DQo+ID4gKyNpbmNsdWRlIDxsaW51eC9kZWxheS5oPg0KPiA+ICAjaW5j
bHVkZSA8bGludXgvdHlwZXMuaD4NCj4gPiAgI2luY2x1ZGUgPGxpbnV4L3NsYWIuaD4NCj4gPiAg
I2luY2x1ZGUgPGxpbnV4L2lvLmg+DQo+ID4gQEAgLTM1LDYgKzM2LDggQEANCj4gPg0KPiA+ICBz
dHJ1Y3QgZnNsX2lmY19jdHJsICpmc2xfaWZjX2N0cmxfZGV2Ow0KPiA+IEVYUE9SVF9TWU1CT0wo
ZnNsX2lmY19jdHJsX2Rldik7DQo+ID4gKyNkZWZpbmUgRlNMX0lGQ19WMV8zXzAJMHgwMTAzMDAw
MA0KPiA+ICsjZGVmaW5lIElGQ19USU1FT1VUX01TRUNTCTEwMDAwMCAvKiAxMDBtcyAqLw0KPiAN
Cj4gV2hhdCBkb2VzIHRoZSAiTVNFQ1MiIG1lYW4gaW4gSUZDX1RJTUVPVVRfTVNFQ1M/ICBJdCdz
IGEgdW5pdCB3aXRob3V0IGENCj4gcXVhbnRpdHkuDQoNClllcywgSSBhZ3JlZS4gSSB3aWxsIHJl
bmFtZSBpdCB0byBJRkNfV0FJVF9JVFIuDQoNCj4gDQo+ID4NCj4gPiAgLyoNCj4gPiAgICogY29u
dmVydF9pZmNfYWRkcmVzcyAtIGNvbnZlcnQgdGhlIGJhc2UgYWRkcmVzcyBAQCAtMzA5LDYgKzMx
MiwxNjMNCj4gPiBAQCBlcnI6DQo+ID4gIAlyZXR1cm4gcmV0Ow0KPiA+ICB9DQo+ID4NCj4gPiAr
I2lmZGVmIENPTkZJR19QTV9TTEVFUA0KPiA+ICsvKiBzYXZlIGlmYyByZWdpc3RlcnMgKi8NCj4g
PiArc3RhdGljIGludCBmc2xfaWZjX3N1c3BlbmQoc3RydWN0IGRldmljZSAqZGV2KSB7DQo+ID4g
KwlzdHJ1Y3QgZnNsX2lmY19jdHJsICpjdHJsID0gZGV2X2dldF9kcnZkYXRhKGRldik7DQo+ID4g
KwlzdHJ1Y3QgZnNsX2lmY19yZWdzIF9faW9tZW0gKmlmYyA9IGN0cmwtPnJlZ3M7DQo+ID4gKwlf
X2JlMzIgbmFuZF9ldnRlcl9pbnRyX2VuLCBjbV9ldnRlcl9pbnRyX2VuLCBub3JfZXZ0ZXJfaW50
cl9lbiwNCj4gPiArCQkJCQkJCSBncGNtX2V2dGVyX2ludHJfZW4NCj4gPiA7DQo+IA0KPiBzL19f
YmUzMi91MzIvIGFzIHRoZXkndmUgYWxyZWFkeSBiZWVuIGNvbnZlcnRlZCB0byBob3N0IGVuZGlh
bm5lc3MuDQo+IA0KPiBBbHNvIHBsZWFzZSByZXBlYXQgdGhlIHR5cGUgb24gYSBuZXcgbGluZSBy
YXRoZXIgdGhhbiB1c2UgY29udGludWF0aW9uIGxpbmVzDQo+IHRvIGRlY2xhcmUgbW9yZSB2YXJp
YWJsZXMgKGFuZCBkb24ndCBpbmRlbnQgY29udGludWF0aW9uIGxpbmVzIHNvIGZhcikuDQo+IA0K
DQpPa2F5LCB3aWxsIHRha2UgY2FyZSBvZiB0aGlzIGluIHRoZSBuZXh0IHZlcnNpb24uDQoNCj4g
PiArDQo+ID4gKwljdHJsLT5zYXZlZF9yZWdzID0ga3phbGxvYyhzaXplb2Yoc3RydWN0IGZzbF9p
ZmNfcmVncyksDQo+ID4gR0ZQX0tFUk5FTCk7DQo+ID4gKwlpZiAoIWN0cmwtPnNhdmVkX3JlZ3Mp
DQo+ID4gKwkJcmV0dXJuIC1FTk9NRU07DQo+IA0KPiBBbGxvY2F0ZSBtZW1vcnkgYXQgcHJvYmUg
dGltZSwgbm90IGhlcmUuDQo+IA0KDQpCdXQsIHdoeSBhbGxvY2F0ZSBtZW1vcnkgYXQgdGhlIHBy
b2JlIHdoZW4gaXQgaXMgbm90IGtub3duIGF0IHRoYXQgdGltZSB3aGV0aGVyDQpkZWVwIHNsZWVw
IHN0YXRlIHdvdWxkIGJlIHJlcXVpcmVkIG9yIG5vdD8gSXMgdGhhdCBiZWNhdXNlIHdlIHdhbnQg
dG8gc2F2ZSB0aW1lDQp3aGlsZSBnb2luZyB0byBkZWVwIHNsZWVwPw0KDQo+ID4gKwljbV9ldnRl
cl9pbnRyX2VuID0gaWZjX2luMzIoJmlmYy0+Y21fZXZ0ZXJfaW50cl9lbik7DQo+ID4gKwluYW5k
X2V2dGVyX2ludHJfZW4gPSBpZmNfaW4zMigmaWZjLT5pZmNfbmFuZC5uYW5kX2V2dGVyX2ludHJf
ZW4pOw0KPiA+ICsJbm9yX2V2dGVyX2ludHJfZW4gPSBpZmNfaW4zMigmaWZjLT5pZmNfbm9yLm5v
cl9ldnRlcl9pbnRyX2VuKTsNCj4gPiArCWdwY21fZXZ0ZXJfaW50cl9lbiA9IGlmY19pbjMyKCZp
ZmMtDQo+ID5pZmNfZ3BjbS5ncGNtX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsNCj4gPiArLyogSUZD
IGludGVycnVwdHMgZGlzYWJsZWQgKi8NCj4gPiArDQo+ID4gKwlpZmNfb3V0MzIoMHgwLCAmaWZj
LT5jbV9ldnRlcl9pbnRyX2VuKTsNCj4gDQo+IEluZGVudCB0aGUgY29tbWVudHMgdGhlIHNhbWUg
YXMgdGhlIGNvZGUuDQo+IA0KDQpPa2F5Lg0KDQo+ID4gKwlpZmNfb3V0MzIoMHgwLCAmaWZjLT5p
ZmNfbmFuZC5uYW5kX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsJaWZjX291dDMyKDB4MCwgJmlmYy0+
aWZjX25vci5ub3JfZXZ0ZXJfaW50cl9lbik7DQo+ID4gKwlpZmNfb3V0MzIoMHgwLCAmaWZjLT5p
ZmNfZ3BjbS5ncGNtX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsNCj4gPiArCW1lbWNweV9mcm9taW8o
Y3RybC0+c2F2ZWRfcmVncywgaWZjLCBzaXplb2Yoc3RydWN0IGZzbF9pZmNfcmVncykpOw0KPiA+
ICsNCj4gPiArLyogc2F2ZSB0aGUgaW50ZXJydXB0IHZhbHVlcyAqLw0KPiA+ICsJY3RybC0+c2F2
ZWRfcmVncy0+Y21fZXZ0ZXJfaW50cl9lbiA9IGNtX2V2dGVyX2ludHJfZW47DQo+ID4gKwljdHJs
LT5zYXZlZF9yZWdzLT5pZmNfbmFuZC5uYW5kX2V2dGVyX2ludHJfZW4gPQ0KPiBuYW5kX2V2dGVy
X2ludHJfZW47DQo+ID4gKwljdHJsLT5zYXZlZF9yZWdzLT5pZmNfbm9yLm5vcl9ldnRlcl9pbnRy
X2VuID0gbm9yX2V2dGVyX2ludHJfZW47DQo+ID4gKwljdHJsLT5zYXZlZF9yZWdzLT5pZmNfZ3Bj
bS5ncGNtX2V2dGVyX2ludHJfZW4gPQ0KPiBncGNtX2V2dGVyX2ludHJfZW47DQo+IA0KPiBXaHkg
ZGlkbid0IHlvdSB1c2UgdGhlIG1lbWNweV9mcm9taW8oKSB0byBzYXZlIHRoZXNlLCBhbmQgY2xl
YXIgaW50cl9lbg0KPiBsYXRlcj8NCj4gDQoNCkkgdXNlZCBpdCB3aGVuZXZlciBJIGRpZCBhIHdy
aXRlL3JlYWQgb24gaW9tZW0uIEluIHRoaXMgY2FzZSwgYm90aCBtZW1vcmllcyANCmFyZSBub24g
aW9tZW0uDQoNCj4gVGhhdCBzYWlkLCBJIHN0aWxsIGRvbid0IGxpa2UgdGhpcyBhcHByb2FjaC4g
IEknZCByYXRoZXIgc2VlIHRoZSBuYW5kIGRyaXZlciBzYXZlDQo+IHRoZSByZWdpc3RlcnMgaXQg
Y2FyZXMgYWJvdXQsIGFuZCB0aGlzIGRyaXZlciB3b3VsZG4ndCBoYXZlIHRvIGRvIG11Y2ggb3Ro
ZXINCj4gdGhhbiBxdWllc2NlIHRoZSByZXN0IG9mIHRoZSBpbnRlcnJ1cHRzLg0KPiANCg0KT2th
eSwgd2Ugd2lsbCBhbmFseXplIHRoZSByZXF1aXJlZCBjaGFuZ2VzIGFuZCBpbmNsdWRlIHRoZW0u
DQoNCj4gPiArDQo+ID4gKwlyZXR1cm4gMDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArLyogcmVzdG9y
ZSBpZmMgcmVnaXN0ZXJzICovDQo+ID4gK3N0YXRpYyBpbnQgZnNsX2lmY19yZXN1bWUoc3RydWN0
IGRldmljZSAqZGV2KSB7DQo+ID4gKwlzdHJ1Y3QgZnNsX2lmY19jdHJsICpjdHJsID0gZGV2X2dl
dF9kcnZkYXRhKGRldik7DQo+ID4gKwlzdHJ1Y3QgZnNsX2lmY19yZWdzIF9faW9tZW0gKmlmYyA9
IGN0cmwtPnJlZ3M7DQo+ID4gKwlzdHJ1Y3QgZnNsX2lmY19yZWdzICpzYXZkX3JlZ3MgPSBjdHJs
LT5zYXZlZF9yZWdzOw0KPiA+ICsJdWludDMyX3QgdmVyID0gMCwgbmNmZ3IsIHRpbWVvdXQsIGlm
Y19iYW5rLCBpOw0KPiANCj4gcy9zYXZkL3NhdmVkLw0KPiANCg0KT2theS4NCg0KPiA+ICsNCj4g
PiArLyoNCj4gPiArICogSUZDIGludGVycnVwdHMgZGlzYWJsZWQNCj4gPiArICovDQo+ID4gKwlp
ZmNfb3V0MzIoMHgwLCAmaWZjLT5jbV9ldnRlcl9pbnRyX2VuKTsNCj4gPiArCWlmY19vdXQzMigw
eDAsICZpZmMtPmlmY19uYW5kLm5hbmRfZXZ0ZXJfaW50cl9lbik7DQo+ID4gKwlpZmNfb3V0MzIo
MHgwLCAmaWZjLT5pZmNfbm9yLm5vcl9ldnRlcl9pbnRyX2VuKTsNCj4gPiArCWlmY19vdXQzMigw
eDAsICZpZmMtPmlmY19ncGNtLmdwY21fZXZ0ZXJfaW50cl9lbik7DQo+ID4gKw0KPiA+ICsNCj4g
PiArCWlmIChjdHJsLT5zYXZlZF9yZWdzKSB7DQo+ID4gKwkJZm9yIChpZmNfYmFuayA9IDA7IGlm
Y19iYW5rIDwgRlNMX0lGQ19CQU5LX0NPVU5UOw0KPiA+IGlmY19iYW5rKyspIHsNCj4gPiArCQkJ
aWZjX291dDMyKHNhdmRfcmVncy0+Y3Nwcl9jc1tpZmNfYmFua10uY3Nwcl9leHQsDQo+ID4gKwkJ
CQkJJmlmYy0+Y3Nwcl9jc1tpZmNfYmFua10uY3Nwcl9leHQpOw0KPiA+ICsJCQlpZmNfb3V0MzIo
c2F2ZF9yZWdzLT5jc3ByX2NzW2lmY19iYW5rXS5jc3ByLA0KPiA+ICsJCQkJCSZpZmMtPmNzcHJf
Y3NbaWZjX2JhbmtdLmNzcHIpOw0KPiA+ICsJCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5hbWFza19j
c1tpZmNfYmFua10uYW1hc2ssDQo+ID4gKwkJCQkJJmlmYy0+YW1hc2tfY3NbaWZjX2JhbmtdLmFt
YXNrKTsNCj4gPiArCQkJaWZjX291dDMyKHNhdmRfcmVncy0+Y3Nvcl9jc1tpZmNfYmFua10uY3Nv
cl9leHQsDQo+ID4gKwkJCQkJJmlmYy0+Y3Nvcl9jc1tpZmNfYmFua10uY3Nvcl9leHQpOw0KPiA+
ICsJCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5jc29yX2NzW2lmY19iYW5rXS5jc29yLA0KPiA+ICsJ
CQkJCSZpZmMtPmNzb3JfY3NbaWZjX2JhbmtdLmNzb3IpOw0KPiANCj4gQWxpZ24gY29udGludWF0
aW9uIGxpbmVzIHRoZSB3YXkgcGF0Y2h3b3JrIHN1Z2dlc3RzICgiJmlmYyIgYWxpZ25lZCB3aXRo
DQo+ICJzYXZkIikuDQoNCk9rYXksIEkgd2lsbCB0YWtlIGNhcmUgb2YgdGhpcyBpbiB0aGUgbmV4
dCBwYXRjaC4NCg0KPiANCj4gRG9lcyByZXN1bWUgZnJvbSBkZWVwIHNsZWVwIGdvIHZpYSBVLUJv
b3QgKHdoaWNoIHdvdWxkIGluaXRpYWxpemUgdGhlc2UNCj4gcmVnaXN0ZXJzKSBvbiB0aGVzZSBj
aGlwcz8NCg0KWWVzLCBkZWVwIHNsZWVwIHJlc3VtZSBnb2VzIHZpYSB1LWJvb3QgYW5kIHRoZXNl
IHJlZ2lzdGVycyBzaG91bGQgYmUgaW5pdGlhbGl6ZWQgDQpCeSB1LWJvb3QuDQoNCj4gDQo+ID4g
KwkJCWZvciAoaSA9IDA7IGkgPCA0OyBpKyspIHsNCj4gPiArCQkJCWlmY19vdXQzMihzYXZkX3Jl
Z3MNCj4gPiAtPmZ0aW1fY3NbaWZjX2JhbmtdLmZ0aW1baV0sDQo+ID4gKwkJCQkJJmlmYy0+ZnRp
bV9jc1tpZmNfYmFua10uZnRpbVtpXSk7DQo+ID4gKwkJCX0NCj4gPiArCQl9DQo+ID4gKwkJaWZj
X291dDMyKHNhdmRfcmVncy0+aWZjX2djciwgJmlmYy0+aWZjX2djcik7DQo+ID4gKwkJaWZjX291
dDMyKHNhdmRfcmVncy0+Y21fZXZ0ZXJfZW4sICZpZmMtPmNtX2V2dGVyX2VuKTsNCj4gPiArDQo+
ID4gKy8qDQo+ID4gKyogSUZDIGNvbnRyb2xsZXIgTkFORCBtYWNoaW5lIHJlZ2lzdGVycyAqLw0K
PiA+ICsJCWlmY19vdXQzMihzYXZkX3JlZ3MtPmlmY19uYW5kLm5jZmdyLCAmaWZjLT5pZmNfbmFu
ZC5uY2Zncik7DQo+ID4gKwkJaWZjX291dDMyKHNhdmRfcmVncy0+aWZjX25hbmQubmFuZF9mY3Iw
LA0KPiA+ICsJCQkJCQkmaWZjLT5pZmNfbmFuZC5uYW5kX2ZjcjApOw0KPiA+ICsJCWlmY19vdXQz
MihzYXZkX3JlZ3MtPmlmY19uYW5kLm5hbmRfZmNyMSwNCj4gPiArCQkJCQkJJmlmYy0+aWZjX25h
bmQubmFuZF9mY3IxKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5yb3cw
LCAmaWZjLT5pZmNfbmFuZC5yb3cwKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNf
bmFuZC5yb3cxLCAmaWZjLT5pZmNfbmFuZC5yb3cxKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9y
ZWdzLT5pZmNfbmFuZC5jb2wwLCAmaWZjLT5pZmNfbmFuZC5jb2wwKTsNCj4gPiArCQlpZmNfb3V0
MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5jb2wxLCAmaWZjLT5pZmNfbmFuZC5jb2wxKTsNCj4gPiAr
CQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5yb3cyLCAmaWZjLT5pZmNfbmFuZC5yb3cy
KTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5jb2wyLCAmaWZjLT5pZmNf
bmFuZC5jb2wyKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5yb3czLCAm
aWZjLT5pZmNfbmFuZC5yb3czKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFu
ZC5jb2wzLCAmaWZjLT5pZmNfbmFuZC5jb2wzKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdz
LT5pZmNfbmFuZC5uYW5kX2ZiY3IsDQo+ID4gKwkJCQkJCSZpZmMtPmlmY19uYW5kLm5hbmRfZmJj
cik7DQo+ID4gKwkJaWZjX291dDMyKHNhdmRfcmVncy0+aWZjX25hbmQubmFuZF9maXIwLA0KPiA+
ICsJCQkJCQkmaWZjLT5pZmNfbmFuZC5uYW5kX2ZpcjApOw0KPiA+ICsJCWlmY19vdXQzMihzYXZk
X3JlZ3MtPmlmY19uYW5kLm5hbmRfZmlyMSwNCj4gPiArCQkJCQkJJmlmYy0+aWZjX25hbmQubmFu
ZF9maXIxKTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5uYW5kX2ZpcjIs
DQo+ID4gKwkJCQkJCSZpZmMtPmlmY19uYW5kLm5hbmRfZmlyMik7DQo+ID4gKwkJaWZjX291dDMy
KHNhdmRfcmVncy0+aWZjX25hbmQubmFuZF9jc2VsLA0KPiA+ICsJCQkJCQkmaWZjLT5pZmNfbmFu
ZC5uYW5kX2NzZWwpOw0KPiA+ICsJCWlmY19vdXQzMihzYXZkX3JlZ3MtPmlmY19uYW5kLm5hbmRz
ZXFfc3RydCwNCj4gPiArCQkJCQkJJmlmYw0KPiA+IC0+aWZjX25hbmQubmFuZHNlcV9zdHJ0KTsN
Cj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5uYW5kX2V2dGVyX2VuLA0KPiA+
ICsJCQkJCQkmaWZjDQo+ID4gLT5pZmNfbmFuZC5uYW5kX2V2dGVyX2VuKTsNCj4gPiArCQlpZmNf
b3V0MzIoc2F2ZF9yZWdzLT5pZmNfbmFuZC5uYW5uZGNyLCAmaWZjDQo+ID4gLT5pZmNfbmFuZC5u
YW5uZGNyKTsNCj4gDQo+IE1hbnkgb2YgdGhlc2UgYXJlIGVpdGhlciBpbml0aWFsaXplZCBieSB0
aGUgTkFORCBkcml2ZXIgZm9yIGVhY2ggdHJhbnNhY3Rpb24sDQo+IG9yIGFyZSBub3QgdXNlZCBh
dCBhbGwuDQo+IA0KDQpZZXMsIHdpbGwgYW5hbHl6ZSB0aGUgcmVnaXN0ZXJzIHVzZWQgYW5kIHRh
a2UgY2FyZSBvZiB0aGVtLg0KDQo+ID4gKw0KPiA+ICsvKg0KPiA+ICsqIElGQyBjb250cm9sbGVy
IE5PUiBtYWNoaW5lIHJlZ2lzdGVycyAqLw0KPiA+ICsJCWlmY19vdXQzMihzYXZkX3JlZ3MtPmlm
Y19ub3Iubm9yX2V2dGVyX2VuLA0KPiA+ICsJCQkJCSZpZmMtPmlmY19ub3Iubm9yX2V2dGVyX2Vu
KTsNCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfbm9yLm5vcmNyLCAmaWZjLT5pZmNf
bm9yLm5vcmNyKTsNCj4gDQo+IFdoYXQgdXNlcyB0aGVzZT8NCj4gDQoNClRoZXNlIHJlZ2lzdGVy
cyBhcmUgbm90IHVzZWQgYXMgc3VjaCwgYnV0IHdlIHdvdWxkIGxpa2UgdG8gcmV0YWluIHRoZWly
IHZhbHVlIGFzIHRoZXkNCmNhbiBiZSBvZiBoZWxwIGluIGNhc2Ugb2YgZXJyb3IgY29uZGl0aW9u
cy4NCg0KPiA+ICsNCj4gPiArLyoNCj4gPiArICogSUZDIGNvbnRyb2xsZXIgR1BDTSBNYWNoaW5l
IHJlZ2lzdGVycyAgKi8NCj4gPiArCQlpZmNfb3V0MzIoc2F2ZF9yZWdzLT5pZmNfZ3BjbS5ncGNt
X2V2dGVyX2VuLA0KPiA+ICsJCQkJCSZpZmMtPmlmY19ncGNtLmdwY21fZXZ0ZXJfZW4pOw0KPiA+
ICsNCj4gPiArDQo+ID4gKw0KPiA+ICsvKg0KPiA+ICsgKiBJRkMgaW50ZXJydXB0cyBlbmFibGVk
DQo+ID4gKyAqLw0KPiA+ICsJaWZjX291dDMyKGN0cmwtPnNhdmVkX3JlZ3MtPmNtX2V2dGVyX2lu
dHJfZW4sICZpZmMNCj4gPiAtPmNtX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsJaWZjX291dDMyKGN0
cmwtPnNhdmVkX3JlZ3MtPmlmY19uYW5kLm5hbmRfZXZ0ZXJfaW50cl9lbiwNCj4gPiArCQkJCQkm
aWZjLT5pZmNfbmFuZC5uYW5kX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsJaWZjX291dDMyKGN0cmwt
PnNhdmVkX3JlZ3MtPmlmY19ub3Iubm9yX2V2dGVyX2ludHJfZW4sDQo+ID4gKwkJCQkJJmlmYy0+
aWZjX25vci5ub3JfZXZ0ZXJfaW50cl9lbik7DQo+ID4gKwlpZmNfb3V0MzIoY3RybC0+c2F2ZWRf
cmVncy0+aWZjX2dwY20uZ3BjbV9ldnRlcl9pbnRyX2VuLA0KPiA+ICsJCQkJCSZpZmMtDQo+ID5p
ZmNfZ3BjbS5ncGNtX2V2dGVyX2ludHJfZW4pOw0KPiA+ICsNCj4gPiArCQlrZnJlZShjdHJsLT5z
YXZlZF9yZWdzKTsNCj4gPiArCQljdHJsLT5zYXZlZF9yZWdzID0gTlVMTDsNCj4gPiArCX0NCj4g
DQo+IEJhZCBpbmRlbnRhdGlvbg0KPiANCg0KV2lsbCB0YWtlIGNhcmUuDQoNCj4gPiArDQo+ID4g
Kwl2ZXIgPSBpZmNfaW4zMigmY3RybC0+cmVncy0+aWZjX3Jldik7DQo+ID4gKwluY2ZnciA9IGlm
Y19pbjMyKCZpZmMtPmlmY19uYW5kLm5jZmdyKTsNCj4gPiArCWlmICh2ZXIgPj0gRlNMX0lGQ19W
MV8zXzApIHsNCj4gPiArDQo+ID4gKwkJaWZjX291dDMyKG5jZmdyIHwgSUZDX05BTkRfU1JBTV9J
TklUX0VOLA0KPiA+ICsJCQkJCSZpZmMtPmlmY19uYW5kLm5jZmdyKTsNCj4gPiArCQkvKiB3YWl0
IGZvciAgU1JBTV9JTklUIGJpdCB0byBiZSBjbGVhciBvciB0aW1lb3V0ICovDQo+ID4gKwkJdGlt
ZW91dCA9IElGQ19USU1FT1VUX01TRUNTOw0KPiA+ICsJCXdoaWxlICgoaWZjX2luMzIoJmlmYy0+
aWZjX25hbmQubmNmZ3IpICYNCj4gPiArCQkJCQlJRkNfTkFORF9TUkFNX0lOSVRfRU4pICYmDQo+
IHRpbWVvdXQpDQo+ID4gew0KPiA+ICsJCQljcHVfcmVsYXgoKTsNCj4gPiArCQkJdGltZW91dC0t
Ow0KPiA+ICsJCX0NCj4gDQo+IEhvdyBjYW4gdGhpcyB0aW1lb3V0IGJlIGluIG1pbGxpc2Vjb25k
cyBvciBhbnkgb3RoZXIgcmVhbCB1bml0IG9mIHRpbWUsIGlmIGl0J3MNCj4gYWN0dWFsbHkgbWVh
c3VyaW5nIGxvb3AgaXRlcmF0aW9ucyB3aXRoIG5vIHVkZWxheSgpIG9yIHNpbWlsYXI/DQo+IA0K
DQpZZXMsIGl0J3Mgbm90IGluIG1pbGxpc2Vjb25kIGFueSBsb25nZXIuIFdpbGwgY2hhbmdlIHRo
ZSBuYW1lIHRvIElGQ19XQUlUX0lUUg0KDQo+IElzIGl0IHJlYWxseSBuZWNlc3NhcnkgdG8gc3Bp
biBoZXJlIHJhdGhlciB0aGFuIHdhaXRpbmcgZm9yIGFuIGludGVycnVwdCBsaWtlDQo+IG5vcm1h
bD8NCj4gDQoNCkFyZW4ndCB0aGUgZ2xvYmFsIGludGVycnVwdHMgZGlzYWJsZWQgYXQgdGhpcyBz
dGFnZT8gQ2FuIHdlIHVzZSB0aGUgaW50ZXJydXB0IGJhc2VkDQp3YWl0cyBpbiB0aGUgZGVlcCBz
bGVlcCBjb2RlPyBXZSB1c2VkIGl0IGJhc2VkIG9uIHRoZSBhc3N1bXB0aW9uIHRoYXQgaW50ZXJy
dXB0cw0KY2Fubm90IGJlIHVzZWQgaGVyZS4NCg0KPiA+ICsNCj4gPiArCQlpZiAoIXRpbWVvdXQp
DQo+ID4gKwkJCWRldl9lcnIoY3RybC0+ZGV2LCAiVGltZW91dCB3YWl0aW5nIGZvciBJRkMgU1JB
TQ0KPiA+IElOSVQiKTsNCj4gPiArCX0NCj4gDQo+IFUtYm9vdCBkb2VzIHRoaXMgd2hlbiB0aGUg
dmVyc2lvbiBpcyA+IDEuMS4wIC0tIHdoeSA+PSAxLjMuMCBoZXJlPyAgQXJlIHRoZXJlDQo+IGFu
eSB2ZXJzaW9ucyBpbiBiZXR3ZWVuIDEuMS4wIGFuZCAxLjMuMD8NCg0KQmVjYXVzZSBvbmx5IEI0
IGFuZCBUNCBhcmUgYmFzZWQgb24gMS4xLjAgd2hpY2ggZG8gbm90IHN1cHBvcnQgZGVlcCBzbGVl
cC4NClRoZSBmaXJzdCBib2FyZCB3aGljaCBzdXBwb3J0cyBkZWVwIHNsZWVwIGlzIFQxMDQwIHdo
aWNoIGhhcyB2ZXJzaW9uIDEuMy4wLg0KDQo+IA0KPiBBbHNvLCBob3cgZGlkIExpbnV4IGFuZCBV
LUJvb3QgZW5kIHVwIGhhdmluZyBvcHBvc2l0ZSBhcmd1bWVudCBvcmRlcmluZw0KPiBmb3IgaWZj
X291dDMyKCk/IDotKA0KPiANCj4gLVNjb3R0DQoNCkkgZ3Vlc3MgaXQgaGFwcGVuZWQgaW4gdGhl
IHBhdGNoOg0KaHR0cHM6Ly9wYXRjaHdvcmsub3psYWJzLm9yZy9wYXRjaC80NzQ3MTkvDQpMZXQn
cyBrZWVwIGl0IHRoYXQgd2F5IG5vdz8gQ2hhbmdpbmcgaXQgd291bGQgcmVxdWlyZSBzaWduaWZp
Y2FudCB3b3JrLg0KDQotUmFnaGF2DQoNCg==
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-17 14:40 ` Raghav Dogra
@ 2016-02-17 23:19 ` Leo Li
2016-02-18 1:11 ` Scott Wood
2016-02-18 1:19 ` Scott Wood
1 sibling, 1 reply; 7+ messages in thread
From: Leo Li @ 2016-02-17 23:19 UTC (permalink / raw)
To: Raghav Dogra
Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org, Prabhakar Kushwaha
On Wed, Feb 17, 2016 at 8:40 AM, Raghav Dogra <raghav.dogra@nxp.com> wrote:
>
>
>> -----Original Message-----
>> From: Scott Wood [mailto:oss@buserror.net]
>> Sent: Tuesday, February 16, 2016 2:05 PM
>> To: Raghav Dogra <raghav.dogra@nxp.com>; linuxppc-dev@lists.ozlabs.org
>> Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>> Subject: Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
>>
>> On Mon, 2016-02-15 at 11:44 +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: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>> > Signed-off-by: Raghav Dogra <raghav.dogra@nxp.com>
Similar comment as last time, that we should involve the MTD guys.
>> > ---
>> > Changes for v3: Replace spin_event_timeout() with arch independent
>> > macro
>> >
>> > Based on
>> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> > branch "master"
>> >
>> > drivers/memory/fsl_ifc.c | 165
>> > +++++++++++++++++++++++++++++++++++++++++++++++
>> > include/linux/fsl_ifc.h | 6 ++
>> > 2 files changed, 171 insertions(+)
>> >
>> > diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c index
>> > acd1460..fa028bd 100644
>> > --- a/drivers/memory/fsl_ifc.c
>> > +++ b/drivers/memory/fsl_ifc.c
>> > @@ -24,6 +24,7 @@
>> > #include <linux/compiler.h>
>> > #include <linux/sched.h>
>> > #include <linux/spinlock.h>
>> > +#include <linux/delay.h>
>> > #include <linux/types.h>
>> > #include <linux/slab.h>
>> > #include <linux/io.h>
>> > @@ -35,6 +36,8 @@
>> >
>> > 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 */
>>
>> What does the "MSECS" mean in IFC_TIMEOUT_MSECS? It's a unit without a
>> quantity.
>
> Yes, I agree. I will rename it to IFC_WAIT_ITR.
>
>>
>> >
>> > /*
>> > * convert_ifc_address - convert the base address @@ -309,6 +312,163
>> > @@ 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;
>> > + __be32 nand_evter_intr_en, cm_evter_intr_en, nor_evter_intr_en,
>> > + gpcm_evter_intr_en
>> > ;
>>
>> s/__be32/u32/ as they've already been converted to host endianness.
>>
>> Also please repeat the type on a new line rather than use continuation lines
>> to declare more variables (and don't indent continuation lines so far).
>>
>
> Okay, will take care of this in the next version.
>
>> > +
>> > + ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs),
>> > GFP_KERNEL);
>> > + if (!ctrl->saved_regs)
>> > + return -ENOMEM;
>>
>> Allocate memory at probe time, not here.
>>
>
> But, why allocate memory at the probe when it is not known at that time whether
> deep sleep state would be required or not? Is that because we want to save time
> while going to deep sleep?
>
>> > + cm_evter_intr_en = ifc_in32(&ifc->cm_evter_intr_en);
>> > + nand_evter_intr_en = ifc_in32(&ifc->ifc_nand.nand_evter_intr_en);
>> > + nor_evter_intr_en = ifc_in32(&ifc->ifc_nor.nor_evter_intr_en);
>> > + gpcm_evter_intr_en = ifc_in32(&ifc-
>> >ifc_gpcm.gpcm_evter_intr_en);
>> > +
>> > +/* IFC interrupts disabled */
>> > +
>> > + ifc_out32(0x0, &ifc->cm_evter_intr_en);
>>
>> Indent the comments the same as the code.
>>
>
> Okay.
>
>> > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
>> > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
>> > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
>> > +
>> > + memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct fsl_ifc_regs));
>> > +
>> > +/* save the interrupt values */
>> > + ctrl->saved_regs->cm_evter_intr_en = cm_evter_intr_en;
>> > + ctrl->saved_regs->ifc_nand.nand_evter_intr_en =
>> nand_evter_intr_en;
>> > + ctrl->saved_regs->ifc_nor.nor_evter_intr_en = nor_evter_intr_en;
>> > + ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en =
>> gpcm_evter_intr_en;
>>
>> Why didn't you use the memcpy_fromio() to save these, and clear intr_en
>> later?
>>
>
> I used it whenever I did a write/read on iomem. In this case, both memories
> are non iomem.
>
>> That said, I still don't like this approach. I'd rather see the nand driver save
>> the registers it cares about, and this driver wouldn't have to do much other
>> than quiesce the rest of the interrupts.
>>
>
> Okay, we will analyze the required changes and include them.
>
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +/* 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;
>> > + struct fsl_ifc_regs *savd_regs = ctrl->saved_regs;
>> > + uint32_t ver = 0, ncfgr, timeout, ifc_bank, i;
>>
>> s/savd/saved/
>>
>
> Okay.
>
>> > +
>> > +/*
>> > + * IFC interrupts disabled
>> > + */
>> > + ifc_out32(0x0, &ifc->cm_evter_intr_en);
>> > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
>> > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
>> > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
>> > +
>> > +
>> > + if (ctrl->saved_regs) {
>> > + for (ifc_bank = 0; ifc_bank < FSL_IFC_BANK_COUNT;
>> > ifc_bank++) {
>> > + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr_ext,
>> > + &ifc->cspr_cs[ifc_bank].cspr_ext);
>> > + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr,
>> > + &ifc->cspr_cs[ifc_bank].cspr);
>> > + ifc_out32(savd_regs->amask_cs[ifc_bank].amask,
>> > + &ifc->amask_cs[ifc_bank].amask);
>> > + ifc_out32(savd_regs->csor_cs[ifc_bank].csor_ext,
>> > + &ifc->csor_cs[ifc_bank].csor_ext);
>> > + ifc_out32(savd_regs->csor_cs[ifc_bank].csor,
>> > + &ifc->csor_cs[ifc_bank].csor);
>>
>> Align continuation lines the way patchwork suggests ("&ifc" aligned with
>> "savd").
>
> Okay, I will take care of this in the next patch.
>
>>
>> Does resume from deep sleep go via U-Boot (which would initialize these
>> registers) on these chips?
>
> Yes, deep sleep resume goes via u-boot and these registers should be initialized
> By u-boot.
>
>>
>> > + for (i = 0; i < 4; i++) {
>> > + ifc_out32(savd_regs
>> > ->ftim_cs[ifc_bank].ftim[i],
>> > + &ifc->ftim_cs[ifc_bank].ftim[i]);
>> > + }
>> > + }
>> > + ifc_out32(savd_regs->ifc_gcr, &ifc->ifc_gcr);
>> > + ifc_out32(savd_regs->cm_evter_en, &ifc->cm_evter_en);
>> > +
>> > +/*
>> > +* IFC controller NAND machine registers */
>> > + ifc_out32(savd_regs->ifc_nand.ncfgr, &ifc->ifc_nand.ncfgr);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fcr0,
>> > + &ifc->ifc_nand.nand_fcr0);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fcr1,
>> > + &ifc->ifc_nand.nand_fcr1);
>> > + ifc_out32(savd_regs->ifc_nand.row0, &ifc->ifc_nand.row0);
>> > + ifc_out32(savd_regs->ifc_nand.row1, &ifc->ifc_nand.row1);
>> > + ifc_out32(savd_regs->ifc_nand.col0, &ifc->ifc_nand.col0);
>> > + ifc_out32(savd_regs->ifc_nand.col1, &ifc->ifc_nand.col1);
>> > + ifc_out32(savd_regs->ifc_nand.row2, &ifc->ifc_nand.row2);
>> > + ifc_out32(savd_regs->ifc_nand.col2, &ifc->ifc_nand.col2);
>> > + ifc_out32(savd_regs->ifc_nand.row3, &ifc->ifc_nand.row3);
>> > + ifc_out32(savd_regs->ifc_nand.col3, &ifc->ifc_nand.col3);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fbcr,
>> > + &ifc->ifc_nand.nand_fbcr);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fir0,
>> > + &ifc->ifc_nand.nand_fir0);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fir1,
>> > + &ifc->ifc_nand.nand_fir1);
>> > + ifc_out32(savd_regs->ifc_nand.nand_fir2,
>> > + &ifc->ifc_nand.nand_fir2);
>> > + ifc_out32(savd_regs->ifc_nand.nand_csel,
>> > + &ifc->ifc_nand.nand_csel);
>> > + ifc_out32(savd_regs->ifc_nand.nandseq_strt,
>> > + &ifc
>> > ->ifc_nand.nandseq_strt);
>> > + ifc_out32(savd_regs->ifc_nand.nand_evter_en,
>> > + &ifc
>> > ->ifc_nand.nand_evter_en);
>> > + ifc_out32(savd_regs->ifc_nand.nanndcr, &ifc
>> > ->ifc_nand.nanndcr);
>>
>> Many of these are either initialized by the NAND driver for each transaction,
>> or are not used at all.
>>
>
> Yes, will analyze the registers used and take care of them.
>
>> > +
>> > +/*
>> > +* IFC controller NOR machine registers */
>> > + ifc_out32(savd_regs->ifc_nor.nor_evter_en,
>> > + &ifc->ifc_nor.nor_evter_en);
>> > + ifc_out32(savd_regs->ifc_nor.norcr, &ifc->ifc_nor.norcr);
>>
>> What uses these?
>>
>
> These registers are not used as such, but we would like to retain their value as they
> can be of help in case of error conditions.
>
>> > +
>> > +/*
>> > + * IFC controller GPCM Machine registers */
>> > + ifc_out32(savd_regs->ifc_gpcm.gpcm_evter_en,
>> > + &ifc->ifc_gpcm.gpcm_evter_en);
>> > +
>> > +
>> > +
>> > +/*
>> > + * IFC interrupts enabled
>> > + */
>> > + ifc_out32(ctrl->saved_regs->cm_evter_intr_en, &ifc
>> > ->cm_evter_intr_en);
>> > + ifc_out32(ctrl->saved_regs->ifc_nand.nand_evter_intr_en,
>> > + &ifc->ifc_nand.nand_evter_intr_en);
>> > + ifc_out32(ctrl->saved_regs->ifc_nor.nor_evter_intr_en,
>> > + &ifc->ifc_nor.nor_evter_intr_en);
>> > + ifc_out32(ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en,
>> > + &ifc-
>> >ifc_gpcm.gpcm_evter_intr_en);
>> > +
>> > + kfree(ctrl->saved_regs);
>> > + ctrl->saved_regs = NULL;
>> > + }
>>
>> Bad indentation
>>
>
> Will take care.
>
>> > +
>> > + ver = ifc_in32(&ctrl->regs->ifc_rev);
>> > + ncfgr = ifc_in32(&ifc->ifc_nand.ncfgr);
>> > + if (ver >= FSL_IFC_V1_3_0) {
>> > +
>> > + ifc_out32(ncfgr | IFC_NAND_SRAM_INIT_EN,
>> > + &ifc->ifc_nand.ncfgr);
>> > + /* wait for SRAM_INIT bit to be clear or timeout */
>> > + timeout = IFC_TIMEOUT_MSECS;
>> > + while ((ifc_in32(&ifc->ifc_nand.ncfgr) &
>> > + IFC_NAND_SRAM_INIT_EN) &&
>> timeout)
>> > {
>> > + cpu_relax();
>> > + timeout--;
>> > + }
>>
>> How can this timeout be in milliseconds or any other real unit of time, if it's
>> actually measuring loop iterations with no udelay() or similar?
>>
>
> Yes, it's not in millisecond any longer. Will change the name to IFC_WAIT_ITR
>
>> Is it really necessary to spin here rather than waiting for an interrupt like
>> normal?
>>
>
> Aren't the global interrupts disabled at this stage? Can we use the interrupt based
> waits in the deep sleep code? We used it based on the assumption that interrupts
> cannot be used here.
At the resume() stage, interrupts are already enabled. But the
problem of using interrupt based wait here is that we cannot give a
correct return value at this point. And it can also defeat the
ordering of resume() callbacks for dependent devices.
Regards,
Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-17 23:19 ` Leo Li
@ 2016-02-18 1:11 ` Scott Wood
0 siblings, 0 replies; 7+ messages in thread
From: Scott Wood @ 2016-02-18 1:11 UTC (permalink / raw)
To: Leo Li, Raghav Dogra; +Cc: linuxppc-dev@lists.ozlabs.org, Prabhakar Kushwaha
On Wed, 2016-02-17 at 17:19 -0600, Leo Li wrote:
> On Wed, Feb 17, 2016 at 8:40 AM, Raghav Dogra <raghav.dogra@nxp.com> wrote:
> >
> > > Is it really necessary to spin here rather than waiting for an interrupt
> > > like
> > > normal?
> > >
> >
> > Aren't the global interrupts disabled at this stage? Can we use the
> > interrupt based
> > waits in the deep sleep code? We used it based on the assumption that
> > interrupts
> > cannot be used here.
>
> At the resume() stage, interrupts are already enabled. But the
> problem of using interrupt based wait here is that we cannot give a
> correct return value at this point. And it can also defeat the
> ordering of resume() callbacks for dependent devices.
I didn't say to return from the resume() function before the operation is
done, just to have the resume() function wait for the interrupt. At the very
least it would make it easier to reuse existing code once this is moved to the
NAND driver, if we don't need a special way of waiting for this operation.
-Scott
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-17 14:40 ` Raghav Dogra
2016-02-17 23:19 ` Leo Li
@ 2016-02-18 1:19 ` Scott Wood
2016-04-14 23:07 ` Leo Li
1 sibling, 1 reply; 7+ messages in thread
From: Scott Wood @ 2016-02-18 1:19 UTC (permalink / raw)
To: Raghav Dogra, linuxppc-dev@lists.ozlabs.org; +Cc: Prabhakar Kushwaha
On Wed, 2016-02-17 at 14:40 +0000, Raghav Dogra wrote:
>
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Tuesday, February 16, 2016 2:05 PM
> > To: Raghav Dogra <raghav.dogra@nxp.com>; linuxppc-dev@lists.ozlabs.org
> > Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > Subject: Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
> >
> > On Mon, 2016-02-15 at 11:44 +0530, Raghav Dogra wrote:
> > >
> > > +
> > > + ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs),
> > > GFP_KERNEL);
> > > + if (!ctrl->saved_regs)
> > > + return -ENOMEM;
> >
> > Allocate memory at probe time, not here.
> >
>
> But, why allocate memory at the probe when it is not known at that time
> whether
> deep sleep state would be required or not? Is that because we want to save
> time
> while going to deep sleep?
We also want to avoid potential failures here. We can also keep the code
simpler by embedding this into the ctrl struct itself, and not dynamically
allocating it at all.
> > >
> > > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
> > > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
> > > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
> > > +
> > > + memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct
> > > fsl_ifc_regs));
> > > +
> > > +/* save the interrupt values */
> > > + ctrl->saved_regs->cm_evter_intr_en = cm_evter_intr_en;
> > > + ctrl->saved_regs->ifc_nand.nand_evter_intr_en =
> > nand_evter_intr_en;
> > > + ctrl->saved_regs->ifc_nor.nor_evter_intr_en =
> > > nor_evter_intr_en;
> > > + ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en =
> > gpcm_evter_intr_en;
> >
> > Why didn't you use the memcpy_fromio() to save these, and clear intr_en
> > later?
> >
>
> I used it whenever I did a write/read on iomem. In this case, both memories
> are non iomem.
Huh? &ifc->ifc_nand.nand_evter_intr_en and such are iomem.
> > > +
> > > +/*
> > > + * IFC interrupts disabled
> > > + */
> > > + ifc_out32(0x0, &ifc->cm_evter_intr_en);
> > > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
> > > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
> > > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
> > > +
> > > +
> > > + if (ctrl->saved_regs) {
> > > + for (ifc_bank = 0; ifc_bank < FSL_IFC_BANK_COUNT;
> > > ifc_bank++) {
> > > + ifc_out32(savd_regs
> > > ->cspr_cs[ifc_bank].cspr_ext,
> > > + &ifc
> > > ->cspr_cs[ifc_bank].cspr_ext);
> > > + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr,
> > > + &ifc->cspr_cs[ifc_bank].cspr);
> > > + ifc_out32(savd_regs->amask_cs[ifc_bank].amask,
> > > + &ifc
> > > ->amask_cs[ifc_bank].amask);
> > > + ifc_out32(savd_regs
> > > ->csor_cs[ifc_bank].csor_ext,
> > > + &ifc
> > > ->csor_cs[ifc_bank].csor_ext);
> > > + ifc_out32(savd_regs->csor_cs[ifc_bank].csor,
> > > + &ifc->csor_cs[ifc_bank].csor);
> >
> > Align continuation lines the way patchwork suggests ("&ifc" aligned with
> > "savd").
>
> Okay, I will take care of this in the next patch.
>
> >
> > Does resume from deep sleep go via U-Boot (which would initialize these
> > registers) on these chips?
>
> Yes, deep sleep resume goes via u-boot and these registers should be
> initialized
> By u-boot.
So then we don't need to save/restore them.
>
> > > +
> > > +/*
> > > +* IFC controller NOR machine registers */
> > > + ifc_out32(savd_regs->ifc_nor.nor_evter_en,
> > > + &ifc->ifc_nor.nor_evter_en);
> > > + ifc_out32(savd_regs->ifc_nor.norcr, &ifc
> > > ->ifc_nor.norcr);
> >
> > What uses these?
> >
>
> These registers are not used as such, but we would like to retain their
> value as they
> can be of help in case of error conditions.
I don't follow. Neither of those registers reports errors, and the registers
that *do* report errors are generally w1c and thus you can't save/restore
them.
> > >
> > > +
> > > + ver = ifc_in32(&ctrl->regs->ifc_rev);
> > > + ncfgr = ifc_in32(&ifc->ifc_nand.ncfgr);
> > > + if (ver >= FSL_IFC_V1_3_0) {
> > > +
> > > + ifc_out32(ncfgr | IFC_NAND_SRAM_INIT_EN,
> > > + &ifc->ifc_nand.ncfgr);
> > > + /* wait for SRAM_INIT bit to be clear or timeout */
> > > + timeout = IFC_TIMEOUT_MSECS;
> > > + while ((ifc_in32(&ifc->ifc_nand.ncfgr) &
> > > + IFC_NAND_SRAM_INIT_EN) &&
> > timeout)
> > > {
> > > + cpu_relax();
> > > + timeout--;
> > > + }
> >
> > How can this timeout be in milliseconds or any other real unit of time, if
> > it's
> > actually measuring loop iterations with no udelay() or similar?
> >
>
> Yes, it's not in millisecond any longer. Will change the name to
> IFC_WAIT_ITR
What does ITR mean? And my complaint was not just about naming -- this type
of delay loop is inherently unpredictable. Future chips might go through
100,000 loops a lot faster than current chips. Use a timeout that reflects
actual time.
> > > +
> > > + if (!timeout)
> > > + dev_err(ctrl->dev, "Timeout waiting for IFC
> > > SRAM
> > > INIT");
> > > + }
> >
> > U-boot does this when the version is > 1.1.0 -- why >= 1.3.0 here? Are
> > there
> > any versions in between 1.1.0 and 1.3.0?
>
> Because only B4 and T4 are based on 1.1.0 which do not support deep sleep.
> The first board which supports deep sleep is T1040 which has version 1.3.0.
That's a lousy excuse for making the code look like only >= 1.3.0 needs SRAM
init. BTW, we should be doing this SRAM init on regular boot as well, since
we shouldn't rely on it having happened in the bootloader.
> > Also, how did Linux and U-Boot end up having opposite argument ordering
> > for ifc_out32()? :-(
> >
> > -Scott
>
> I guess it happened in the patch:
> https://patchwork.ozlabs.org/patch/474719/
> Let's keep it that way now? Changing it would require significant work.
I'm not suggesting that it be changed now -- just grumbling, and hoping that
we're more careful next time.
-Scott
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
2016-02-18 1:19 ` Scott Wood
@ 2016-04-14 23:07 ` Leo Li
0 siblings, 0 replies; 7+ messages in thread
From: Leo Li @ 2016-04-14 23:07 UTC (permalink / raw)
To: Scott Wood
Cc: Raghav Dogra, linuxppc-dev@lists.ozlabs.org, Prabhakar Kushwaha
Hi Raghav,
Are we planning to send a new version of this patch? Btw, I see that
the current patch covers NOR/GPCM related registers, but the driver is
only built when IFC NAND is enabled right now. Do we want to change
the Kconfig to make it not depending on NAND?
Regards,
Leo
On Wed, Feb 17, 2016 at 7:19 PM, Scott Wood <oss@buserror.net> wrote:
> On Wed, 2016-02-17 at 14:40 +0000, Raghav Dogra wrote:
>>
>> > -----Original Message-----
>> > From: Scott Wood [mailto:oss@buserror.net]
>> > Sent: Tuesday, February 16, 2016 2:05 PM
>> > To: Raghav Dogra <raghav.dogra@nxp.com>; linuxppc-dev@lists.ozlabs.org
>> > Cc: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>> > Subject: Re: [PATCH][v3] drivers/memory: Add deep sleep support for IFC
>> >
>> > On Mon, 2016-02-15 at 11:44 +0530, Raghav Dogra wrote:
>> > >
>> > > +
>> > > + ctrl->saved_regs = kzalloc(sizeof(struct fsl_ifc_regs),
>> > > GFP_KERNEL);
>> > > + if (!ctrl->saved_regs)
>> > > + return -ENOMEM;
>> >
>> > Allocate memory at probe time, not here.
>> >
>>
>> But, why allocate memory at the probe when it is not known at that time
>> whether
>> deep sleep state would be required or not? Is that because we want to save
>> time
>> while going to deep sleep?
>
> We also want to avoid potential failures here. We can also keep the code
> simpler by embedding this into the ctrl struct itself, and not dynamically
> allocating it at all.
>
>> > >
>> > > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
>> > > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
>> > > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
>> > > +
>> > > + memcpy_fromio(ctrl->saved_regs, ifc, sizeof(struct
>> > > fsl_ifc_regs));
>> > > +
>> > > +/* save the interrupt values */
>> > > + ctrl->saved_regs->cm_evter_intr_en = cm_evter_intr_en;
>> > > + ctrl->saved_regs->ifc_nand.nand_evter_intr_en =
>> > nand_evter_intr_en;
>> > > + ctrl->saved_regs->ifc_nor.nor_evter_intr_en =
>> > > nor_evter_intr_en;
>> > > + ctrl->saved_regs->ifc_gpcm.gpcm_evter_intr_en =
>> > gpcm_evter_intr_en;
>> >
>> > Why didn't you use the memcpy_fromio() to save these, and clear intr_en
>> > later?
>> >
>>
>> I used it whenever I did a write/read on iomem. In this case, both memories
>> are non iomem.
>
> Huh? &ifc->ifc_nand.nand_evter_intr_en and such are iomem.
>
>> > > +
>> > > +/*
>> > > + * IFC interrupts disabled
>> > > + */
>> > > + ifc_out32(0x0, &ifc->cm_evter_intr_en);
>> > > + ifc_out32(0x0, &ifc->ifc_nand.nand_evter_intr_en);
>> > > + ifc_out32(0x0, &ifc->ifc_nor.nor_evter_intr_en);
>> > > + ifc_out32(0x0, &ifc->ifc_gpcm.gpcm_evter_intr_en);
>> > > +
>> > > +
>> > > + if (ctrl->saved_regs) {
>> > > + for (ifc_bank = 0; ifc_bank < FSL_IFC_BANK_COUNT;
>> > > ifc_bank++) {
>> > > + ifc_out32(savd_regs
>> > > ->cspr_cs[ifc_bank].cspr_ext,
>> > > + &ifc
>> > > ->cspr_cs[ifc_bank].cspr_ext);
>> > > + ifc_out32(savd_regs->cspr_cs[ifc_bank].cspr,
>> > > + &ifc->cspr_cs[ifc_bank].cspr);
>> > > + ifc_out32(savd_regs->amask_cs[ifc_bank].amask,
>> > > + &ifc
>> > > ->amask_cs[ifc_bank].amask);
>> > > + ifc_out32(savd_regs
>> > > ->csor_cs[ifc_bank].csor_ext,
>> > > + &ifc
>> > > ->csor_cs[ifc_bank].csor_ext);
>> > > + ifc_out32(savd_regs->csor_cs[ifc_bank].csor,
>> > > + &ifc->csor_cs[ifc_bank].csor);
>> >
>> > Align continuation lines the way patchwork suggests ("&ifc" aligned with
>> > "savd").
>>
>> Okay, I will take care of this in the next patch.
>>
>> >
>> > Does resume from deep sleep go via U-Boot (which would initialize these
>> > registers) on these chips?
>>
>> Yes, deep sleep resume goes via u-boot and these registers should be
>> initialized
>> By u-boot.
>
> So then we don't need to save/restore them.
>
>>
>
>> > > +
>> > > +/*
>> > > +* IFC controller NOR machine registers */
>> > > + ifc_out32(savd_regs->ifc_nor.nor_evter_en,
>> > > + &ifc->ifc_nor.nor_evter_en);
>> > > + ifc_out32(savd_regs->ifc_nor.norcr, &ifc
>> > > ->ifc_nor.norcr);
>> >
>> > What uses these?
>> >
>>
>> These registers are not used as such, but we would like to retain their
>> value as they
>> can be of help in case of error conditions.
>
> I don't follow. Neither of those registers reports errors, and the registers
> that *do* report errors are generally w1c and thus you can't save/restore
> them.
>
>> > >
>> > > +
>> > > + ver = ifc_in32(&ctrl->regs->ifc_rev);
>> > > + ncfgr = ifc_in32(&ifc->ifc_nand.ncfgr);
>> > > + if (ver >= FSL_IFC_V1_3_0) {
>> > > +
>> > > + ifc_out32(ncfgr | IFC_NAND_SRAM_INIT_EN,
>> > > + &ifc->ifc_nand.ncfgr);
>> > > + /* wait for SRAM_INIT bit to be clear or timeout */
>> > > + timeout = IFC_TIMEOUT_MSECS;
>> > > + while ((ifc_in32(&ifc->ifc_nand.ncfgr) &
>> > > + IFC_NAND_SRAM_INIT_EN) &&
>> > timeout)
>> > > {
>> > > + cpu_relax();
>> > > + timeout--;
>> > > + }
>> >
>> > How can this timeout be in milliseconds or any other real unit of time, if
>> > it's
>> > actually measuring loop iterations with no udelay() or similar?
>> >
>>
>> Yes, it's not in millisecond any longer. Will change the name to
>> IFC_WAIT_ITR
>
> What does ITR mean? And my complaint was not just about naming -- this type
> of delay loop is inherently unpredictable. Future chips might go through
> 100,000 loops a lot faster than current chips. Use a timeout that reflects
> actual time.
>
>
>> > > +
>> > > + if (!timeout)
>> > > + dev_err(ctrl->dev, "Timeout waiting for IFC
>> > > SRAM
>> > > INIT");
>> > > + }
>> >
>> > U-boot does this when the version is > 1.1.0 -- why >= 1.3.0 here? Are
>> > there
>> > any versions in between 1.1.0 and 1.3.0?
>>
>> Because only B4 and T4 are based on 1.1.0 which do not support deep sleep.
>> The first board which supports deep sleep is T1040 which has version 1.3.0.
>
> That's a lousy excuse for making the code look like only >= 1.3.0 needs SRAM
> init. BTW, we should be doing this SRAM init on regular boot as well, since
> we shouldn't rely on it having happened in the bootloader.
>
>> > Also, how did Linux and U-Boot end up having opposite argument ordering
>> > for ifc_out32()? :-(
>> >
>> > -Scott
>>
>> I guess it happened in the patch:
>> https://patchwork.ozlabs.org/patch/474719/
>> Let's keep it that way now? Changing it would require significant work.
>
> I'm not suggesting that it be changed now -- just grumbling, and hoping that
> we're more careful next time.
>
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
- Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-04-14 23:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-15 6:14 [PATCH][v3] drivers/memory: Add deep sleep support for IFC Raghav Dogra
2016-02-16 8:34 ` Scott Wood
2016-02-17 14:40 ` Raghav Dogra
2016-02-17 23:19 ` Leo Li
2016-02-18 1:11 ` Scott Wood
2016-02-18 1:19 ` Scott Wood
2016-04-14 23:07 ` Leo Li
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).