| Message ID | 1447637775-9887-2-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, On 2015? 11? 16? 10:36, Krzysztof Kozlowski wrote: > Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS > so it is built also on ARMv7. This does not bring any kind of benefit. > There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like > multi_v7 for ARMv7). > > Instead build clock drivers only for respective SoC's architecture. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/clk/samsung/Kconfig | 13 +++++++++++++ > drivers/clk/samsung/Makefile | 4 ++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig > index 84196ecdaa12..5f138fc4d84d 100644 > --- a/drivers/clk/samsung/Kconfig > +++ b/drivers/clk/samsung/Kconfig > @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG > bool > select COMMON_CLK > > +# ARMv7 SoCs: > config S3C2410_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > > +# ARMv8 SoCs: > +config EXYNOS5433_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > + > +config EXYNOS7_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile > index 5f6833ea355d..a31332a24ef4 100644 > --- a/drivers/clk/samsung/Makefile > +++ b/drivers/clk/samsung/Makefile > @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o > obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o > obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o > obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o > -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o > +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o > obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o > -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o > +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o > obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o > obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o > obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o > Looks good to me. Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 16 November 2015 07:06 AM, Krzysztof Kozlowski wrote: > Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS > so it is built also on ARMv7. This does not bring any kind of benefit. > There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like > multi_v7 for ARMv7). > > Instead build clock drivers only for respective SoC's architecture. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/clk/samsung/Kconfig | 13 +++++++++++++ > drivers/clk/samsung/Makefile | 4 ++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig > index 84196ecdaa12..5f138fc4d84d 100644 > --- a/drivers/clk/samsung/Kconfig > +++ b/drivers/clk/samsung/Kconfig > @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG > bool > select COMMON_CLK > > +# ARMv7 SoCs: > config S3C2410_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > > +# ARMv8 SoCs: > +config EXYNOS5433_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > + > +config EXYNOS7_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile > index 5f6833ea355d..a31332a24ef4 100644 > --- a/drivers/clk/samsung/Makefile > +++ b/drivers/clk/samsung/Makefile > @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o > obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o > obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o > obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o > -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o > +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o > obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o > -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o > +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o > obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o > obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o > obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o > So in this approach we need to add separate config for clock support of each ARM64 Exynos64 SoC. Is this fine? Can we club compilation of each ARM64 Exynos SoC clock file under EXYNOS7_COMMON_CLK? As for all ARM64 SoC there is single defconfig and binary. Thanks, Pankaj Dubey -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.11.2015 13:31, pankaj.dubey wrote: > > > On Monday 16 November 2015 07:06 AM, Krzysztof Kozlowski wrote: >> Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS >> so it is built also on ARMv7. This does not bring any kind of benefit. >> There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like >> multi_v7 for ARMv7). >> >> Instead build clock drivers only for respective SoC's architecture. >> >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> --- >> drivers/clk/samsung/Kconfig | 13 +++++++++++++ >> drivers/clk/samsung/Makefile | 4 ++-- >> 2 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig >> index 84196ecdaa12..5f138fc4d84d 100644 >> --- a/drivers/clk/samsung/Kconfig >> +++ b/drivers/clk/samsung/Kconfig >> @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG >> bool >> select COMMON_CLK >> >> +# ARMv7 SoCs: >> config S3C2410_COMMON_CLK >> bool >> select COMMON_CLK_SAMSUNG >> @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK >> bool >> select COMMON_CLK_SAMSUNG >> >> +# ARMv8 SoCs: >> +config EXYNOS5433_COMMON_CLK >> + bool >> + depends on ARM64 || COMPILE_TEST >> + default ARCH_EXYNOS >> + select COMMON_CLK_SAMSUNG >> + >> +config EXYNOS7_COMMON_CLK >> + bool >> + depends on ARM64 || COMPILE_TEST >> + default ARCH_EXYNOS >> + select COMMON_CLK_SAMSUNG >> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile >> index 5f6833ea355d..a31332a24ef4 100644 >> --- a/drivers/clk/samsung/Makefile >> +++ b/drivers/clk/samsung/Makefile >> @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o >> obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o >> obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o >> obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o >> -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o >> +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o >> obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o >> obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o >> obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o >> -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o >> +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o >> obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o >> obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o >> obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o >> > > So in this approach we need to add separate config for clock support of > each ARM64 Exynos64 SoC. Is this fine? > > Can we club compilation of each ARM64 Exynos SoC clock file under > EXYNOS7_COMMON_CLK? As for all ARM64 SoC there is single defconfig and > binary. Yes, it can be one config symbol for all clocks of ARMv8 Exynos SoCs. From my point of view both has some advantages and disadvantages (kernel size, granularity, number of Kconfig symbols etc.) and I don't mind choosing different than I selected before. Any opinion from Samsung clock maintainers? Which do you prefer? Best regards, Krzysztof it even looks -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/11/15 05:39, Krzysztof Kozlowski wrote: > On 17.11.2015 13:31, pankaj.dubey wrote: >> On Monday 16 November 2015 07:06 AM, Krzysztof Kozlowski wrote: >>> Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS >>> so it is built also on ARMv7. This does not bring any kind of benefit. >>> There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like >>> multi_v7 for ARMv7). >>> >>> Instead build clock drivers only for respective SoC's architecture. >>> >>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >>> --- >>> drivers/clk/samsung/Kconfig | 13 +++++++++++++ >>> drivers/clk/samsung/Makefile | 4 ++-- >>> 2 files changed, 15 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig >>> index 84196ecdaa12..5f138fc4d84d 100644 >>> --- a/drivers/clk/samsung/Kconfig >>> +++ b/drivers/clk/samsung/Kconfig >>> @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG >>> bool >>> select COMMON_CLK >>> >>> +# ARMv7 SoCs: >>> config S3C2410_COMMON_CLK >>> bool >>> select COMMON_CLK_SAMSUNG >>> @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK >>> bool >>> select COMMON_CLK_SAMSUNG >>> >>> +# ARMv8 SoCs: >>> +config EXYNOS5433_COMMON_CLK >>> + bool >>> + depends on ARM64 || COMPILE_TEST >>> + default ARCH_EXYNOS >>> + select COMMON_CLK_SAMSUNG >>> + >>> +config EXYNOS7_COMMON_CLK >>> + bool >>> + depends on ARM64 || COMPILE_TEST >>> + default ARCH_EXYNOS >>> + select COMMON_CLK_SAMSUNG >>> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile >>> index 5f6833ea355d..a31332a24ef4 100644 >>> --- a/drivers/clk/samsung/Makefile >>> +++ b/drivers/clk/samsung/Makefile >>> @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o >>> obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o >>> obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o >>> obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o >>> -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o >>> +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o >>> obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o >>> obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o >>> obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o >>> -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o >>> +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o >>> obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o >>> obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o >>> obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o >>> >> >> So in this approach we need to add separate config for clock support of >> each ARM64 Exynos64 SoC. Is this fine? >> >> Can we club compilation of each ARM64 Exynos SoC clock file under >> EXYNOS7_COMMON_CLK? As for all ARM64 SoC there is single defconfig and >> binary. > > Yes, it can be one config symbol for all clocks of ARMv8 Exynos SoCs. > From my point of view both has some advantages and disadvantages (kernel > size, granularity, number of Kconfig symbols etc.) and I don't mind > choosing different than I selected before. > > Any opinion from Samsung clock maintainers? Which do you prefer? It would have been a bit unfortunate to not be able to exclude the unneeded clk drivers from build. From my side both patches look like a step in right direction. For the $subject patch: Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Hi Krzysztof, On 11/16/2015 07:06 AM, Krzysztof Kozlowski wrote: > Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS > so it is built also on ARMv7. This does not bring any kind of benefit. > There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like > multi_v7 for ARMv7). > > Instead build clock drivers only for respective SoC's architecture. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Tested $SUBJECT patch on exynos7-espresso board, so Tested-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > drivers/clk/samsung/Kconfig | 13 +++++++++++++ > drivers/clk/samsung/Makefile | 4 ++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig > index 84196ecdaa12..5f138fc4d84d 100644 > --- a/drivers/clk/samsung/Kconfig > +++ b/drivers/clk/samsung/Kconfig > @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG > bool > select COMMON_CLK > > +# ARMv7 SoCs: > config S3C2410_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > > +# ARMv8 SoCs: > +config EXYNOS5433_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > + > +config EXYNOS7_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile > index 5f6833ea355d..a31332a24ef4 100644 > --- a/drivers/clk/samsung/Makefile > +++ b/drivers/clk/samsung/Makefile > @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o > obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o > obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o > obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o > -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o > +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o > obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o > obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o > -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o > +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o > obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o > obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o > obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof, Good idea, just a couple of nits inline. Other than that: Acked-by: Tomasz Figa <tomasz.figa@gmail.com> 2015-11-16 10:36 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS > so it is built also on ARMv7. This does not bring any kind of benefit. > There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like > multi_v7 for ARMv7). > > Instead build clock drivers only for respective SoC's architecture. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/clk/samsung/Kconfig | 13 +++++++++++++ > drivers/clk/samsung/Makefile | 4 ++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig > index 84196ecdaa12..5f138fc4d84d 100644 > --- a/drivers/clk/samsung/Kconfig > +++ b/drivers/clk/samsung/Kconfig > @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG > bool > select COMMON_CLK > > +# ARMv7 SoCs: nit: I'm not aware of any recent upgrade of the S3C24xx line-up to ARMv7 cores. ;) I'd suggest "32-bit ARM SoCs" or just "ARM SoCs"... > config S3C2410_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK > bool > select COMMON_CLK_SAMSUNG > > +# ARMv8 SoCs: and then here "64-bit ARM SoCs" or "ARM64 SoCs", whichever you prefer. I'd lean towards simple "ARM" and "ARM64". > +config EXYNOS5433_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS nit: bool and default can be combined into def_bool ARCH_EXYNOS > + select COMMON_CLK_SAMSUNG > + > +config EXYNOS7_COMMON_CLK > + bool > + depends on ARM64 || COMPILE_TEST > + default ARCH_EXYNOS nit: See above. However, I don't think we can disable compilation of particular 64-bit SoCs, so maybe there isn't much sense in splitting their clock drivers into separate symbols? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19.11.2015 13:18, Tomasz Figa wrote: > Hi Krzysztof, > > Good idea, just a couple of nits inline. Other than that: > > Acked-by: Tomasz Figa <tomasz.figa@gmail.com> > > 2015-11-16 10:36 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: >> Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS >> so it is built also on ARMv7. This does not bring any kind of benefit. >> There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like >> multi_v7 for ARMv7). >> >> Instead build clock drivers only for respective SoC's architecture. >> >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> --- >> drivers/clk/samsung/Kconfig | 13 +++++++++++++ >> drivers/clk/samsung/Makefile | 4 ++-- >> 2 files changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig >> index 84196ecdaa12..5f138fc4d84d 100644 >> --- a/drivers/clk/samsung/Kconfig >> +++ b/drivers/clk/samsung/Kconfig >> @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG >> bool >> select COMMON_CLK >> >> +# ARMv7 SoCs: > > nit: I'm not aware of any recent upgrade of the S3C24xx line-up to > ARMv7 cores. ;) I'd suggest "32-bit ARM SoCs" or just "ARM SoCs"... okay > >> config S3C2410_COMMON_CLK >> bool >> select COMMON_CLK_SAMSUNG >> @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK >> bool >> select COMMON_CLK_SAMSUNG >> >> +# ARMv8 SoCs: > > and then here "64-bit ARM SoCs" or "ARM64 SoCs", whichever you prefer. > I'd lean towards simple "ARM" and "ARM64". ARM64 sounds good. > >> +config EXYNOS5433_COMMON_CLK >> + bool >> + depends on ARM64 || COMPILE_TEST >> + default ARCH_EXYNOS > > nit: bool and default can be combined into def_bool ARCH_EXYNOS > Right. >> + select COMMON_CLK_SAMSUNG >> + >> +config EXYNOS7_COMMON_CLK >> + bool >> + depends on ARM64 || COMPILE_TEST >> + default ARCH_EXYNOS > > nit: See above. > > However, I don't think we can disable compilation of particular 64-bit > SoCs, so maybe there isn't much sense in splitting their clock drivers > into separate symbols? To me it does not really matter. Indeed as you said one cannot disable building of one particular Exynos SoCs. However we could still want not build some parts of such SoCs (like clock, pinctrl etc). I don't see much benefit for such case except when someone would like to drastically reduce the size of kernel image (for whatever reasons he has.). On the other hand having separate symbols causes duplication and obfuscates a little the Kconfig/Makefile. I like keeping things simple so one symbol for all ARM64 Exynos clocks sounds good. Sylwester preferred current approach. You and Pankaj seem to prefer one symbol-way. Should we make a voting? :) Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-11-19 13:51 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: > On 19.11.2015 13:18, Tomasz Figa wrote: >> However, I don't think we can disable compilation of particular 64-bit >> SoCs, so maybe there isn't much sense in splitting their clock drivers >> into separate symbols? > > To me it does not really matter. Indeed as you said one cannot disable > building of one particular Exynos SoCs. > > However we could still want not build some parts of such SoCs (like > clock, pinctrl etc). I don't see much benefit for such case except when > someone would like to drastically reduce the size of kernel image (for > whatever reasons he has.). Can we really build a kernel that support selected Exynos SoC without its clock driver? Actually I don't think we even allow deselecting clock drivers currently, because they are not visible in menuconfig. Unless there is a clear goal to separate ARCH level Kconfig symbol for particular ARM64-based Exynos SoCs, I don't think it makes any sense to keep the clock-related symbols separate. > > On the other hand having separate symbols causes duplication and > obfuscates a little the Kconfig/Makefile. I like keeping things simple > so one symbol for all ARM64 Exynos clocks sounds good. > > Sylwester preferred current approach. You and Pankaj seem to prefer one > symbol-way. Hmm, I read Sylwester's post as a reply to your original message and not Pankaj's. Sylwester, could you clarify? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
W dniu 19.11.2015 o 18:16, Tomasz Figa pisze: > 2015-11-19 13:51 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: >> On 19.11.2015 13:18, Tomasz Figa wrote: >>> However, I don't think we can disable compilation of particular 64-bit >>> SoCs, so maybe there isn't much sense in splitting their clock drivers >>> into separate symbols? >> >> To me it does not really matter. Indeed as you said one cannot disable >> building of one particular Exynos SoCs. >> >> However we could still want not build some parts of such SoCs (like >> clock, pinctrl etc). I don't see much benefit for such case except when >> someone would like to drastically reduce the size of kernel image (for >> whatever reasons he has.). > > Can we really build a kernel that support selected Exynos SoC without > its clock driver? Actually I don't think we even allow deselecting > clock drivers currently, because they are not visible in menuconfig. > Unless there is a clear goal to separate ARCH level Kconfig symbol for > particular ARM64-based Exynos SoCs, I don't think it makes any sense > to keep the clock-related symbols separate. That is reasonable and very convincing. I'll wait for Sylwester reply before re-spinning. BR, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/11/15 10:16, Tomasz Figa wrote: > 2015-11-19 13:51 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>: >> > On 19.11.2015 13:18, Tomasz Figa wrote: >>> >> However, I don't think we can disable compilation of particular >>> >> 64-bit SoCs, so maybe there isn't much sense in splitting their >>> >> clock drivers into separate symbols? >> > >> > To me it does not really matter. Indeed as you said one cannot >> > disable building of one particular Exynos SoCs. >> > >> > However we could still want not build some parts of such SoCs (like >> > clock, pinctrl etc). I don't see much benefit for such case except >> > when someone would like to drastically reduce the size of kernel >> > image (for whatever reasons he has.). > > Can we really build a kernel that support selected Exynos SoC without > its clock driver? Actually I don't think we even allow deselecting > clock drivers currently, because they are not visible in menuconfig. > Unless there is a clear goal to separate ARCH level Kconfig symbol for > particular ARM64-based Exynos SoCs, I don't think it makes any sense > to keep the clock-related symbols separate. > >> > >> > On the other hand having separate symbols causes duplication and >> > obfuscates a little the Kconfig/Makefile. I like keeping things >> > simple so one symbol for all ARM64 Exynos clocks sounds good. >> > >> > Sylwester preferred current approach. You and Pankaj seem to prefer >> > one symbol-way. > > Hmm, I read Sylwester's post as a reply to your original message and > not Pankaj's. Sylwester, could you clarify? OK, let's just use a common clk Kconfig symbol for Exynos ARM64. What I tried to say is that with addition of support for few more of those SoCs the kernel image size can easily grow by 1MB order, due to just clk drivers inclusion. Perhaps it's not a big issue with current hardware configuration. Maybe in long term we should think about splitting CMU drivers into a built-in critical clocks part and the rest in loadable modules.
diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig index 84196ecdaa12..5f138fc4d84d 100644 --- a/drivers/clk/samsung/Kconfig +++ b/drivers/clk/samsung/Kconfig @@ -2,6 +2,7 @@ config COMMON_CLK_SAMSUNG bool select COMMON_CLK +# ARMv7 SoCs: config S3C2410_COMMON_CLK bool select COMMON_CLK_SAMSUNG @@ -24,3 +25,15 @@ config S3C2443_COMMON_CLK bool select COMMON_CLK_SAMSUNG +# ARMv8 SoCs: +config EXYNOS5433_COMMON_CLK + bool + depends on ARM64 || COMPILE_TEST + default ARCH_EXYNOS + select COMMON_CLK_SAMSUNG + +config EXYNOS7_COMMON_CLK + bool + depends on ARM64 || COMPILE_TEST + default ARCH_EXYNOS + select COMMON_CLK_SAMSUNG diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 5f6833ea355d..a31332a24ef4 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile @@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o obj-$(CONFIG_SOC_EXYNOS5260) += clk-exynos5260.o obj-$(CONFIG_SOC_EXYNOS5410) += clk-exynos5410.o obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o -obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos5433.o +obj-$(CONFIG_EXYNOS5433_COMMON_CLK) += clk-exynos5433.o obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-clkout.o -obj-$(CONFIG_ARCH_EXYNOS7) += clk-exynos7.o +obj-$(CONFIG_EXYNOS7_COMMON_CLK) += clk-exynos7.o obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o
Currently the Exynos5433 (ARMv8 SoC) clock driver depends on ARCH_EXYNOS so it is built also on ARMv7. This does not bring any kind of benefit. There won't be a single kernel image for ARMv7 and ARMv8 SoCs (like multi_v7 for ARMv7). Instead build clock drivers only for respective SoC's architecture. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/clk/samsung/Kconfig | 13 +++++++++++++ drivers/clk/samsung/Makefile | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-)