diff mbox

[v5,9/9] drivers: soc: Add support for Exynos PMU driver

Message ID 1447406983-27835-10-git-send-email-pankaj.dubey@samsung.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Pankaj Dubey Nov. 13, 2015, 9:29 a.m. UTC
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/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
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

Comments

Krzysztof Kozlowski Nov. 13, 2015, 10:28 a.m. UTC | #1
On 13.11.2015 18:29, 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

Why ifdef CONFIG_ARM? This already depends on ARCH_EXYNOS. If you want
to limit to ARMv7 then add the dependency to Kconfig (just like
EXYNOS_SROM).

Rest looks ok - nice renames.

Best regards,
Krzysztof

> 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
> 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-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Nov. 13, 2015, 3:36 p.m. UTC | #2
On 13 November 2015 at 15:58, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
>
> On 13.11.2015 18:29, 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
>
> Why ifdef CONFIG_ARM? This already depends on ARCH_EXYNOS. If you want
> to limit to ARMv7 then add the dependency to Kconfig (just like
> EXYNOS_SROM).
>

This is required, so that 32-bit based Exynos SoC's PMU should not get
compiled when we are compiling for ARM64 and vice-versa.

For example: In future, I have plan to add exynos7 PMU support as -

ifdef CONFIG_ARM64
obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos7-pmu.o
endif

Thus preventing compilation of ARM64 based SoCs PMU data files when we
are compiling for ARM.

Only exynos-pmu.c will be shared and compiled in both cases.

Thanks,
Pankaj Dubey

>
> Rest looks ok - nice renames.
>
> Best regards,
> Krzysztof
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Nov. 14, 2015, 11:30 a.m. UTC | #3
W dniu 14.11.2015 o 00:36, Pankaj Dubey pisze:
> On 13 November 2015 at 15:58, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>>
>> On 13.11.2015 18:29, 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
>>
>> Why ifdef CONFIG_ARM? This already depends on ARCH_EXYNOS. If you want
>> to limit to ARMv7 then add the dependency to Kconfig (just like
>> EXYNOS_SROM).
>>
> 
> This is required, so that 32-bit based Exynos SoC's PMU should not get
> compiled when we are compiling for ARM64 and vice-versa.
> 
> For example: In future, I have plan to add exynos7 PMU support as -
> 
> ifdef CONFIG_ARM64
> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos7-pmu.o
> endif
> 
> Thus preventing compilation of ARM64 based SoCs PMU data files when we
> are compiling for ARM.
> 
> Only exynos-pmu.c will be shared and compiled in both cases.

But this actually spreads Kconfig configuration over different places:
Kconfig and Makefile. It confuses and makes it more difficult in the
future to maintain and extend.

This should be in Kconfig. If you want to limit this per architecture
(is it really needed?) then maybe define different EXYNOS_PMU config
symbols?

Best regards,
Krzysztof



--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Nov. 17, 2015, 3:44 a.m. UTC | #4
On Saturday 14 November 2015 05:00 PM, Krzysztof Kozlowski wrote:
> W dniu 14.11.2015 o 00:36, Pankaj Dubey pisze:
>> On 13 November 2015 at 15:58, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>>
>>> On 13.11.2015 18:29, 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
>>>
>>> Why ifdef CONFIG_ARM? This already depends on ARCH_EXYNOS. If you want
>>> to limit to ARMv7 then add the dependency to Kconfig (just like
>>> EXYNOS_SROM).
>>>
>>
>> This is required, so that 32-bit based Exynos SoC's PMU should not get
>> compiled when we are compiling for ARM64 and vice-versa.
>>
>> For example: In future, I have plan to add exynos7 PMU support as -
>>
>> ifdef CONFIG_ARM64
>> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos7-pmu.o
>> endif
>>
>> Thus preventing compilation of ARM64 based SoCs PMU data files when we
>> are compiling for ARM.
>>
>> Only exynos-pmu.c will be shared and compiled in both cases.
>
> But this actually spreads Kconfig configuration over different places:
> Kconfig and Makefile. It confuses and makes it more difficult in the
> future to maintain and extend.
>

I could not understand your point here. Will you please explain what 
issue you can see in this approach?

> This should be in Kconfig. If you want to limit this per architecture
> (is it really needed?) then maybe define different EXYNOS_PMU config
> symbols?
>

Do you mean we need to have separate configs for enabling PMU on ARM and 
ARM64 based Exynos SoCs?

Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Nov. 17, 2015, 4:28 a.m. UTC | #5
On 17.11.2015 12:44, pankaj.dubey wrote:
> 
> 
> On Saturday 14 November 2015 05:00 PM, Krzysztof Kozlowski wrote:
>> W dniu 14.11.2015 o 00:36, Pankaj Dubey pisze:
>>> On 13 November 2015 at 15:58, Krzysztof Kozlowski
>>> <k.kozlowski@samsung.com> wrote:
>>>>
>>>> On 13.11.2015 18:29, 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
>>>>
>>>> Why ifdef CONFIG_ARM? This already depends on ARCH_EXYNOS. If you want
>>>> to limit to ARMv7 then add the dependency to Kconfig (just like
>>>> EXYNOS_SROM).
>>>>
>>>
>>> This is required, so that 32-bit based Exynos SoC's PMU should not get
>>> compiled when we are compiling for ARM64 and vice-versa.
>>>
>>> For example: In future, I have plan to add exynos7 PMU support as -
>>>
>>> ifdef CONFIG_ARM64
>>> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos7-pmu.o
>>> endif
>>>
>>> Thus preventing compilation of ARM64 based SoCs PMU data files when we
>>> are compiling for ARM.
>>>
>>> Only exynos-pmu.c will be shared and compiled in both cases.
>>
>> But this actually spreads Kconfig configuration over different places:
>> Kconfig and Makefile. It confuses and makes it more difficult in the
>> future to maintain and extend.
>>
> 
> I could not understand your point here. Will you please explain what
> issue you can see in this approach?

You are spreading Kconfig symbols over different places. These should be
put only in one - in Kconfig.

Some time later someone will add COMPILE_TEST to Kconfig and wonder why
it does not work... oh yeah, because dependency was put not only in
Kconfig but also in Makefile. One place for dependency: Kconfig.

> 
>> This should be in Kconfig. If you want to limit this per architecture
>> (is it really needed?) then maybe define different EXYNOS_PMU config
>> symbols?
>>
> 
> Do you mean we need to have separate configs for enabling PMU on ARM and
> ARM64 based Exynos SoCs?

Yes. If you want to separate ARMv7 from ARMv8 then yes. Just like we
have for other drivers - usually per SoC but this can be per architecture.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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