Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1261160 > unrolled thread
| Started by | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| First post | 2015-11-03 03:30 +0100 |
| Last post | 2015-11-06 01:50 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-11-03 03:30 +0100
Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver Pankaj Dubey <pankaj.dubey@samsung.com> - 2015-11-05 06:40 +0100
Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-11-06 01:50 +0100
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-11-03 03:30 +0100 |
| Subject | Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver |
| Message-ID | <qqzwd-4Xh-1@gated-at.bofh.it> |
On 26.10.2015 21:55, Pankaj Dubey wrote:
> This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
> to "drivers/soc/samsung". This driver is mainly used for setting misc
> bits of register from PMU IP of Exynos SoC which will be required to
> configure before Suspend/Resume. Currently all these settings are done
> in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC
> support, there is a need of this PMU driver in driver/* folder.
>
> This driver uses existing DT binding information and there should
> be no functionality change in the supported platforms.
>
> Signed-off-by: Amit Daniel Kachhap <amitdanielk@gmail.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/Kconfig | 1 +
> arch/arm/mach-exynos/Makefile | 4 +---
> drivers/soc/samsung/Kconfig | 4 ++++
> drivers/soc/samsung/Makefile | 4 ++++
> arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c | 0
> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h | 0
> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c | 0
> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c | 0
> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c | 0
> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c | 0
> 10 files changed, 10 insertions(+), 3 deletions(-)
> rename arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c (100%)
> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h (100%)
> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c (100%)
> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c (100%)
> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c (100%)
> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c (100%)
>
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 83c85f5..874cb38 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
> select ARM_GIC
> select COMMON_CLK_SAMSUNG
> select EXYNOS_THERMAL
> + select EXYNOS_PMU
> select EXYNOS_SROM if PM
> select HAVE_ARM_SCU if SMP
> select HAVE_S3C2410_I2C if I2C
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 2d58063..34d29df 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -9,9 +9,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += -I$(srctree)/$(src)/include -I$(srctree)
>
> # Core
>
> -obj-$(CONFIG_ARCH_EXYNOS) += exynos.o pmu.o exynos-smc.o firmware.o \
> - exynos3250-pmu.o exynos4-pmu.o \
> - exynos5250-pmu.o exynos5420-pmu.o
> +obj-$(CONFIG_ARCH_EXYNOS) += exynos.o exynos-smc.o firmware.o
>
> obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
> obj-$(CONFIG_PM_SLEEP) += suspend.o
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2833b5b..f545d6c 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -10,4 +10,8 @@ config EXYNOS_SROM
> bool
> depends on ARM && ARCH_EXYNOS && PM
>
> +config EXYNOS_PMU
> + bool
> + depends on ARCH_EXYNOS
> +
> endmenu
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 9c554d5..26fb489 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1 +1,5 @@
> obj-$(CONFIG_EXYNOS_SROM) += exynos-srom.o
> +ifdef CONFIG_ARM
> +obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
> + exynos5250-pmu.o exynos5420-pmu.o
> +endif
> diff --git a/arch/arm/mach-exynos/pmu.c b/drivers/soc/samsung/exynos-pmu.c
> similarity index 100%
> rename from arch/arm/mach-exynos/pmu.c
> rename to drivers/soc/samsung/exynos-pmu.c
> diff --git a/arch/arm/mach-exynos/exynos-pmu.h b/drivers/soc/samsung/exynos-pmu.h
1. Please reorder the exynos_sys_powerdown_conf() to be after the
statics. I am thinking also about adding EXPORT_SYMBOL... but maybe this
would be over-thinking.
2. I think the proper location of everything is drivers/power/reset/.
Although I don't have strong opinion.
3. Please cc linux-pm and arm-soc guys (Arnd, Olof, Kevin) on next
iteration.
Best regards,
Krzysztof
> similarity index 100%
> rename from arch/arm/mach-exynos/exynos-pmu.h
> rename to drivers/soc/samsung/exynos-pmu.h
> diff --git a/arch/arm/mach-exynos/exynos3250-pmu.c b/drivers/soc/samsung/exynos3250-pmu.c
> similarity index 100%
> rename from arch/arm/mach-exynos/exynos3250-pmu.c
> rename to drivers/soc/samsung/exynos3250-pmu.c
> diff --git a/arch/arm/mach-exynos/exynos4-pmu.c b/drivers/soc/samsung/exynos4-pmu.c
> similarity index 100%
> rename from arch/arm/mach-exynos/exynos4-pmu.c
> rename to drivers/soc/samsung/exynos4-pmu.c
> diff --git a/arch/arm/mach-exynos/exynos5250-pmu.c b/drivers/soc/samsung/exynos5250-pmu.c
> similarity index 100%
> rename from arch/arm/mach-exynos/exynos5250-pmu.c
> rename to drivers/soc/samsung/exynos5250-pmu.c
> diff --git a/arch/arm/mach-exynos/exynos5420-pmu.c b/drivers/soc/samsung/exynos5420-pmu.c
> similarity index 100%
> rename from arch/arm/mach-exynos/exynos5420-pmu.c
> rename to drivers/soc/samsung/exynos5420-pmu.c
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Pankaj Dubey <pankaj.dubey@samsung.com> |
|---|---|
| Date | 2015-11-05 06:40 +0100 |
| Message-ID | <qrlrb-2q8-1@gated-at.bofh.it> |
| In reply to | #1261160 |
Hi Krzysztof,
On Tuesday 03 November 2015 07:52 AM, Krzysztof Kozlowski wrote:
> On 26.10.2015 21:55, Pankaj Dubey wrote:
>> This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
>> to "drivers/soc/samsung". This driver is mainly used for setting misc
>> bits of register from PMU IP of Exynos SoC which will be required to
>> configure before Suspend/Resume. Currently all these settings are done
>> in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC
>> support, there is a need of this PMU driver in driver/* folder.
>>
>> This driver uses existing DT binding information and there should
>> be no functionality change in the supported platforms.
>>
>> Signed-off-by: Amit Daniel Kachhap <amitdanielk@gmail.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/Kconfig | 1 +
>> arch/arm/mach-exynos/Makefile | 4 +---
>> drivers/soc/samsung/Kconfig | 4 ++++
>> drivers/soc/samsung/Makefile | 4 ++++
>> arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c | 0
>> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h | 0
>> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c | 0
>> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c | 0
>> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c | 0
>> {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c | 0
>> 10 files changed, 10 insertions(+), 3 deletions(-)
>> rename arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c (100%)
>> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h (100%)
>> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c (100%)
>> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c (100%)
>> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c (100%)
>> rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c (100%)
>>
>
> 1. Please reorder the exynos_sys_powerdown_conf() to be after the
> statics. I am thinking also about adding EXPORT_SYMBOL... but maybe this
> would be over-thinking.
>
I could not understand your point of reordering, will you please explain
this.
> 2. I think the proper location of everything is drivers/power/reset/.
> Although I don't have strong opinion.
>
There has been discussion about the proper location for this driver,
initial attempt was done in "drivers/mfd" folder but then we realized
that this driver is not exactly fitting in MFD category.
There was suggestion from Catalin Marinas [1], [2] to move it to
"drivers/power" or a more suitable place other than mfd. As I received
comments from Bartlomiej [3] and other members also (sorry I could not
produce all links as it was quite more than a year back), I feel driver
is very much SoC specific and hence decided to move it here.
1: https://lkml.org/lkml/2014/4/28/879
2:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/252018.html
3:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244690.html
> 3. Please cc linux-pm and arm-soc guys (Arnd, Olof, Kevin) on next
> iteration.
>
Ok will keep them in CC in next revision.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-11-06 01:50 +0100 |
| Message-ID | <qrDo6-5L5-15@gated-at.bofh.it> |
| In reply to | #1262930 |
On 05.11.2015 14:31, Pankaj Dubey wrote: > Hi Krzysztof, > > On Tuesday 03 November 2015 07:52 AM, Krzysztof Kozlowski wrote: > >> >> 1. Please reorder the exynos_sys_powerdown_conf() to be after the >> statics. I am thinking also about adding EXPORT_SYMBOL... but maybe this >> would be over-thinking. >> > > I could not understand your point of reordering, will you please explain > this. Usually static functions are put at beginning of a file and the externally linkable ones (including exportable) are at the end. This allows quick finding of what is exported by this unit. Mixing static-nonstatic-static brings confusion/ This is a driver so everything except it is static, so I see two choices: 1. Put it in front, just after pmu_context definition. 2. Put it in back, just before the probe. > >> 2. I think the proper location of everything is drivers/power/reset/. >> Although I don't have strong opinion. >> > > There has been discussion about the proper location for this driver, > initial attempt was done in "drivers/mfd" folder but then we realized > that this driver is not exactly fitting in MFD category. > There was suggestion from Catalin Marinas [1], [2] to move it to > "drivers/power" or a more suitable place other than mfd. As I received > comments from Bartlomiej [3] and other members also (sorry I could not > produce all links as it was quite more than a year back), I feel driver > is very much SoC specific and hence decided to move it here. > > 1: https://lkml.org/lkml/2014/4/28/879 > 2: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/252018.html > > 3: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244690.html In drivers/power/reset there are already very-SoC-specific reset handlers. All of them are non-reusable outside of some SoC family. However I don't think it really matters - both locations (soc and power) seem fine to me. Looking at Bart's comments I see that you did not resolve all of them. One was left: "what happens if exynos_sys_powerdown_conf() is called while there are no platform devices binded to a driver but driver itself is loaded." Looking at the code NULL pointer exception will happen on pmu_context dereference. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web