On 04/11/2014 03:32 PM, Chanwoo Choi wrote:
> Hi,
>
> On 04/11/2014 10:46 AM, Olof Johansson wrote:
>> On Thu, Apr 10, 2014 at 06:37:12PM +0900, Chanwoo Choi wrote:
>>> This patch add Exynos3250's SoC ID. Exynos 3250 is System-On-Chip(SoC) that
>>> is based on the 32-bit RISC processor for Smartphone. Exynos3250 uses
>>> Cortex-A7
>>> dual cores and has a target speed of 1.0GHz.
>>>
>>> Signed-off-by: Chanwoo Choi <cw00.c...@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
>>> ---
>>> arch/arm/mach-exynos/Kconfig | 22 ++++++++++++++++++++++
>>> arch/arm/mach-exynos/exynos.c | 1 +
>>> arch/arm/plat-samsung/include/plat/cpu.h | 10 ++++++++++
>>> 3 files changed, 33 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>>> index fc8bf18..6da8a68 100644
>>> --- a/arch/arm/mach-exynos/Kconfig
>>> +++ b/arch/arm/mach-exynos/Kconfig
>>> @@ -11,6 +11,17 @@ if ARCH_EXYNOS
>>>
>>> menu "SAMSUNG EXYNOS SoCs Support"
>>>
>>> +config ARCH_EXYNOS3
>>> + bool "SAMSUNG EXYNOS3"
>>> + select ARM_AMBA
>>> + select CLKSRC_OF
>>> + select HAVE_ARM_SCU if SMP
>>> + select HAVE_SMP
>>> + select PINCTRL
>>> + select PM_GENERIC_DOMAINS if PM_RUNTIME
>>> + help
>>> + Samsung EXYNOS3 SoCs based systems
>>> +
>>> config ARCH_EXYNOS4
>>> bool "SAMSUNG EXYNOS4"
>>> default y
>>> @@ -41,6 +52,17 @@ config ARCH_EXYNOS5
>>>
>>> comment "EXYNOS SoCs"
>>>
>>> +config SOC_EXYNOS3250
>>> + bool "SAMSUNG EXYNOS3250"
>>> + default y
>>> + depends on ARCH_EXYNOS3
>>> + select ARCH_HAS_BANDGAP
>>> + select ARM_CPU_SUSPEND if PM
>>> + select PINCTRL_EXYNOS
>>> + select SAMSUNG_DMADEV
>>> + help
>>> + Enable EXYNOS3250 CPU support
>>> +
>>> config CPU_EXYNOS4210
>>> bool "SAMSUNG EXYNOS4210"
>>> default y
>>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>>> index b32a907..b134868 100644
>>> --- a/arch/arm/mach-exynos/exynos.c
>>> +++ b/arch/arm/mach-exynos/exynos.c
>>> @@ -370,6 +370,7 @@ static void __init exynos_dt_machine_init(void)
>>> }
>>>
>>> static char const *exynos_dt_compat[] __initconst = {
>>> + "samsung,exynos3250",
>>
>> Please consider samsung,exynos3 instead, so you don't have to update this
>> table
>> for every SoC. We've talked about this before..
>
> This patchset included only exynos3250.dtsi without exynos3.dtsi.
> So, I added only "samsung,exynos3250" compatible name.
>
> Do you prefer to add SoC version as following?
> + "samsung,exynos3",
> + "samsung,exynos3250",
>
> or ?
> + "samsung,exynos3",
>
>>
>>> "samsung,exynos4",
>>> "samsung,exynos4210",
>>> "samsung,exynos4212",
>>> diff --git a/arch/arm/plat-samsung/include/plat/cpu.h
>>> b/arch/arm/plat-samsung/include/plat/cpu.h
>>> index 5992b8d..3d808f6b 100644
>>> --- a/arch/arm/plat-samsung/include/plat/cpu.h
>>> +++ b/arch/arm/plat-samsung/include/plat/cpu.h
>>> @@ -43,6 +43,9 @@ extern unsigned long samsung_cpu_id;
>>> #define S5PV210_CPU_ID 0x43110000
>>> #define S5PV210_CPU_MASK 0xFFFFF000
>>>
>>> +#define EXYNOS3250_SOC_ID 0xE3472000
>>> +#define EXYNOS3_SOC_MASK 0xFFFFF000
>>> +
>>> #define EXYNOS4210_CPU_ID 0x43210000
>>> #define EXYNOS4212_CPU_ID 0x43220000
>>> #define EXYNOS4412_CPU_ID 0xE4412200
>>> @@ -68,6 +71,7 @@ IS_SAMSUNG_CPU(s5p6440, S5P6440_CPU_ID, S5P64XX_CPU_MASK)
>>> IS_SAMSUNG_CPU(s5p6450, S5P6450_CPU_ID, S5P64XX_CPU_MASK)
>>> IS_SAMSUNG_CPU(s5pc100, S5PC100_CPU_ID, S5PC100_CPU_MASK)
>>> IS_SAMSUNG_CPU(s5pv210, S5PV210_CPU_ID, S5PV210_CPU_MASK)
>>> +IS_SAMSUNG_CPU(exynos3250, EXYNOS3250_SOC_ID, EXYNOS3_SOC_MASK)
>>> IS_SAMSUNG_CPU(exynos4210, EXYNOS4210_CPU_ID, EXYNOS4_CPU_MASK)
>>> IS_SAMSUNG_CPU(exynos4212, EXYNOS4212_CPU_ID, EXYNOS4_CPU_MASK)
>>> IS_SAMSUNG_CPU(exynos4412, EXYNOS4412_CPU_ID, EXYNOS4_CPU_MASK)
>>> @@ -126,6 +130,12 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID,
>>> EXYNOS5_SOC_MASK)
>>> # define soc_is_s5pv210() 0
>>> #endif
>>>
>>> +#if defined(CONFIG_SOC_EXYNOS3250)
>>> +# define soc_is_exynos3250() is_samsung_exynos3250()
>>> +#else
>>> +# define soc_is_exynos3250() 0
>>> +#endif
>>
>> In general, I think we have too much code littered with soc_is_<foo>() going
>> on, so please try to avoid adding more for this SoC. Especially in cases
>> where
>> you just want to bail out of certain features where we might already have
>> function pointers to control if a function is called or not, such as the
>> firmware interfaces.
>>
>
> Do you prefer dt helper function such as following function instead of new
> soc_is_xx() ?
> - of_machine_is_compatible("samsung,exynos3250")
>
I think of_machine_is_compatible() is not proper alternative method.
of_machine_is_compatible can be only used if CONFIG_OF is enabled.
Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
