| Message ID | 1404878455-31518-4-git-send-email-pankaj.dubey@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Pankaj Dubey wrote: > > This patch modifies Exynos Power Management Unit (PMU) initialization > implementation in following way: > > - Added platform driver support and probe function where Exynos PMU > driver will register itself as syscon provider with syscon framework. > - Added platform struct exynos_pmu_data to hold platform specific data. > - For each SoC's PMU support now we can add platform data and statically > bind PMU configuration and SoC specific initialization function. > - Separate each SoC's PMU initialization function and make it as part of > platform data. > - It also removes uses of soc_is_exynosXYZ(). > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > --- > arch/arm/mach-exynos/Kconfig | 1 + > arch/arm/mach-exynos/pmu.c | 185 +++++++++++++++++++++++++++++++++--------- > 2 files changed, 146 insertions(+), 40 deletions(-) > Looks good to me, will apply this and 4/4. Thanks, Kukjin > diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig > index 8f9b66c..a2944ac 100644 > --- a/arch/arm/mach-exynos/Kconfig > +++ b/arch/arm/mach-exynos/Kconfig > @@ -24,6 +24,7 @@ menuconfig ARCH_EXYNOS > select PM_GENERIC_DOMAINS if PM_RUNTIME > select S5P_DEV_MFC > select SRAM > + select MFD_SYSCON > help > Support for SAMSUNG EXYNOS SoCs (EXYNOS4/5) > > diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c > index ff9d23f..5b76728 100644 > --- a/arch/arm/mach-exynos/pmu.c > +++ b/arch/arm/mach-exynos/pmu.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd. > + * Copyright (c) 2011-2014 Samsung Electronics Co., Ltd. > * http://www.samsung.com/ > * > * EXYNOS - CPU PMU(Power Management Unit) support > @@ -10,12 +10,28 @@ > */ > > #include <linux/io.h> > -#include <linux/kernel.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/mfd/syscon.h> > > #include "common.h" > #include "regs-pmu.h" > > -static const struct exynos_pmu_conf *exynos_pmu_config; > +struct exynos_pmu_data { > + const struct exynos_pmu_conf *pmu_config; > + const struct exynos_pmu_conf *pmu_config_extra; > + > + void (*pmu_init)(void); > + void (*powerdown_conf)(enum sys_powerdown); > +}; > + > +struct exynos_pmu_context { > + struct device *dev; > + const struct exynos_pmu_data *pmu_data; > +}; > + > +static struct exynos_pmu_context *pmu_context; > > static const struct exynos_pmu_conf exynos4210_pmu_config[] = { > /* { .offset = offset, .val = { AFTR, LPA, SLEEP } */ > @@ -335,7 +351,7 @@ static unsigned int const exynos5_list_diable_wfi_wfe[] = { > EXYNOS5_ISP_ARM_OPTION, > }; > > -static void exynos5_init_pmu(void) > +static void exynos5_powerdown_conf(enum sys_powerdown mode) > { > unsigned int i; > unsigned int tmp; > @@ -372,51 +388,140 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode) > { > unsigned int i; > > - if (soc_is_exynos5250()) > - exynos5_init_pmu(); > + const struct exynos_pmu_data *pmu_data = pmu_context->pmu_data; > > - for (i = 0; (exynos_pmu_config[i].offset != PMU_TABLE_END) ; i++) > - pmu_raw_writel(exynos_pmu_config[i].val[mode], > - exynos_pmu_config[i].offset); > + if (pmu_data->powerdown_conf) > + pmu_data->powerdown_conf(mode); > > - if (soc_is_exynos4412()) { > - for (i = 0; exynos4412_pmu_config[i].offset != PMU_TABLE_END ; i++) > - pmu_raw_writel(exynos4412_pmu_config[i].val[mode], > - exynos4412_pmu_config[i].offset); > + if (pmu_data->pmu_config) { > + for (i = 0; (pmu_data->pmu_config[i].offset != PMU_TABLE_END) ; i++) > + pmu_raw_writel(pmu_data->pmu_config[i].val[mode], > + pmu_data->pmu_config[i].offset); > + } > + > + if (pmu_data->pmu_config_extra) { > + for (i = 0; pmu_data->pmu_config_extra[i].offset != PMU_TABLE_END; i++) > + pmu_raw_writel(pmu_data->pmu_config_extra[i].val[mode], > + pmu_data->pmu_config_extra[i].offset); > } > } > > -static int __init exynos_pmu_init(void) > +static void exynos5250_pmu_init(void) > { > unsigned int value; > + /* > + * When SYS_WDTRESET is set, watchdog timer reset request > + * is ignored by power management unit. > + */ > + value = pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); > + value &= ~EXYNOS5_SYS_WDTRESET; > + pmu_raw_writel(value, EXYNOS5_AUTO_WDTRESET_DISABLE); > + > + value = pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); > + value &= ~EXYNOS5_SYS_WDTRESET; > + pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST); > +} > + > +static const struct exynos_pmu_data exynos4210_pmu_data = { > + .pmu_config = exynos4210_pmu_config, > +}; > + > +static const struct exynos_pmu_data exynos4212_pmu_data = { > + .pmu_config = exynos4x12_pmu_config, > +}; > + > +static const struct exynos_pmu_data exynos4412_pmu_data = { > + .pmu_config = exynos4x12_pmu_config, > + .pmu_config_extra = exynos4412_pmu_config, > +}; > + > +static const struct exynos_pmu_data exynos5250_pmu_data = { > + .pmu_config = exynos5250_pmu_config, > + .pmu_init = exynos5250_pmu_init, > + .powerdown_conf = exynos5_powerdown_conf, > +}; > + > +static const struct regmap_config pmu_regmap_config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > +}; > > - exynos_pmu_config = exynos4210_pmu_config; > - > - if (soc_is_exynos4210()) { > - exynos_pmu_config = exynos4210_pmu_config; > - pr_info("EXYNOS4210 PMU Initialize\n"); > - } else if (soc_is_exynos4212() || soc_is_exynos4412()) { > - exynos_pmu_config = exynos4x12_pmu_config; > - pr_info("EXYNOS4x12 PMU Initialize\n"); > - } else if (soc_is_exynos5250()) { > - /* > - * When SYS_WDTRESET is set, watchdog timer reset request > - * is ignored by power management unit. > - */ > - value = pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); > - value &= ~EXYNOS5_SYS_WDTRESET; > - pmu_raw_writel(value, EXYNOS5_AUTO_WDTRESET_DISABLE); > - > - value = pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); > - value &= ~EXYNOS5_SYS_WDTRESET; > - pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST); > - > - exynos_pmu_config = exynos5250_pmu_config; > - pr_info("EXYNOS5250 PMU Initialize\n"); > - } else { > - pr_info("EXYNOS: PMU not supported\n"); > +/* > + * PMU platform driver and devicetree bindings. > + */ > +static const struct of_device_id exynos_pmu_of_device_ids[] = { > + { > + .compatible = "samsung,exynos4210-pmu", > + .data = &exynos4210_pmu_data, > + }, { > + .compatible = "samsung,exynos4212-pmu", > + .data = &exynos4212_pmu_data, > + }, { > + .compatible = "samsung,exynos4412-pmu", > + .data = &exynos4412_pmu_data, > + }, { > + .compatible = "samsung,exynos5250-pmu", > + .data = &exynos5250_pmu_data, > + }, > + { /*sentinel*/ }, > +}; > + > +static int exynos_pmu_probe(struct platform_device *pdev) > +{ > + const struct of_device_id *match; > + struct device *dev = &pdev->dev; > + struct regmap *regmap; > + struct resource *res; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + pmu_base_addr = devm_ioremap_resource(dev, res); > + if (IS_ERR(pmu_base_addr)) > + return PTR_ERR(pmu_base_addr); > + > + pmu_context = devm_kzalloc(&pdev->dev, > + sizeof(struct exynos_pmu_context), > + GFP_KERNEL); > + if (!pmu_context) { > + dev_err(dev, "Cannot allocate memory.\n"); > + return -ENOMEM; > + } > + pmu_context->dev = dev; > + > + regmap = devm_regmap_init_mmio(dev, pmu_base_addr, > + &pmu_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(dev, "regmap init failed\n"); > + return PTR_ERR(regmap); > } > > + devm_syscon_register(dev, regmap); > + > + match = of_match_node(exynos_pmu_of_device_ids, pdev->dev.of_node); > + > + pmu_context->pmu_data = match->data; > + > + if (pmu_context->pmu_data->pmu_init) > + pmu_context->pmu_data->pmu_init(); > + > + platform_set_drvdata(pdev, pmu_context); > + > + dev_dbg(dev, "Exynos PMU Driver probe done\n"); > return 0; > } > -arch_initcall(exynos_pmu_init); > + > +static struct platform_driver exynos_pmu_driver = { > + .driver = { > + .name = "exynos-pmu", > + .owner = THIS_MODULE, > + .of_match_table = exynos_pmu_of_device_ids, > + }, > + .probe = exynos_pmu_probe, > +}; > + > +static int __init exynos_pmu_init(void) > +{ > + return platform_driver_register(&exynos_pmu_driver); > + > +} > +postcore_initcall(exynos_pmu_init); > -- > 1.7.9.5
Hi Kukjin, On Friday, July 25, 2014 Kukjin Kim wrote: > Subject: RE: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support for > Exynos PMU > > Pankaj Dubey wrote: > > > > This patch modifies Exynos Power Management Unit (PMU) initialization > > implementation in following way: > > > > - Added platform driver support and probe function where Exynos PMU > > driver will register itself as syscon provider with syscon framework. > > - Added platform struct exynos_pmu_data to hold platform specific data. > > - For each SoC's PMU support now we can add platform data and statically > > bind PMU configuration and SoC specific initialization function. > > - Separate each SoC's PMU initialization function and make it as part of > > platform data. > > - It also removes uses of soc_is_exynosXYZ(). > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > > Reviewed-by: Tomasz Figa <t.figa@samsung.com> > > --- > > arch/arm/mach-exynos/Kconfig | 1 + > > arch/arm/mach-exynos/pmu.c | 185 > +++++++++++++++++++++++++++++++++--------- > > 2 files changed, 146 insertions(+), 40 deletions(-) > > > Looks good to me, will apply this and 4/4. > We need to hold these two patches until dependent patch [1] from Tomasz Figa gets merged. [1]: mfd: syscon: Decouple syscon interface from syscon devices https://lkml.org/lkml/2014/6/24/188 Thanks, Pankaj Dubey > Thanks, > Kukjin > > > diff --git a/arch/arm/mach-exynos/Kconfig > > b/arch/arm/mach-exynos/Kconfig index 8f9b66c..a2944ac 100644 > > --- a/arch/arm/mach-exynos/Kconfig > > +++ b/arch/arm/mach-exynos/Kconfig > > @@ -24,6 +24,7 @@ menuconfig ARCH_EXYNOS > > select PM_GENERIC_DOMAINS if PM_RUNTIME > > select S5P_DEV_MFC > > select SRAM > > + select MFD_SYSCON > > help > > Support for SAMSUNG EXYNOS SoCs (EXYNOS4/5) > > > > diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c > > index ff9d23f..5b76728 100644 > > --- a/arch/arm/mach-exynos/pmu.c > > +++ b/arch/arm/mach-exynos/pmu.c > > @@ -1,5 +1,5 @@ > > /* > > - * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd. > > + * Copyright (c) 2011-2014 Samsung Electronics Co., Ltd. > > * http://www.samsung.com/ > > * > > * EXYNOS - CPU PMU(Power Management Unit) support @@ -10,12 +10,28 > > @@ > > */ > > > > #include <linux/io.h> > > -#include <linux/kernel.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <linux/mfd/syscon.h> > > > > #include "common.h" > > #include "regs-pmu.h" > > > > -static const struct exynos_pmu_conf *exynos_pmu_config; > > +struct exynos_pmu_data { > > + const struct exynos_pmu_conf *pmu_config; > > + const struct exynos_pmu_conf *pmu_config_extra; > > + > > + void (*pmu_init)(void); > > + void (*powerdown_conf)(enum sys_powerdown); }; > > + > > +struct exynos_pmu_context { > > + struct device *dev; > > + const struct exynos_pmu_data *pmu_data; }; > > + > > +static struct exynos_pmu_context *pmu_context; > > > > static const struct exynos_pmu_conf exynos4210_pmu_config[] = { > > /* { .offset = offset, .val = { AFTR, LPA, SLEEP } */ @@ -335,7 > > +351,7 @@ static unsigned int const exynos5_list_diable_wfi_wfe[] = { > > EXYNOS5_ISP_ARM_OPTION, > > }; > > > > -static void exynos5_init_pmu(void) > > +static void exynos5_powerdown_conf(enum sys_powerdown mode) > > { > > unsigned int i; > > unsigned int tmp; > > @@ -372,51 +388,140 @@ void exynos_sys_powerdown_conf(enum > > sys_powerdown mode) { > > unsigned int i; > > > > - if (soc_is_exynos5250()) > > - exynos5_init_pmu(); > > + const struct exynos_pmu_data *pmu_data = pmu_context->pmu_data; > > > > - for (i = 0; (exynos_pmu_config[i].offset != PMU_TABLE_END) ; i++) > > - pmu_raw_writel(exynos_pmu_config[i].val[mode], > > - exynos_pmu_config[i].offset); > > + if (pmu_data->powerdown_conf) > > + pmu_data->powerdown_conf(mode); > > > > - if (soc_is_exynos4412()) { > > - for (i = 0; exynos4412_pmu_config[i].offset != PMU_TABLE_END ; > i++) > > - pmu_raw_writel(exynos4412_pmu_config[i].val[mode], > > - exynos4412_pmu_config[i].offset); > > + if (pmu_data->pmu_config) { > > + for (i = 0; (pmu_data->pmu_config[i].offset != PMU_TABLE_END) > ; i++) > > + pmu_raw_writel(pmu_data->pmu_config[i].val[mode], > > + pmu_data->pmu_config[i].offset); > > + } > > + > > + if (pmu_data->pmu_config_extra) { > > + for (i = 0; pmu_data->pmu_config_extra[i].offset != > PMU_TABLE_END; i++) > > + pmu_raw_writel(pmu_data->pmu_config_extra[i].val[mode], > > + pmu_data->pmu_config_extra[i].offset); > > } > > } > > > > -static int __init exynos_pmu_init(void) > > +static void exynos5250_pmu_init(void) > > { > > unsigned int value; > > + /* > > + * When SYS_WDTRESET is set, watchdog timer reset request > > + * is ignored by power management unit. > > + */ > > + value = pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); > > + value &= ~EXYNOS5_SYS_WDTRESET; > > + pmu_raw_writel(value, EXYNOS5_AUTO_WDTRESET_DISABLE); > > + > > + value = pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); > > + value &= ~EXYNOS5_SYS_WDTRESET; > > + pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST); } > > + > > +static const struct exynos_pmu_data exynos4210_pmu_data = { > > + .pmu_config = exynos4210_pmu_config, > > +}; > > + > > +static const struct exynos_pmu_data exynos4212_pmu_data = { > > + .pmu_config = exynos4x12_pmu_config, > > +}; > > + > > +static const struct exynos_pmu_data exynos4412_pmu_data = { > > + .pmu_config = exynos4x12_pmu_config, > > + .pmu_config_extra = exynos4412_pmu_config, > > +}; > > + > > +static const struct exynos_pmu_data exynos5250_pmu_data = { > > + .pmu_config = exynos5250_pmu_config, > > + .pmu_init = exynos5250_pmu_init, > > + .powerdown_conf = exynos5_powerdown_conf, > > +}; > > + > > +static const struct regmap_config pmu_regmap_config = { > > + .reg_bits = 32, > > + .val_bits = 32, > > + .reg_stride = 4, > > +}; > > > > - exynos_pmu_config = exynos4210_pmu_config; > > - > > - if (soc_is_exynos4210()) { > > - exynos_pmu_config = exynos4210_pmu_config; > > - pr_info("EXYNOS4210 PMU Initialize\n"); > > - } else if (soc_is_exynos4212() || soc_is_exynos4412()) { > > - exynos_pmu_config = exynos4x12_pmu_config; > > - pr_info("EXYNOS4x12 PMU Initialize\n"); > > - } else if (soc_is_exynos5250()) { > > - /* > > - * When SYS_WDTRESET is set, watchdog timer reset request > > - * is ignored by power management unit. > > - */ > > - value = > pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); > > - value &= ~EXYNOS5_SYS_WDTRESET; > > - pmu_raw_writel(value, > EXYNOS5_AUTO_WDTRESET_DISABLE); > > - > > - value = > pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); > > - value &= ~EXYNOS5_SYS_WDTRESET; > > - pmu_raw_writel(value, > EXYNOS5_MASK_WDTRESET_REQUEST); > > - > > - exynos_pmu_config = exynos5250_pmu_config; > > - pr_info("EXYNOS5250 PMU Initialize\n"); > > - } else { > > - pr_info("EXYNOS: PMU not supported\n"); > > +/* > > + * PMU platform driver and devicetree bindings. > > + */ > > +static const struct of_device_id exynos_pmu_of_device_ids[] = { > > + { > > + .compatible = "samsung,exynos4210-pmu", > > + .data = &exynos4210_pmu_data, > > + }, { > > + .compatible = "samsung,exynos4212-pmu", > > + .data = &exynos4212_pmu_data, > > + }, { > > + .compatible = "samsung,exynos4412-pmu", > > + .data = &exynos4412_pmu_data, > > + }, { > > + .compatible = "samsung,exynos5250-pmu", > > + .data = &exynos5250_pmu_data, > > + }, > > + { /*sentinel*/ }, > > +}; > > + > > +static int exynos_pmu_probe(struct platform_device *pdev) { > > + const struct of_device_id *match; > > + struct device *dev = &pdev->dev; > > + struct regmap *regmap; > > + struct resource *res; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + pmu_base_addr = devm_ioremap_resource(dev, res); > > + if (IS_ERR(pmu_base_addr)) > > + return PTR_ERR(pmu_base_addr); > > + > > + pmu_context = devm_kzalloc(&pdev->dev, > > + sizeof(struct exynos_pmu_context), > > + GFP_KERNEL); > > + if (!pmu_context) { > > + dev_err(dev, "Cannot allocate memory.\n"); > > + return -ENOMEM; > > + } > > + pmu_context->dev = dev; > > + > > + regmap = devm_regmap_init_mmio(dev, pmu_base_addr, > > + &pmu_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(dev, "regmap init failed\n"); > > + return PTR_ERR(regmap); > > } > > > > + devm_syscon_register(dev, regmap); > > + > > + match = of_match_node(exynos_pmu_of_device_ids, pdev->dev.of_node); > > + > > + pmu_context->pmu_data = match->data; > > + > > + if (pmu_context->pmu_data->pmu_init) > > + pmu_context->pmu_data->pmu_init(); > > + > > + platform_set_drvdata(pdev, pmu_context); > > + > > + dev_dbg(dev, "Exynos PMU Driver probe done\n"); > > return 0; > > } > > -arch_initcall(exynos_pmu_init); > > + > > +static struct platform_driver exynos_pmu_driver = { > > + .driver = { > > + .name = "exynos-pmu", > > + .owner = THIS_MODULE, > > + .of_match_table = exynos_pmu_of_device_ids, > > + }, > > + .probe = exynos_pmu_probe, > > +}; > > + > > +static int __init exynos_pmu_init(void) { > > + return platform_driver_register(&exynos_pmu_driver); > > + > > +} > > +postcore_initcall(exynos_pmu_init); > > -- > > 1.7.9.5
Hi Pankaj, Kukjin, On 25.07.2014 07:32, Pankaj Dubey wrote: > Hi Kukjin, > > On Friday, July 25, 2014 Kukjin Kim wrote: [snip] >>> >> Looks good to me, will apply this and 4/4. >> > > We need to hold these two patches until dependent patch [1] from Tomasz Figa > gets merged. > > [1]: mfd: syscon: Decouple syscon interface from syscon devices > https://lkml.org/lkml/2014/6/24/188 That RFC patch had few comments from Arnd needed to be addressed, so it needs a new revision. Pankaj, If I remember correctly, we had talked about this and the conclusion was that you would take care of addressing the comments and sending new version of the patch. Any update on this or have I missed something? Best regards, Tomasz
Hi Tomasz, On Friday, July 25, 2014 Tomasz Figa wrote: > To: Pankaj Dubey; 'Kukjin Kim'; linux-arm-kernel@lists.infradead.org; linux- > samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: linux@arm.linux.org.uk; t.figa@samsung.com; vikas.sajjan@samsung.com; > joshi@samsung.com; naushad@samsung.com; thomas.ab@samsung.com; > chow.kim@samsung.com > Subject: Re: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support for > Exynos PMU > > Hi Pankaj, Kukjin, > > On 25.07.2014 07:32, Pankaj Dubey wrote: > > Hi Kukjin, > > > > On Friday, July 25, 2014 Kukjin Kim wrote: > > [snip] > > >>> > >> Looks good to me, will apply this and 4/4. > >> > > > > We need to hold these two patches until dependent patch [1] from > > Tomasz Figa gets merged. > > > > [1]: mfd: syscon: Decouple syscon interface from syscon devices > > https://lkml.org/lkml/2014/6/24/188 > > That RFC patch had few comments from Arnd needed to be addressed, so it needs a > new revision. > > Pankaj, If I remember correctly, we had talked about this and the conclusion was that > you would take care of addressing the comments and sending new version of the > patch. Any update on this or have I missed something? > Well, I don't think we concluded as such anything. Since this patch needs to get in so that Exynos PMU and PM related changes can go in, I discussed with you saying that I am not able to understand about Arnd's comments and if possible and time permits I will look into it. Meanwhile I got busy with some other official work, so could not get time to look into it. > Best regards, > Tomasz
Hi, On Monday, July 28, 2014 08:40:52 AM Pankaj Dubey wrote: > Hi Tomasz, > > On Friday, July 25, 2014 Tomasz Figa wrote: > > > To: Pankaj Dubey; 'Kukjin Kim'; linux-arm-kernel@lists.infradead.org; > linux- > > samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: linux@arm.linux.org.uk; t.figa@samsung.com; vikas.sajjan@samsung.com; > > joshi@samsung.com; naushad@samsung.com; thomas.ab@samsung.com; > > chow.kim@samsung.com > > Subject: Re: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support for > > Exynos PMU > > > > Hi Pankaj, Kukjin, > > > > On 25.07.2014 07:32, Pankaj Dubey wrote: > > > Hi Kukjin, > > > > > > On Friday, July 25, 2014 Kukjin Kim wrote: > > > > [snip] > > > > >>> > > >> Looks good to me, will apply this and 4/4. > > >> > > > > > > We need to hold these two patches until dependent patch [1] from > > > Tomasz Figa gets merged. > > > > > > [1]: mfd: syscon: Decouple syscon interface from syscon devices > > > https://lkml.org/lkml/2014/6/24/188 > > > > That RFC patch had few comments from Arnd needed to be addressed, so it > needs a > > new revision. > > > > Pankaj, If I remember correctly, we had talked about this and the > conclusion was that > > you would take care of addressing the comments and sending new version of > the > > patch. Any update on this or have I missed something? > > > > Well, I don't think we concluded as such anything. > Since this patch needs to get in so that Exynos PMU and PM related changes > can go in, > I discussed with you saying that I am not able to understand about Arnd's > comments and > if possible and time permits I will look into it. Meanwhile I got busy with > some other > official work, so could not get time to look into it. Tomasz/Pankaj, could we please get some agreement on what needs to be done and who should do the pending work? syscon patch is blocking PMU cleanup patches which in turn are blocking PMU support additions for new SoCs (Exynos5420/5800 and Exynos3250 PMU patches). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi Bart, On 18.08.2014 19:42, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Monday, July 28, 2014 08:40:52 AM Pankaj Dubey wrote: >> Hi Tomasz, >> >> On Friday, July 25, 2014 Tomasz Figa wrote: >> >>> To: Pankaj Dubey; 'Kukjin Kim'; linux-arm-kernel@lists.infradead.org; >> linux- >>> samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org >>> Cc: linux@arm.linux.org.uk; t.figa@samsung.com; vikas.sajjan@samsung.com; >>> joshi@samsung.com; naushad@samsung.com; thomas.ab@samsung.com; >>> chow.kim@samsung.com >>> Subject: Re: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support for >>> Exynos PMU >>> >>> Hi Pankaj, Kukjin, >>> >>> On 25.07.2014 07:32, Pankaj Dubey wrote: >>>> Hi Kukjin, >>>> >>>> On Friday, July 25, 2014 Kukjin Kim wrote: >>> >>> [snip] >>> >>>>>> >>>>> Looks good to me, will apply this and 4/4. >>>>> >>>> >>>> We need to hold these two patches until dependent patch [1] from >>>> Tomasz Figa gets merged. >>>> >>>> [1]: mfd: syscon: Decouple syscon interface from syscon devices >>>> https://lkml.org/lkml/2014/6/24/188 >>> >>> That RFC patch had few comments from Arnd needed to be addressed, so it >> needs a >>> new revision. >>> >>> Pankaj, If I remember correctly, we had talked about this and the >> conclusion was that >>> you would take care of addressing the comments and sending new version of >> the >>> patch. Any update on this or have I missed something? >>> >> >> Well, I don't think we concluded as such anything. >> Since this patch needs to get in so that Exynos PMU and PM related changes >> can go in, >> I discussed with you saying that I am not able to understand about Arnd's >> comments and >> if possible and time permits I will look into it. Meanwhile I got busy with >> some other >> official work, so could not get time to look into it. > > Tomasz/Pankaj, could we please get some agreement on what needs to be > done and who should do the pending work? > > syscon patch is blocking PMU cleanup patches which in turn are blocking > PMU support additions for new SoCs (Exynos5420/5800 and Exynos3250 PMU > patches). Leaving alone the matter who is going to take care of it for now, the remaining work to do is to further decouple syscon from struct device, which means providing of_ API to register a syscon provider on a device tree node even before driver model is available yet. I believe it should be quite straightforward on top of my RFC and should require only saving syscon's of_node directly in syscon struct, adding appropriate API and extending the look-up loops to handle cases when syscon's dev is NULL. Best regards, Tomasz
+Arnd, Lee Jones Hi Tomasz, On Tuesday, August 19, 2014 Tomasz Figa wrote: > > Hi Bart, > > On 18.08.2014 19:42, Bartlomiej Zolnierkiewicz wrote: > > > > Hi, > > > > On Monday, July 28, 2014 08:40:52 AM Pankaj Dubey wrote: > >> Hi Tomasz, > >> > >> On Friday, July 25, 2014 Tomasz Figa wrote: > >> > >>> To: Pankaj Dubey; 'Kukjin Kim'; > >>> linux-arm-kernel@lists.infradead.org; > >> linux- > >>> samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org > >>> Cc: linux@arm.linux.org.uk; t.figa@samsung.com; > >>> vikas.sajjan@samsung.com; joshi@samsung.com; naushad@samsung.com; > >>> thomas.ab@samsung.com; chow.kim@samsung.com > >>> Subject: Re: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support > >>> for Exynos PMU > >>> > >>> Hi Pankaj, Kukjin, > >>> > >>> On 25.07.2014 07:32, Pankaj Dubey wrote: > >>>> Hi Kukjin, > >>>> > >>>> On Friday, July 25, 2014 Kukjin Kim wrote: > >>> > >>> [snip] > >>> > >>>>>> > >>>>> Looks good to me, will apply this and 4/4. > >>>>> > >>>> > >>>> We need to hold these two patches until dependent patch [1] from > >>>> Tomasz Figa gets merged. > >>>> > >>>> [1]: mfd: syscon: Decouple syscon interface from syscon devices > >>>> https://lkml.org/lkml/2014/6/24/188 > >>> > >>> That RFC patch had few comments from Arnd needed to be addressed, so > >>> it > >> needs a > >>> new revision. > >>> > >>> Pankaj, If I remember correctly, we had talked about this and the > >> conclusion was that > >>> you would take care of addressing the comments and sending new > >>> version of > >> the > >>> patch. Any update on this or have I missed something? > >>> > >> > >> Well, I don't think we concluded as such anything. > >> Since this patch needs to get in so that Exynos PMU and PM related > >> changes can go in, I discussed with you saying that I am not able to > >> understand about Arnd's comments and if possible and time permits I > >> will look into it. Meanwhile I got busy with some other official > >> work, so could not get time to look into it. > > > > Tomasz/Pankaj, could we please get some agreement on what needs to be > > done and who should do the pending work? > > > > syscon patch is blocking PMU cleanup patches which in turn are > > blocking PMU support additions for new SoCs (Exynos5420/5800 and > > Exynos3250 PMU patches). > > Leaving alone the matter who is going to take care of it for now, the remaining work > to do is to further decouple syscon from struct device, which means providing of_ > API to register a syscon provider on a device tree node even before driver model is > available yet. > As per Arnd's comment on your RFC patch he mentioned - "I believe the part you are missing is that with the approach I suggested, there would be no registration function at all." I think he is not in favor of adding such registration function at all. So do you think adding such function will really solve the problem? Further even Lee Jones agreed to Arnd's point of making syscon independent of device, but he also mentioned that it can be done in subsequent patch. So in IMHO, your RFC patch can be taken as is, and any further improvement suggested by Arnd can be done in subsequent patches, because as I can see in 3.17-rc1 still has user of syscon_regmap_lookup_by_pdevname (clps711x.c) so we can't completely make it independent of platform_device as of now and also the changes required as per Arnd's suggestions requires considerable effort and time. > I believe it should be quite straightforward on top of my RFC and should require only > saving syscon's of_node directly in syscon struct, adding appropriate API and > extending the look-up loops to handle cases when syscon's dev is NULL. > > Best regards, > Tomasz Thanks, Pankaj Dubey
On 21.08.2014 16:07, Pankaj Dubey wrote: > +Arnd, Lee Jones > > Hi Tomasz, > > On Tuesday, August 19, 2014 Tomasz Figa wrote: > >> >> Hi Bart, >> >> On 18.08.2014 19:42, Bartlomiej Zolnierkiewicz wrote: >>> >>> Hi, >>> >>> On Monday, July 28, 2014 08:40:52 AM Pankaj Dubey wrote: >>>> Hi Tomasz, >>>> >>>> On Friday, July 25, 2014 Tomasz Figa wrote: >>>> >>>>> To: Pankaj Dubey; 'Kukjin Kim'; >>>>> linux-arm-kernel@lists.infradead.org; >>>> linux- >>>>> samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org >>>>> Cc: linux@arm.linux.org.uk; t.figa@samsung.com; >>>>> vikas.sajjan@samsung.com; joshi@samsung.com; naushad@samsung.com; >>>>> thomas.ab@samsung.com; chow.kim@samsung.com >>>>> Subject: Re: [PATCH v7 3/4] ARM: EXYNOS: Add platform driver support >>>>> for Exynos PMU >>>>> >>>>> Hi Pankaj, Kukjin, >>>>> >>>>> On 25.07.2014 07:32, Pankaj Dubey wrote: >>>>>> Hi Kukjin, >>>>>> >>>>>> On Friday, July 25, 2014 Kukjin Kim wrote: >>>>> >>>>> [snip] >>>>> >>>>>>>> >>>>>>> Looks good to me, will apply this and 4/4. >>>>>>> >>>>>> >>>>>> We need to hold these two patches until dependent patch [1] from >>>>>> Tomasz Figa gets merged. >>>>>> >>>>>> [1]: mfd: syscon: Decouple syscon interface from syscon devices >>>>>> https://lkml.org/lkml/2014/6/24/188 >>>>> >>>>> That RFC patch had few comments from Arnd needed to be addressed, so >>>>> it >>>> needs a >>>>> new revision. >>>>> >>>>> Pankaj, If I remember correctly, we had talked about this and the >>>> conclusion was that >>>>> you would take care of addressing the comments and sending new >>>>> version of >>>> the >>>>> patch. Any update on this or have I missed something? >>>>> >>>> >>>> Well, I don't think we concluded as such anything. >>>> Since this patch needs to get in so that Exynos PMU and PM related >>>> changes can go in, I discussed with you saying that I am not able to >>>> understand about Arnd's comments and if possible and time permits I >>>> will look into it. Meanwhile I got busy with some other official >>>> work, so could not get time to look into it. >>> >>> Tomasz/Pankaj, could we please get some agreement on what needs to be >>> done and who should do the pending work? >>> >>> syscon patch is blocking PMU cleanup patches which in turn are >>> blocking PMU support additions for new SoCs (Exynos5420/5800 and >>> Exynos3250 PMU patches). >> >> Leaving alone the matter who is going to take care of it for now, the > remaining work >> to do is to further decouple syscon from struct device, which means > providing of_ >> API to register a syscon provider on a device tree node even before driver > model is >> available yet. >> > > As per Arnd's comment on your RFC patch he mentioned - > "I believe the part you are missing is that with the approach I suggested, > there would be no registration function at all." > > I think he is not in favor of adding such registration function at all. So > do you think > adding such function will really solve the problem? > > Further even Lee Jones agreed to Arnd's point of making syscon independent > of device, > but he also mentioned that it can be done in subsequent patch. Let's look again at the original thread then... I believe Lee agreed with my proposed solution or at least he quoted my e-mail and pointed that further work addressing Arnd's comments could be done in follow up patches. I also think that we should rather make one step as a time, especially this patch is required for further clean-up of Exynos. However there was also a reply from Michal Simek, which pointed out that even with my patch the syscon is still bound to driver model and for his use case he would need a purely OF-based version of the API. That's why I think my patch should be re-spun with changes I mentioned in my previous message in this thread. > > So in IMHO, your RFC patch can be taken as is, and any further improvement > suggested > by Arnd can be done in subsequent patches, because as I can see in 3.17-rc1 > still > has user of syscon_regmap_lookup_by_pdevname (clps711x.c) so we can't > completely > make it independent of platform_device as of now and also the changes > required > as per Arnd's suggestions requires considerable effort and time. Agreed. However we can still provide OF-only syscon registration function and modify look-up functions to allow syscons without struct device pointer, just with OF node. Best regards, Tomasz
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index 8f9b66c..a2944ac 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -24,6 +24,7 @@ menuconfig ARCH_EXYNOS select PM_GENERIC_DOMAINS if PM_RUNTIME select S5P_DEV_MFC select SRAM + select MFD_SYSCON help Support for SAMSUNG EXYNOS SoCs (EXYNOS4/5) diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c index ff9d23f..5b76728 100644 --- a/arch/arm/mach-exynos/pmu.c +++ b/arch/arm/mach-exynos/pmu.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd. + * Copyright (c) 2011-2014 Samsung Electronics Co., Ltd. * http://www.samsung.com/ * * EXYNOS - CPU PMU(Power Management Unit) support @@ -10,12 +10,28 @@ */ #include <linux/io.h> -#include <linux/kernel.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> #include "common.h" #include "regs-pmu.h" -static const struct exynos_pmu_conf *exynos_pmu_config; +struct exynos_pmu_data { + const struct exynos_pmu_conf *pmu_config; + const struct exynos_pmu_conf *pmu_config_extra; + + void (*pmu_init)(void); + void (*powerdown_conf)(enum sys_powerdown); +}; + +struct exynos_pmu_context { + struct device *dev; + const struct exynos_pmu_data *pmu_data; +}; + +static struct exynos_pmu_context *pmu_context; static const struct exynos_pmu_conf exynos4210_pmu_config[] = { /* { .offset = offset, .val = { AFTR, LPA, SLEEP } */ @@ -335,7 +351,7 @@ static unsigned int const exynos5_list_diable_wfi_wfe[] = { EXYNOS5_ISP_ARM_OPTION, }; -static void exynos5_init_pmu(void) +static void exynos5_powerdown_conf(enum sys_powerdown mode) { unsigned int i; unsigned int tmp; @@ -372,51 +388,140 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode) { unsigned int i; - if (soc_is_exynos5250()) - exynos5_init_pmu(); + const struct exynos_pmu_data *pmu_data = pmu_context->pmu_data; - for (i = 0; (exynos_pmu_config[i].offset != PMU_TABLE_END) ; i++) - pmu_raw_writel(exynos_pmu_config[i].val[mode], - exynos_pmu_config[i].offset); + if (pmu_data->powerdown_conf) + pmu_data->powerdown_conf(mode); - if (soc_is_exynos4412()) { - for (i = 0; exynos4412_pmu_config[i].offset != PMU_TABLE_END ; i++) - pmu_raw_writel(exynos4412_pmu_config[i].val[mode], - exynos4412_pmu_config[i].offset); + if (pmu_data->pmu_config) { + for (i = 0; (pmu_data->pmu_config[i].offset != PMU_TABLE_END) ; i++) + pmu_raw_writel(pmu_data->pmu_config[i].val[mode], + pmu_data->pmu_config[i].offset); + } + + if (pmu_data->pmu_config_extra) { + for (i = 0; pmu_data->pmu_config_extra[i].offset != PMU_TABLE_END; i++) + pmu_raw_writel(pmu_data->pmu_config_extra[i].val[mode], + pmu_data->pmu_config_extra[i].offset); } } -static int __init exynos_pmu_init(void) +static void exynos5250_pmu_init(void) { unsigned int value; + /* + * When SYS_WDTRESET is set, watchdog timer reset request + * is ignored by power management unit. + */ + value = pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); + value &= ~EXYNOS5_SYS_WDTRESET; + pmu_raw_writel(value, EXYNOS5_AUTO_WDTRESET_DISABLE); + + value = pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); + value &= ~EXYNOS5_SYS_WDTRESET; + pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST); +} + +static const struct exynos_pmu_data exynos4210_pmu_data = { + .pmu_config = exynos4210_pmu_config, +}; + +static const struct exynos_pmu_data exynos4212_pmu_data = { + .pmu_config = exynos4x12_pmu_config, +}; + +static const struct exynos_pmu_data exynos4412_pmu_data = { + .pmu_config = exynos4x12_pmu_config, + .pmu_config_extra = exynos4412_pmu_config, +}; + +static const struct exynos_pmu_data exynos5250_pmu_data = { + .pmu_config = exynos5250_pmu_config, + .pmu_init = exynos5250_pmu_init, + .powerdown_conf = exynos5_powerdown_conf, +}; + +static const struct regmap_config pmu_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, +}; - exynos_pmu_config = exynos4210_pmu_config; - - if (soc_is_exynos4210()) { - exynos_pmu_config = exynos4210_pmu_config; - pr_info("EXYNOS4210 PMU Initialize\n"); - } else if (soc_is_exynos4212() || soc_is_exynos4412()) { - exynos_pmu_config = exynos4x12_pmu_config; - pr_info("EXYNOS4x12 PMU Initialize\n"); - } else if (soc_is_exynos5250()) { - /* - * When SYS_WDTRESET is set, watchdog timer reset request - * is ignored by power management unit. - */ - value = pmu_raw_readl(EXYNOS5_AUTO_WDTRESET_DISABLE); - value &= ~EXYNOS5_SYS_WDTRESET; - pmu_raw_writel(value, EXYNOS5_AUTO_WDTRESET_DISABLE); - - value = pmu_raw_readl(EXYNOS5_MASK_WDTRESET_REQUEST); - value &= ~EXYNOS5_SYS_WDTRESET; - pmu_raw_writel(value, EXYNOS5_MASK_WDTRESET_REQUEST); - - exynos_pmu_config = exynos5250_pmu_config; - pr_info("EXYNOS5250 PMU Initialize\n"); - } else { - pr_info("EXYNOS: PMU not supported\n"); +/* + * PMU platform driver and devicetree bindings. + */ +static const struct of_device_id exynos_pmu_of_device_ids[] = { + { + .compatible = "samsung,exynos4210-pmu", + .data = &exynos4210_pmu_data, + }, { + .compatible = "samsung,exynos4212-pmu", + .data = &exynos4212_pmu_data, + }, { + .compatible = "samsung,exynos4412-pmu", + .data = &exynos4412_pmu_data, + }, { + .compatible = "samsung,exynos5250-pmu", + .data = &exynos5250_pmu_data, + }, + { /*sentinel*/ }, +}; + +static int exynos_pmu_probe(struct platform_device *pdev) +{ + const struct of_device_id *match; + struct device *dev = &pdev->dev; + struct regmap *regmap; + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + pmu_base_addr = devm_ioremap_resource(dev, res); + if (IS_ERR(pmu_base_addr)) + return PTR_ERR(pmu_base_addr); + + pmu_context = devm_kzalloc(&pdev->dev, + sizeof(struct exynos_pmu_context), + GFP_KERNEL); + if (!pmu_context) { + dev_err(dev, "Cannot allocate memory.\n"); + return -ENOMEM; + } + pmu_context->dev = dev; + + regmap = devm_regmap_init_mmio(dev, pmu_base_addr, + &pmu_regmap_config); + if (IS_ERR(regmap)) { + dev_err(dev, "regmap init failed\n"); + return PTR_ERR(regmap); } + devm_syscon_register(dev, regmap); + + match = of_match_node(exynos_pmu_of_device_ids, pdev->dev.of_node); + + pmu_context->pmu_data = match->data; + + if (pmu_context->pmu_data->pmu_init) + pmu_context->pmu_data->pmu_init(); + + platform_set_drvdata(pdev, pmu_context); + + dev_dbg(dev, "Exynos PMU Driver probe done\n"); return 0; } -arch_initcall(exynos_pmu_init); + +static struct platform_driver exynos_pmu_driver = { + .driver = { + .name = "exynos-pmu", + .owner = THIS_MODULE, + .of_match_table = exynos_pmu_of_device_ids, + }, + .probe = exynos_pmu_probe, +}; + +static int __init exynos_pmu_init(void) +{ + return platform_driver_register(&exynos_pmu_driver); + +} +postcore_initcall(exynos_pmu_init);