[PATCH 0/6] gpmi/mxs-dma runtime pm patch set

[PATCH 0/6] gpmi/mxs-dma runtime pm patch set

[Han Xu]

enable the system pm and runtime pm for mxs-dma and refine the gpmi
runtime pm code

Han Xu (6):
  dmaengine: mxs: change the way to register probe function
  dmaengine: mxs: add the remove function
  dmaengine: mxs: add the power management functions
  dmaengine: mxs: switch from dma_coherent to dma_pool
  mtd: rawnand: gpmi: refine the runtime pm ops
  mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

 drivers/dma/mxs-dma.c                      | 155 ++++++++++++++++++---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |  60 ++++----
 2 files changed, 167 insertions(+), 48 deletions(-)

-- 
2.17.1

[PATCH 0/6] gpmi/mxs-dma runtime pm patch set

[Han Xu]

enable the system pm and runtime pm for mxs-dma and refine the gpmi
runtime pm code

Han Xu (6):
  dmaengine: mxs: change the way to register probe function
  dmaengine: mxs: add the remove function
  dmaengine: mxs: add the power management functions
  dmaengine: mxs: switch from dma_coherent to dma_pool
  mtd: rawnand: gpmi: refine the runtime pm ops
  mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

 drivers/dma/mxs-dma.c                      | 155 ++++++++++++++++++---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |  60 ++++----
 2 files changed, 167 insertions(+), 48 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/6] gpmi/mxs-dma runtime pm patch set

[Han Xu]

enable the system pm and runtime pm for mxs-dma and refine the gpmi
runtime pm code

Han Xu (6):
  dmaengine: mxs: change the way to register probe function
  dmaengine: mxs: add the remove function
  dmaengine: mxs: add the power management functions
  dmaengine: mxs: switch from dma_coherent to dma_pool
  mtd: rawnand: gpmi: refine the runtime pm ops
  mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

 drivers/dma/mxs-dma.c                      | 155 ++++++++++++++++++---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |  60 ++++----
 2 files changed, 167 insertions(+), 48 deletions(-)

-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Han Xu]

change the way to register probe function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 3039bba0e4d5..9deaaf4fc58f 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -760,7 +760,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
 				     ofdma->of_node);
 }
 
-static int __init mxs_dma_probe(struct platform_device *pdev)
+static int mxs_dma_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	const struct platform_device_id *id_entry;
@@ -869,10 +869,7 @@ static struct platform_driver mxs_dma_driver = {
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
+	.probe		= mxs_dma_probe,
 };
 
-static int __init mxs_dma_module_init(void)
-{
-	return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
-}
-subsys_initcall(mxs_dma_module_init);
+module_platform_driver(mxs_dma_driver);
-- 
2.17.1

[PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Han Xu]

change the way to register probe function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 3039bba0e4d5..9deaaf4fc58f 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -760,7 +760,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
 				     ofdma->of_node);
 }
 
-static int __init mxs_dma_probe(struct platform_device *pdev)
+static int mxs_dma_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	const struct platform_device_id *id_entry;
@@ -869,10 +869,7 @@ static struct platform_driver mxs_dma_driver = {
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
+	.probe		= mxs_dma_probe,
 };
 
-static int __init mxs_dma_module_init(void)
-{
-	return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
-}
-subsys_initcall(mxs_dma_module_init);
+module_platform_driver(mxs_dma_driver);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Han Xu]

change the way to register probe function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 3039bba0e4d5..9deaaf4fc58f 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -760,7 +760,7 @@ static struct dma_chan *mxs_dma_xlate(struct of_phandle_args *dma_spec,
 				     ofdma->of_node);
 }
 
-static int __init mxs_dma_probe(struct platform_device *pdev)
+static int mxs_dma_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	const struct platform_device_id *id_entry;
@@ -869,10 +869,7 @@ static struct platform_driver mxs_dma_driver = {
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
+	.probe		= mxs_dma_probe,
 };
 
-static int __init mxs_dma_module_init(void)
-{
-	return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe);
-}
-subsys_initcall(mxs_dma_module_init);
+module_platform_driver(mxs_dma_driver);
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Fabio Estevam]

On Tue, Jan 14, 2020 at 6:48 PM Han Xu <han.xu@nxp.com> wrote:
>
> change the way to register probe function for mxs-dma

Please provide the reasoning for such change in the commit log.

Re: [PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Fabio Estevam]

On Tue, Jan 14, 2020 at 6:48 PM Han Xu <han.xu@nxp.com> wrote:
>
> change the way to register probe function for mxs-dma

Please provide the reasoning for such change in the commit log.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] dmaengine: mxs: change the way to register probe function

[Fabio Estevam]

On Tue, Jan 14, 2020 at 6:48 PM Han Xu <han.xu@nxp.com> wrote:
>
> change the way to register probe function for mxs-dma

Please provide the reasoning for such change in the commit log.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/6] dmaengine: mxs: add the remove function

[Han Xu]

add the remove function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 9deaaf4fc58f..b458f06f9067 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -863,6 +863,22 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int mxs_dma_remove(struct platform_device *pdev)
+{
+	struct mxs_dma_engine *mxs_dma = platform_get_drvdata(pdev);
+	int i;
+
+	dma_async_device_unregister(&mxs_dma->dma_device);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		tasklet_kill(&mxs_chan->tasklet);
+	}
+
+	return 0;
+}
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
@@ -870,6 +886,7 @@ static struct platform_driver mxs_dma_driver = {
 	},
 	.id_table	= mxs_dma_ids,
 	.probe		= mxs_dma_probe,
+	.remove		= mxs_dma_remove,
 };
 
 module_platform_driver(mxs_dma_driver);
-- 
2.17.1

[PATCH 2/6] dmaengine: mxs: add the remove function

[Han Xu]

add the remove function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 9deaaf4fc58f..b458f06f9067 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -863,6 +863,22 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int mxs_dma_remove(struct platform_device *pdev)
+{
+	struct mxs_dma_engine *mxs_dma = platform_get_drvdata(pdev);
+	int i;
+
+	dma_async_device_unregister(&mxs_dma->dma_device);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		tasklet_kill(&mxs_chan->tasklet);
+	}
+
+	return 0;
+}
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
@@ -870,6 +886,7 @@ static struct platform_driver mxs_dma_driver = {
 	},
 	.id_table	= mxs_dma_ids,
 	.probe		= mxs_dma_probe,
+	.remove		= mxs_dma_remove,
 };
 
 module_platform_driver(mxs_dma_driver);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] dmaengine: mxs: add the remove function

[Han Xu]

add the remove function for mxs-dma

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 9deaaf4fc58f..b458f06f9067 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -863,6 +863,22 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int mxs_dma_remove(struct platform_device *pdev)
+{
+	struct mxs_dma_engine *mxs_dma = platform_get_drvdata(pdev);
+	int i;
+
+	dma_async_device_unregister(&mxs_dma->dma_device);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		tasklet_kill(&mxs_chan->tasklet);
+	}
+
+	return 0;
+}
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
@@ -870,6 +886,7 @@ static struct platform_driver mxs_dma_driver = {
 	},
 	.id_table	= mxs_dma_ids,
 	.probe		= mxs_dma_probe,
+	.remove		= mxs_dma_remove,
 };
 
 module_platform_driver(mxs_dma_driver);
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

add the power management functions and leverage the runtime pm for
system suspend/resume

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 90 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index b458f06f9067..251492c5ea58 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -25,6 +25,7 @@
 #include <linux/of_dma.h>
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/irq.h>
 
@@ -39,6 +40,8 @@
 #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
 #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
 
+#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
+
 #define HW_APBHX_CTRL0				0x000
 #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
 #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
@@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
 	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
@@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (ret)
 		goto err_irq;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
-	if (ret)
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
 		goto err_clk;
+	}
 
 	mxs_dma_reset_chan(chan);
 
@@ -458,6 +464,7 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 
 	mxs_dma_disable_chan(chan);
 
@@ -466,7 +473,9 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
 			mxs_chan->ccw, mxs_chan->ccw_phys);
 
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 }
 
 /*
@@ -689,14 +698,32 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 	return mxs_chan->status;
 }
 
-static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+static int mxs_dma_init_rpm(struct mxs_dma_engine *mxs_dma)
 {
+	struct device *dev = &mxs_dma->pdev->dev;
+
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, MXS_DMA_RPM_TIMEOUT);
+	pm_runtime_use_autosuspend(dev);
+
+	return 0;
+}
+
+static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+{
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
+	ret = mxs_dma_init_rpm(mxs_dma);
 	if (ret)
 		return ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
 	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
 		goto err_out;
@@ -714,7 +741,8 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
 err_out:
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return ret;
 }
 
@@ -821,11 +849,13 @@ static int mxs_dma_probe(struct platform_device *pdev)
 			&mxs_dma->dma_device.channels);
 	}
 
+	platform_set_drvdata(pdev, mxs_dma);
+	mxs_dma->pdev = pdev;
+
 	ret = mxs_dma_init(mxs_dma);
 	if (ret)
 		return ret;
 
-	mxs_dma->pdev = pdev;
 	mxs_dma->dma_device.dev = &pdev->dev;
 
 	/* mxs_dma gets 65535 bytes maximum sg size */
@@ -879,9 +909,62 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mxs_dma_pm_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
+}
+
+static int mxs_dma_pm_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = mxs_dma_init(mxs_dma);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+#endif
+
+int mxs_dma_runtime_suspend(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(mxs_dma->clk);
+
+	return 0;
+}
+
+int mxs_dma_runtime_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(mxs_dma->clk);
+	if (ret) {
+		dev_err(&mxs_dma->pdev->dev, "failed to enable the clock\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops mxs_dma_pm_ops = {
+	SET_RUNTIME_PM_OPS(mxs_dma_runtime_suspend,
+			   mxs_dma_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(mxs_dma_pm_suspend, mxs_dma_pm_resume)
+};
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.pm = &mxs_dma_pm_ops,
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
-- 
2.17.1

[PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

add the power management functions and leverage the runtime pm for
system suspend/resume

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 90 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index b458f06f9067..251492c5ea58 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -25,6 +25,7 @@
 #include <linux/of_dma.h>
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/irq.h>
 
@@ -39,6 +40,8 @@
 #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
 #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
 
+#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
+
 #define HW_APBHX_CTRL0				0x000
 #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
 #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
@@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
 	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
@@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (ret)
 		goto err_irq;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
-	if (ret)
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
 		goto err_clk;
+	}
 
 	mxs_dma_reset_chan(chan);
 
@@ -458,6 +464,7 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 
 	mxs_dma_disable_chan(chan);
 
@@ -466,7 +473,9 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
 			mxs_chan->ccw, mxs_chan->ccw_phys);
 
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 }
 
 /*
@@ -689,14 +698,32 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 	return mxs_chan->status;
 }
 
-static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+static int mxs_dma_init_rpm(struct mxs_dma_engine *mxs_dma)
 {
+	struct device *dev = &mxs_dma->pdev->dev;
+
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, MXS_DMA_RPM_TIMEOUT);
+	pm_runtime_use_autosuspend(dev);
+
+	return 0;
+}
+
+static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+{
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
+	ret = mxs_dma_init_rpm(mxs_dma);
 	if (ret)
 		return ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
 	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
 		goto err_out;
@@ -714,7 +741,8 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
 err_out:
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return ret;
 }
 
@@ -821,11 +849,13 @@ static int mxs_dma_probe(struct platform_device *pdev)
 			&mxs_dma->dma_device.channels);
 	}
 
+	platform_set_drvdata(pdev, mxs_dma);
+	mxs_dma->pdev = pdev;
+
 	ret = mxs_dma_init(mxs_dma);
 	if (ret)
 		return ret;
 
-	mxs_dma->pdev = pdev;
 	mxs_dma->dma_device.dev = &pdev->dev;
 
 	/* mxs_dma gets 65535 bytes maximum sg size */
@@ -879,9 +909,62 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mxs_dma_pm_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
+}
+
+static int mxs_dma_pm_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = mxs_dma_init(mxs_dma);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+#endif
+
+int mxs_dma_runtime_suspend(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(mxs_dma->clk);
+
+	return 0;
+}
+
+int mxs_dma_runtime_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(mxs_dma->clk);
+	if (ret) {
+		dev_err(&mxs_dma->pdev->dev, "failed to enable the clock\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops mxs_dma_pm_ops = {
+	SET_RUNTIME_PM_OPS(mxs_dma_runtime_suspend,
+			   mxs_dma_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(mxs_dma_pm_suspend, mxs_dma_pm_resume)
+};
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.pm = &mxs_dma_pm_ops,
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

add the power management functions and leverage the runtime pm for
system suspend/resume

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 90 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index b458f06f9067..251492c5ea58 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -25,6 +25,7 @@
 #include <linux/of_dma.h>
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/irq.h>
 
@@ -39,6 +40,8 @@
 #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
 #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
 
+#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
+
 #define HW_APBHX_CTRL0				0x000
 #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
 #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
@@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
 	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
@@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (ret)
 		goto err_irq;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
-	if (ret)
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
 		goto err_clk;
+	}
 
 	mxs_dma_reset_chan(chan);
 
@@ -458,6 +464,7 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+	struct device *dev = &mxs_dma->pdev->dev;
 
 	mxs_dma_disable_chan(chan);
 
@@ -466,7 +473,9 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
 			mxs_chan->ccw, mxs_chan->ccw_phys);
 
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 }
 
 /*
@@ -689,14 +698,32 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 	return mxs_chan->status;
 }
 
-static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+static int mxs_dma_init_rpm(struct mxs_dma_engine *mxs_dma)
 {
+	struct device *dev = &mxs_dma->pdev->dev;
+
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, MXS_DMA_RPM_TIMEOUT);
+	pm_runtime_use_autosuspend(dev);
+
+	return 0;
+}
+
+static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+{
+	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	ret = clk_prepare_enable(mxs_dma->clk);
+	ret = mxs_dma_init_rpm(mxs_dma);
 	if (ret)
 		return ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
 	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
 		goto err_out;
@@ -714,7 +741,8 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
 err_out:
-	clk_disable_unprepare(mxs_dma->clk);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 	return ret;
 }
 
@@ -821,11 +849,13 @@ static int mxs_dma_probe(struct platform_device *pdev)
 			&mxs_dma->dma_device.channels);
 	}
 
+	platform_set_drvdata(pdev, mxs_dma);
+	mxs_dma->pdev = pdev;
+
 	ret = mxs_dma_init(mxs_dma);
 	if (ret)
 		return ret;
 
-	mxs_dma->pdev = pdev;
 	mxs_dma->dma_device.dev = &pdev->dev;
 
 	/* mxs_dma gets 65535 bytes maximum sg size */
@@ -879,9 +909,62 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mxs_dma_pm_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
+}
+
+static int mxs_dma_pm_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = mxs_dma_init(mxs_dma);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+#endif
+
+int mxs_dma_runtime_suspend(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(mxs_dma->clk);
+
+	return 0;
+}
+
+int mxs_dma_runtime_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(mxs_dma->clk);
+	if (ret) {
+		dev_err(&mxs_dma->pdev->dev, "failed to enable the clock\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops mxs_dma_pm_ops = {
+	SET_RUNTIME_PM_OPS(mxs_dma_runtime_suspend,
+			   mxs_dma_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(mxs_dma_pm_suspend, mxs_dma_pm_resume)
+};
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.pm = &mxs_dma_pm_ops,
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> add the power management functions and leverage the runtime pm for
> system suspend/resume
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index b458f06f9067..251492c5ea58 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_dma.h>
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <asm/irq.h>
>  
> @@ -39,6 +40,8 @@
>  #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
>  #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
>  
> +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> +
>  #define HW_APBHX_CTRL0				0x000
>  #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
>  #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
> @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  {
>  	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
>  	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> +	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
>  	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	if (ret)
>  		goto err_irq;
>  
> -	ret = clk_prepare_enable(mxs_dma->clk);
> -	if (ret)
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable clock\n");
>  		goto err_clk;

From looking at other DMA drivers I know we are in good company here,
but I think this is wrong. Doing pm_runtime_get_sync() in
alloc_chan_resources() and going to autosuspend in free_chan_resources()
effectively disables runtime_pm as clients normally acquire their
channels during driver probe and release them only in driver remove.

In the next patch you release the DMA channels in the GPMI nand drivers
runtime_suspend hook just to somehow trigger the runtime_suspend of the
DMA driver.

What you should do instead is to make sure the hook runtime_pm to the
DMA drivers activity phases, like for example the pl330 driver does.
Then you wouldn't have to care about manually putting the DMA driver into
suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> add the power management functions and leverage the runtime pm for
> system suspend/resume
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index b458f06f9067..251492c5ea58 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_dma.h>
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <asm/irq.h>
>  
> @@ -39,6 +40,8 @@
>  #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
>  #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
>  
> +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> +
>  #define HW_APBHX_CTRL0				0x000
>  #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
>  #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
> @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  {
>  	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
>  	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> +	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
>  	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	if (ret)
>  		goto err_irq;
>  
> -	ret = clk_prepare_enable(mxs_dma->clk);
> -	if (ret)
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable clock\n");
>  		goto err_clk;

From looking at other DMA drivers I know we are in good company here,
but I think this is wrong. Doing pm_runtime_get_sync() in
alloc_chan_resources() and going to autosuspend in free_chan_resources()
effectively disables runtime_pm as clients normally acquire their
channels during driver probe and release them only in driver remove.

In the next patch you release the DMA channels in the GPMI nand drivers
runtime_suspend hook just to somehow trigger the runtime_suspend of the
DMA driver.

What you should do instead is to make sure the hook runtime_pm to the
DMA drivers activity phases, like for example the pl330 driver does.
Then you wouldn't have to care about manually putting the DMA driver into
suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> add the power management functions and leverage the runtime pm for
> system suspend/resume
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index b458f06f9067..251492c5ea58 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_dma.h>
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <asm/irq.h>
>  
> @@ -39,6 +40,8 @@
>  #define dma_is_apbh(mxs_dma)	((mxs_dma)->type == MXS_DMA_APBH)
>  #define apbh_is_old(mxs_dma)	((mxs_dma)->dev_id == IMX23_DMA)
>  
> +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> +
>  #define HW_APBHX_CTRL0				0x000
>  #define BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
>  #define BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
> @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  {
>  	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
>  	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> +	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
>  	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	if (ret)
>  		goto err_irq;
>  
> -	ret = clk_prepare_enable(mxs_dma->clk);
> -	if (ret)
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable clock\n");
>  		goto err_clk;

From looking at other DMA drivers I know we are in good company here,
but I think this is wrong. Doing pm_runtime_get_sync() in
alloc_chan_resources() and going to autosuspend in free_chan_resources()
effectively disables runtime_pm as clients normally acquire their
channels during driver probe and release them only in driver remove.

In the next patch you release the DMA channels in the GPMI nand drivers
runtime_suspend hook just to somehow trigger the runtime_suspend of the
DMA driver.

What you should do instead is to make sure the hook runtime_pm to the
DMA drivers activity phases, like for example the pl330 driver does.
Then you wouldn't have to care about manually putting the DMA driver into
suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > add the power management functions and leverage the runtime pm for
> > system suspend/resume
> >
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > ---
> >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > index b458f06f9067..251492c5ea58 100644
> > --- a/drivers/dma/mxs-dma.c
> > +++ b/drivers/dma/mxs-dma.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/of_dma.h>
> >  #include <linux/list.h>
> >  #include <linux/dma/mxs-dma.h>
> > +#include <linux/pm_runtime.h>
> >
> >  #include <asm/irq.h>
> >
> > @@ -39,6 +40,8 @@
> >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> >
> > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > +
> >  #define HW_APBHX_CTRL0                               0x000
> >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >  {
> >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > +     struct device *dev = &mxs_dma->pdev->dev;
> >       int ret;
> >
> >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >       if (ret)
> >               goto err_irq;
> >
> > -     ret = clk_prepare_enable(mxs_dma->clk);
> > -     if (ret)
> > +     ret = pm_runtime_get_sync(dev);
> > +     if (ret < 0) {
> > +             dev_err(dev, "Failed to enable clock\n");
> >               goto err_clk;
>
> From looking at other DMA drivers I know we are in good company here,
> but I think this is wrong. Doing pm_runtime_get_sync() in
> alloc_chan_resources() and going to autosuspend in free_chan_resources()
> effectively disables runtime_pm as clients normally acquire their
> channels during driver probe and release them only in driver remove.

Thanks for the comments.
That's why I moved acquire_dma_resource from the probe to
runtime_resume in the gpmi driver, this change won't disable the
runtime_pm function and the incremental counter always balanced.

>
> In the next patch you release the DMA channels in the GPMI nand drivers
> runtime_suspend hook just to somehow trigger the runtime_suspend of the
> DMA driver.
>
> What you should do instead is to make sure the hook runtime_pm to the
> DMA drivers activity phases, like for example the pl330 driver does.
> Then you wouldn't have to care about manually putting the DMA driver into
> suspend from the GPMI NAND driver.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > add the power management functions and leverage the runtime pm for
> > system suspend/resume
> >
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > ---
> >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > index b458f06f9067..251492c5ea58 100644
> > --- a/drivers/dma/mxs-dma.c
> > +++ b/drivers/dma/mxs-dma.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/of_dma.h>
> >  #include <linux/list.h>
> >  #include <linux/dma/mxs-dma.h>
> > +#include <linux/pm_runtime.h>
> >
> >  #include <asm/irq.h>
> >
> > @@ -39,6 +40,8 @@
> >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> >
> > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > +
> >  #define HW_APBHX_CTRL0                               0x000
> >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >  {
> >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > +     struct device *dev = &mxs_dma->pdev->dev;
> >       int ret;
> >
> >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >       if (ret)
> >               goto err_irq;
> >
> > -     ret = clk_prepare_enable(mxs_dma->clk);
> > -     if (ret)
> > +     ret = pm_runtime_get_sync(dev);
> > +     if (ret < 0) {
> > +             dev_err(dev, "Failed to enable clock\n");
> >               goto err_clk;
>
> From looking at other DMA drivers I know we are in good company here,
> but I think this is wrong. Doing pm_runtime_get_sync() in
> alloc_chan_resources() and going to autosuspend in free_chan_resources()
> effectively disables runtime_pm as clients normally acquire their
> channels during driver probe and release them only in driver remove.

Thanks for the comments.
That's why I moved acquire_dma_resource from the probe to
runtime_resume in the gpmi driver, this change won't disable the
runtime_pm function and the incremental counter always balanced.

>
> In the next patch you release the DMA channels in the GPMI nand drivers
> runtime_suspend hook just to somehow trigger the runtime_suspend of the
> DMA driver.
>
> What you should do instead is to make sure the hook runtime_pm to the
> DMA drivers activity phases, like for example the pl330 driver does.
> Then you wouldn't have to care about manually putting the DMA driver into
> suspend from the GPMI NAND driver.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Han Xu]

On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > add the power management functions and leverage the runtime pm for
> > system suspend/resume
> >
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > ---
> >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 90 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > index b458f06f9067..251492c5ea58 100644
> > --- a/drivers/dma/mxs-dma.c
> > +++ b/drivers/dma/mxs-dma.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/of_dma.h>
> >  #include <linux/list.h>
> >  #include <linux/dma/mxs-dma.h>
> > +#include <linux/pm_runtime.h>
> >
> >  #include <asm/irq.h>
> >
> > @@ -39,6 +40,8 @@
> >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> >
> > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > +
> >  #define HW_APBHX_CTRL0                               0x000
> >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >  {
> >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > +     struct device *dev = &mxs_dma->pdev->dev;
> >       int ret;
> >
> >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> >       if (ret)
> >               goto err_irq;
> >
> > -     ret = clk_prepare_enable(mxs_dma->clk);
> > -     if (ret)
> > +     ret = pm_runtime_get_sync(dev);
> > +     if (ret < 0) {
> > +             dev_err(dev, "Failed to enable clock\n");
> >               goto err_clk;
>
> From looking at other DMA drivers I know we are in good company here,
> but I think this is wrong. Doing pm_runtime_get_sync() in
> alloc_chan_resources() and going to autosuspend in free_chan_resources()
> effectively disables runtime_pm as clients normally acquire their
> channels during driver probe and release them only in driver remove.

Thanks for the comments.
That's why I moved acquire_dma_resource from the probe to
runtime_resume in the gpmi driver, this change won't disable the
runtime_pm function and the incremental counter always balanced.

>
> In the next patch you release the DMA channels in the GPMI nand drivers
> runtime_suspend hook just to somehow trigger the runtime_suspend of the
> DMA driver.
>
> What you should do instead is to make sure the hook runtime_pm to the
> DMA drivers activity phases, like for example the pl330 driver does.
> Then you wouldn't have to care about manually putting the DMA driver into
> suspend from the GPMI NAND driver.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Thu, Jan 16, 2020 at 10:36:33AM -0600, Han Xu wrote:
> On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > > add the power management functions and leverage the runtime pm for
> > > system suspend/resume
> > >
> > > Signed-off-by: Han Xu <han.xu@nxp.com>
> > > ---
> > >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> > >  1 file changed, 90 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > index b458f06f9067..251492c5ea58 100644
> > > --- a/drivers/dma/mxs-dma.c
> > > +++ b/drivers/dma/mxs-dma.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/of_dma.h>
> > >  #include <linux/list.h>
> > >  #include <linux/dma/mxs-dma.h>
> > > +#include <linux/pm_runtime.h>
> > >
> > >  #include <asm/irq.h>
> > >
> > > @@ -39,6 +40,8 @@
> > >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> > >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> > >
> > > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > > +
> > >  #define HW_APBHX_CTRL0                               0x000
> > >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> > >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >  {
> > >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> > >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > > +     struct device *dev = &mxs_dma->pdev->dev;
> > >       int ret;
> > >
> > >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >       if (ret)
> > >               goto err_irq;
> > >
> > > -     ret = clk_prepare_enable(mxs_dma->clk);
> > > -     if (ret)
> > > +     ret = pm_runtime_get_sync(dev);
> > > +     if (ret < 0) {
> > > +             dev_err(dev, "Failed to enable clock\n");
> > >               goto err_clk;
> >
> > From looking at other DMA drivers I know we are in good company here,
> > but I think this is wrong. Doing pm_runtime_get_sync() in
> > alloc_chan_resources() and going to autosuspend in free_chan_resources()
> > effectively disables runtime_pm as clients normally acquire their
> > channels during driver probe and release them only in driver remove.
> 
> Thanks for the comments.
> That's why I moved acquire_dma_resource from the probe to
> runtime_resume in the gpmi driver, this change won't disable the
> runtime_pm function and the incremental counter always balanced.

Yes, that's what I've written a few lines further down:

> 
> >
> > In the next patch you release the DMA channels in the GPMI nand drivers
> > runtime_suspend hook just to somehow trigger the runtime_suspend of the
> > DMA driver.

And I consider doing this a crude hack. Here is what I suggested doing
instead:

> >
> > What you should do instead is to make sure the hook runtime_pm to the
> > DMA drivers activity phases, like for example the pl330 driver does.
> > Then you wouldn't have to care about manually putting the DMA driver into
> > suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Thu, Jan 16, 2020 at 10:36:33AM -0600, Han Xu wrote:
> On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > > add the power management functions and leverage the runtime pm for
> > > system suspend/resume
> > >
> > > Signed-off-by: Han Xu <han.xu@nxp.com>
> > > ---
> > >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> > >  1 file changed, 90 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > index b458f06f9067..251492c5ea58 100644
> > > --- a/drivers/dma/mxs-dma.c
> > > +++ b/drivers/dma/mxs-dma.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/of_dma.h>
> > >  #include <linux/list.h>
> > >  #include <linux/dma/mxs-dma.h>
> > > +#include <linux/pm_runtime.h>
> > >
> > >  #include <asm/irq.h>
> > >
> > > @@ -39,6 +40,8 @@
> > >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> > >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> > >
> > > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > > +
> > >  #define HW_APBHX_CTRL0                               0x000
> > >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> > >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >  {
> > >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> > >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > > +     struct device *dev = &mxs_dma->pdev->dev;
> > >       int ret;
> > >
> > >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >       if (ret)
> > >               goto err_irq;
> > >
> > > -     ret = clk_prepare_enable(mxs_dma->clk);
> > > -     if (ret)
> > > +     ret = pm_runtime_get_sync(dev);
> > > +     if (ret < 0) {
> > > +             dev_err(dev, "Failed to enable clock\n");
> > >               goto err_clk;
> >
> > From looking at other DMA drivers I know we are in good company here,
> > but I think this is wrong. Doing pm_runtime_get_sync() in
> > alloc_chan_resources() and going to autosuspend in free_chan_resources()
> > effectively disables runtime_pm as clients normally acquire their
> > channels during driver probe and release them only in driver remove.
> 
> Thanks for the comments.
> That's why I moved acquire_dma_resource from the probe to
> runtime_resume in the gpmi driver, this change won't disable the
> runtime_pm function and the incremental counter always balanced.

Yes, that's what I've written a few lines further down:

> 
> >
> > In the next patch you release the DMA channels in the GPMI nand drivers
> > runtime_suspend hook just to somehow trigger the runtime_suspend of the
> > DMA driver.

And I consider doing this a crude hack. Here is what I suggested doing
instead:

> >
> > What you should do instead is to make sure the hook runtime_pm to the
> > DMA drivers activity phases, like for example the pl330 driver does.
> > Then you wouldn't have to care about manually putting the DMA driver into
> > suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] dmaengine: mxs: add the power management functions

[Sascha Hauer]

On Thu, Jan 16, 2020 at 10:36:33AM -0600, Han Xu wrote:
> On Wed, Jan 15, 2020 at 2:03 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Wed, Jan 15, 2020 at 05:44:00AM +0800, Han Xu wrote:
> > > add the power management functions and leverage the runtime pm for
> > > system suspend/resume
> > >
> > > Signed-off-by: Han Xu <han.xu@nxp.com>
> > > ---
> > >  drivers/dma/mxs-dma.c | 97 +++++++++++++++++++++++++++++++++++++++----
> > >  1 file changed, 90 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > > index b458f06f9067..251492c5ea58 100644
> > > --- a/drivers/dma/mxs-dma.c
> > > +++ b/drivers/dma/mxs-dma.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/of_dma.h>
> > >  #include <linux/list.h>
> > >  #include <linux/dma/mxs-dma.h>
> > > +#include <linux/pm_runtime.h>
> > >
> > >  #include <asm/irq.h>
> > >
> > > @@ -39,6 +40,8 @@
> > >  #define dma_is_apbh(mxs_dma) ((mxs_dma)->type == MXS_DMA_APBH)
> > >  #define apbh_is_old(mxs_dma) ((mxs_dma)->dev_id == IMX23_DMA)
> > >
> > > +#define MXS_DMA_RPM_TIMEOUT 50 /* ms */
> > > +
> > >  #define HW_APBHX_CTRL0                               0x000
> > >  #define BM_APBH_CTRL0_APB_BURST8_EN          (1 << 29)
> > >  #define BM_APBH_CTRL0_APB_BURST_EN           (1 << 28)
> > > @@ -416,6 +419,7 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >  {
> > >       struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
> > >       struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
> > > +     struct device *dev = &mxs_dma->pdev->dev;
> > >       int ret;
> > >
> > >       mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> > > @@ -431,9 +435,11 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
> > >       if (ret)
> > >               goto err_irq;
> > >
> > > -     ret = clk_prepare_enable(mxs_dma->clk);
> > > -     if (ret)
> > > +     ret = pm_runtime_get_sync(dev);
> > > +     if (ret < 0) {
> > > +             dev_err(dev, "Failed to enable clock\n");
> > >               goto err_clk;
> >
> > From looking at other DMA drivers I know we are in good company here,
> > but I think this is wrong. Doing pm_runtime_get_sync() in
> > alloc_chan_resources() and going to autosuspend in free_chan_resources()
> > effectively disables runtime_pm as clients normally acquire their
> > channels during driver probe and release them only in driver remove.
> 
> Thanks for the comments.
> That's why I moved acquire_dma_resource from the probe to
> runtime_resume in the gpmi driver, this change won't disable the
> runtime_pm function and the incremental counter always balanced.

Yes, that's what I've written a few lines further down:

> 
> >
> > In the next patch you release the DMA channels in the GPMI nand drivers
> > runtime_suspend hook just to somehow trigger the runtime_suspend of the
> > DMA driver.

And I consider doing this a crude hack. Here is what I suggested doing
instead:

> >
> > What you should do instead is to make sure the hook runtime_pm to the
> > DMA drivers activity phases, like for example the pl330 driver does.
> > Then you wouldn't have to care about manually putting the DMA driver into
> > suspend from the GPMI NAND driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Han Xu]

create one dma_pool dedicate for mxs-dma to avoid the
"alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
frequently alloc/free resource ops in runtime pm.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 251492c5ea58..dfee41ae1981 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -26,6 +26,7 @@
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
 #include <linux/pm_runtime.h>
+#include <linux/dmapool.h>
 
 #include <asm/irq.h>
 
@@ -121,6 +122,7 @@ struct mxs_dma_chan {
 	enum dma_status			status;
 	unsigned int			flags;
 	bool				reset;
+	struct dma_pool			*ccw_pool;
 #define MXS_DMA_SG_LOOP			(1 << 0)
 #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
 };
@@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
-					   CCW_BLOCK_SIZE,
-					   &mxs_chan->ccw_phys, GFP_KERNEL);
+	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
+					GFP_ATOMIC,
+					&mxs_chan->ccw_phys);
+
 	if (!mxs_chan->ccw) {
 		ret = -ENOMEM;
 		goto err_alloc;
@@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 err_clk:
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 err_irq:
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 err_alloc:
 	return ret;
 }
@@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
@@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	const struct mxs_dma_type *dma_type;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
+	struct dma_pool *ccw_pool;
 	int ret, i;
 
 	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
@@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
 		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
 			     (unsigned long) mxs_chan);
 
-
 		/* Add the channel to mxs_chan list */
 		list_add_tail(&mxs_chan->chan.device_node,
 			&mxs_dma->dma_device.channels);
@@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
 
 	mxs_dma->dma_device.dev = &pdev->dev;
 
+	/* create the dma pool */
+	ccw_pool = dma_pool_create("ccw_pool",
+				   mxs_dma->dma_device.dev,
+				   CCW_BLOCK_SIZE, 32, 0);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		mxs_chan->ccw_pool = ccw_pool;
+	}
+
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
 	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
@@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	int i;
 
 	dma_async_device_unregister(&mxs_dma->dma_device);
+	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);
 
 	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
 		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
 
 		tasklet_kill(&mxs_chan->tasklet);
+		mxs_chan->ccw_pool = NULL;
 	}
 
 	return 0;
-- 
2.17.1

[PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Han Xu]

create one dma_pool dedicate for mxs-dma to avoid the
"alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
frequently alloc/free resource ops in runtime pm.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 251492c5ea58..dfee41ae1981 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -26,6 +26,7 @@
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
 #include <linux/pm_runtime.h>
+#include <linux/dmapool.h>
 
 #include <asm/irq.h>
 
@@ -121,6 +122,7 @@ struct mxs_dma_chan {
 	enum dma_status			status;
 	unsigned int			flags;
 	bool				reset;
+	struct dma_pool			*ccw_pool;
 #define MXS_DMA_SG_LOOP			(1 << 0)
 #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
 };
@@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
-					   CCW_BLOCK_SIZE,
-					   &mxs_chan->ccw_phys, GFP_KERNEL);
+	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
+					GFP_ATOMIC,
+					&mxs_chan->ccw_phys);
+
 	if (!mxs_chan->ccw) {
 		ret = -ENOMEM;
 		goto err_alloc;
@@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 err_clk:
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 err_irq:
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 err_alloc:
 	return ret;
 }
@@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
@@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	const struct mxs_dma_type *dma_type;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
+	struct dma_pool *ccw_pool;
 	int ret, i;
 
 	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
@@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
 		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
 			     (unsigned long) mxs_chan);
 
-
 		/* Add the channel to mxs_chan list */
 		list_add_tail(&mxs_chan->chan.device_node,
 			&mxs_dma->dma_device.channels);
@@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
 
 	mxs_dma->dma_device.dev = &pdev->dev;
 
+	/* create the dma pool */
+	ccw_pool = dma_pool_create("ccw_pool",
+				   mxs_dma->dma_device.dev,
+				   CCW_BLOCK_SIZE, 32, 0);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		mxs_chan->ccw_pool = ccw_pool;
+	}
+
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
 	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
@@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	int i;
 
 	dma_async_device_unregister(&mxs_dma->dma_device);
+	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);
 
 	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
 		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
 
 		tasklet_kill(&mxs_chan->tasklet);
+		mxs_chan->ccw_pool = NULL;
 	}
 
 	return 0;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Han Xu]

create one dma_pool dedicate for mxs-dma to avoid the
"alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
frequently alloc/free resource ops in runtime pm.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 251492c5ea58..dfee41ae1981 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -26,6 +26,7 @@
 #include <linux/list.h>
 #include <linux/dma/mxs-dma.h>
 #include <linux/pm_runtime.h>
+#include <linux/dmapool.h>
 
 #include <asm/irq.h>
 
@@ -121,6 +122,7 @@ struct mxs_dma_chan {
 	enum dma_status			status;
 	unsigned int			flags;
 	bool				reset;
+	struct dma_pool			*ccw_pool;
 #define MXS_DMA_SG_LOOP			(1 << 0)
 #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
 };
@@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	struct device *dev = &mxs_dma->pdev->dev;
 	int ret;
 
-	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
-					   CCW_BLOCK_SIZE,
-					   &mxs_chan->ccw_phys, GFP_KERNEL);
+	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
+					GFP_ATOMIC,
+					&mxs_chan->ccw_phys);
+
 	if (!mxs_chan->ccw) {
 		ret = -ENOMEM;
 		goto err_alloc;
@@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 err_clk:
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 err_irq:
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 err_alloc:
 	return ret;
 }
@@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 
-	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
-			mxs_chan->ccw, mxs_chan->ccw_phys);
+	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
+		      mxs_chan->ccw_phys);
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
@@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
 	const struct mxs_dma_type *dma_type;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
+	struct dma_pool *ccw_pool;
 	int ret, i;
 
 	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
@@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
 		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
 			     (unsigned long) mxs_chan);
 
-
 		/* Add the channel to mxs_chan list */
 		list_add_tail(&mxs_chan->chan.device_node,
 			&mxs_dma->dma_device.channels);
@@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
 
 	mxs_dma->dma_device.dev = &pdev->dev;
 
+	/* create the dma pool */
+	ccw_pool = dma_pool_create("ccw_pool",
+				   mxs_dma->dma_device.dev,
+				   CCW_BLOCK_SIZE, 32, 0);
+
+	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
+		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
+
+		mxs_chan->ccw_pool = ccw_pool;
+	}
+
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
 	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
@@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
 	int i;
 
 	dma_async_device_unregister(&mxs_dma->dma_device);
+	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);
 
 	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
 		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
 
 		tasklet_kill(&mxs_chan->tasklet);
+		mxs_chan->ccw_pool = NULL;
 	}
 
 	return 0;
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:01AM +0800, Han Xu wrote:
> create one dma_pool dedicate for mxs-dma to avoid the
> "alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
> frequently alloc/free resource ops in runtime pm.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 251492c5ea58..dfee41ae1981 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -26,6 +26,7 @@
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/dmapool.h>
>  
>  #include <asm/irq.h>
>  
> @@ -121,6 +122,7 @@ struct mxs_dma_chan {
>  	enum dma_status			status;
>  	unsigned int			flags;
>  	bool				reset;
> +	struct dma_pool			*ccw_pool;
>  #define MXS_DMA_SG_LOOP			(1 << 0)
>  #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
>  };
> @@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
> -	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> -					   CCW_BLOCK_SIZE,
> -					   &mxs_chan->ccw_phys, GFP_KERNEL);
> +	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
> +					GFP_ATOMIC,
> +					&mxs_chan->ccw_phys);

Why GFP_ATOMIC?

> +
>  	if (!mxs_chan->ccw) {
>  		ret = -ENOMEM;
>  		goto err_alloc;
> @@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  err_clk:
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  err_irq:
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  err_alloc:
>  	return ret;
>  }
> @@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
>  
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  
>  	pm_runtime_mark_last_busy(dev);
>  	pm_runtime_put_autosuspend(dev);
> @@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  	const struct mxs_dma_type *dma_type;
>  	struct mxs_dma_engine *mxs_dma;
>  	struct resource *iores;
> +	struct dma_pool *ccw_pool;
>  	int ret, i;
>  
>  	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
> @@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
>  			     (unsigned long) mxs_chan);
>  
> -
>  		/* Add the channel to mxs_chan list */
>  		list_add_tail(&mxs_chan->chan.device_node,
>  			&mxs_dma->dma_device.channels);
> @@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  
>  	mxs_dma->dma_device.dev = &pdev->dev;
>  
> +	/* create the dma pool */
> +	ccw_pool = dma_pool_create("ccw_pool",
> +				   mxs_dma->dma_device.dev,
> +				   CCW_BLOCK_SIZE, 32, 0);
> +
> +	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
> +		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
> +
> +		mxs_chan->ccw_pool = ccw_pool;
> +	}

ccw_pool is the same for every channel, it should be a member of
struct mxs_dma_engine and not of struct mcs_dma_chan.

> +
>  	/* mxs_dma gets 65535 bytes maximum sg size */
>  	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
>  	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
> @@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
>  	int i;
>  
>  	dma_async_device_unregister(&mxs_dma->dma_device);
> +	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);

It doesn't seem to make a big difference, but I would do this after
killing the tasklets, not before. Otherwise we would have to prove that
no tasklet is still accessing the dma_pool.

>  
>  	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
>  		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
>  
>  		tasklet_kill(&mxs_chan->tasklet);
> +		mxs_chan->ccw_pool = NULL;

There's no point in resetting this to NULL, mxs_chan is never going to
be touched again.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:01AM +0800, Han Xu wrote:
> create one dma_pool dedicate for mxs-dma to avoid the
> "alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
> frequently alloc/free resource ops in runtime pm.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 251492c5ea58..dfee41ae1981 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -26,6 +26,7 @@
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/dmapool.h>
>  
>  #include <asm/irq.h>
>  
> @@ -121,6 +122,7 @@ struct mxs_dma_chan {
>  	enum dma_status			status;
>  	unsigned int			flags;
>  	bool				reset;
> +	struct dma_pool			*ccw_pool;
>  #define MXS_DMA_SG_LOOP			(1 << 0)
>  #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
>  };
> @@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
> -	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> -					   CCW_BLOCK_SIZE,
> -					   &mxs_chan->ccw_phys, GFP_KERNEL);
> +	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
> +					GFP_ATOMIC,
> +					&mxs_chan->ccw_phys);

Why GFP_ATOMIC?

> +
>  	if (!mxs_chan->ccw) {
>  		ret = -ENOMEM;
>  		goto err_alloc;
> @@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  err_clk:
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  err_irq:
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  err_alloc:
>  	return ret;
>  }
> @@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
>  
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  
>  	pm_runtime_mark_last_busy(dev);
>  	pm_runtime_put_autosuspend(dev);
> @@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  	const struct mxs_dma_type *dma_type;
>  	struct mxs_dma_engine *mxs_dma;
>  	struct resource *iores;
> +	struct dma_pool *ccw_pool;
>  	int ret, i;
>  
>  	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
> @@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
>  			     (unsigned long) mxs_chan);
>  
> -
>  		/* Add the channel to mxs_chan list */
>  		list_add_tail(&mxs_chan->chan.device_node,
>  			&mxs_dma->dma_device.channels);
> @@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  
>  	mxs_dma->dma_device.dev = &pdev->dev;
>  
> +	/* create the dma pool */
> +	ccw_pool = dma_pool_create("ccw_pool",
> +				   mxs_dma->dma_device.dev,
> +				   CCW_BLOCK_SIZE, 32, 0);
> +
> +	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
> +		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
> +
> +		mxs_chan->ccw_pool = ccw_pool;
> +	}

ccw_pool is the same for every channel, it should be a member of
struct mxs_dma_engine and not of struct mcs_dma_chan.

> +
>  	/* mxs_dma gets 65535 bytes maximum sg size */
>  	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
>  	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
> @@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
>  	int i;
>  
>  	dma_async_device_unregister(&mxs_dma->dma_device);
> +	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);

It doesn't seem to make a big difference, but I would do this after
killing the tasklets, not before. Otherwise we would have to prove that
no tasklet is still accessing the dma_pool.

>  
>  	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
>  		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
>  
>  		tasklet_kill(&mxs_chan->tasklet);
> +		mxs_chan->ccw_pool = NULL;

There's no point in resetting this to NULL, mxs_chan is never going to
be touched again.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] dmaengine: mxs: switch from dma_coherent to dma_pool

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:01AM +0800, Han Xu wrote:
> create one dma_pool dedicate for mxs-dma to avoid the
> "alloc_contig_range: [xxx, xxx) PFNs busy" warning message during
> frequently alloc/free resource ops in runtime pm.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/dma/mxs-dma.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index 251492c5ea58..dfee41ae1981 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -26,6 +26,7 @@
>  #include <linux/list.h>
>  #include <linux/dma/mxs-dma.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/dmapool.h>
>  
>  #include <asm/irq.h>
>  
> @@ -121,6 +122,7 @@ struct mxs_dma_chan {
>  	enum dma_status			status;
>  	unsigned int			flags;
>  	bool				reset;
> +	struct dma_pool			*ccw_pool;
>  #define MXS_DMA_SG_LOOP			(1 << 0)
>  #define MXS_DMA_USE_SEMAPHORE		(1 << 1)
>  };
> @@ -422,9 +424,10 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  	struct device *dev = &mxs_dma->pdev->dev;
>  	int ret;
>  
> -	mxs_chan->ccw = dma_alloc_coherent(mxs_dma->dma_device.dev,
> -					   CCW_BLOCK_SIZE,
> -					   &mxs_chan->ccw_phys, GFP_KERNEL);
> +	mxs_chan->ccw = dma_pool_zalloc(mxs_chan->ccw_pool,
> +					GFP_ATOMIC,
> +					&mxs_chan->ccw_phys);

Why GFP_ATOMIC?

> +
>  	if (!mxs_chan->ccw) {
>  		ret = -ENOMEM;
>  		goto err_alloc;
> @@ -454,8 +457,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
>  err_clk:
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  err_irq:
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  err_alloc:
>  	return ret;
>  }
> @@ -470,8 +473,8 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
>  
>  	free_irq(mxs_chan->chan_irq, mxs_dma);
>  
> -	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
> -			mxs_chan->ccw, mxs_chan->ccw_phys);
> +	dma_pool_free(mxs_chan->ccw_pool, mxs_chan->ccw,
> +		      mxs_chan->ccw_phys);
>  
>  	pm_runtime_mark_last_busy(dev);
>  	pm_runtime_put_autosuspend(dev);
> @@ -796,6 +799,7 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  	const struct mxs_dma_type *dma_type;
>  	struct mxs_dma_engine *mxs_dma;
>  	struct resource *iores;
> +	struct dma_pool *ccw_pool;
>  	int ret, i;
>  
>  	mxs_dma = devm_kzalloc(&pdev->dev, sizeof(*mxs_dma), GFP_KERNEL);
> @@ -843,7 +847,6 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  		tasklet_init(&mxs_chan->tasklet, mxs_dma_tasklet,
>  			     (unsigned long) mxs_chan);
>  
> -
>  		/* Add the channel to mxs_chan list */
>  		list_add_tail(&mxs_chan->chan.device_node,
>  			&mxs_dma->dma_device.channels);
> @@ -858,6 +861,17 @@ static int mxs_dma_probe(struct platform_device *pdev)
>  
>  	mxs_dma->dma_device.dev = &pdev->dev;
>  
> +	/* create the dma pool */
> +	ccw_pool = dma_pool_create("ccw_pool",
> +				   mxs_dma->dma_device.dev,
> +				   CCW_BLOCK_SIZE, 32, 0);
> +
> +	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
> +		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
> +
> +		mxs_chan->ccw_pool = ccw_pool;
> +	}

ccw_pool is the same for every channel, it should be a member of
struct mxs_dma_engine and not of struct mcs_dma_chan.

> +
>  	/* mxs_dma gets 65535 bytes maximum sg size */
>  	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
>  	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
> @@ -899,11 +913,13 @@ static int mxs_dma_remove(struct platform_device *pdev)
>  	int i;
>  
>  	dma_async_device_unregister(&mxs_dma->dma_device);
> +	dma_pool_destroy(mxs_dma->mxs_chans[0].ccw_pool);

It doesn't seem to make a big difference, but I would do this after
killing the tasklets, not before. Otherwise we would have to prove that
no tasklet is still accessing the dma_pool.

>  
>  	for (i = 0; i < MXS_DMA_CHANNELS; i++) {
>  		struct mxs_dma_chan *mxs_chan = &mxs_dma->mxs_chans[i];
>  
>  		tasklet_kill(&mxs_chan->tasklet);
> +		mxs_chan->ccw_pool = NULL;

There's no point in resetting this to NULL, mxs_chan is never going to
be touched again.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Han Xu]

several changes for runtime code in gpmi-nand driver

- Always invoke runtime get/put in same function to balance the usage
counter.

- leverage the runtime pm for system pm, move acquire dma to runtime pm
to acquire dma only when needed.

- add pm_runtime_dont_use_autosuspend in err path. If driver failed to
probe before runtime pm timeout, such as NAND not mounted in socket,
runtime suspend won't be called without the change.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index fcc7325f2a10..73644c96fa9b 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
 	 */
 	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
 
-	return 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
 	/* Set *all* chip selects to use layout 0. */
 	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
 
-	ret = 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
 	if (ret)
 		goto exit_regs;
 
-	ret = acquire_dma_channels(this);
-	if (ret)
-		goto exit_regs;
-
 	ret = gpmi_get_clks(this);
 	if (ret)
 		goto exit_clock;
@@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_acquire_resources;
 
-	ret = __gpmi_enable_clk(this, true);
-	if (ret)
-		goto exit_nfc_init;
-
+	pm_runtime_enable(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 
 	ret = gpmi_init(this);
 	if (ret)
@@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_nfc_init;
 
-	pm_runtime_mark_last_busy(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
-
 	dev_info(this->dev, "driver registered.\n");
 
 	return 0;
 
 exit_nfc_init:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	release_resources(this);
 exit_acquire_resources:
@@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 {
 	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
 
-	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	nand_release(&this->nand);
@@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int gpmi_pm_suspend(struct device *dev)
 {
-	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	release_dma_channels(this);
-	return 0;
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
 }
 
 static int gpmi_pm_resume(struct device *dev)
@@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 	int ret;
 
-	ret = acquire_dma_channels(this);
-	if (ret < 0)
+	ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		dev_err(this->dev, "Error in resume %d\n", ret);
 		return ret;
+	}
 
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
@@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
+#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
+#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)
+
+static int gpmi_runtime_suspend(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 
-	return __gpmi_enable_clk(this, false);
+	gpmi_disable_clk(this);
+	release_dma_channels(this);
+
+	return 0;
 }
 
-static int __maybe_unused gpmi_runtime_resume(struct device *dev)
+static int gpmi_runtime_resume(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	return __gpmi_enable_clk(this, true);
+	ret = gpmi_enable_clk(this);
+	if (ret)
+		return ret;
+
+	ret = acquire_dma_channels(this);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static const struct dev_pm_ops gpmi_pm_ops = {
-- 
2.17.1

[PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Han Xu]

several changes for runtime code in gpmi-nand driver

- Always invoke runtime get/put in same function to balance the usage
counter.

- leverage the runtime pm for system pm, move acquire dma to runtime pm
to acquire dma only when needed.

- add pm_runtime_dont_use_autosuspend in err path. If driver failed to
probe before runtime pm timeout, such as NAND not mounted in socket,
runtime suspend won't be called without the change.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index fcc7325f2a10..73644c96fa9b 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
 	 */
 	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
 
-	return 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
 	/* Set *all* chip selects to use layout 0. */
 	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
 
-	ret = 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
 	if (ret)
 		goto exit_regs;
 
-	ret = acquire_dma_channels(this);
-	if (ret)
-		goto exit_regs;
-
 	ret = gpmi_get_clks(this);
 	if (ret)
 		goto exit_clock;
@@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_acquire_resources;
 
-	ret = __gpmi_enable_clk(this, true);
-	if (ret)
-		goto exit_nfc_init;
-
+	pm_runtime_enable(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 
 	ret = gpmi_init(this);
 	if (ret)
@@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_nfc_init;
 
-	pm_runtime_mark_last_busy(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
-
 	dev_info(this->dev, "driver registered.\n");
 
 	return 0;
 
 exit_nfc_init:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	release_resources(this);
 exit_acquire_resources:
@@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 {
 	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
 
-	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	nand_release(&this->nand);
@@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int gpmi_pm_suspend(struct device *dev)
 {
-	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	release_dma_channels(this);
-	return 0;
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
 }
 
 static int gpmi_pm_resume(struct device *dev)
@@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 	int ret;
 
-	ret = acquire_dma_channels(this);
-	if (ret < 0)
+	ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		dev_err(this->dev, "Error in resume %d\n", ret);
 		return ret;
+	}
 
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
@@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
+#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
+#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)
+
+static int gpmi_runtime_suspend(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 
-	return __gpmi_enable_clk(this, false);
+	gpmi_disable_clk(this);
+	release_dma_channels(this);
+
+	return 0;
 }
 
-static int __maybe_unused gpmi_runtime_resume(struct device *dev)
+static int gpmi_runtime_resume(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	return __gpmi_enable_clk(this, true);
+	ret = gpmi_enable_clk(this);
+	if (ret)
+		return ret;
+
+	ret = acquire_dma_channels(this);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static const struct dev_pm_ops gpmi_pm_ops = {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Han Xu]

several changes for runtime code in gpmi-nand driver

- Always invoke runtime get/put in same function to balance the usage
counter.

- leverage the runtime pm for system pm, move acquire dma to runtime pm
to acquire dma only when needed.

- add pm_runtime_dont_use_autosuspend in err path. If driver failed to
probe before runtime pm timeout, such as NAND not mounted in socket,
runtime suspend won't be called without the change.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index fcc7325f2a10..73644c96fa9b 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
 	 */
 	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
 
-	return 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
 	/* Set *all* chip selects to use layout 0. */
 	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
 
-	ret = 0;
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
 	pm_runtime_put_autosuspend(this->dev);
@@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
 	if (ret)
 		goto exit_regs;
 
-	ret = acquire_dma_channels(this);
-	if (ret)
-		goto exit_regs;
-
 	ret = gpmi_get_clks(this);
 	if (ret)
 		goto exit_clock;
@@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_acquire_resources;
 
-	ret = __gpmi_enable_clk(this, true);
-	if (ret)
-		goto exit_nfc_init;
-
+	pm_runtime_enable(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 
 	ret = gpmi_init(this);
 	if (ret)
@@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_nfc_init;
 
-	pm_runtime_mark_last_busy(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
-
 	dev_info(this->dev, "driver registered.\n");
 
 	return 0;
 
 exit_nfc_init:
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	release_resources(this);
 exit_acquire_resources:
@@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 {
 	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
 
-	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	nand_release(&this->nand);
@@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int gpmi_pm_suspend(struct device *dev)
 {
-	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	release_dma_channels(this);
-	return 0;
+	ret = pm_runtime_force_suspend(dev);
+
+	return ret;
 }
 
 static int gpmi_pm_resume(struct device *dev)
@@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 	int ret;
 
-	ret = acquire_dma_channels(this);
-	if (ret < 0)
+	ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		dev_err(this->dev, "Error in resume %d\n", ret);
 		return ret;
+	}
 
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
@@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_SLEEP */
 
-static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
+#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
+#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)
+
+static int gpmi_runtime_suspend(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
 
-	return __gpmi_enable_clk(this, false);
+	gpmi_disable_clk(this);
+	release_dma_channels(this);
+
+	return 0;
 }
 
-static int __maybe_unused gpmi_runtime_resume(struct device *dev)
+static int gpmi_runtime_resume(struct device *dev)
 {
 	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
 
-	return __gpmi_enable_clk(this, true);
+	ret = gpmi_enable_clk(this);
+	if (ret)
+		return ret;
+
+	ret = acquire_dma_channels(this);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static const struct dev_pm_ops gpmi_pm_ops = {
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:02AM +0800, Han Xu wrote:
> several changes for runtime code in gpmi-nand driver
> 
> - Always invoke runtime get/put in same function to balance the usage
> counter.
> 
> - leverage the runtime pm for system pm, move acquire dma to runtime pm
> to acquire dma only when needed.
> 
> - add pm_runtime_dont_use_autosuspend in err path. If driver failed to
> probe before runtime pm timeout, such as NAND not mounted in socket,
> runtime suspend won't be called without the change.

Using a bullet list in a commit message is often a sign that the patch
should be split into multiple patches...

> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index fcc7325f2a10..73644c96fa9b 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
>  	 */
>  	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
>  
> -	return 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);
> @@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
>  	/* Set *all* chip selects to use layout 0. */
>  	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>  
> -	ret = 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);

While I agree that this "ret = 0" is unnecessary because 'ret' holds the
successful return value of the last function called, I still think it's
nice to make it explicit that this is the success path of this function.

If you disagree please at least make this a separate patch.

> @@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
>  	if (ret)
>  		goto exit_regs;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret)
> -		goto exit_regs;
> -
>  	ret = gpmi_get_clks(this);
>  	if (ret)
>  		goto exit_clock;
> @@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_acquire_resources;
>  
> -	ret = __gpmi_enable_clk(this, true);
> -	if (ret)
> -		goto exit_nfc_init;
> -
> +	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
>  	pm_runtime_use_autosuspend(&pdev->dev);
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
>  
>  	ret = gpmi_init(this);
>  	if (ret)
> @@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_nfc_init;
>  
> -	pm_runtime_mark_last_busy(&pdev->dev);
> -	pm_runtime_put_autosuspend(&pdev->dev);
> -
>  	dev_info(this->dev, "driver registered.\n");
>  
>  	return 0;
>  
>  exit_nfc_init:
> -	pm_runtime_put(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	release_resources(this);
>  exit_acquire_resources:
> @@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  {
>  	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
>  
> -	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	nand_release(&this->nand);
> @@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  #ifdef CONFIG_PM_SLEEP
>  static int gpmi_pm_suspend(struct device *dev)
>  {
> -	struct gpmi_nand_data *this = dev_get_drvdata(dev);
> +	int ret;
>  
> -	release_dma_channels(this);
> -	return 0;
> +	ret = pm_runtime_force_suspend(dev);
> +
> +	return ret;
>  }
>  
>  static int gpmi_pm_resume(struct device *dev)
> @@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
>  	struct gpmi_nand_data *this = dev_get_drvdata(dev);
>  	int ret;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret < 0)
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret) {
> +		dev_err(this->dev, "Error in resume %d\n", ret);
>  		return ret;
> +	}
>  
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
> @@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  
> -static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
> +#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
> +#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)

These defines do not add any value.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:02AM +0800, Han Xu wrote:
> several changes for runtime code in gpmi-nand driver
> 
> - Always invoke runtime get/put in same function to balance the usage
> counter.
> 
> - leverage the runtime pm for system pm, move acquire dma to runtime pm
> to acquire dma only when needed.
> 
> - add pm_runtime_dont_use_autosuspend in err path. If driver failed to
> probe before runtime pm timeout, such as NAND not mounted in socket,
> runtime suspend won't be called without the change.

Using a bullet list in a commit message is often a sign that the patch
should be split into multiple patches...

> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index fcc7325f2a10..73644c96fa9b 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
>  	 */
>  	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
>  
> -	return 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);
> @@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
>  	/* Set *all* chip selects to use layout 0. */
>  	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>  
> -	ret = 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);

While I agree that this "ret = 0" is unnecessary because 'ret' holds the
successful return value of the last function called, I still think it's
nice to make it explicit that this is the success path of this function.

If you disagree please at least make this a separate patch.

> @@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
>  	if (ret)
>  		goto exit_regs;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret)
> -		goto exit_regs;
> -
>  	ret = gpmi_get_clks(this);
>  	if (ret)
>  		goto exit_clock;
> @@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_acquire_resources;
>  
> -	ret = __gpmi_enable_clk(this, true);
> -	if (ret)
> -		goto exit_nfc_init;
> -
> +	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
>  	pm_runtime_use_autosuspend(&pdev->dev);
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
>  
>  	ret = gpmi_init(this);
>  	if (ret)
> @@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_nfc_init;
>  
> -	pm_runtime_mark_last_busy(&pdev->dev);
> -	pm_runtime_put_autosuspend(&pdev->dev);
> -
>  	dev_info(this->dev, "driver registered.\n");
>  
>  	return 0;
>  
>  exit_nfc_init:
> -	pm_runtime_put(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	release_resources(this);
>  exit_acquire_resources:
> @@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  {
>  	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
>  
> -	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	nand_release(&this->nand);
> @@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  #ifdef CONFIG_PM_SLEEP
>  static int gpmi_pm_suspend(struct device *dev)
>  {
> -	struct gpmi_nand_data *this = dev_get_drvdata(dev);
> +	int ret;
>  
> -	release_dma_channels(this);
> -	return 0;
> +	ret = pm_runtime_force_suspend(dev);
> +
> +	return ret;
>  }
>  
>  static int gpmi_pm_resume(struct device *dev)
> @@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
>  	struct gpmi_nand_data *this = dev_get_drvdata(dev);
>  	int ret;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret < 0)
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret) {
> +		dev_err(this->dev, "Error in resume %d\n", ret);
>  		return ret;
> +	}
>  
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
> @@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  
> -static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
> +#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
> +#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)

These defines do not add any value.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] mtd: rawnand: gpmi: refine the runtime pm ops

[Sascha Hauer]

On Wed, Jan 15, 2020 at 05:44:02AM +0800, Han Xu wrote:
> several changes for runtime code in gpmi-nand driver
> 
> - Always invoke runtime get/put in same function to balance the usage
> counter.
> 
> - leverage the runtime pm for system pm, move acquire dma to runtime pm
> to acquire dma only when needed.
> 
> - add pm_runtime_dont_use_autosuspend in err path. If driver failed to
> probe before runtime pm timeout, such as NAND not mounted in socket,
> runtime suspend won't be called without the change.

Using a bullet list in a commit message is often a sign that the patch
should be split into multiple patches...

> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 56 +++++++++++-----------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index fcc7325f2a10..73644c96fa9b 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -183,7 +183,6 @@ static int gpmi_init(struct gpmi_nand_data *this)
>  	 */
>  	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
>  
> -	return 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);
> @@ -556,7 +555,6 @@ static int bch_set_geometry(struct gpmi_nand_data *this)
>  	/* Set *all* chip selects to use layout 0. */
>  	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>  
> -	ret = 0;
>  err_out:
>  	pm_runtime_mark_last_busy(this->dev);
>  	pm_runtime_put_autosuspend(this->dev);

While I agree that this "ret = 0" is unnecessary because 'ret' holds the
successful return value of the last function called, I still think it's
nice to make it explicit that this is the success path of this function.

If you disagree please at least make this a separate patch.

> @@ -1213,10 +1211,6 @@ static int acquire_resources(struct gpmi_nand_data *this)
>  	if (ret)
>  		goto exit_regs;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret)
> -		goto exit_regs;
> -
>  	ret = gpmi_get_clks(this);
>  	if (ret)
>  		goto exit_clock;
> @@ -2656,15 +2650,9 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_acquire_resources;
>  
> -	ret = __gpmi_enable_clk(this, true);
> -	if (ret)
> -		goto exit_nfc_init;
> -
> +	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
>  	pm_runtime_use_autosuspend(&pdev->dev);
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> -	pm_runtime_get_sync(&pdev->dev);
>  
>  	ret = gpmi_init(this);
>  	if (ret)
> @@ -2674,15 +2662,12 @@ static int gpmi_nand_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto exit_nfc_init;
>  
> -	pm_runtime_mark_last_busy(&pdev->dev);
> -	pm_runtime_put_autosuspend(&pdev->dev);
> -
>  	dev_info(this->dev, "driver registered.\n");
>  
>  	return 0;
>  
>  exit_nfc_init:
> -	pm_runtime_put(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	release_resources(this);
>  exit_acquire_resources:
> @@ -2694,7 +2679,6 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  {
>  	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
>  
> -	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	nand_release(&this->nand);
> @@ -2706,10 +2690,11 @@ static int gpmi_nand_remove(struct platform_device *pdev)
>  #ifdef CONFIG_PM_SLEEP
>  static int gpmi_pm_suspend(struct device *dev)
>  {
> -	struct gpmi_nand_data *this = dev_get_drvdata(dev);
> +	int ret;
>  
> -	release_dma_channels(this);
> -	return 0;
> +	ret = pm_runtime_force_suspend(dev);
> +
> +	return ret;
>  }
>  
>  static int gpmi_pm_resume(struct device *dev)
> @@ -2717,9 +2702,11 @@ static int gpmi_pm_resume(struct device *dev)
>  	struct gpmi_nand_data *this = dev_get_drvdata(dev);
>  	int ret;
>  
> -	ret = acquire_dma_channels(this);
> -	if (ret < 0)
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret) {
> +		dev_err(this->dev, "Error in resume %d\n", ret);
>  		return ret;
> +	}
>  
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
> @@ -2743,18 +2730,33 @@ static int gpmi_pm_resume(struct device *dev)
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  
> -static int __maybe_unused gpmi_runtime_suspend(struct device *dev)
> +#define gpmi_enable_clk(x)	__gpmi_enable_clk(x, true)
> +#define gpmi_disable_clk(x)	__gpmi_enable_clk(x, false)

These defines do not add any value.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Han Xu]

set the correct pinctrl state in system pm suspend/resume ops

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 73644c96fa9b..de1e3dbb2eb1 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/dma/mxs-dma.h>
 #include "gpmi-nand.h"
 #include "gpmi-regs.h"
@@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
 {
 	int ret;
 
+	pinctrl_pm_select_sleep_state(dev);
 	ret = pm_runtime_force_suspend(dev);
 
 	return ret;
@@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
 		return ret;
 	}
 
+	pinctrl_pm_select_default_state(dev);
+
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
 	if (ret) {
-- 
2.17.1

[PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Han Xu]

set the correct pinctrl state in system pm suspend/resume ops

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 73644c96fa9b..de1e3dbb2eb1 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/dma/mxs-dma.h>
 #include "gpmi-nand.h"
 #include "gpmi-regs.h"
@@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
 {
 	int ret;
 
+	pinctrl_pm_select_sleep_state(dev);
 	ret = pm_runtime_force_suspend(dev);
 
 	return ret;
@@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
 		return ret;
 	}
 
+	pinctrl_pm_select_default_state(dev);
+
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
 	if (ret) {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Han Xu]

set the correct pinctrl state in system pm suspend/resume ops

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 73644c96fa9b..de1e3dbb2eb1 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/dma/mxs-dma.h>
 #include "gpmi-nand.h"
 #include "gpmi-regs.h"
@@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
 {
 	int ret;
 
+	pinctrl_pm_select_sleep_state(dev);
 	ret = pm_runtime_force_suspend(dev);
 
 	return ret;
@@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
 		return ret;
 	}
 
+	pinctrl_pm_select_default_state(dev);
+
 	/* re-init the GPMI registers */
 	ret = gpmi_init(this);
 	if (ret) {
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Esben Haabendal]

Han Xu <han.xu@nxp.com> writes:

> set the correct pinctrl state in system pm suspend/resume ops
>
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 73644c96fa9b..de1e3dbb2eb1 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -15,6 +15,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/dma/mxs-dma.h>
>  #include "gpmi-nand.h"
>  #include "gpmi-regs.h"
> @@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
>  {
>  	int ret;
>  
> +	pinctrl_pm_select_sleep_state(dev);
>  	ret = pm_runtime_force_suspend(dev);
>  
>  	return ret;
> @@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	pinctrl_pm_select_default_state(dev);
> +
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
>  	if (ret) {

Acked-by: Esben Haabendal <esben@geanix.com>

Re: [PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Esben Haabendal]

Han Xu <han.xu@nxp.com> writes:

> set the correct pinctrl state in system pm suspend/resume ops
>
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 73644c96fa9b..de1e3dbb2eb1 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -15,6 +15,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/dma/mxs-dma.h>
>  #include "gpmi-nand.h"
>  #include "gpmi-regs.h"
> @@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
>  {
>  	int ret;
>  
> +	pinctrl_pm_select_sleep_state(dev);
>  	ret = pm_runtime_force_suspend(dev);
>  
>  	return ret;
> @@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	pinctrl_pm_select_default_state(dev);
> +
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
>  	if (ret) {

Acked-by: Esben Haabendal <esben@geanix.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] mtd: rawnand: gpmi: set the pinctrl state for suspend/reusme

[Esben Haabendal]

Han Xu <han.xu@nxp.com> writes:

> set the correct pinctrl state in system pm suspend/resume ops
>
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 73644c96fa9b..de1e3dbb2eb1 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -15,6 +15,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/dma/mxs-dma.h>
>  #include "gpmi-nand.h"
>  #include "gpmi-regs.h"
> @@ -2692,6 +2693,7 @@ static int gpmi_pm_suspend(struct device *dev)
>  {
>  	int ret;
>  
> +	pinctrl_pm_select_sleep_state(dev);
>  	ret = pm_runtime_force_suspend(dev);
>  
>  	return ret;
> @@ -2708,6 +2710,8 @@ static int gpmi_pm_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	pinctrl_pm_select_default_state(dev);
> +
>  	/* re-init the GPMI registers */
>  	ret = gpmi_init(this);
>  	if (ret) {

Acked-by: Esben Haabendal <esben@geanix.com>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/