[PATCH 0/2] ARM: davinci: cleanup and rework PM initializaion

[PATCH 0/2] ARM: davinci: cleanup and rework PM initializaion

[Kevin Hilman]

This series removes the fake platform_device used to initialize PM on
legacy davinci platforms, and adds PM support for DT platforms.

Sekhar: the 2nd patch should probably be squashed into the first upon
approval, but was kept separated in order to make the functional
changes in the first patch more readable.

Tested with legacy boot on da850-evm, and DT boot on da850-lcdk.  Used
RTC to wake from suspend-to-RAM: e.g. rtcwake -m mem -s 4 -d /dev/rtc0

Kevin Hilman (2):
  ARM: davinci: PM: rework init, support DT platforms
  ARM: davinci: PM: rework init: cleanup

 arch/arm/mach-davinci/board-da850-evm.c    | 17 +----
 arch/arm/mach-davinci/board-mityomapl138.c | 16 +----
 arch/arm/mach-davinci/common.c             |  1 -
 arch/arm/mach-davinci/da850.c              | 38 ------------
 arch/arm/mach-davinci/da8xx-dt.c           |  1 +
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 -
 arch/arm/mach-davinci/pm.c                 | 99 +++++++++++++++++-------------
 7 files changed, 58 insertions(+), 115 deletions(-)

-- 
2.9.3

[PATCH 1/2] ARM: davinci: PM: rework init, support DT platforms

[Kevin Hilman]

Remove fake platform device used for PM init.  Move pdata values which
are common across all current platforms into pm.c.

Also add PM support for DT platforms (vi da8xx-dt.c)

Suggested-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/mach-davinci/board-da850-evm.c    | 17 +-------
 arch/arm/mach-davinci/board-mityomapl138.c | 16 +-------
 arch/arm/mach-davinci/common.c             |  1 -
 arch/arm/mach-davinci/da850.c              | 38 ------------------
 arch/arm/mach-davinci/da8xx-dt.c           |  1 +
 arch/arm/mach-davinci/include/mach/da8xx.h |  1 -
 arch/arm/mach-davinci/pm.c                 | 64 ++++++++++++++++++------------
 7 files changed, 41 insertions(+), 97 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 8e4539f69fdc..4e88723c23c7 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -196,18 +196,6 @@ static struct platform_device da850_evm_norflash_device = {
 	.resource	= da850_evm_norflash_resource,
 };
 
-static struct davinci_pm_config da850_pm_pdata = {
-	.sleepcount = 128,
-};
-
-static struct platform_device da850_pm_device = {
-	.name           = "pm-davinci",
-	.dev = {
-		.platform_data	= &da850_pm_pdata,
-	},
-	.id             = -1,
-};
-
 /* DA850/OMAP-L138 EVM includes a 512 MByte large-page NAND flash
  * (128K blocks). It may be used instead of the (default) SPI flash
  * to boot, using TI's tools to install the secondary boot loader
@@ -1453,10 +1441,7 @@ static __init void da850_evm_init(void)
 	if (ret)
 		pr_warn("%s: cpuidle registration failed: %d\n", __func__, ret);
 
-	ret = da850_register_pm(&da850_pm_device);
-	if (ret)
-		pr_warn("%s: suspend registration failed: %d\n", __func__, ret);
-
+	davinci_pm_init();
 	da850_vpif_init();
 
 	ret = spi_register_board_info(da850evm_spi_info,
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index bc4e63fa9808..9e7388ba413c 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -498,18 +498,6 @@ static void __init mityomapl138_config_emac(void)
 		pr_warn("emac registration failed: %d\n", ret);
 }
 
-static struct davinci_pm_config da850_pm_pdata = {
-	.sleepcount = 128,
-};
-
-static struct platform_device da850_pm_device = {
-	.name	= "pm-davinci",
-	.dev = {
-		.platform_data  = &da850_pm_pdata,
-	},
-	.id	= -1,
-};
-
 static void __init mityomapl138_init(void)
 {
 	int ret;
@@ -555,9 +543,7 @@ static void __init mityomapl138_init(void)
 	if (ret)
 		pr_warn("cpuidle registration failed: %d\n", ret);
 
-	ret = da850_register_pm(&da850_pm_device);
-	if (ret)
-		pr_warn("suspend registration failed: %d\n", ret);
+	davinci_pm_init();
 }
 
 #ifdef CONFIG_SERIAL_8250_CONSOLE
diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
index 049025f6d531..9f9fbfa6da0d 100644
--- a/arch/arm/mach-davinci/common.c
+++ b/arch/arm/mach-davinci/common.c
@@ -118,6 +118,5 @@ void __init davinci_common_init(struct davinci_soc_info *soc_info)
 void __init davinci_init_late(void)
 {
 	davinci_cpufreq_init();
-	davinci_pm_init();
 	davinci_clk_disable_unused();
 }
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index ed3d0e9f72ac..28771923199f 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1172,44 +1172,6 @@ static int da850_round_armrate(struct clk *clk, unsigned long rate)
 }
 #endif
 
-int __init da850_register_pm(struct platform_device *pdev)
-{
-	int ret;
-	struct davinci_pm_config *pdata = pdev->dev.platform_data;
-
-	ret = davinci_cfg_reg(DA850_RTC_ALARM);
-	if (ret)
-		return ret;
-
-	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
-	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
-	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;
-
-	pdata->cpupll_reg_base = ioremap(DA8XX_PLL0_BASE, SZ_4K);
-	if (!pdata->cpupll_reg_base)
-		return -ENOMEM;
-
-	pdata->ddrpll_reg_base = ioremap(DA850_PLL1_BASE, SZ_4K);
-	if (!pdata->ddrpll_reg_base) {
-		ret = -ENOMEM;
-		goto no_ddrpll_mem;
-	}
-
-	pdata->ddrpsc_reg_base = ioremap(DA8XX_PSC1_BASE, SZ_4K);
-	if (!pdata->ddrpsc_reg_base) {
-		ret = -ENOMEM;
-		goto no_ddrpsc_mem;
-	}
-
-	return platform_device_register(pdev);
-
-no_ddrpsc_mem:
-	iounmap(pdata->ddrpll_reg_base);
-no_ddrpll_mem:
-	iounmap(pdata->cpupll_reg_base);
-	return ret;
-}
-
 /* VPIF resource, platform data */
 static u64 da850_vpif_dma_mask = DMA_BIT_MASK(32);
 
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e9274aa8..aed44dcdfd30 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -46,6 +46,7 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 static void __init da850_init_machine(void)
 {
 	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
+	davinci_pm_init();
 }
 
 static const char *const da850_boards_compat[] __initconst = {
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index f9f9713aacdd..3d7a13789661 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -101,7 +101,6 @@ int da8xx_register_gpio(void *pdata);
 int da850_register_cpufreq(char *async_clk);
 int da8xx_register_cpuidle(void);
 void __iomem *da8xx_get_mem_ctlr(void);
-int da850_register_pm(struct platform_device *pdev);
 int da850_register_sata(unsigned long refclkpn);
 int da850_register_vpif(void);
 int da850_register_vpif_display
diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c
index 8929569b1f8a..fc6a5710b3fa 100644
--- a/arch/arm/mach-davinci/pm.c
+++ b/arch/arm/mach-davinci/pm.c
@@ -23,13 +23,18 @@
 #include <mach/da8xx.h>
 #include "sram.h"
 #include <mach/pm.h>
+#include <mach/mux.h>
 
 #include "clock.h"
+#include "psc.h"
 
+#define DA850_PLL1_BASE		0x01e1a000
 #define DEEPSLEEP_SLEEPCOUNT_MASK	0xFFFF
+#define DEEPSLEEP_SLEEPCOUNT		128
 
 static void (*davinci_sram_suspend) (struct davinci_pm_config *);
-static struct davinci_pm_config *pdata;
+static struct davinci_pm_config pm_config;
+static struct davinci_pm_config *pdata = &pm_config;
 
 static void davinci_sram_push(void *dest, void *src, unsigned int size)
 {
@@ -117,17 +122,38 @@ static const struct platform_suspend_ops davinci_pm_ops = {
 	.valid		= suspend_valid_only_mem,
 };
 
-static int __init davinci_pm_probe(struct platform_device *pdev)
+int __init davinci_pm_init(void)
 {
-	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(&pdev->dev, "cannot get platform data\n");
-		return -ENOENT;
+	int ret;
+
+	ret = davinci_cfg_reg(DA850_RTC_ALARM);
+	if (ret)
+		return ret;
+
+	pdata->sleepcount = DEEPSLEEP_SLEEPCOUNT;
+	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
+	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
+	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;
+
+	pdata->cpupll_reg_base = ioremap(DA8XX_PLL0_BASE, SZ_4K);
+	if (!pdata->cpupll_reg_base)
+		return -ENOMEM;
+
+	pdata->ddrpll_reg_base = ioremap(DA850_PLL1_BASE, SZ_4K);
+	if (!pdata->ddrpll_reg_base) {
+		ret = -ENOMEM;
+		goto no_ddrpll_mem;
+	}
+
+	pdata->ddrpsc_reg_base = ioremap(DA8XX_PSC1_BASE, SZ_4K);
+	if (!pdata->ddrpsc_reg_base) {
+		ret = -ENOMEM;
+		goto no_ddrpsc_mem;
 	}
 
 	davinci_sram_suspend = sram_alloc(davinci_cpu_suspend_sz, NULL);
 	if (!davinci_sram_suspend) {
-		dev_err(&pdev->dev, "cannot allocate SRAM memory\n");
+		pr_err("PM: cannot allocate SRAM memory\n");
 		return -ENOMEM;
 	}
 
@@ -136,23 +162,9 @@ static int __init davinci_pm_probe(struct platform_device *pdev)
 
 	suspend_set_ops(&davinci_pm_ops);
 
-	return 0;
-}
-
-static int __exit davinci_pm_remove(struct platform_device *pdev)
-{
-	sram_free(davinci_sram_suspend, davinci_cpu_suspend_sz);
-	return 0;
-}
-
-static struct platform_driver davinci_pm_driver = {
-	.driver = {
-		.name	 = "pm-davinci",
-	},
-	.remove = __exit_p(davinci_pm_remove),
-};
-
-int __init davinci_pm_init(void)
-{
-	return platform_driver_probe(&davinci_pm_driver, davinci_pm_probe);
+no_ddrpsc_mem:
+	iounmap(pdata->ddrpll_reg_base);
+no_ddrpll_mem:
+	iounmap(pdata->cpupll_reg_base);
+	return ret;
 }
-- 
2.9.3

[PATCH 2/2] ARM: davinci: PM: rework init: cleanup

[Kevin Hilman]

A follow-on cleanup renaming 'pdata' since it is no longer
platform_data.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/mach-davinci/pm.c | 61 +++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c
index fc6a5710b3fa..1f2ac36efe11 100644
--- a/arch/arm/mach-davinci/pm.c
+++ b/arch/arm/mach-davinci/pm.c
@@ -34,7 +34,6 @@
 
 static void (*davinci_sram_suspend) (struct davinci_pm_config *);
 static struct davinci_pm_config pm_config;
-static struct davinci_pm_config *pdata = &pm_config;
 
 static void davinci_sram_push(void *dest, void *src, unsigned int size)
 {
@@ -46,58 +45,58 @@ static void davinci_pm_suspend(void)
 {
 	unsigned val;
 
-	if (pdata->cpupll_reg_base != pdata->ddrpll_reg_base) {
+	if (pm_config.cpupll_reg_base != pm_config.ddrpll_reg_base) {
 
 		/* Switch CPU PLL to bypass mode */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val &= ~(PLLCTL_PLLENSRC | PLLCTL_PLLEN);
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 
 		udelay(PLL_BYPASS_TIME);
 
 		/* Powerdown CPU PLL */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val |= PLLCTL_PLLPWRDN;
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 	}
 
 	/* Configure sleep count in deep sleep register */
-	val = __raw_readl(pdata->deepsleep_reg);
+	val = __raw_readl(pm_config.deepsleep_reg);
 	val &= ~DEEPSLEEP_SLEEPCOUNT_MASK,
-	val |= pdata->sleepcount;
-	__raw_writel(val, pdata->deepsleep_reg);
+	val |= pm_config.sleepcount;
+	__raw_writel(val, pm_config.deepsleep_reg);
 
 	/* System goes to sleep in this call */
-	davinci_sram_suspend(pdata);
+	davinci_sram_suspend(&pm_config);
 
-	if (pdata->cpupll_reg_base != pdata->ddrpll_reg_base) {
+	if (pm_config.cpupll_reg_base != pm_config.ddrpll_reg_base) {
 
 		/* put CPU PLL in reset */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val &= ~PLLCTL_PLLRST;
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 
 		/* put CPU PLL in power down */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val &= ~PLLCTL_PLLPWRDN;
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 
 		/* wait for CPU PLL reset */
 		udelay(PLL_RESET_TIME);
 
 		/* bring CPU PLL out of reset */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val |= PLLCTL_PLLRST;
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 
 		/* Wait for CPU PLL to lock */
 		udelay(PLL_LOCK_TIME);
 
 		/* Remove CPU PLL from bypass mode */
-		val = __raw_readl(pdata->cpupll_reg_base + PLLCTL);
+		val = __raw_readl(pm_config.cpupll_reg_base + PLLCTL);
 		val &= ~PLLCTL_PLLENSRC;
 		val |= PLLCTL_PLLEN;
-		__raw_writel(val, pdata->cpupll_reg_base + PLLCTL);
+		__raw_writel(val, pm_config.cpupll_reg_base + PLLCTL);
 	}
 }
 
@@ -130,23 +129,23 @@ int __init davinci_pm_init(void)
 	if (ret)
 		return ret;
 
-	pdata->sleepcount = DEEPSLEEP_SLEEPCOUNT;
-	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
-	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
-	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;
+	pm_config.sleepcount = DEEPSLEEP_SLEEPCOUNT;
+	pm_config.ddr2_ctlr_base = da8xx_get_mem_ctlr();
+	pm_config.deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
+	pm_config.ddrpsc_num = DA8XX_LPSC1_EMIF3C;
 
-	pdata->cpupll_reg_base = ioremap(DA8XX_PLL0_BASE, SZ_4K);
-	if (!pdata->cpupll_reg_base)
+	pm_config.cpupll_reg_base = ioremap(DA8XX_PLL0_BASE, SZ_4K);
+	if (!pm_config.cpupll_reg_base)
 		return -ENOMEM;
 
-	pdata->ddrpll_reg_base = ioremap(DA850_PLL1_BASE, SZ_4K);
-	if (!pdata->ddrpll_reg_base) {
+	pm_config.ddrpll_reg_base = ioremap(DA850_PLL1_BASE, SZ_4K);
+	if (!pm_config.ddrpll_reg_base) {
 		ret = -ENOMEM;
 		goto no_ddrpll_mem;
 	}
 
-	pdata->ddrpsc_reg_base = ioremap(DA8XX_PSC1_BASE, SZ_4K);
-	if (!pdata->ddrpsc_reg_base) {
+	pm_config.ddrpsc_reg_base = ioremap(DA8XX_PSC1_BASE, SZ_4K);
+	if (!pm_config.ddrpsc_reg_base) {
 		ret = -ENOMEM;
 		goto no_ddrpsc_mem;
 	}
@@ -163,8 +162,8 @@ int __init davinci_pm_init(void)
 	suspend_set_ops(&davinci_pm_ops);
 
 no_ddrpsc_mem:
-	iounmap(pdata->ddrpll_reg_base);
+	iounmap(pm_config.ddrpll_reg_base);
 no_ddrpll_mem:
-	iounmap(pdata->cpupll_reg_base);
+	iounmap(pm_config.cpupll_reg_base);
 	return ret;
 }
-- 
2.9.3

[PATCH 1/2] ARM: davinci: PM: rework init, support DT platforms

[Sekhar Nori]

Hi Kevin,

Looks good to me overall, I have some minor comments.

On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
> Remove fake platform device used for PM init.  Move pdata values which
> are common across all current platforms into pm.c.
> 
> Also add PM support for DT platforms (vi da8xx-dt.c)

Can you please separate out PM enabling on DT platform to a separate
patch? Its a small change, but it will be nice to separate it from rest
of the cleanup.

> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
> index f9f9713aacdd..3d7a13789661 100644
> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
> @@ -101,7 +101,6 @@ int da8xx_register_gpio(void *pdata);
>  int da850_register_cpufreq(char *async_clk);
>  int da8xx_register_cpuidle(void);
>  void __iomem *da8xx_get_mem_ctlr(void);
> -int da850_register_pm(struct platform_device *pdev);
>  int da850_register_sata(unsigned long refclkpn);
>  int da850_register_vpif(void);
>  int da850_register_vpif_display
> diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c
> index 8929569b1f8a..fc6a5710b3fa 100644
> --- a/arch/arm/mach-davinci/pm.c
> +++ b/arch/arm/mach-davinci/pm.c
> @@ -23,13 +23,18 @@
>  #include <mach/da8xx.h>
>  #include "sram.h"
>  #include <mach/pm.h>
> +#include <mach/mux.h>

Can you please add the mux.h inclusion above pm.h? Looks like the sram.h
inclusion is out of place already, but since you are touching this part,
can you please move it below along with rest of the local includes.

I see that linux/ includes are not sorted as well, but lets keep that
aside until someone needs to touch them.

>  #include "clock.h"
> +#include "psc.h"
>  
> +#define DA850_PLL1_BASE		0x01e1a000
>  #define DEEPSLEEP_SLEEPCOUNT_MASK	0xFFFF
> +#define DEEPSLEEP_SLEEPCOUNT		128
>  
>  static void (*davinci_sram_suspend) (struct davinci_pm_config *);
> -static struct davinci_pm_config *pdata;
> +static struct davinci_pm_config pm_config;
> +static struct davinci_pm_config *pdata = &pm_config;
>  
>  static void davinci_sram_push(void *dest, void *src, unsigned int size)
>  {
> @@ -117,17 +122,38 @@ static const struct platform_suspend_ops davinci_pm_ops = {
>  	.valid		= suspend_valid_only_mem,
>  };
>  
> -static int __init davinci_pm_probe(struct platform_device *pdev)
> +int __init davinci_pm_init(void)
>  {
> -	pdata = pdev->dev.platform_data;
> -	if (!pdata) {
> -		dev_err(&pdev->dev, "cannot get platform data\n");
> -		return -ENOENT;
> +	int ret;
> +
> +	ret = davinci_cfg_reg(DA850_RTC_ALARM);
> +	if (ret)
> +		return ret;
> +
> +	pdata->sleepcount = DEEPSLEEP_SLEEPCOUNT;
> +	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
> +	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
> +	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;

Some of these could be statically initialized in pm_config. Can you
please move the constants to static initialization.

Thanks,
Sekhar

[PATCH 2/2] ARM: davinci: PM: rework init: cleanup

[Sekhar Nori]

On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
> A follow-on cleanup renaming 'pdata' since it is no longer
> platform_data.
> 
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>

Looks good to me. Thanks for separating this out. Makes it much easier
to review.

In the subject line though, instead of just calling it cleanup, how
about a bit more descriptive statement like "remove references to pdata".

Thanks,
Sekhar

[PATCH 1/2] ARM: davinci: PM: rework init, support DT platforms

[Kevin Hilman]

Sekhar Nori <nsekhar@ti.com> writes:

> Hi Kevin,
>
> Looks good to me overall, I have some minor comments.
>
> On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
>> Remove fake platform device used for PM init.  Move pdata values which
>> are common across all current platforms into pm.c.
>> 
>> Also add PM support for DT platforms (vi da8xx-dt.c)
>
> Can you please separate out PM enabling on DT platform to a separate
> patch? Its a small change, but it will be nice to separate it from rest
> of the cleanup.

Yes, will do.

>> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
>> index f9f9713aacdd..3d7a13789661 100644
>> --- a/arch/arm/mach-davinci/include/mach/da8xx.h
>> +++ b/arch/arm/mach-davinci/include/mach/da8xx.h
>> @@ -101,7 +101,6 @@ int da8xx_register_gpio(void *pdata);
>>  int da850_register_cpufreq(char *async_clk);
>>  int da8xx_register_cpuidle(void);
>>  void __iomem *da8xx_get_mem_ctlr(void);
>> -int da850_register_pm(struct platform_device *pdev);
>>  int da850_register_sata(unsigned long refclkpn);
>>  int da850_register_vpif(void);
>>  int da850_register_vpif_display
>> diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c
>> index 8929569b1f8a..fc6a5710b3fa 100644
>> --- a/arch/arm/mach-davinci/pm.c
>> +++ b/arch/arm/mach-davinci/pm.c
>> @@ -23,13 +23,18 @@
>>  #include <mach/da8xx.h>
>>  #include "sram.h"
>>  #include <mach/pm.h>
>> +#include <mach/mux.h>
>
> Can you please add the mux.h inclusion above pm.h? Looks like the sram.h
> inclusion is out of place already, but since you are touching this part,
> can you please move it below along with rest of the local includes.
>
> I see that linux/ includes are not sorted as well, but lets keep that
> aside until someone needs to touch them.
>
>>  #include "clock.h"
>> +#include "psc.h"
>>  
>> +#define DA850_PLL1_BASE		0x01e1a000
>>  #define DEEPSLEEP_SLEEPCOUNT_MASK	0xFFFF
>> +#define DEEPSLEEP_SLEEPCOUNT		128
>>  
>>  static void (*davinci_sram_suspend) (struct davinci_pm_config *);
>> -static struct davinci_pm_config *pdata;
>> +static struct davinci_pm_config pm_config;
>> +static struct davinci_pm_config *pdata = &pm_config;
>>  
>>  static void davinci_sram_push(void *dest, void *src, unsigned int size)
>>  {
>> @@ -117,17 +122,38 @@ static const struct platform_suspend_ops davinci_pm_ops = {
>>  	.valid		= suspend_valid_only_mem,
>>  };
>>  
>> -static int __init davinci_pm_probe(struct platform_device *pdev)
>> +int __init davinci_pm_init(void)
>>  {
>> -	pdata = pdev->dev.platform_data;
>> -	if (!pdata) {
>> -		dev_err(&pdev->dev, "cannot get platform data\n");
>> -		return -ENOENT;
>> +	int ret;
>> +
>> +	ret = davinci_cfg_reg(DA850_RTC_ALARM);
>> +	if (ret)
>> +		return ret;
>> +
>> +	pdata->sleepcount = DEEPSLEEP_SLEEPCOUNT;
>> +	pdata->ddr2_ctlr_base = da8xx_get_mem_ctlr();
>> +	pdata->deepsleep_reg = DA8XX_SYSCFG1_VIRT(DA8XX_DEEPSLEEP_REG);
>> +	pdata->ddrpsc_num = DA8XX_LPSC1_EMIF3C;
>
> Some of these could be statically initialized in pm_config. Can you
> please move the constants to static initialization.

Yes.

Kevin

[PATCH 2/2] ARM: davinci: PM: rework init: cleanup

[Kevin Hilman]

Sekhar Nori <nsekhar@ti.com> writes:

> On Tuesday 15 November 2016 04:34 AM, Kevin Hilman wrote:
>> A follow-on cleanup renaming 'pdata' since it is no longer
>> platform_data.
>> 
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>
> Looks good to me. Thanks for separating this out. Makes it much easier
> to review.
>
> In the subject line though, instead of just calling it cleanup, how
> about a bit more descriptive statement like "remove references to pdata".

Sure.  respin coming right up.

Kevin