arm64: Kconfig: Update ARCH_EXYNOS select configs
Subsystems (11)
v2 cover arm64: Kconfig: Update ARCH_EXYNOS select configs
This is v2 of the series of patches that modularizes a number of core ARCH_EXYNOS drivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from the ARCH_EXYNOS series of "select XXX". This includes setting the following configs as tristate:
* COMMON_CLK_SAMSUNG
* EXYNOS_ARM64_COMMON_CLK
* PINCTRL_SAMSUNG
* PINCTRL_EXYNOS
* EXYNOS_PMU_ARM64
* EXYNOS_PM_DOMAINS
Additionally, it introduces the config EXYNOS_PMU_ARM64 and EXYNOS_PMU_ARM which was previously EXYNOS_PMU and EXYNOS_PMU_ARM_DRIVERS respectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies on pmu_base_addr under arch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the CLK_OF_DECLARE() changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.
Lastly, this series is based off of [1] since there are dependencies on EXYNOS_CHIPID from that series..
[1] https://lore.kernel.org/lkml/[email protected]/
* From v1:
- Fixed modifying hidden configs
- Modularized all the drivers that were touched
- Removed HAVE_S3C_RTC
- Updated all Samsung ARCH_XXX configs as suggested from reviews
- Rebased on top of 5.15-rc3 and pulled in [1]
Will McVicker (12):
arm64: don't have ARCH_EXYNOS select EXYNOS_CHIPID
timekeeping: add API for getting timekeeping_suspended
clk: samsung: add support for CPU clocks
clk: samsung: exynos5433: update apollo and atlas clock probing
clk: export __clk_lookup
clk: samsung: modularize exynos arm64 clk drivers
clk: samsung: set exynos arm64 clk driver as tristate
pinctrl: samsung: modularize the ARM and ARM64 pinctrls
pinctrl: samsung: set PINCTRL_EXYNOS and PINCTRL_SAMSUNG as tristate
soc: samsung: pmu: modularize the Exynos ARMv8 PMU driver
soc: samsung: pm_domains: modularize EXYNOS_PM_DOMAINS
ARM: rtc: remove HAVE_S3C_RTC in favor of direct dependencies
arch/arm/Kconfig | 1 - arch/arm/mach-exynos/Kconfig | 6 +- arch/arm/mach-s3c/Kconfig.s3c64xx | 1 - arch/arm/mach-s5pv210/Kconfig | 3 - arch/arm64/Kconfig.platforms | 6 - drivers/clk/clk.c | 1 + drivers/clk/samsung/Kconfig | 5 +- drivers/clk/samsung/Makefile | 3 +- drivers/clk/samsung/clk-cpu.c | 28 +- drivers/clk/samsung/clk-cpu.h | 2 +- drivers/clk/samsung/clk-exynos5433.c | 465 ++++++++---------- drivers/clk/samsung/clk-exynos7.c | 177 +++---- drivers/clk/samsung/clk-pll.c | 6 +- drivers/clk/samsung/clk.c | 35 +- drivers/clk/samsung/clk.h | 50 +- drivers/pinctrl/samsung/Kconfig | 5 +- drivers/pinctrl/samsung/Makefile | 13 +- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 102 ++-- .../pinctrl/samsung/pinctrl-exynos-arm64.c | 73 +-- drivers/pinctrl/samsung/pinctrl-exynos.c | 17 +- drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- drivers/rtc/Kconfig | 10 +- drivers/soc/samsung/Kconfig | 18 +- drivers/soc/samsung/Makefile | 8 +- drivers/soc/samsung/exynos-pmu.c | 13 +- drivers/soc/samsung/exynos-pmu.h | 2 +- drivers/soc/samsung/pm_domains.c | 12 +- include/linux/soc/samsung/exynos-pmu.h | 2 +- include/linux/timekeeping.h | 1 + kernel/time/timekeeping.c | 11 + 30 files changed, 553 insertions(+), 534 deletions(-)
Signature · 1 line
2.33.0.685.g46640cef36-goog
PATCH v2 1/12 arm64: don't have ARCH_EXYNOS select EXYNOS_CHIPID
Now that EXYNOS_CHIPID can be a module and is enabled by default via ARCH_EXYNOS, we don't need to have ARCH_EXYNOS directly select it. So remove that.
--- arch/arm64/Kconfig.platforms | 1 - 1 file changed, 1 deletion(-)
arch/arm64/Kconfig.platforms +0 -1
@@ -92,7 +92,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" select COMMON_CLK_SAMSUNG - select EXYNOS_CHIPID select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select EXYNOS_PMU select HAVE_S3C_RTC if RTC_CLASS
Signature · 1 line
2.33.0.685.g46640cef36-goog
Krzysztof Kozlowski
AppliedRe: (subset) [PATCH v2 01/12] arm64: don't have ARCH_EXYNOS select EXYNOS_CHIPID
Quoted 5 lines · Will McVicker
Now that
EXYNOS_CHIPIDcan be a module and is enabled by default viaARCH_EXYNOS, we don't need to haveARCH_EXYNOSdirectly select it. So remove that.
Applied, thanks!
[01/12] arm64: don't have ARCH_EXYNOS select EXYNOS_CHIPID
commit: 6ad4185220e6e3b7ebac7c7a5f55197aedd000da
Best regards,
Signature · 1 line
Krzysztof Kozlowski <[email protected]>
Krzysztof Kozlowski
AppliedRe: [PATCH v2 01/12] arm64: don't have ARCH_EXYNOS select EXYNOS_CHIPID
In reply to Will McVickerQuoted 15 lines · Will McVicker
Now that
EXYNOS_CHIPIDcan be a module and is enabled by default viaARCH_EXYNOS, we don't need to haveARCH_EXYNOSdirectly select it. So remove that.Signed-off-by Will McVicker <[email protected]>--- arch/arm64/Kconfig.platforms | 1 - 1 file changed, 1 deletion(-)arch/arm64/Kconfig.platforms +0
-0
arch/arm64/Kconfig.platforms config ARCH_BRCMSTB · line 97
Quoted 7 lines
arch/arm64/Kconfig.platforms +0
-1@@ -92,7 +92,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" select COMMON_CLK_SAMSUNG - select EXYNOS_CHIPID select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select EXYNOS_PMU select HAVE_S3C_RTC if RTC_CLASS
This actually should be part of my series converting ChipID driver to a module:
https://lore.kernel.org/linux-samsung-soc/[email protected]/T/#t
Applied it, thanks.
Best regards,
Krzysztof
PATCH v2 2/12 timekeeping: add API for getting timekeeping_suspended
In reply to Will McVickerThis allows modules to access the value of timekeeping_suspended without giving them write access to the variable.
--- include/linux/timekeeping.h | 1 + kernel/time/timekeeping.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
include/linux/timekeeping.h +1 -0
@@ -8,6 +8,7 @@/* Included from linux/ktime.h */ void timekeeping_init (void ); +extern bool timekeeping_is_suspended (void );extern int timekeeping_suspended;/* Architecture timer tick functions: */
kernel/time/timekeeping.c +11 -0
@@ -57,6 +57,17 @@ static struct timekeeper shadow_timekeeper;/* flag for if timekeeping is suspended */ int __read_mostly timekeeping_suspended; +/** +* timekeeping_is_suspended - query for timekeeping_suspended +* +* Returns the true/false based on the value of timekeeping_suspened. +*/ +bool timekeeping_is_suspended (void ) +{ +return timekeeping_suspended ? true : false; +} +EXPORT_SYMBOL_GPL (timekeeping_is_suspended); +/** * struct tk_fast - NMI safe timekeeper * @seq: Sequence counter for protecting updates. The lowest bit
Signature · 1 line
2.33.0.685.g46640cef36-goog
Re: [PATCH v2 02/12] timekeeping: add API for getting timekeeping_suspended
Quoted 4 lines · Will McVicker
This allows modules to access the value of
timekeeping_suspendedwithout giving them write access to the variable.
It's important to cover "the why" not "the what" in these commit messages, so you might add a note as to what code will be the user of this (the samsung/clk-pll.c code changed later in this series).
thanks
-john
Re: [PATCH v2 02/12] timekeeping: add API for getting timekeeping_suspended
Quoted 7 lines · John Stultz
Quoted 4 lines · Will McVicker
This allows modules to access the value of
timekeeping_suspendedwithout giving them write access to the variable.
Quoted 6 lines
It's important to cover "the why" not "the what" in these commit messages, so you might add a note as to what code will be the user of this (the
samsung/clk-pll.ccode changed later in this series).thanks
-john
Thanks John for the tip. I will try to be better at that in the followup.
For this specific patch, I am adding this new API because the Samsung PLL driver (drivers/clk/samsung/clk-pll.c) currently is using the variable 'timekeeping_suspended' to detect timeouts before the clocksource is initialized or timekeeping itself is suspended. My patch series aims to modularize the Samsung PLL driver. So to keep the driver's functionality intact, I need to add this additional API.
--Will
Re: [PATCH v2 02/12] timekeeping: add API for getting timekeeping_suspended
Quoted 7 lines · Will McVicker
Quoted 6 lines · John Stultz
Quoted 4 lines · Will McVicker
This allows modules to access the value of
timekeeping_suspendedwithout giving them write access to the variable.
Quoted 8 lines
Quoted 6 lines
It's important to cover "the why" not "the what" in these commit messages, so you might add a note as to what code will be the user of this (the
samsung/clk-pll.ccode changed later in this series).thanks
-johnThanks John for the tip. I will try to be better at that in the followup.
I have to remind myself regularly as well. :) Apologies if my quick reply above seemed curt (as it does to me re-reading it now). Wasn't my intent.
Quoted 6 lines
For this specific patch, I am adding this new API because the Samsung PLL driver (
drivers/clk/samsung/clk-pll.c) currently is using the variable 'timekeeping_suspended' to detect timeouts before the clocksource is initialized or timekeeping itself is suspended. My patch series aims to modularize the Samsung PLL driver. So to keep the driver's functionality intact, I need to add this additional API.
Sounds good!
Another small/medium suggestion: Since you're adding a new interface for non-core users of timekeeping_suspended, it might be good to switch the other users as well (seems like just drivers/clk/ti/clkctrl.c and kernel/sched/clock.c), then also remove the extern in include/linux/timekeeping.h (so there's one consistent method to access it)? I know it's a sort of scope creep, so apologies for asking, but it would make the series more attractive if it's not leaving something for others to clean up later.
thanks
-john
Re: [PATCH v2 02/12] timekeeping: add API for getting timekeeping_suspended
Thanks for the suggestion. That makes sense. I'll look into that for the next version.
--Will
Quoted 42 lines · John Stultz
Quoted 7 lines · Will McVicker
Quoted 6 lines · John Stultz
Quoted 4 lines · Will McVicker
This allows modules to access the value of
timekeeping_suspendedwithout giving them write access to the variable.Quoted 8 lines
Quoted 6 lines
It's important to cover "the why" not "the what" in these commit messages, so you might add a note as to what code will be the user of this (the
samsung/clk-pll.ccode changed later in this series).thanks
-johnThanks John for the tip. I will try to be better at that in the followup.
I have to remind myself regularly as well. :) Apologies if my quick reply above seemed curt (as it does to me re-reading it now). Wasn't my intent.
Quoted 6 lines
For this specific patch, I am adding this new API because the Samsung PLL driver (
drivers/clk/samsung/clk-pll.c) currently is using the variable 'timekeeping_suspended' to detect timeouts before the clocksource is initialized or timekeeping itself is suspended. My patch series aims to modularize the Samsung PLL driver. So to keep the driver's functionality intact, I need to add this additional API.Sounds good!
Another small/medium suggestion: Since you're adding a new interface for non-core users of
timekeeping_suspended, it might be good to switch the other users as well (seems like justdrivers/clk/ti/clkctrl.candkernel/sched/clock.c), then also remove the extern ininclude/linux/timekeeping.h(so there's one consistent method to access it)? I know it's a sort of scope creep, so apologies for asking, but it would make the series more attractive if it's not leaving something for others to clean up later.thanks
-john
PATCH v2 3/12 clk: samsung: add support for CPU clocks
In reply to Will McVickerAdds 'struct samsung_cpu_clock' and correpsonding code to the samsung common clk driver. This allows drivers to register their CPU clocks with the samsung_cmu_register_one() API.
--- drivers/clk/samsung/clk-cpu.c | 26 ++++++++++++++++++++++++++ drivers/clk/samsung/clk.c | 2 ++ drivers/clk/samsung/clk.h | 26 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)
drivers/clk/samsung/clk-cpu.c +26 -0
@@ -469,3 +469,29 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,kfree (cpuclk);return ret; } + +void samsung_clk_register_cpu (struct samsung_clk_provider *ctx, +const struct samsung_cpu_clock *list,unsigned int nr_clk) +{ +unsigned int idx; +unsigned int num_cfgs; +struct clk *parent_clk, *alt_parent_clk; +const struct clk_hw *parent_clk_hw =NULL ; +const struct clk_hw *alt_parent_clk_hw =NULL ; + +for (idx =0 ; idx < nr_clk; idx++, list++) { +/* find count of configuration rates in cfg */ +for (num_cfgs =0 ; list->cfg[num_cfgs].prate !=0 ; ) + num_cfgs++; + + parent_clk =__clk_lookup (list->parent_name); +if (parent_clk) + parent_clk_hw =__clk_get_hw (parent_clk); + alt_parent_clk =__clk_lookup (list->alt_parent_name); +if (alt_parent_clk) + alt_parent_clk_hw =__clk_get_hw (alt_parent_clk); + +exynos_register_cpu_clock (ctx, list->id, list->name, parent_clk_hw, + alt_parent_clk_hw, list->offset, list->cfg, num_cfgs, list->flags); + } +}
drivers/clk/samsung/clk.c +2 -0
@@ -378,6 +378,8 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(samsung_clk_extended_sleep_init (reg_base, cmu->clk_regs, cmu->nr_clk_regs, cmu->suspend_regs, cmu->nr_suspend_regs); +if (cmu->cpu_clks) +samsung_clk_register_cpu (ctx, cmu->cpu_clks, cmu->nr_cpu_clks);samsung_clk_of_add_provider (np, ctx);
drivers/clk/samsung/clk.h +26 -0
@@ -271,6 +271,27 @@ struct samsung_pll_clock {__PLL (_typ, _id, _name, _pname,CLK_GET_RATE_NOCACHE , _lock, \ _con, _rtable) +struct samsung_cpu_clock { +unsigned int id; +const char *name; +const char *parent_name; +const char *alt_parent_name; +unsigned long flags; +int offset; +const struct exynos_cpuclk_cfg_data *cfg; +}; + +#define CPU_CLK (_id, _name, _pname, _apname, _flags, _offset, _cfg) \ + { \ + .id = _id, \ + .name = _name, \ + .parent_name = _pname, \ + .alt_parent_name = _apname, \ + .flags = _flags, \ + .offset = _offset, \ + .cfg = _cfg, \ + } +struct samsung_clock_reg_cache {struct list_head node;void __iomem *reg_base;
@@ -301,6 +322,9 @@ struct samsung_cmu_info {unsigned int nr_fixed_factor_clks;/* total number of clocks with IDs assigned*/ unsigned int nr_clk_ids; +/* list of cpu clocks and respective count */ +const struct samsung_cpu_clock *cpu_clks; +unsigned int nr_cpu_clks;/* list and number of clocks registers */ const unsigned long *clk_regs;
@@ -350,6 +374,8 @@ extern void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,extern void __init samsung_clk_register_pll (struct samsung_clk_provider *ctx,const struct samsung_pll_clock *pll_list,unsigned int nr_clk,void __iomem *base); +extern void __init samsung_clk_register_cpu (struct samsung_clk_provider *ctx, +const struct samsung_cpu_clock *list,unsigned int nr_clk); extern struct samsung_clk_provider __init *samsung_cmu_register_one( struct device_node *,
Signature · 1 line
2.33.0.685.g46640cef36-goog
PATCH v2 4/12 clk: samsung: exynos5433: update apollo and atlas clock probing
In reply to Will McVickerUse the samsung common clk driver to initialize and probe the apollo and atlas clocks. This removes their dedicated init functions and uses the platform driver to handle the probing.
--- drivers/clk/samsung/clk-exynos5433.c | 130 ++++++++++----------------- 1 file changed, 49 insertions(+), 81 deletions(-)
drivers/clk/samsung/clk-exynos5433.c +49 -81
@@ -3675,47 +3675,28 @@ static const struct exynos_cpuclk_cfg_data exynos5433_apolloclk_d[] __initconst { 0 }, }; -static void __init exynos5433_cmu_apollo_init (struct device_node *np) -{ -void __iomem *reg_base; -struct samsung_clk_provider *ctx; -struct clk_hw **hws; - - reg_base =of_iomap (np,0 ); -if (!reg_base) { -panic ("%s: failed to map registers\n" , __func__); -return ; - } - - ctx =samsung_clk_init (np, reg_base,APOLLO_NR_CLK ); -if (!ctx) { -panic ("%s: unable to allocate ctx\n" , __func__); -return ; - } - -samsung_clk_register_pll (ctx, apollo_pll_clks, -ARRAY_SIZE (apollo_pll_clks), reg_base); -samsung_clk_register_mux (ctx, apollo_mux_clks, -ARRAY_SIZE (apollo_mux_clks)); -samsung_clk_register_div (ctx, apollo_div_clks, -ARRAY_SIZE (apollo_div_clks)); -samsung_clk_register_gate (ctx, apollo_gate_clks, -ARRAY_SIZE (apollo_gate_clks)); - - hws = ctx->clk_data.hws; - -exynos_register_cpu_clock (ctx,CLK_SCLK_APOLLO ,"apolloclk" , - hws[CLK_MOUT_APOLLO_PLL ], hws[CLK_MOUT_BUS_PLL_APOLLO_USER ],0x200 , - exynos5433_apolloclk_d,ARRAY_SIZE (exynos5433_apolloclk_d), -CLK_CPU_HAS_E5433_REGS_LAYOUT ); - -samsung_clk_sleep_init (reg_base, apollo_clk_regs, -ARRAY_SIZE (apollo_clk_regs)); - -samsung_clk_of_add_provider (np, ctx); -} -CLK_OF_DECLARE (exynos5433_cmu_apollo,"samsung,exynos5433-cmu-apollo" , - exynos5433_cmu_apollo_init); +static const struct samsung_cpu_clock apollo_cpu_clks[]__initconst = { +CPU_CLK (CLK_SCLK_APOLLO ,"apolloclk" ,"mout_apollo_pll" , +"mout_bus_pll_apollo_user" , +CLK_CPU_HAS_E5433_REGS_LAYOUT ,0x200 , + exynos5433_apolloclk_d), +}; + +static const struct samsung_cmu_info apollo_cmu_info__initconst = { + .pll_clks = apollo_pll_clks, + .nr_pll_clks =ARRAY_SIZE (apollo_pll_clks), + .mux_clks = apollo_mux_clks, + .nr_mux_clks =ARRAY_SIZE (apollo_mux_clks), + .div_clks = apollo_div_clks, + .nr_div_clks =ARRAY_SIZE (apollo_div_clks), + .gate_clks = apollo_gate_clks, + .nr_gate_clks =ARRAY_SIZE (apollo_gate_clks), + .cpu_clks = apollo_cpu_clks, + .nr_cpu_clks =ARRAY_SIZE (apollo_cpu_clks), + .nr_clk_ids =APOLLO_NR_CLK , + .clk_regs = apollo_clk_regs, + .nr_clk_regs =ARRAY_SIZE (apollo_clk_regs), +};/* * Register offset definitions for CMU_ATLAS
@@ -3932,47 +3913,28 @@ static const struct exynos_cpuclk_cfg_data exynos5433_atlasclk_d[] __initconst = { 0 }, }; -static void __init exynos5433_cmu_atlas_init (struct device_node *np) -{ -void __iomem *reg_base; -struct samsung_clk_provider *ctx; -struct clk_hw **hws; - - reg_base =of_iomap (np,0 ); -if (!reg_base) { -panic ("%s: failed to map registers\n" , __func__); -return ; - } - - ctx =samsung_clk_init (np, reg_base,ATLAS_NR_CLK ); -if (!ctx) { -panic ("%s: unable to allocate ctx\n" , __func__); -return ; - } - -samsung_clk_register_pll (ctx, atlas_pll_clks, -ARRAY_SIZE (atlas_pll_clks), reg_base); -samsung_clk_register_mux (ctx, atlas_mux_clks, -ARRAY_SIZE (atlas_mux_clks)); -samsung_clk_register_div (ctx, atlas_div_clks, -ARRAY_SIZE (atlas_div_clks)); -samsung_clk_register_gate (ctx, atlas_gate_clks, -ARRAY_SIZE (atlas_gate_clks)); - - hws = ctx->clk_data.hws; - -exynos_register_cpu_clock (ctx,CLK_SCLK_ATLAS ,"atlasclk" , - hws[CLK_MOUT_ATLAS_PLL ], hws[CLK_MOUT_BUS_PLL_ATLAS_USER ],0x200 , - exynos5433_atlasclk_d,ARRAY_SIZE (exynos5433_atlasclk_d), -CLK_CPU_HAS_E5433_REGS_LAYOUT ); - -samsung_clk_sleep_init (reg_base, atlas_clk_regs, -ARRAY_SIZE (atlas_clk_regs)); +static const struct samsung_cpu_clock atlas_cpu_clks[]__initconst = { +CPU_CLK (CLK_SCLK_ATLAS ,"atlasclk" ,"mout_atlas_pll" , +"mout_bus_pll_atlas_user" , +CLK_CPU_HAS_E5433_REGS_LAYOUT ,0x200 , + exynos5433_atlasclk_d), +}; -samsung_clk_of_add_provider (np, ctx); -} -CLK_OF_DECLARE (exynos5433_cmu_atlas,"samsung,exynos5433-cmu-atlas" , - exynos5433_cmu_atlas_init); +static const struct samsung_cmu_info atlas_cmu_info__initconst = { + .pll_clks = atlas_pll_clks, + .nr_pll_clks =ARRAY_SIZE (atlas_pll_clks), + .mux_clks = atlas_mux_clks, + .nr_mux_clks =ARRAY_SIZE (atlas_mux_clks), + .div_clks = atlas_div_clks, + .nr_div_clks =ARRAY_SIZE (atlas_div_clks), + .gate_clks = atlas_gate_clks, + .nr_gate_clks =ARRAY_SIZE (atlas_gate_clks), + .cpu_clks = atlas_cpu_clks, + .nr_cpu_clks =ARRAY_SIZE (atlas_cpu_clks), + .nr_clk_ids =ATLAS_NR_CLK , + .clk_regs = atlas_clk_regs, + .nr_clk_regs =ARRAY_SIZE (atlas_clk_regs), +};/* * Register offset definitions for CMU_MSCL
@@ -5700,6 +5662,12 @@ static const struct of_device_id exynos5433_cmu_of_match[] = { }, { .compatible = "samsung,exynos5433-cmu-imem", .data = &imem_cmu_info, + }, { + .compatible = "samsung,exynos5433-cmu-atlas", + .data = &atlas_cmu_info, + }, { + .compatible = "samsung,exynos5433-cmu-apollo", + .data = &apollo_cmu_info, }, { }, };
Signature · 1 line
2.33.0.685.g46640cef36-goog
PATCH v2 5/12 clk: export __clk_lookup
In reply to Will McVickerThis symbol is needed to modularize the samsung clk drivers. It's used to get the clock using the clock name.
--- drivers/clk/clk.c | 1 + 1 file changed, 1 insertion(+)
drivers/clk/clk.c +1 -0
@@ -612,6 +612,7 @@ struct clk *__clk_lookup(const char *name)return !core ?NULL : core->hw->clk; } +EXPORT_SYMBOL_GPL (__clk_lookup); static void clk_core_get_boundaries(struct clk_core *core, unsigned long *min_rate,
Signature · 1 line
2.33.0.685.g46640cef36-goog
Stephen Boyd
maintainerRe: [PATCH v2 05/12] clk: export __clk_lookup
Quoting Will McVicker (2021-09-28 16:56:22)
Quoted 13 lines
This symbol is needed to modularize the samsung clk drivers. It's used to get the clock using the clock name.
Signed-off-by Will McVicker <[email protected]>--- drivers/clk/clk.c | 1 + 1 file changed, 1 insertion(+)drivers/clk/clk.c +0
-0
drivers/clk/clk.c struct clk *__clk_lookup(const char *name) · line 615
Quoted 4 lines
drivers/clk/clk.c +1
-0@@ -612,6 +612,7 @@ struct clk *__clk_lookup(const char *name)return !core ?NULL : core->hw->clk; } +EXPORT_SYMBOL_GPL (__clk_lookup);
I'd prefer we deleted this API. Can you make the samsung clk driver stop using it instead?
PATCH v2 6/12 clk: samsung: modularize exynos arm64 clk drivers
In reply to Will McVickerThis modularizes the Exynos ARM64 clock drivers. The change consolidates the clock devices into the of match table so that the platform driver probes all the clocks (per driver) as well as exports the necessary functions for each driver.
With this, we now have 3 separate kernel modules (pending Kconfig changes):
1) clk-common-exynos.ko: the common exynos clock driver
2) clk-exynos5433.ko: the Exynos5433 clock driver
3) clk-exynos7.ko: the Exynos7 clock driver
--- drivers/clk/samsung/Makefile | 3 +- drivers/clk/samsung/clk-cpu.c | 2 +- drivers/clk/samsung/clk-cpu.h | 2 +- drivers/clk/samsung/clk-exynos5433.c | 349 +++++++++++++-------------- drivers/clk/samsung/clk-exynos7.c | 177 ++++++-------- drivers/clk/samsung/clk-pll.c | 6 +- drivers/clk/samsung/clk.c | 33 ++- drivers/clk/samsung/clk.h | 26 +- 8 files changed, 287 insertions(+), 311 deletions(-)
drivers/clk/samsung/Makefile +2 -1
@@ -3,7 +3,8 @@ # Samsung Clock specific Makefile # -obj-$(CONFIG_COMMON_CLK) += clk.o clk-pll.o clk-cpu.o +obj-$(CONFIG_COMMON_CLK_SAMSUNG) += clk-common-exynos.o +clk-common-exynos-y += clk.o clk-pll.o clk-cpu.o obj-$(CONFIG_EXYNOS_3250_COMMON_CLK) += clk-exynos3250.o obj-$(CONFIG_EXYNOS_4_COMMON_CLK) += clk-exynos4.o obj-$(CONFIG_EXYNOS_4_COMMON_CLK) += clk-exynos4412-isp.o
drivers/clk/samsung/clk-cpu.c +1 -1
@@ -400,7 +400,7 @@ static int exynos5433_cpuclk_notifier_cb(struct notifier_block *nb, }/* helper function to register a CPU clock */ -int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, +int exynos_register_cpu_clock(struct samsung_clk_provider *ctx, unsigned int lookup_id, const char *name, const struct clk_hw *parent, const struct clk_hw *alt_parent, unsigned long offset, const struct exynos_cpuclk_cfg_data *cfg,
drivers/clk/samsung/clk-cpu.h +1 -1
@@ -62,7 +62,7 @@ struct exynos_cpuclk {#define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) }; -int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, +int exynos_register_cpu_clock(struct samsung_clk_provider *ctx, unsigned int lookup_id, const char *name, const struct clk_hw *parent, const struct clk_hw *alt_parent, unsigned long offset,
drivers/clk/samsung/clk-exynos5433.c +162 -187
1,139 lines not on this page · raw message
drivers/clk/samsung/clk-exynos7.c +79 -98
344 lines not on this page · raw message
drivers/clk/samsung/clk-pll.c +4 -2
@@ -6,6 +6,7 @@* This file contains the utility functions to register the pll clocks. */ +#include <linux/module.h> #include <linux/errno.h> #include <linux/hrtimer.h> #include <linux/iopoll.h>
@@ -93,7 +94,7 @@ static int samsung_pll_lock_wait(struct samsung_clk_pll *pll,* Exynos SoC variants. Single register read time was usually in range * 0.4...1.5 us, never less than 0.4 us. */ -if (pll_early_timeout || timekeeping_suspended) { +if (pll_early_timeout ||timekeeping_is_suspended ()) { i =PLL_TIMEOUT_LOOPS ;while (i-- >0 ) {if (readl_relaxed (pll->con_reg) & reg_mask)
@@ -1392,7 +1393,7 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,samsung_clk_add_lookup (ctx, &pll->hw, pll_clk->id); } -void __init samsung_clk_register_pll (struct samsung_clk_provider *ctx, +void samsung_clk_register_pll (struct samsung_clk_provider *ctx,const struct samsung_pll_clock *pll_list,unsigned int nr_pll,void __iomem *base) {
@@ -1401,3 +1402,4 @@ void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx,for (cnt =0 ; cnt < nr_pll; cnt++)_samsung_clk_register_pll (ctx, &pll_list[cnt], base); } +EXPORT_SYMBOL_GPL (samsung_clk_register_pll);
drivers/clk/samsung/clk.c +25 -8
@@ -13,6 +13,7 @@#include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/io.h> +#include <linux/module.h> #include <linux/of_address.h> #include <linux/syscore_ops.h>
@@ -27,6 +28,7 @@ void samsung_clk_save(void __iomem *base,for (; num_regs >0 ; --num_regs, ++rd) rd->value =readl (base + rd->offset); } +EXPORT_SYMBOL_GPL (samsung_clk_save); void samsung_clk_restore(void __iomem *base, const struct samsung_clk_reg_dump *rd,
@@ -35,6 +37,7 @@ void samsung_clk_restore(void __iomem *base,for (; num_regs >0 ; --num_regs, ++rd)writel (rd->value, base + rd->offset); } +EXPORT_SYMBOL_GPL (samsung_clk_restore); struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump( const unsigned long *rdump,
@@ -52,9 +55,10 @@ struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(return rd; } +EXPORT_SYMBOL_GPL (samsung_clk_alloc_reg_dump);/* setup the essentials required to support clock lookup using ccf */ -struct samsung_clk_provider *__init samsung_clk_init (struct device_node *np, +struct samsung_clk_provider *samsung_clk_init (struct device_node *np,void __iomem *base,unsigned long nr_clks) {struct samsung_clk_provider *ctx;
@@ -74,7 +78,7 @@ struct samsung_clk_provider *__init samsung_clk_init(struct device_node *np,return ctx; } -void __init samsung_clk_of_add_provider (struct device_node *np, +void samsung_clk_of_add_provider (struct device_node *np,struct samsung_clk_provider *ctx) {if (np) {
@@ -83,6 +87,7 @@ void __init samsung_clk_of_add_provider(struct device_node *np,panic ("could not register clk provider\n" ); } } +EXPORT_SYMBOL_GPL (samsung_clk_of_add_provider);/* add a clock instance to the clock lookup table used for dt based lookup */ void samsung_clk_add_lookup(struct samsung_clk_provider *ctx,
@@ -123,7 +128,7 @@ void __init samsung_clk_register_alias(struct samsung_clk_provider *ctx, }/* register a list of fixed clocks */ -void __init samsung_clk_register_fixed_rate (struct samsung_clk_provider *ctx, +void samsung_clk_register_fixed_rate (struct samsung_clk_provider *ctx,const struct samsung_fixed_rate_clock *list,unsigned int nr_clk) {
@@ -151,9 +156,10 @@ void __init samsung_clk_register_fixed_rate(struct samsung_clk_provider *ctx, __func__, list->name); } } +EXPORT_SYMBOL_GPL (samsung_clk_register_fixed_rate);/* register a list of fixed factor clocks */ -void __init samsung_clk_register_fixed_factor (struct samsung_clk_provider *ctx, +void samsung_clk_register_fixed_factor (struct samsung_clk_provider *ctx,const struct samsung_fixed_factor_clock *list,unsigned int nr_clk) {struct clk_hw *clk_hw;
@@ -171,9 +177,10 @@ void __init samsung_clk_register_fixed_factor(struct samsung_clk_provider *ctx,samsung_clk_add_lookup (ctx, clk_hw, list->id); } } +EXPORT_SYMBOL_GPL (samsung_clk_register_fixed_factor);/* register a list of mux clocks */ -void __init samsung_clk_register_mux (struct samsung_clk_provider *ctx, +void samsung_clk_register_mux (struct samsung_clk_provider *ctx,const struct samsung_mux_clock *list,unsigned int nr_clk) {
@@ -194,9 +201,10 @@ void __init samsung_clk_register_mux(struct samsung_clk_provider *ctx,samsung_clk_add_lookup (ctx, clk_hw, list->id); } } +EXPORT_SYMBOL_GPL (samsung_clk_register_mux);/* register a list of div clocks */ -void __init samsung_clk_register_div (struct samsung_clk_provider *ctx, +void samsung_clk_register_div (struct samsung_clk_provider *ctx,const struct samsung_div_clock *list,unsigned int nr_clk) {
@@ -224,9 +232,10 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,samsung_clk_add_lookup (ctx, clk_hw, list->id); } } +EXPORT_SYMBOL_GPL (samsung_clk_register_div);/* register a list of gate clocks */ -void __init samsung_clk_register_gate (struct samsung_clk_provider *ctx, +void samsung_clk_register_gate (struct samsung_clk_provider *ctx,const struct samsung_gate_clock *list,unsigned int nr_clk) {
@@ -246,6 +255,7 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,samsung_clk_add_lookup (ctx, clk_hw, list->id); } } +EXPORT_SYMBOL_GPL (samsung_clk_register_gate);/* * obtain the clock speed of all external fixed clock sources from device
@@ -342,7 +352,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,* Common function which registers plls, muxes, dividers and gates * for each CMU. It also add CMU register list to register cache. */ -struct samsung_clk_provider *__init samsung_cmu_register_one ( +struct samsung_clk_provider *samsung_cmu_register_one (struct device_node *np,const struct samsung_cmu_info *cmu) {
@@ -385,3 +395,10 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(return ctx; } +EXPORT_SYMBOL_GPL (samsung_cmu_register_one); + +MODULE_DESCRIPTION ("Samsung Exynos clock controller support" ); +MODULE_AUTHOR ("Thomas Abraham <[email protected]>" ); +MODULE_AUTHOR ("Marek Szyprowski <[email protected]>" ); +MODULE_AUTHOR ("Bartlomiej Zolnierkiewicz <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );
drivers/clk/samsung/clk.h +13 -13
@@ -222,7 +222,7 @@ struct samsung_gate_clock {#define GATE (_id, cname, pname, o, b, f, gf) \ __GATE(_id, cname, pname, o, b, f, gf) -#define PNAME (x) static const char * const x[] __initconst +#define PNAME (x) static const char * const x[]/** * struct samsung_clk_reg_dump: register dump of clock controller registers.
@@ -337,10 +337,10 @@ struct samsung_cmu_info {const char *clk_name; }; -extern struct samsung_clk_provider *__init samsung_clk_init ( +extern struct samsung_clk_provider *samsung_clk_init (struct device_node *np,void __iomem *base,unsigned long nr_clks); -extern void __init samsung_clk_of_add_provider (struct device_node *np, +extern void samsung_clk_of_add_provider (struct device_node *np,struct samsung_clk_provider *ctx); extern void __init samsung_clk_of_register_fixed_ext( struct samsung_clk_provider *ctx,
@@ -354,32 +354,32 @@ extern void samsung_clk_add_lookup(struct samsung_clk_provider *ctx,extern void __init samsung_clk_register_alias (struct samsung_clk_provider *ctx,const struct samsung_clock_alias *list,unsigned int nr_clk); -extern void __init samsung_clk_register_fixed_rate ( +extern void samsung_clk_register_fixed_rate (struct samsung_clk_provider *ctx,const struct samsung_fixed_rate_clock *clk_list,unsigned int nr_clk); -extern void __init samsung_clk_register_fixed_factor ( +extern void samsung_clk_register_fixed_factor (struct samsung_clk_provider *ctx,const struct samsung_fixed_factor_clock *list,unsigned int nr_clk); -extern void __init samsung_clk_register_mux (struct samsung_clk_provider *ctx, +extern void samsung_clk_register_mux (struct samsung_clk_provider *ctx,const struct samsung_mux_clock *clk_list,unsigned int nr_clk); -extern void __init samsung_clk_register_div (struct samsung_clk_provider *ctx, +extern void samsung_clk_register_div (struct samsung_clk_provider *ctx,const struct samsung_div_clock *clk_list,unsigned int nr_clk); -extern void __init samsung_clk_register_gate (struct samsung_clk_provider *ctx, +extern void samsung_clk_register_gate (struct samsung_clk_provider *ctx,const struct samsung_gate_clock *clk_list,unsigned int nr_clk); -extern void __init samsung_clk_register_pll (struct samsung_clk_provider *ctx, +extern void samsung_clk_register_pll (struct samsung_clk_provider *ctx,const struct samsung_pll_clock *pll_list,unsigned int nr_clk,void __iomem *base); -extern void __init samsung_clk_register_cpu (struct samsung_clk_provider *ctx, +extern void samsung_clk_register_cpu (struct samsung_clk_provider *ctx,const struct samsung_cpu_clock *list,unsigned int nr_clk); -extern struct samsung_clk_provider __init *samsung_cmu_register_one ( -struct device_node *, -const struct samsung_cmu_info *); +extern struct samsung_clk_provider *samsung_cmu_register_one ( +struct device_node *np, +const struct samsung_cmu_info *cmu);extern unsigned long _get_rate (const char *clk_name);
Signature · 1 line
2.33.0.685.g46640cef36-goog
PATCH v2 7/12 clk: samsung: set exynos arm64 clk driver as tristate
In reply to Will McVickerThis sets the COMMON_CLK_SAMSUNG and EXYNOS_ARM64_COMMON_CLK drivers as tristate so that we can compile them as modules.
--- arch/arm/mach-exynos/Kconfig | 1 - arch/arm/mach-s3c/Kconfig.s3c64xx | 1 - arch/arm/mach-s5pv210/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/clk/samsung/Kconfig | 5 +++-- 5 files changed, 3 insertions(+), 6 deletions(-)
arch/arm/mach-exynos/Kconfig +0 -1
@@ -12,7 +12,6 @@ menuconfig ARCH_EXYNOS select ARM_AMBA select ARM_GIC select EXYNOS_IRQ_COMBINER - select COMMON_CLK_SAMSUNG select EXYNOS_THERMAL select EXYNOS_PMU select EXYNOS_SROM
arch/arm/mach-s3c/Kconfig.s3c64xx +0 -1
@@ -9,7 +9,6 @@ menuconfig ARCH_S3C64XX select ARM_AMBA select ARM_VIC select CLKSRC_SAMSUNG_PWM - select COMMON_CLK_SAMSUNG select GPIO_SAMSUNG if ATAGS select GPIOLIB select HAVE_S3C2410_I2C if I2C
arch/arm/mach-s5pv210/Kconfig +0 -1
@@ -10,7 +10,6 @@ config ARCH_S5PV210 depends on ARCH_MULTI_V7 select ARM_VIC select CLKSRC_SAMSUNG_PWM - select COMMON_CLK_SAMSUNG select GPIOLIB select HAVE_S3C2410_I2C if I2C select HAVE_S3C_RTC if RTC_CLASS
arch/arm64/Kconfig.platforms +0 -1
@@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select COMMON_CLK_SAMSUNG select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select EXYNOS_PMU select HAVE_S3C_RTC if RTC_CLASS
drivers/clk/samsung/Kconfig +3 -2
@@ -1,7 +1,8 @@ # SPDX-License-Identifier: GPL-2.0-only # Recent Exynos platforms should just select COMMON_CLK_SAMSUNG: config COMMON_CLK_SAMSUNG - bool "Samsung Exynos clock controller support" if COMPILE_TEST + tristate "Samsung Exynos clock controller support" + default y if ARCH_EXYNOS || ARCH_S5PV210 || ARCH_S3C64XX select S3C64XX_COMMON_CLK if ARM && ARCH_S3C64XX select S5PV210_COMMON_CLK if ARM && ARCH_S5PV210 select EXYNOS_3250_COMMON_CLK if ARM && SOC_EXYNOS3250
@@ -70,7 +71,7 @@ config EXYNOS_5420_COMMON_CLK Exynos5420 SoCs. Choose Y here only if you build for this SoC. config EXYNOS_ARM64_COMMON_CLK - bool "Samsung Exynos ARMv8-family clock controller support" if COMPILE_TEST + tristate "Samsung Exynos ARMv8-family clock controller support" depends on COMMON_CLK_SAMSUNG config EXYNOS_AUDSS_CLK_CON
Signature · 1 line
2.33.0.685.g46640cef36-goog
Re: [PATCH v2 07/12] clk: samsung: set exynos arm64 clk driver as tristate
Quoted 2 lines · Will McVicker
This sets the
COMMON_CLK_SAMSUNGandEXYNOS_ARM64_COMMON_CLKdrivers as tristate so that we can compile them as modules.
This is not the only thing you are doing. You are removing the enforced built-in and this must be here explained and tested. The policy so far was that all S3C, S5P and Exynos supported platforms select (enforce) necessary drivers. Please explain in commit msg reasoning why this is being changed and what's the impact (whether it actually works).
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
Best regards,
Krzysztof
PATCH v2 8/12 pinctrl: samsung: modularize the ARM and ARM64 pinctrls
In reply to Will McVickerThis patch modularizes the Samsung Exynos ARM and ARM64 pinctrl drivers. It creates 2 kernel modules (pending Kconfig changes):
1) pinctrl-samsung.ko: common pinctrl driver for all the samsung pinctrl drivers.
2) pinctrl_exynos.ko: ARM and ARM64 pinctrl driver.
--- drivers/pinctrl/samsung/Makefile | 13 +-- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 102 ++++++++++-------- .../pinctrl/samsung/pinctrl-exynos-arm64.c | 73 +++++++------ drivers/pinctrl/samsung/pinctrl-exynos.c | 17 +-- drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- 5 files changed, 117 insertions(+), 99 deletions(-)
drivers/pinctrl/samsung/Makefile +7 -6
@@ -1,9 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 # Samsung pin control drivers -obj-$(CONFIG_PINCTRL_SAMSUNG) += pinctrl-samsung.o -obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl-exynos.o -obj-$(CONFIG_PINCTRL_EXYNOS_ARM) += pinctrl-exynos-arm.o -obj-$(CONFIG_PINCTRL_EXYNOS_ARM64) += pinctrl-exynos-arm64.o -obj-$(CONFIG_PINCTRL_S3C24XX) += pinctrl-s3c24xx.o -obj-$(CONFIG_PINCTRL_S3C64XX) += pinctrl-s3c64xx.o +obj-$(CONFIG_PINCTRL_SAMSUNG) += pinctrl-samsung.o +obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl_exynos.o +pinctrl_exynos-y += pinctrl-exynos.o +pinctrl_exynos-$(CONFIG_PINCTRL_EXYNOS_ARM) += pinctrl-exynos-arm.o +pinctrl_exynos-$(CONFIG_PINCTRL_EXYNOS_ARM64) += pinctrl-exynos-arm64.o +obj-$(CONFIG_PINCTRL_S3C24XX) += pinctrl-s3c24xx.o +obj-$(CONFIG_PINCTRL_S3C64XX) += pinctrl-s3c64xx.o
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +55 -47
377 lines not on this page · raw message
drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +38 -35
269 lines not on this page · raw message
drivers/pinctrl/samsung/pinctrl-exynos.c +11 -6
@@ -20,6 +20,7 @@#include <linux/irqchip/chained_irq.h> #include <linux/of.h> #include <linux/of_irq.h> +#include <linux/module.h> #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/regmap.h>
@@ -207,7 +208,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd)/* * irq_chip for gpio interrupts. */ -static const struct exynos_irq_chip exynos_gpio_irq_chip__initconst = { +static const struct exynos_irq_chip exynos_gpio_irq_chip = { .chip = { .name ="exynos_gpio_irq_chip" , .irq_unmask = exynos_irq_unmask,
@@ -275,7 +276,7 @@ struct exynos_eint_gpio_save {* exynos_eint_gpio_init() - setup handling of external gpio interrupts. * @d: driver data of samsung pinctrl driver. */ -__init int exynos_eint_gpio_init (struct samsung_pinctrl_drv_data *d) +int exynos_eint_gpio_init (struct samsung_pinctrl_drv_data *d) {struct samsung_pin_bank *bank;struct device *dev = d->dev;
@@ -399,7 +400,7 @@ static u32 eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED;/* * irq_chip for wakeup interrupts */ -static const struct exynos_irq_chip s5pv210_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip s5pv210_wkup_irq_chip = { .chip = { .name ="s5pv210_wkup_irq_chip" , .irq_unmask = exynos_irq_unmask,
@@ -419,7 +420,7 @@ static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = { .set_eint_wakeup_mask = s5pv210_pinctrl_set_eint_wakeup_mask, }; -static const struct exynos_irq_chip exynos4210_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip exynos4210_wkup_irq_chip = { .chip = { .name ="exynos4210_wkup_irq_chip" , .irq_unmask = exynos_irq_unmask,
@@ -438,7 +439,7 @@ static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = { .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, }; -static const struct exynos_irq_chip exynos7_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip exynos7_wkup_irq_chip = { .chip = { .name ="exynos7_wkup_irq_chip" , .irq_unmask = exynos_irq_unmask,
@@ -521,7 +522,7 @@ static void exynos_irq_demux_eint16_31(struct irq_desc *desc)* exynos_eint_wkup_init() - setup handling of external wakeup interrupts. * @d: driver data of samsung pinctrl driver. */ -__init int exynos_eint_wkup_init (struct samsung_pinctrl_drv_data *d) +int exynos_eint_wkup_init (struct samsung_pinctrl_drv_data *d) {struct device *dev = d->dev;struct device_node *wkup_np =NULL ;
@@ -760,3 +761,7 @@ exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata,return ctrl; } + +MODULE_DESCRIPTION ("Pinctrl common driver for Exynos, S3C24XX, and S3C64XX SoCs" ); +MODULE_AUTHOR ("Thomas Abraham <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );
drivers/pinctrl/samsung/pinctrl-samsung.c +6 -5
@@ -18,6 +18,7 @@#include <linux/init.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/module.h> #include <linux/slab.h> #include <linux/err.h> #include <linux/gpio/driver.h>
@@ -1299,8 +1300,8 @@ static struct platform_driver samsung_pinctrl_driver = { }, }; -static int __init samsung_pinctrl_drv_register (void ) -{ -return platform_driver_register (&samsung_pinctrl_driver); -} -postcore_initcall (samsung_pinctrl_drv_register); +module_platform_driver (samsung_pinctrl_driver); + +MODULE_DESCRIPTION ("Pinctrl common driver for Exynos, S3C24XX, and S3C64XX SoCs" ); +MODULE_AUTHOR ("Thomas Abraham <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.33.0.685.g46640cef36-goog
RE: [PATCH v2 08/12] pinctrl: samsung: modularize the ARM and ARM64 pinctrls
Quoted 2 lines
This patch modularizes the Samsung Exynos ARM and ARM64 pinctrl drivers. It creates 2 kernel modules (pending
Kconfigchanges):
Hmm. I think we can use pinctrl-samsung.ko as a super module because chip-specific objects can be selectable by Kconfig option. So, we don't need to separate them.
Quoted 42 lines
1) pinctrl-samsung.ko: common pinctrl driver for all the samsung pinctrl drivers.
2) pinctrl_exynos.ko: ARM and ARM64 pinctrl driver.Signed-off-by Will McVicker <[email protected]>--- drivers/pinctrl/samsung/Makefile | 13 +-- drivers/pinctrl/samsung/pinctrl-exynos-arm.c | 102 ++++++++++-------- .../pinctrl/samsung/pinctrl-exynos-arm64.c | 73 +++++++------ drivers/pinctrl/samsung/pinctrl-exynos.c | 17 +-- drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- 5 files changed, 117 insertions(+), 99 deletions(-)drivers/pinctrl/samsung/Makefile +7
-6@@ -1,9 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 # Samsung pin control drivers -obj-$(CONFIG_PINCTRL_SAMSUNG) += pinctrl-samsung.o -obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl-exynos.o -obj-$(CONFIG_PINCTRL_EXYNOS_ARM) += pinctrl-exynos-arm.o -obj-$(CONFIG_PINCTRL_EXYNOS_ARM64) += pinctrl-exynos-arm64.o -obj-$(CONFIG_PINCTRL_S3C24XX) += pinctrl-s3c24xx.o -obj-$(CONFIG_PINCTRL_S3C64XX) += pinctrl-s3c64xx.o +obj-$(CONFIG_PINCTRL_SAMSUNG) += pinctrl-samsung.o +obj-$(CONFIG_PINCTRL_EXYNOS) += pinctrl_exynos.o +pinctrl_exynos-y += pinctrl-exynos.o +pinctrl_exynos-$(CONFIG_PINCTRL_EXYNOS_ARM) += pinctrl-exynos-arm.o +pinctrl_exynos-$(CONFIG_PINCTRL_EXYNOS_ARM64) += pinctrl-exynos-arm64.o +obj-$(CONFIG_PINCTRL_S3C24XX) += pinctrl-s3c24xx.o +obj-$(CONFIG_PINCTRL_S3C64XX) += pinctrl-s3c64xx.odrivers/pinctrl/samsung/pinctrl-exynos-arm.c +0
-0
drivers/pinctrl/samsung/pinctrl-exynos-arm.c s5pv210_retention_init(struct samsung_pinctrl_drv_data · line 90
Quoted 9 lines
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-1@@ -83,12 +83,12 @@ s5pv210_retention_init(struct samsung_pinctrl_drv_data *drvdata, return ctrl; } -static const struct samsung_retention_data s5pv210_retention_data __initconst = { +static const struct samsung_retention_data s5pv210_retention_data = { .init = s5pv210_retention_init, };
Any reason why do we need to drop __initconst and __init? AFAIK, they will be automatically converted in case of MODULE.
Quoted 835 lines
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0/* pin banks of s5pv210 pin-controller */ -static const structsamsung_pin_bank_data s5pv210_pin_bank[]__initconst = { +static const struct samsung_pin_bank_data s5pv210_pin_bank[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(4, 0x020, "gpa1", 0x04), @@ -126,7 +126,7 @@static const struct
samsung_pin_bank_datas5pv210_pin_bank[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-2EXYNOS_PIN_BANK_EINTW(8, 0xc60, "gph3", 0x0c), }; -static const struct samsung_pin_ctrl s5pv210_pin_ctrl[]__initconst = { +static const struct samsung_pin_ctrl s5pv210_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = s5pv210_pin_bank,@@ -139,16 +139,17 @@ static const struct samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data s5pv210_of_data__initconst = { +const struct samsung_pinctrl_of_match_data s5pv210_of_data = { .ctrl = s5pv210_pin_ctrl, .num_ctrl =ARRAY_SIZE (s5pv210_pin_ctrl), }; +EXPORT_SYMBOL_GPL (s5pv210_of_data);/* Pad retention control code for accessing PMU regmap */ static atomic_t exynos_shared_retention_refcnt; /* pin banks of exynos3250 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos3250_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos3250_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -160,7 +161,7 @@static const struct
samsung_pin_bank_dataexynos3250_pin_banks0[]__initconst= };/* pin banks of exynos3250 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos3250_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos3250_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTN (8 ,0x120 ,"gpe0" ), EXYNOS_PIN_BANK_EINTN(8, 0x140, "gpe1"), @@ -196,7 +197,7 @@ staticconst u32 exynos3250_retention_regs[] = {
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +0
-1S5P_PAD_RET_SPI_OPTION, }; -static const struct samsung_retention_data exynos3250_retention_data
__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +2
-0+static const struct samsung_retention_data exynos3250_retention_data = +{ .regs = exynos3250_retention_regs, .nr_regs =ARRAY_SIZE (exynos3250_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -208,7 +209,7 @@ static const struct samsung_retention_data
exynos3250_retention_data__initconst
* Samsung pinctrl driver data for Exynos3250 SoC. Exynos3250 SoC
includes
* two gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos3250_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos3250_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos3250_pin_banks0,@@ -229,13 +230,14 @@ static const struct samsung_pin_ctrl exynos3250_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos3250_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos3250_of_data = { .ctrl = exynos3250_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos3250_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos3250_of_data); /* pin banks of exynos4210 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos4210_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos4210_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -256,7 +258,7 @@static const struct
samsung_pin_bank_dataexynos4210_pin_banks0[]__initconst= };/* pin banks of exynos4210 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos4210_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos4210_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpj0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(5, 0x020, "gpj1", 0x04), @@ -281,7 +283,7 @@static const struct
samsung_pin_bank_dataexynos4210_pin_banks1[]__initconst= };/* pin banks of exynos4210 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos4210_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos4210_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTN(7, 0x000, "gpz"), }; @@ -296,7 +298,7 @@static const u32 exynos4_retention_regs[] = {
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +0
-1S5P_PAD_RET_EBIB_OPTION, }; -static const struct samsung_retention_data exynos4_retention_data
__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +3
-1+static const struct samsung_retention_data exynos4_retention_data = { .regs = exynos4_retention_regs, .nr_regs =ARRAY_SIZE (exynos4_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -309,7 +311,7 @@ static const u32 exynos4_audio_retention_regs[] = { S5P_PAD_RET_MAUDIO_OPTION, }; -static const struct samsung_retention_data exynos4_audio_retention_data __initconst = { +static const struct samsung_retention_data exynos4_audio_retention_data += { .regs = exynos4_audio_retention_regs, .nr_regs =ARRAY_SIZE (exynos4_audio_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -320,7 +322,7 @@ static const struct samsung_retention_data
exynos4_audio_retention_data__initco
* Samsung pinctrl driver data for Exynos4210 SoC. Exynos4210 SoC
includes
* three gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos4210_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos4210_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos4210_pin_banks0,@@ -346,13 +348,14 @@ static const struct samsung_pin_ctrl exynos4210_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos4210_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos4210_of_data = { .ctrl = exynos4210_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos4210_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos4210_of_data); /* pin banks of exynos4x12 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos4x12_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos4x12_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -370,7 +373,7 @@static const struct
samsung_pin_bank_dataexynos4x12_pin_banks0[]__initconst= };/* pin banks of exynos4x12 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos4x12_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos4x12_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x040 ,"gpk0" ,0x08 ), EXYNOS_PIN_BANK_EINTG(7, 0x060, "gpk1", 0x0c), @@ -398,13 +401,13@@ static const struct
samsung_pin_bank_dataexynos4x12_pin_banks1[]__initconst= };/* pin banks of exynos4x12 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos4x12_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos4x12_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x000 ,"gpz" ,0x00 ), }; /* pin banks of exynos4x12 pin-controller 3 */ -static const struct
samsung_pin_bank_dataexynos4x12_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos4x12_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpv0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpv1", 0x04), @@ -417,7 +420,7 @@static const struct
samsung_pin_bank_dataexynos4x12_pin_banks3[]__initconst=
* Samsung pinctrl driver data for Exynos4x12 SoC. Exynos4x12 SoC
includes
* four gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos4x12_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos4x12_pin_banks0,@@ -453,13 +456,14 @@ static const struct samsung_pin_ctrl exynos4x12_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos4x12_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos4x12_of_data = { .ctrl = exynos4x12_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos4x12_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos4x12_of_data); /* pin banks of exynos5250 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos5250_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos5250_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -489,7 +493,7 @@static const struct
samsung_pin_bank_dataexynos5250_pin_banks0[]__initconst= };/* pin banks of exynos5250 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos5250_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5250_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpe0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(2, 0x020, "gpe1", 0x04), @@ -503,7 +507,7 @@static const struct
samsung_pin_bank_dataexynos5250_pin_banks1[]__initconst= };/* pin banks of exynos5250 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos5250_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5250_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpv0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpv1", 0x04), @@ -513,7 +517,7 @@static const struct
samsung_pin_bank_dataexynos5250_pin_banks2[]__initconst= };/* pin banks of exynos5250 pin-controller 3 */ -static const struct
samsung_pin_bank_dataexynos5250_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5250_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpz", 0x00), }; @@ -522,7 +526,7@@ static const struct
samsung_pin_bank_dataexynos5250_pin_banks3[]__initconst=
* Samsung pinctrl driver data for Exynos5250 SoC. Exynos5250 SoC
includes
* four gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos5250_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos5250_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos5250_pin_banks0,@@ -558,13 +562,14 @@ static const struct samsung_pin_ctrl exynos5250_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos5250_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos5250_of_data = { .ctrl = exynos5250_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos5250_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos5250_of_data); /* pin banks of exynos5260 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos5260_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos5260_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (4 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(7, 0x020, "gpa1", 0x04), @@ -590,7 +595,7 @@static const struct
samsung_pin_bank_dataexynos5260_pin_banks0[]__initconst= };/* pin banks of exynos5260 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos5260_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5260_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x000 ,"gpc0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpc1", 0x04), @@ -600,7 +605,7 @@static const struct
samsung_pin_bank_dataexynos5260_pin_banks1[]__initconst= };/* pin banks of exynos5260 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos5260_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5260_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x000 ,"gpz0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(4, 0x020, "gpz1", 0x04), @@ -610,7 +615,7 @@static const struct
samsung_pin_bank_dataexynos5260_pin_banks2[]__initconst=
* Samsung pinctrl driver data for Exynos5260 SoC. Exynos5260 SoC
includes
* three gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos5260_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos5260_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos5260_pin_banks0,@@ -636,13 +641,14 @@ static const struct samsung_pin_ctrl exynos5260_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos5260_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos5260_of_data = { .ctrl = exynos5260_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos5260_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos5260_of_data); /* pin banks of exynos5410 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos5410_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos5410_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -682,7 +688,7 @@static const struct
samsung_pin_bank_dataexynos5410_pin_banks0[]__initconst= };/* pin banks of exynos5410 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos5410_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5410_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (5 ,0x000 ,"gpj0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpj1", 0x04), @@ -696,7 +702,7 @@static const struct
samsung_pin_bank_dataexynos5410_pin_banks1[]__initconst= };/* pin banks of exynos5410 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos5410_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5410_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpv0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpv1", 0x04), @@ -706,7 +712,7 @@static const struct
samsung_pin_bank_dataexynos5410_pin_banks2[]__initconst= };/* pin banks of exynos5410 pin-controller 3 */ -static const struct
samsung_pin_bank_dataexynos5410_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5410_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpz", 0x00), }; @@ -715,7 +721,7@@ static const struct
samsung_pin_bank_dataexynos5410_pin_banks3[]__initconst=
* Samsung pinctrl driver data for Exynos5410 SoC. Exynos5410 SoC
includes
* four gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos5410_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +4
-1+static const struct samsung_pin_ctrl exynos5410_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos5410_pin_banks0,@@ -748,13 +754,14 @@ static const struct samsung_pin_ctrl exynos5410_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos5410_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos5410_of_data = { .ctrl = exynos5410_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos5410_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos5410_of_data); /* pin banks of exynos5420 pin-controller 0 */ -static const structsamsung_pin_bank_data exynos5420_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos5420_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpy7" ,0x00 ), EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), @@ -764,7 +771,7 @@static const struct
samsung_pin_bank_dataexynos5420_pin_banks0[]__initconst= };/* pin banks of exynos5420 pin-controller 1 */ -static const struct
samsung_pin_bank_dataexynos5420_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5420_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpc0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpc1", 0x04), @@ -782,7 +789,7 @@static const struct
samsung_pin_bank_dataexynos5420_pin_banks1[]__initconst= };/* pin banks of exynos5420 pin-controller 2 */ -static const struct
samsung_pin_bank_dataexynos5420_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5420_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpe0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(2, 0x020, "gpe1", 0x04), @@ -795,7 +802,7 @@static const struct
samsung_pin_bank_dataexynos5420_pin_banks2[]__initconst= };/* pin banks of exynos5420 pin-controller 3 */ -static const struct
samsung_pin_bank_dataexynos5420_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5420_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), @@ -809,7 +816,7 @@static const struct
samsung_pin_bank_dataexynos5420_pin_banks3[]__initconst= };/* pin banks of exynos5420 pin-controller 4 */ -static const struct
samsung_pin_bank_dataexynos5420_pin_banks4[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +1
-0+static const struct samsung_pin_bank_data exynos5420_pin_banks4[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpz", 0x00), }; @@ -830,7 +837,7@@ static const u32 exynos5420_retention_regs[] = {
drivers/pinctrl/samsung/pinctrl-exynos-arm.c +0
-1EXYNOS5420_PAD_RET_DRAM_COREBLK_OPTION, }; -static const struct samsung_retention_data exynos5420_retention_data
__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +2
-0+static const struct samsung_retention_data exynos5420_retention_data = +{ .regs = exynos5420_retention_regs, .nr_regs =ARRAY_SIZE (exynos5420_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -842,7 +849,7 @@ static const struct samsung_retention_data
exynos5420_retention_data__initconst
* Samsung pinctrl driver data for Exynos5420 SoC. Exynos5420 SoC
includes
* four gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos5420_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm.c +3
-1+static const struct samsung_pin_ctrl exynos5420_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos5420_pin_banks0,@@ -887,7 +894,8 @@ static const struct samsung_pin_ctrl exynos5420_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos5420_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos5420_of_data = { .ctrl = exynos5420_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos5420_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos5420_of_data);drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +0
-0@@ -62,7 +62,7 @@ static const struct samsung_pin_bank_type
exynos850_bank_type_alive= { staticatomic_texynos_shared_retention_refcnt;/* pin banks of exynos5433 pin-controller - ALIVE */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks0[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTW (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS5433_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), @@ -76,32 +76,32@@ static const struct
samsung_pin_bank_dataexynos5433_pin_banks0[]__initconst= };/* pin banks of exynos5433 pin-controller - AUD */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (7 ,0x000 ,"gpz0" ,0x00 ),EXYNOS5433_PIN_BANK_EINTG (4 ,0x020 ,"gpz1" ,0x04 ), }; /* pin banks of exynos5433 pin-controller - CPIF */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (2 ,0x000 ,"gpv6" ,0x00 ), }; /* pin banks of exynos5433 pin-controller - eSE */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (3 ,0x000 ,"gpj2" ,0x00 ), }; /* pin banks of exynos5433 pin-controller - FINGER */ -static conststruct
samsung_pin_bank_dataexynos5433_pin_banks4[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks4[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (4 ,0x000 ,"gpd5" ,0x00 ), }; /* pin banks of exynos5433 pin-controller - FSYS */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks5[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +2
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks5[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (6 ,0x000 ,"gph1" ,0x00 ), EXYNOS5433_PIN_BANK_EINTG(7, 0x020, "gpr4", 0x04), @@ -112,19 +112,19 @@ static const struct samsung_pin_bank_dataexynos5433_pin_banks5[]
__initconst= };/* pin banks of exynos5433 pin-controller - IMEM */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks6[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks6[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (8 ,0x000 ,"gpf0" ,0x00 ), }; /* pin banks of exynos5433 pin-controller - NFC */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks7[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks7[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (3 ,0x000 ,"gpj0" ,0x00 ), }; /* pin banks of exynos5433 pin-controller - PERIC */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks8[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos5433_pin_banks8[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG (6 ,0x000 ,"gpv7" ,0x00 ), EXYNOS5433_PIN_BANK_EINTG(5, 0x020, "gpb0", 0x04), @@ -146,7 +146,7@@ static const struct
samsung_pin_bank_dataexynos5433_pin_banks8[]__initconst= };/* pin banks of exynos5433 pin-controller - TOUCH */ -static const struct
samsung_pin_bank_dataexynos5433_pin_banks9[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +2
-1+static const struct samsung_pin_bank_data exynos5433_pin_banks9[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS5433_PIN_BANK_EINTG(3, 0x000, "gpj1", 0x00), }; @@ -165,7 +165,7 @@ static const u32 exynos5433_retention_regs[] = {EXYNOS5433_PAD_RETENTION_FSYSGENIO_OPTION , }; -static const struct samsung_retention_data exynos5433_retention_data
__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +6
-2+static const struct samsung_retention_data exynos5433_retention_data = +{ .regs = exynos5433_retention_regs, .nr_regs =ARRAY_SIZE (exynos5433_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -178,7 +178,7 @@ static const u32 exynos5433_audio_retention_regs[] = { EXYNOS5433_PAD_RETENTION_AUD_OPTION, }; -static const struct samsung_retention_data exynos5433_audio_retention_data __initconst = { +static const struct samsung_retention_data +exynos5433_audio_retention_data = { .regs = exynos5433_audio_retention_regs, .nr_regs =ARRAY_SIZE (exynos5433_audio_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -192,7 +192,7 @@ static const u32 exynos5433_fsys_retention_regs[] = { EXYNOS5433_PAD_RETENTION_MMC2_OPTION, }; -static const struct samsung_retention_data exynos5433_fsys_retention_data __initconst = { +static const struct samsung_retention_data +exynos5433_fsys_retention_data = { .regs = exynos5433_fsys_retention_regs, .nr_regs =ARRAY_SIZE (exynos5433_fsys_retention_regs), .value =EXYNOS_WAKEUP_FROM_LOWPWR ,@@ -203,7 +203,7 @@ static const struct samsung_retention_data
exynos5433_fsys_retention_data__init
* Samsung pinctrl driver data for Exynos5433 SoC. Exynos5433 SoC
includes
* ten gpio/pin-mux/pinconfig controllers.
*/
-static const structsamsung_pin_ctrlexynos5433_pin_ctrl[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +4
-1+static const struct samsung_pin_ctrl exynos5433_pin_ctrl[] = { {/* pin-controller instance 0 data */ .pin_banks = exynos5433_pin_banks0,@@ -288,13 +288,14 @@ static const struct samsung_pin_ctrl exynos5433_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos5433_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos5433_of_data = { .ctrl = exynos5433_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos5433_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos5433_of_data); /* pin banks of exynos7 pin-controller - ALIVE */ -static const structsamsung_pin_bank_data exynos7_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos7_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTW (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), @@ -303,7 +304,7 @@static const struct
samsung_pin_bank_dataexynos7_pin_banks0[]__initconst= { };/* pin banks of exynos7 pin-controller - BUS0 */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (5 ,0x000 ,"gpb0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpc0", 0x04), @@ -323,37 +324,37@@ static const struct
samsung_pin_bank_dataexynos7_pin_banks1[]__initconst= { };/* pin banks of exynos7 pin-controller - NFC */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (3 ,0x000 ,"gpj0" ,0x00 ), }; /* pin banks of exynos7 pin-controller - TOUCH */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (3 ,0x000 ,"gpj1" ,0x00 ), }; /* pin banks of exynos7 pin-controller - FF */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks4[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks4[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (4 ,0x000 ,"gpg4" ,0x00 ), }; /* pin banks of exynos7 pin-controller - ESE */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks5[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks5[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (5 ,0x000 ,"gpv7" ,0x00 ), }; /* pin banks of exynos7 pin-controller - FSYS0 */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks6[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks6[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x000 ,"gpr4" ,0x00 ), }; /* pin banks of exynos7 pin-controller - FSYS1 */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks7[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks7[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (4 ,0x000 ,"gpr0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpr1", 0x04), @@ -362,7 +363,7 @@static const struct
samsung_pin_bank_dataexynos7_pin_banks7[]__initconst= { };/* pin banks of exynos7 pin-controller - BUS1 */ -static const struct
samsung_pin_bank_dataexynos7_pin_banks8[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos7_pin_banks8[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (8 ,0x020 ,"gpf0" ,0x00 ), EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpf1", 0x04), @@ -376,13 +377,13@@ static const struct
samsung_pin_bank_dataexynos7_pin_banks8[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +0
-1EXYNOS_PIN_BANK_EINTG(3, 0x140, "gpv6", 0x24), }; -static const struct samsung_pin_bank_data exynos7_pin_banks9[]
__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +5
-2+static const struct samsung_pin_bank_data exynos7_pin_banks9[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS_PIN_BANK_EINTG (7 ,0x000 ,"gpz0" ,0x00 ),EXYNOS_PIN_BANK_EINTG (4 ,0x020 ,"gpz1" ,0x04 ), }; -static const struct samsung_pin_ctrl exynos7_pin_ctrl[]__initconst = { +static const struct samsung_pin_ctrl exynos7_pin_ctrl[] = { {/* pin-controller instance 0 Alive data */ .pin_banks = exynos7_pin_banks0,@@ -436,13 +437,14 @@ static const struct samsung_pin_ctrl exynos7_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos7_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos7_of_data = { .ctrl = exynos7_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos7_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos7_of_data); /* pin banks of exynos850 pin-controller 0 (ALIVE) */ -static conststruct samsung_pin_bank_data exynos850_pin_banks0[]__initconst = { +static const struct samsung_pin_bank_data exynos850_pin_banks0[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTW (8 ,0x000 ,"gpa0" ,0x00 ), EXYNOS850_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), @@ -453,7 +455,7@@ static const struct
samsung_pin_bank_dataexynos850_pin_banks0[]__initconst= { };/* pin banks of exynos850 pin-controller 1 (CMGP) */ -static const struct
samsung_pin_bank_dataexynos850_pin_banks1[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +2
-0+static const struct samsung_pin_bank_data exynos850_pin_banks1[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTW (1 ,0x000 ,"gpm0" ,0x00 ), EXYNOS850_PIN_BANK_EINTW(1, 0x020, "gpm1", 0x04), @@ -466,27 +468,27 @@ static const struct samsung_pin_bank_data exynos850_pin_banks1[]
__initconst= { };/* pin banks of exynos850 pin-controller 2 (AUD) */ -static const struct
samsung_pin_bank_dataexynos850_pin_banks2[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos850_pin_banks2[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTG (5 ,0x000 ,"gpb0" ,0x00 ),EXYNOS850_PIN_BANK_EINTG (5 ,0x020 ,"gpb1" ,0x04 ), }; /* pin banks of exynos850 pin-controller 3 (HSI) */ -static const struct
samsung_pin_bank_dataexynos850_pin_banks3[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos850_pin_banks3[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTG (6 ,0x000 ,"gpf2" ,0x00 ), }; /* pin banks of exynos850 pin-controller 4 (CORE) */ -static const struct
samsung_pin_bank_dataexynos850_pin_banks4[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos850_pin_banks4[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTG (4 ,0x000 ,"gpf0" ,0x00 ),EXYNOS850_PIN_BANK_EINTG (8 ,0x020 ,"gpf1" ,0x04 ), }; /* pin banks of exynos850 pin-controller 5 (PERI) */ -static const struct
samsung_pin_bank_dataexynos850_pin_banks5[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +1
-0+static const struct samsung_pin_bank_data exynos850_pin_banks5[] = {/* Must start with EINTG banks, ordered by EINT group number. */ EXYNOS850_PIN_BANK_EINTG (2 ,0x000 ,"gpg0" ,0x00 ), EXYNOS850_PIN_BANK_EINTG(6, 0x020, "gpp0", 0x04), @@ -499,7 +501,7@@ static const struct
samsung_pin_bank_dataexynos850_pin_banks5[]__initconst= {drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +3
-2EXYNOS850_PIN_BANK_EINTG(6, 0x100, "gpc1", 0x20), }; -static const struct samsung_pin_ctrl exynos850_pin_ctrl[]__initconst = { +static const struct samsung_pin_ctrl exynos850_pin_ctrl[] = { {/* pin-controller instance 0 ALIVE data */ .pin_banks = exynos850_pin_banks0,@@ -534,7 +536,8 @@ static const struct samsung_pin_ctrl exynos850_pin_ctrl[] __initconst = { }, }; -const struct samsung_pinctrl_of_match_data exynos850_of_data__initconst = { +const struct samsung_pinctrl_of_match_data exynos850_of_data = { .ctrl = exynos850_pin_ctrl, .num_ctrl =ARRAY_SIZE (exynos850_pin_ctrl), }; +EXPORT_SYMBOL_GPL (exynos850_of_data);drivers/pinctrl/samsung/pinctrl-exynos.c +2
-1@@ -20,6 +20,7 @@#include <linux/irqchip/chained_irq.h> #include <linux/of.h> #include <linux/of_irq.h> +#include <linux/module.h> #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/regmap.h> @@ -207,7 +208,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd)/* * irq_chip for gpio interrupts. */ -static const struct exynos_irq_chip exynos_gpio_irq_chip__initconst = { +static const struct exynos_irq_chip exynos_gpio_irq_chip = { .chip = { .name ="exynos_gpio_irq_chip" ,drivers/pinctrl/samsung/pinctrl-exynos.c +2
-2.irq_unmask = exynos_irq_unmask,@@ -275,7 +276,7 @@ struct exynos_eint_gpio_save {* exynos_eint_gpio_init() - setup handling of external gpio interrupts. * @d: driver data of samsung pinctrl driver. */ -__init int exynos_eint_gpio_init (struct samsung_pinctrl_drv_data *d) +int exynos_eint_gpio_init (struct samsung_pinctrl_drv_data *d) {struct samsung_pin_bank *bank;struct device *dev = d->dev;@@ -399,7 +400,7 @@ static u32 eint_wake_mask_value =EXYNOS_EINT_WAKEUP_MASK_DISABLED ;/* * irq_chip for wakeup interrupts */ -static const struct exynos_irq_chip s5pv210_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip s5pv210_wkup_irq_chip = { .chip = { .name ="s5pv210_wkup_irq_chip" ,drivers/pinctrl/samsung/pinctrl-exynos.c +1
-1.irq_unmask = exynos_irq_unmask,@@ -419,7 +420,7 @@ static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = { .set_eint_wakeup_mask = s5pv210_pinctrl_set_eint_wakeup_mask, }; -static const struct exynos_irq_chip exynos4210_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip exynos4210_wkup_irq_chip = { .chip = {drivers/pinctrl/samsung/pinctrl-exynos.c +1
-1.name = "exynos4210_wkup_irq_chip", .irq_unmask = exynos_irq_unmask,@@ -438,7 +439,7 @@ static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = { .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, }; -static const struct exynos_irq_chip exynos7_wkup_irq_chip__initconst = { +static const struct exynos_irq_chip exynos7_wkup_irq_chip = { .chip = { .name ="exynos7_wkup_irq_chip" ,drivers/pinctrl/samsung/pinctrl-exynos.c +1
-1.irq_unmask = exynos_irq_unmask,@@ -521,7 +522,7 @@ static void exynos_irq_demux_eint16_31(struct irq_desc*desc) * exynos_eint_wkup_init() - setup handling of external wakeup interrupts. * @d: driver data of samsung pinctrl driver. */ -__init int exynos_eint_wkup_init (struct samsung_pinctrl_drv_data *d) +int exynos_eint_wkup_init (struct samsung_pinctrl_drv_data *d) {struct device *dev = d->dev;drivers/pinctrl/samsung/pinctrl-exynos.c +4
-0struct device_node *wkup_np =NULL ;@@ -760,3 +761,7 @@ exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, return ctrl; } + +MODULE_DESCRIPTION("Pinctrl common driver for Exynos, S3C24XX, and +S3C64XX SoCs"); MODULE_AUTHOR(" Thomas Abraham +<[email protected]>"); MODULE_LICENSE("GPL v2");drivers/pinctrl/samsung/pinctrl-samsung.c +1
-0@@ -18,6 +18,7 @@#include <linux/init.h> #include <linux/platform_device.h> #include <linux/io.h> +#include <linux/module.h> #include <linux/slab.h> #include <linux/err.h> #include <linux/gpio/driver.h>
drivers/pinctrl/samsung/pinctrl-samsung.c static struct platform_driver samsung_pinctrl_driver · line 1303
Quoted 10 lines
drivers/pinctrl/samsung/pinctrl-samsung.c +1
-5@@ -1299,8 +1300,8 @@ static struct platform_driver samsung_pinctrl_driver = { }, }; -static int __init samsung_pinctrl_drv_register (void ) -{ -return platform_driver_register (&samsung_pinctrl_driver); -} -postcore_initcall (samsung_pinctrl_drv_register); +module_platform_driver (samsung_pinctrl_driver );
To keep previous initcall order, please do not use module_platform_driver macro.
Best Regards,
Chanho Park
PATCH v2 9/12 pinctrl: samsung: set PINCTRL_EXYNOS and PINCTRL_SAMSUNG as tristate
In reply to Will McVickerMake PINCTRL_EXYNOS and PINCTRL_SAMSUNG tristate so that we can compile them as modules. Also don't have ARCH_EXYNOS directly select them, but use "default y if ARCH_EXYNOS || ARCH_S5PV210" instead.
--- arch/arm/mach-exynos/Kconfig | 1 - arch/arm/mach-s5pv210/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/pinctrl/samsung/Kconfig | 5 +++-- 4 files changed, 3 insertions(+), 5 deletions(-)
arch/arm/mach-exynos/Kconfig +0 -1
@@ -22,7 +22,6 @@ menuconfig ARCH_EXYNOS select HAVE_S3C2410_I2C if I2C select HAVE_S3C_RTC if RTC_CLASS select PINCTRL - select PINCTRL_EXYNOS select PM_GENERIC_DOMAINS if PM select S5P_DEV_MFC select SAMSUNG_MC
arch/arm/mach-s5pv210/Kconfig +0 -1
@@ -14,7 +14,6 @@ config ARCH_S5PV210 select HAVE_S3C2410_I2C if I2C select HAVE_S3C_RTC if RTC_CLASS select PINCTRL - select PINCTRL_EXYNOS select SOC_SAMSUNG help Samsung S5PV210/S5PC110 series based systems
arch/arm64/Kconfig.platforms +0 -1
@@ -95,7 +95,6 @@ config ARCH_EXYNOS select EXYNOS_PMU select HAVE_S3C_RTC if RTC_CLASS select PINCTRL - select PINCTRL_EXYNOS select PM_GENERIC_DOMAINS if PM select SOC_SAMSUNG help
drivers/pinctrl/samsung/Kconfig +3 -2
@@ -3,15 +3,16 @@ # Samsung Pin control drivers # config PINCTRL_SAMSUNG - bool + tristate "Pinctrl common driver for Exynos, S3C24XX, and S3C64XX SoCs" if COMPILE_TEST depends on OF_GPIO select PINMUX select PINCONF config PINCTRL_EXYNOS - bool "Pinctrl common driver part for Samsung Exynos SoCs" + tristate "Pinctrl common driver part for Samsung Exynos ARM and ARM64 SoCs" depends on OF_GPIO depends on ARCH_EXYNOS || ARCH_S5PV210 || COMPILE_TEST + default y if (ARCH_EXYNOS || ARCH_S5PV210) select PINCTRL_SAMSUNG select PINCTRL_EXYNOS_ARM if ARM && (ARCH_EXYNOS || ARCH_S5PV210) select PINCTRL_EXYNOS_ARM64 if ARM64 && ARCH_EXYNOS
Signature · 1 line
2.33.0.685.g46640cef36-goog
PATCH v2 10/12 soc: samsung: pmu: modularize the Exynos ARMv8 PMU driver
In reply to Will McVickerIn order to only modularize the Exynos ARMv8 PMU driver, we have to split it up from the ARM PMU driver. So make the following Kconfig changes:
* EXYNOS_PMU -> EXYNOS_PMU_ARM64
* EXYNOS_PMU_ARM_DRIVERS -> EXYNOS_PMU_ARM
This patch also includes the necessary modularization changes.
Lastly, have EXYNOS_PMU_ARM64 use "default y if ARCH_EXYNOS && ARM64" instead of having ARCH_EXYNOS select it directly.
--- arch/arm/mach-exynos/Kconfig | 2 +- arch/arm64/Kconfig.platforms | 1 - drivers/soc/samsung/Kconfig | 15 +++++++++------ drivers/soc/samsung/Makefile | 8 +++++--- drivers/soc/samsung/exynos-pmu.c | 13 +++++++------ drivers/soc/samsung/exynos-pmu.h | 2 +- include/linux/soc/samsung/exynos-pmu.h | 2 +- 7 files changed, 24 insertions(+), 19 deletions(-)
arch/arm/mach-exynos/Kconfig +1 -1
@@ -13,7 +13,7 @@ menuconfig ARCH_EXYNOS select ARM_GIC select EXYNOS_IRQ_COMBINER select EXYNOS_THERMAL - select EXYNOS_PMU + select EXYNOS_PMU_ARM select EXYNOS_SROM select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select GPIOLIB
arch/arm64/Kconfig.platforms +0 -1
@@ -92,7 +92,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS - select EXYNOS_PMU select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM
drivers/soc/samsung/Kconfig +9 -6
@@ -23,15 +23,18 @@ config EXYNOS_CHIPID Support for Samsung Exynos SoC ChipID and Adaptive Supply Voltage. This driver can also be built as module (exynos_chipid). -config EXYNOS_PMU - bool "Exynos PMU controller driver" if COMPILE_TEST - depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST) - select EXYNOS_PMU_ARM_DRIVERS if ARM && ARCH_EXYNOS +config EXYNOS_PMU_ARM64 + tristate "Exynos PMU controller driver" + depends on ARCH_EXYNOS || (ARM64 && COMPILE_TEST) + default y if (ARCH_EXYNOS && ARM64) + help + Support for Samsung Exynos ARMv8 PMU controller. This driver can be + built-in or as a module (exynos-pmu). # There is no need to enable these drivers for ARMv8 -config EXYNOS_PMU_ARM_DRIVERS +config EXYNOS_PMU_ARM bool "Exynos PMU ARMv7-specific driver extensions" if COMPILE_TEST - depends on EXYNOS_PMU + depends on ARCH_EXYNOS || (ARM && COMPILE_TEST) config EXYNOS_PM_DOMAINS bool "Exynos PM domains" if COMPILE_TEST
drivers/soc/samsung/Makefile +5 -3
@@ -4,10 +4,12 @@ obj-$(CONFIG_EXYNOS_ASV_ARM) += exynos5422-asv.o obj-$(CONFIG_EXYNOS_CHIPID) += exynos_chipid.o exynos_chipid-y += exynos-chipid.o exynos-asv.o -obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o +obj-$(CONFIG_EXYNOS_PMU_ARM64) += exynos-pmu.o + +obj-$(CONFIG_EXYNOS_PMU_ARM) += exynos-pmu32.o +exynos-pmu32-y += exynos-pmu.o exynos3250-pmu.o \ + exynos4-pmu.o exynos5250-pmu.o exynos5420-pmu.o -obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS) += exynos3250-pmu.o exynos4-pmu.o \ - exynos5250-pmu.o exynos5420-pmu.o obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o obj-$(CONFIG_EXYNOS_REGULATOR_COUPLER) += exynos-regulator-coupler.o
drivers/soc/samsung/exynos-pmu.c +7 -6
@@ -8,6 +8,7 @@#include <linux/of.h> #include <linux/of_address.h> #include <linux/of_device.h> +#include <linux/module.h> #include <linux/mfd/core.h> #include <linux/mfd/syscon.h> #include <linux/platform_device.h>
@@ -63,7 +64,7 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)* Split the data between ARM architectures because it is relatively big * and useless on other arch. */ -#ifdef CONFIG_EXYNOS_PMU_ARM_DRIVERS +#ifdef CONFIG_EXYNOS_PMU_ARM #define exynos_pmu_data_arm_ptr (data) (&data)#else #define exynos_pmu_data_arm_ptr (data) NULL
@@ -154,9 +155,9 @@ static struct platform_driver exynos_pmu_driver = { .probe = exynos_pmu_probe, }; -static int __init exynos_pmu_init (void ) -{ -return platform_driver_register (&exynos_pmu_driver); +module_platform_driver (exynos_pmu_driver); -} -postcore_initcall (exynos_pmu_init); +MODULE_DESCRIPTION ("Exynos PMU controller driver" ); +MODULE_AUTHOR ("Marek Szyprowski <[email protected]>" ); +MODULE_AUTHOR ("Krzysztof Kozlowski <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );
drivers/soc/samsung/exynos-pmu.h +1 -1
@@ -28,7 +28,7 @@ struct exynos_pmu_data {extern void __iomem *pmu_base_addr; -#ifdef CONFIG_EXYNOS_PMU_ARM_DRIVERS +#ifdef CONFIG_EXYNOS_PMU_ARM /* list of all exported SoC specific data */ extern const struct exynos_pmu_data exynos3250_pmu_data;extern const struct exynos_pmu_data exynos4210_pmu_data;
include/linux/soc/samsung/exynos-pmu.h +1 -1
@@ -19,7 +19,7 @@ enum sys_powerdown { };extern void exynos_sys_powerdown_conf (enum sys_powerdown mode); -#ifdef CONFIG_EXYNOS_PMU +#if IS_ENABLED (CONFIG_EXYNOS_PMU_ARM64 ) ||IS_ENABLED (CONFIG_EXYNOS_PMU_ARM )extern struct regmap *exynos_get_pmu_regmap (void );#else static inline struct regmap *exynos_get_pmu_regmap (void )
Signature · 1 line
2.33.0.685.g46640cef36-goog
Re: [PATCH v2 10/12] soc: samsung: pmu: modularize the Exynos ARMv8 PMU driver
Quoted 10 lines · Will McVicker
In order to only modularize the Exynos ARMv8 PMU driver, we have to split it up from the ARM PMU driver. So make the following
Kconfigchanges:
*EXYNOS_PMU->EXYNOS_PMU_ARM64
*EXYNOS_PMU_ARM_DRIVERS->EXYNOS_PMU_ARMThis patch also includes the necessary modularization changes.
Lastly, have
EXYNOS_PMU_ARM64use "default y ifARCH_EXYNOS&& ARM64" instead of havingARCH_EXYNOSselect it directly.
The same comments as for clocks patch:
1. You missed to explain the most important change - removed enforcing of built-in.
2. You ignored Geert's comments and broke distros configs for Exynos platforms.
Best regards,
Krzysztof
PATCH v2 11/12 soc: samsung: pm_domains: modularize EXYNOS_PM_DOMAINS
In reply to Will McVickerConvert the Exynos PM Domains driver into a module. This includes setting EXYNOS_PM_DOMAINS as tristate and removing it from being auto-selected by ARCH_EXYNOS. Instead, the config will use "default y if ARCH_EXYNOS" which allows us to set it to a module via the defconfig now.
--- arch/arm/mach-exynos/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/soc/samsung/Kconfig | 3 ++- drivers/soc/samsung/pm_domains.c | 12 +++++++----- 4 files changed, 9 insertions(+), 8 deletions(-)
arch/arm/mach-exynos/Kconfig +0 -1
@@ -15,7 +15,6 @@ menuconfig ARCH_EXYNOS select EXYNOS_THERMAL select EXYNOS_PMU_ARM select EXYNOS_SROM - select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select GPIOLIB select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 select HAVE_ARM_SCU if SMP
arch/arm64/Kconfig.platforms +0 -1
@@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM
drivers/soc/samsung/Kconfig +2 -1
@@ -37,8 +37,9 @@ config EXYNOS_PMU_ARM depends on ARCH_EXYNOS || (ARM && COMPILE_TEST) config EXYNOS_PM_DOMAINS - bool "Exynos PM domains" if COMPILE_TEST + tristate "Exynos PM domains" depends on (ARCH_EXYNOS && PM_GENERIC_DOMAINS) || COMPILE_TEST + default y if ARCH_EXYNOS config SAMSUNG_PM_DEBUG bool "Samsung PM Suspend debug"
drivers/soc/samsung/pm_domains.c +7 -5
@@ -14,6 +14,7 @@#include <linux/slab.h> #include <linux/pm_domain.h> #include <linux/delay.h> +#include <linux/module.h> #include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/pm_runtime.h>
@@ -160,8 +161,9 @@ static struct platform_driver exynos_pd_driver = { } }; -static __init int exynos4_pm_init_power_domain (void ) -{ -return platform_driver_register (&exynos_pd_driver); -} -core_initcall (exynos4_pm_init_power_domain); +module_platform_driver (exynos_pd_driver); + +MODULE_DESCRIPTION ("Exynos PM domains driver" ); +MODULE_AUTHOR ("Marek Szyprowski <[email protected]>" ); +MODULE_AUTHOR ("Krzysztof Kozlowski <[email protected]>" ); +MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.33.0.685.g46640cef36-goog
Re: [PATCH v2 11/12] soc: samsung: pm_domains: modularize EXYNOS_PM_DOMAINS
Quoted 43 lines · Will McVicker
Convert the Exynos PM Domains driver into a module. This includes setting
EXYNOS_PM_DOMAINSas tristate and removing it from being auto-selected byARCH_EXYNOS. Instead, the config will use "default y ifARCH_EXYNOS" which allows us to set it to a module via the defconfig now.Signed-off-by Will McVicker <[email protected]>--- arch/arm/mach-exynos/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/soc/samsung/Kconfig | 3 ++- drivers/soc/samsung/pm_domains.c | 12 +++++++----- 4 files changed, 9 insertions(+), 8 deletions(-)arch/arm/mach-exynos/Kconfig +0
-1@@ -15,7 +15,6 @@ menuconfig ARCH_EXYNOS select EXYNOS_THERMAL select EXYNOS_PMU_ARM select EXYNOS_SROM - select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select GPIOLIB select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 select HAVE_ARM_SCU if SMParch/arm64/Kconfig.platforms +0
-1@@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PMdrivers/soc/samsung/Kconfig +0
-0
drivers/soc/samsung/Kconfig config EXYNOS_PMU_ARM · line 40
Quoted 5 lines
drivers/soc/samsung/Kconfig +1
-1@@ -37,8 +37,9 @@ config EXYNOS_PMU_ARM depends on ARCH_EXYNOS || (ARM && COMPILE_TEST) config EXYNOS_PM_DOMAINS - bool "Exynos PM domains" if COMPILE_TEST + tristate "Exynos PM domains"
+Cc Arnd and Olof,
Unlike in clocks and soc drivers changes, you mentioned the removal of "if", however it is not explained why you do it.
Why is the most important part of commit message, not "what". Because "What" we can easily see. But "why" is sometimes trickier.
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
https://lore.kernel.org/lkml/CAK8P3a1TY+XT1vF=wAh0XA5BXU5Z6Ab1d4DekXbVsN9aj3aL5w@mail.gmail.com/
We follow specific convention or policy and changing it requires some discussion, not silently under the "modularize" hood. It really looks like you want to sneak it in.
P.S. I recommend also to Cc Soc maintainers, because their point of view here is crucial.
drivers/soc/samsung/Kconfig config EXYNOS_PMU_ARM · line 42
Quoted 2 lines
drivers/soc/samsung/Kconfig +1
-0depends on (ARCH_EXYNOS && PM_GENERIC_DOMAINS) || COMPILE_TEST + default y if ARCH_EXYNOS
Best regards,
Krzysztof
PATCH v2 12/12 ARM: rtc: remove HAVE_S3C_RTC in favor of direct dependencies
In reply to Will McVickerThe config HAVE_S3C_RTC is not really needed since we can simply just add the dependencies directly to RTC_DRV_S3C. Also, one less config to keep track of!
--- arch/arm/Kconfig | 1 - arch/arm/mach-exynos/Kconfig | 1 - arch/arm/mach-s5pv210/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/rtc/Kconfig | 10 ++-------- 5 files changed, 2 insertions(+), 12 deletions(-)
arch/arm/Kconfig +0 -1
@@ -475,7 +475,6 @@ config ARCH_S3C24XX select GPIOLIB select GENERIC_IRQ_MULTI_HANDLER select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select NEED_MACH_IO_H select S3C2410_WATCHDOG select SAMSUNG_ATAGS
arch/arm/mach-exynos/Kconfig +0 -1
@@ -19,7 +19,6 @@ menuconfig ARCH_EXYNOS select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 select HAVE_ARM_SCU if SMP select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM select S5P_DEV_MFC
arch/arm/mach-s5pv210/Kconfig +0 -1
@@ -12,7 +12,6 @@ config ARCH_S5PV210 select CLKSRC_SAMSUNG_PWM select GPIOLIB select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select SOC_SAMSUNG help
arch/arm64/Kconfig.platforms +0 -1
@@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM select SOC_SAMSUNG
drivers/rtc/Kconfig +2 -8
@@ -1404,16 +1404,10 @@ config RTC_DRV_OMAP This driver can also be built as a module, if so, module will be called rtc-omap. -config HAVE_S3C_RTC - bool - help - This will include RTC support for Samsung SoCs. If - you want to include RTC support for any machine, kindly - select this in the respective mach-XXXX/Kconfig file. - config RTC_DRV_S3C tristate "Samsung S3C series SoC RTC" - depends on ARCH_S3C64XX || HAVE_S3C_RTC || COMPILE_TEST + depends on ARCH_EXYNOS || ARCH_S3C64XX || ARCH_S3C24XX || ARCH_S5PV210 || \ + COMPILE_TEST help RTC (Realtime Clock) driver for the clock inbuilt into the Samsung S3C24XX series of SoCs. This can provide periodic
Signature · 1 line
2.33.0.685.g46640cef36-goog
Re: [PATCH v2 12/12] ARM: rtc: remove HAVE_S3C_RTC in favor of direct dependencies
Hi,
I'd argue that the subject should be rtc: s3c: ...
Quoted 5 lines · Will McVicker
The config
HAVE_S3C_RTCis not really needed since we can simply just add the dependencies directly toRTC_DRV_S3C. Also, one less config to keep track of!Signed-off-by Will McVicker <[email protected]>
Quoted 82 lines
--- arch/arm/Kconfig | 1 - arch/arm/mach-exynos/Kconfig | 1 - arch/arm/mach-s5pv210/Kconfig | 1 - arch/arm64/Kconfig.platforms | 1 - drivers/rtc/Kconfig | 10 ++-------- 5 files changed, 2 insertions(+), 12 deletions(-)arch/arm/Kconfig +0
-1@@ -475,7 +475,6 @@ config ARCH_S3C24XX select GPIOLIB select GENERIC_IRQ_MULTI_HANDLER select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select NEED_MACH_IO_H select S3C2410_WATCHDOG select SAMSUNG_ATAGSarch/arm/mach-exynos/Kconfig +0
-1@@ -19,7 +19,6 @@ menuconfig ARCH_EXYNOS select HAVE_ARM_ARCH_TIMER if ARCH_EXYNOS5 select HAVE_ARM_SCU if SMP select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM select S5P_DEV_MFCarch/arm/mach-s5pv210/Kconfig +0
-1@@ -12,7 +12,6 @@ config ARCH_S5PV210 select CLKSRC_SAMSUNG_PWM select GPIOLIB select HAVE_S3C2410_I2C if I2C - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select SOC_SAMSUNG helparch/arm64/Kconfig.platforms +0
-1@@ -91,7 +91,6 @@ config ARCH_BRCMSTB config ARCH_EXYNOS bool "ARMv8 based Samsung Exynos SoC family" - select HAVE_S3C_RTC if RTC_CLASS select PINCTRL select PM_GENERIC_DOMAINS if PM select SOC_SAMSUNGdrivers/rtc/Kconfig +2
-8@@ -1404,16 +1404,10 @@ config RTC_DRV_OMAP This driver can also be built as a module, if so, module will be called rtc-omap. -config HAVE_S3C_RTC - bool - help - This will include RTC support for Samsung SoCs. If - you want to include RTC support for any machine, kindly - select this in the respective mach-XXXX/Kconfig file. - config RTC_DRV_S3C tristate "Samsung S3C series SoC RTC" - depends on ARCH_S3C64XX || HAVE_S3C_RTC || COMPILE_TEST + depends on ARCH_EXYNOS || ARCH_S3C64XX || ARCH_S3C24XX || ARCH_S5PV210 || \ + COMPILE_TEST help RTC (Realtime Clock) driver for the clock inbuilt into the Samsung S3C24XX series of SoCs. This can provide periodicSignature · 1 line
2.33.0.685.g46640cef36-goog
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.
Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
Krzysztof
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.
Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
Krzysztof
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
Quoted 6 lines
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
Quoted 3 lines
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
I am not actually changing ANY default build configurations here and I'm not removing any existing configuration. I tried to make it pretty clear in my original patch series commit messages that none of my changes modify the default behavior. The .config is the same with and without my patches. All of these drivers remain enabled as built-in. So if there is a distro that requires all of these drivers to be built-in, then they can continue as is without noticing any difference. IOW, all of these changes are/should be backwards compatible.
I really appreciate yours and John Stultz's comments regarding including the "why" in my commit message wording. I will spend more time on the next series on trying to write a more meaningful commit message, but before that we can surely discuss the "why" here.
As mentioned by Lee and Saravana, our common goal is to make it easier for everyone to contribute upstream. In particular, this series of patches is laying the ground work for distros to have more flexibility in supporting a wider range of platforms without forcing everyone to include unnecessary drivers. You said that upstream supports a generic kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules. This patch series is addressing exactly that -- allow distros to move SoC specific drivers out of the core kernel and into modules. Ultimately, our goal is to be able to directly develop with the upstream kernel on new and old SoCs by not including SoC specific drivers in our generic kernel distro. This helps the upstream community in a number of ways:
(1) It makes the ARM64 generic kernel smaller by converting more drivers into modules
(2) It makes it a lot easier for everyone to develop upstream if they can directly use the upstream kernel without carrying downstream changes.
Quoted 4 lines
Even if there was, I think it is good to have dependencies like
ARCH_EXYNOS, as they let us partition the (19000, as Arnd said recently)Kconfigsymbols into better manageable groups. Without these, we cannot do better than "depends on ARM || ARM64 ||COMPILE_TEST".
My patch series still keeps the dependencies on ARCH_EXYNOS. I am totally fine with "depends on ARCH_EXYNOS" and totally fine with "default ARCH_EXYNOS". The problem we have is that ARCH_EXYNOS forcefully selects SoC specific drivers to be built-in because it just adds more and more SoC-specific drivers to a generic kernel.
I know you are asking for me to only push changes that have proven to work. The theory behind these changes has been proven downstream on other devices and I'm more than willing to help debug any issues that arise out of this patch series, but since I don't have the hardware myself I do need help with device testing these changes. We are not trying to trick upstream in anyway to accept something that is not functional or going to hurt the upstream community. I am more than willing to help upstream and am totally willing to work with upstream to verify all of these changes before they are accepted (feel free to send me any dusty, unused hardware lying around if you want the extra help with device testing).
I hope that helps clarifies things! I will address other patch specific comments in those threads as well.
Thanks,
Will
Quoted 53 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
Krzysztof+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
Quoted 4 lines
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
Quoted 7 lines
Quoted 2 lines
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
One of the steps was problem with distros using everything as modules. They should not receive these drivers as modules.
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Quoted 7 lines
Quoted 4 lines
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
I am not actually changing ANY default build configurations here and I'm not removing any existing configuration.
You are changing not default, but selectability which is part of the enforced configuration to make platforms working. The distros do not always choose defaults but rather all as modules. Kernel configuration is huge and complex, so by mistake they could now even disable potentially essential driver. There is no need to disable for example essential clock driver on a supported Exynos platform.
Quoted 8 lines
I tried to make it pretty
clear in my original patch series commit messages that none of my changes modify the default behavior. The .config is the same with and without my patches. All of these drivers remain enabled as built-in. So if there is a distro that requires all of these drivers to be built-in, then they can continue as is without noticing any difference. IOW, all of these changes are/should be backwards compatible.
I was not referring to default neither to backwards compatibility. Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Is that now a new global approach that all SoC drivers should be allowed to be disabled for ARCH_XXX?
Quoted 11 lines
I really appreciate yours and John Stultz's comments regarding including the "why" in my commit message wording. I will spend more time on the next series on trying to write a more meaningful commit message, but before that we can surely discuss the "why" here.
As mentioned by Lee and Saravana, our common goal is to make it easier for everyone to contribute upstream. In particular, this series of patches is laying the ground work for distros to have more flexibility in supporting a wider range of platforms without forcing everyone to include unnecessary drivers.
The drivers are usually necessary. Actually, you admitted you didn't test patchset, so how do you even know that they are unnecessary? How do
you judge?
Quoted 4 lines
You said that upstream supports a generic
kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules.
Good point, but since having them as modules was not tested, I consider it as theoretical topic.
Quoted 7 lines
This patch series is addressing exactly that -- allow distros to move SoC specific drivers out of the core kernel and into modules. Ultimately, our goal is to be able to directly develop with the upstream kernel on new and old SoCs by not including SoC specific drivers in our generic kernel distro. This helps the upstream community in a number of ways:
Quoted 5 lines
(1) It makes the ARM64 generic kernel smaller by converting more drivers into modules
(2) It makes it a lot easier for everyone to develop upstream if they can directly use the upstream kernel without carrying downstream changes.
I don't understand the point (2) here. Anyone can use upstream kernel for supported and unsupported platforms. How upstream benefits from a change affecting supported platforms made for unsupported, downstream platforms.
Quoted 11 lines
Quoted 5 lines
Even if there was, I think it is good to have dependencies like
ARCH_EXYNOS, as they let us partition the (19000, as Arnd said recently)Kconfigsymbols into better manageable groups. Without these, we cannot do better than "depends on ARM || ARM64 ||COMPILE_TEST".My patch series still keeps the dependencies on
ARCH_EXYNOS. I am totally fine with "depends onARCH_EXYNOS" and totally fine with "defaultARCH_EXYNOS". The problem we have is thatARCH_EXYNOSforcefully selects SoC specific drivers to be built-in because it just adds more and more SoC-specific drivers to a generic kernel.
The selected drivers are essential for supported platforms. We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared.
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
If you want to change it, let me paste from previous discussion:
Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
Everyone else are working like this. NXP, Renesas, Xilinx, TI, Rockchip, AllWinner. Samsung or Google is not special to receive an exception for this.
Quoted 3 lines
I know you are asking for me to only push changes that have proven to work.
Yep, tested.
Quoted 4 lines
The theory behind these changes has been proven downstream on other devices and I'm more than willing to help debug any issues that arise out of this patch series, but since I don't have the hardware myself I do need help with device testing these changes.
Downstream uses very specific Linux "distro" or fork - Android - with changes not present in others. Although tests on other (unsupported/unupstreamed) devices is good, but it's not sufficient. For example I guess that half or most of Odroid devices are running standard Linux distro (Arch, Ubuntu).
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Best regards,
Krzysztof
Quoted 14 lines · Krzysztof Kozlowski
Quoted 5 lines · Will McVicker
Quoted 4 lines · Krzysztof Kozlowski
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
Quoted 7 lines
Quoted 2 lines
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
Quoted 5 lines
One of the steps was problem with distros using everything as modules. They should not receive these drivers as modules.
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Agreed. I absolutely love the work of the GKI developers to turn more drivers into loadable modules, but the "correctness-first" principle is not up for negotiation. If you are uncomfortable with the code or the amount of testing because you think it breaks something, you should reject the patches. Moving core platform functionality is fundamentally hard and it can go wrong in all possible ways where it used to work by accident because the init order was fixed.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
I am not actually changing ANY default build configurations here and I'm not removing any existing configuration.
Quoted 6 lines
You are changing not default, but selectability which is part of the enforced configuration to make platforms working. The distros do not always choose defaults but rather all as modules. Kernel configuration is huge and complex, so by mistake they could now even disable potentially essential driver. There is no need to disable for example essential clock driver on a supported Exynos platform.
I'm not overly worried about the defaults. If the drivers work as loadable modules, I'm happy with them being loadable modules in distros. If they don't work this way, then the patches are broken and should not get merged.
I don't even mind having essential drivers that can be turned off, since we already have a ton of those (e.g. serial ports on most platforms). It's up to distros to know which drivers to enable, though having either reasonable defaults or fail-safe Kconfig dependencies (e.g. making it impossible to turn off but allowing modules) is clearly best.
Quoted 9 lines
Quoted 8 lines
I tried to make it pretty
clear in my original patch series commit messages that none of my changes modify the default behavior. The .config is the same with and without my patches. All of these drivers remain enabled as built-in. So if there is a distro that requires all of these drivers to be built-in, then they can continue as is without noticing any difference. IOW, all of these changes are/should be backwards compatible.
Quoted 7 lines
I was not referring to default neither to backwards compatibility. Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Is that now a new global approach that all SoC drivers should be allowed to be disabled for
ARCH_XXX?
I wouldn't enforce it either way across platforms. I would prefer drivers to be loadable modules where possible (and tested), rather than selected by the platform Kconfig. If you want to ensure the exynos drivers are impossible to turn into a nonworking state, that's up to you.
Quoted 7 lines
Quoted 4 lines
You said that upstream supports a generic
kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules.Good point, but since having them as modules was not tested, I consider it as theoretical topic.
I actually disagree strongly with labelling the kernel as "non-generic" just because it requires platform specific support to be built-in rather than a loadable module. This has never been possible on any platform I'm aware of, and likely never will, except for minor variations of an existing platform.
Look at x86 as an example: there are less than a dozen SoC platforms supported and they are incredibly similar hardware-wise, but the kernel is anything but "generic" in the sense that was mentioned above. Most of the platform specific drivers in arch/x86/platform and the corresponding bits in drivers/{irqchip,clocksource,iommu} are always built-in, and a lot more is hardwired in architecture code as PCI quirks or conditional on cpuid or dmi firmware checks.
Quoted 19 lines
Quoted 10 lines
Quoted 4 lines
Even if there was, I think it is good to have dependencies like
ARCH_EXYNOS, as they let us partition the (19000, as Arnd said recently)Kconfigsymbols into better manageable groups. Without these, we cannot do better than "depends on ARM || ARM64 ||COMPILE_TEST".My patch series still keeps the dependencies on
ARCH_EXYNOS. I am totally fine with "depends onARCH_EXYNOS" and totally fine with "defaultARCH_EXYNOS". The problem we have is thatARCH_EXYNOSforcefully selects SoC specific drivers to be built-in because it just adds more and more SoC-specific drivers to a generic kernel.The selected drivers are essential for supported platforms. We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared.
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 5 lines
If you want to change it, let me paste from previous discussion:
Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
Agreed. I understand that it would be convenient for SoC vendors to never have to upstream their platform code again, and that Android would benefit from this in the short run.
From my upstream perspective, this is absolutely a non-goal. If it becomes easier as a side-effect of making the kernel more modular, that's fine. The actual goal should be to get more people to contribute upstream so devices run code that has been reviewed and integrated into new kernels.
Quoted 4 lines
Quoted 2 lines
I know you are asking for me to only push changes that have proven to work.
Yep, tested.
I'm generally fine with "obviously correct" ones as well, but it's up to you to categorize them ;-)
Arnd
Quoted 134 lines · Arnd Bergmann
Quoted 14 lines · Krzysztof Kozlowski
Quoted 5 lines · Will McVicker
Quoted 4 lines · Krzysztof Kozlowski
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
Quoted 7 lines
Quoted 2 lines
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
Quoted 5 lines
One of the steps was problem with distros using everything as modules. They should not receive these drivers as modules.
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).Agreed. I absolutely love the work of the GKI developers to turn more drivers into loadable modules, but the "correctness-first" principle is not up for negotiation. If you are uncomfortable with the code or the amount of testing because you think it breaks something, you should reject the patches. Moving core platform functionality is fundamentally hard and it can go wrong in all possible ways where it used to work by accident because the init order was fixed.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
I am not actually changing ANY default build configurations here and I'm not removing any existing configuration.
Quoted 6 lines
You are changing not default, but selectability which is part of the enforced configuration to make platforms working. The distros do not always choose defaults but rather all as modules. Kernel configuration is huge and complex, so by mistake they could now even disable potentially essential driver. There is no need to disable for example essential clock driver on a supported Exynos platform.
I'm not overly worried about the defaults. If the drivers work as loadable modules, I'm happy with them being loadable modules in distros. If they don't work this way, then the patches are broken and should not get merged.
I don't even mind having essential drivers that can be turned off, since we already have a ton of those (e.g. serial ports on most platforms). It's up to distros to know which drivers to enable, though having either reasonable defaults or fail-safe
Kconfigdependencies (e.g. making it impossible to turn off but allowing modules) is clearly best.Quoted 9 lines
Quoted 8 lines
I tried to make it pretty
clear in my original patch series commit messages that none of my changes modify the default behavior. The .config is the same with and without my patches. All of these drivers remain enabled as built-in. So if there is a distro that requires all of these drivers to be built-in, then they can continue as is without noticing any difference. IOW, all of these changes are/should be backwards compatible.Quoted 7 lines
I was not referring to default neither to backwards compatibility. Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Is that now a new global approach that all SoC drivers should be allowed to be disabled for
ARCH_XXX?I wouldn't enforce it either way across platforms. I would prefer drivers to be loadable modules where possible (and tested), rather than selected by the platform
Kconfig. If you want to ensure the exynos drivers are impossible to turn into a nonworking state, that's up to you.Quoted 7 lines
Quoted 4 lines
You said that upstream supports a generic
kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules.Good point, but since having them as modules was not tested, I consider it as theoretical topic.
I actually disagree strongly with labelling the kernel as "non-generic" just because it requires platform specific support to be built-in rather than a loadable module. This has never been possible on any platform I'm aware of, and likely never will, except for minor variations of an existing platform.
Look at x86 as an example: there are less than a dozen SoC platforms supported and they are incredibly similar hardware-wise, but the kernel is anything but "generic" in the sense that was mentioned above. Most of the platform specific drivers in
arch/x86/platformand the corresponding bits indrivers/{irqchip,clocksource,iommu}are always built-in, and a lot more is hardwired in architecture code as PCI quirks or conditional on cpuid or dmi firmware checks.Quoted 19 lines
Quoted 10 lines
Quoted 4 lines
Even if there was, I think it is good to have dependencies like
ARCH_EXYNOS, as they let us partition the (19000, as Arnd said recently)Kconfigsymbols into better manageable groups. Without these, we cannot do better than "depends on ARM || ARM64 ||COMPILE_TEST".My patch series still keeps the dependencies on
ARCH_EXYNOS. I am totally fine with "depends onARCH_EXYNOS" and totally fine with "defaultARCH_EXYNOS". The problem we have is thatARCH_EXYNOSforcefully selects SoC specific drivers to be built-in because it just adds more and more SoC-specific drivers to a generic kernel.The selected drivers are essential for supported platforms. We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared.
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 5 lines
If you want to change it, let me paste from previous discussion:
Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
Agreed. I understand that it would be convenient for SoC vendors to never have to upstream their platform code again, and that Android would benefit from this in the short run.
From my upstream perspective, this is absolutely a non-goal. If it becomes easier as a side-effect of making the kernel more modular, that's fine. The actual goal should be to get more people to contribute upstream so devices run code that has been reviewed and integrated into new kernels.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
I know you are asking for me to only push changes that have proven to work.
Yep, tested.
I'm generally fine with "obviously correct" ones as well, but it's up to you to categorize them ;-)
Arnd,
FWIW, I agree with all of your points.
Krzysztof,
It sounds like a lack of testing is your main concern.
How can I help here? What H/W do I need to be able to fully test this?
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 136 lines · Lee Jones
Quoted 134 lines · Arnd Bergmann
Quoted 14 lines · Krzysztof Kozlowski
Quoted 5 lines · Will McVicker
Quoted 4 lines · Krzysztof Kozlowski
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
Quoted 7 lines
Quoted 2 lines
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
Quoted 5 lines
One of the steps was problem with distros using everything as modules. They should not receive these drivers as modules.
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).Agreed. I absolutely love the work of the GKI developers to turn more drivers into loadable modules, but the "correctness-first" principle is not up for negotiation. If you are uncomfortable with the code or the amount of testing because you think it breaks something, you should reject the patches. Moving core platform functionality is fundamentally hard and it can go wrong in all possible ways where it used to work by accident because the init order was fixed.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
Please also explain why Exynos is so special that we deviate from the policy for all SoC that critical SoC-related drivers have to be enabled (built-in or as module).
I am not actually changing ANY default build configurations here and I'm not removing any existing configuration.
Quoted 6 lines
You are changing not default, but selectability which is part of the enforced configuration to make platforms working. The distros do not always choose defaults but rather all as modules. Kernel configuration is huge and complex, so by mistake they could now even disable potentially essential driver. There is no need to disable for example essential clock driver on a supported Exynos platform.
I'm not overly worried about the defaults. If the drivers work as loadable modules, I'm happy with them being loadable modules in distros. If they don't work this way, then the patches are broken and should not get merged.
I don't even mind having essential drivers that can be turned off, since we already have a ton of those (e.g. serial ports on most platforms). It's up to distros to know which drivers to enable, though having either reasonable defaults or fail-safe
Kconfigdependencies (e.g. making it impossible to turn off but allowing modules) is clearly best.Quoted 9 lines
Quoted 8 lines
I tried to make it pretty
clear in my original patch series commit messages that none of my changes modify the default behavior. The .config is the same with and without my patches. All of these drivers remain enabled as built-in. So if there is a distro that requires all of these drivers to be built-in, then they can continue as is without noticing any difference. IOW, all of these changes are/should be backwards compatible.Quoted 7 lines
I was not referring to default neither to backwards compatibility. Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Is that now a new global approach that all SoC drivers should be allowed to be disabled for
ARCH_XXX?I wouldn't enforce it either way across platforms. I would prefer drivers to be loadable modules where possible (and tested), rather than selected by the platform
Kconfig. If you want to ensure the exynos drivers are impossible to turn into a nonworking state, that's up to you.Quoted 7 lines
Quoted 4 lines
You said that upstream supports a generic
kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules.Good point, but since having them as modules was not tested, I consider it as theoretical topic.
I actually disagree strongly with labelling the kernel as "non-generic" just because it requires platform specific support to be built-in rather than a loadable module. This has never been possible on any platform I'm aware of, and likely never will, except for minor variations of an existing platform.
Look at x86 as an example: there are less than a dozen SoC platforms supported and they are incredibly similar hardware-wise, but the kernel is anything but "generic" in the sense that was mentioned above. Most of the platform specific drivers in
arch/x86/platformand the corresponding bits indrivers/{irqchip,clocksource,iommu}are always built-in, and a lot more is hardwired in architecture code as PCI quirks or conditional on cpuid or dmi firmware checks.Quoted 19 lines
Quoted 10 lines
Quoted 4 lines
Even if there was, I think it is good to have dependencies like
ARCH_EXYNOS, as they let us partition the (19000, as Arnd said recently)Kconfigsymbols into better manageable groups. Without these, we cannot do better than "depends on ARM || ARM64 ||COMPILE_TEST".My patch series still keeps the dependencies on
ARCH_EXYNOS. I am totally fine with "depends onARCH_EXYNOS" and totally fine with "defaultARCH_EXYNOS". The problem we have is thatARCH_EXYNOSforcefully selects SoC specific drivers to be built-in because it just adds more and more SoC-specific drivers to a generic kernel.The selected drivers are essential for supported platforms. We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared.
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 5 lines
If you want to change it, let me paste from previous discussion:
Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
Agreed. I understand that it would be convenient for SoC vendors to never have to upstream their platform code again, and that Android would benefit from this in the short run.
From my upstream perspective, this is absolutely a non-goal. If it becomes easier as a side-effect of making the kernel more modular, that's fine. The actual goal should be to get more people to contribute upstream so devices run code that has been reviewed and integrated into new kernels.
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
I know you are asking for me to only push changes that have proven to work.
Yep, tested.
I'm generally fine with "obviously correct" ones as well, but it's up to you to categorize them ;-)
Thanks Arnd!
Quoted 9 lines
Arnd,
FWIW, I agree with all of your points.>
Krzysztof,It sounds like a lack of testing is your main concern.
How can I help here? What H/W do I need to be able to fully test this?
The changes here need to be tested on affected platforms (ARMv7 and ARMv8), when built as a modules on some types of regular distros (e.g. Arch, Ubuntu). From each of such boot I would be happy to see number of new dmesg warnings/errors plus number of probe deferrals.
Since the drivers could be switched to modules (and some distros might do it), they might be hit by surprise regressions in boot performance due to probe deferrals. This should be also checked on these platforms. Geert pointed out before that clocks in many cases are not optional - driver needs them and will wait defer.
Assuming of course that boot succeeds. Minor differences in boot speed should not be a problem, I think, because distro anyway chosen all-module approach so it accepts the penalty.
Best regards,
Krzysztof
Quoted 15 lines · Krzysztof Kozlowski
Quoted 3 lines
It sounds like a lack of testing is your main concern.
How can I help here? What H/W do I need to be able to fully test this?
The changes here need to be tested on affected platforms (ARMv7 and ARMv8), when built as a modules on some types of regular distros (e.g. Arch, Ubuntu). From each of such boot I would be happy to see number of new dmesg warnings/errors plus number of probe deferrals.
Since the drivers could be switched to modules (and some distros might do it), they might be hit by surprise regressions in boot performance due to probe deferrals. This should be also checked on these platforms. Geert pointed out before that clocks in many cases are not optional - driver needs them and will wait defer.
Quoted 3 lines
Assuming of course that boot succeeds. Minor differences in boot speed should not be a problem, I think, because distro anyway chosen all-module approach so it accepts the penalty.
Do you have any suggestions in terms of devboards?
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 16 lines · Lee Jones
Quoted 15 lines · Krzysztof Kozlowski
Quoted 3 lines
It sounds like a lack of testing is your main concern.
How can I help here? What H/W do I need to be able to fully test this?
The changes here need to be tested on affected platforms (ARMv7 and ARMv8), when built as a modules on some types of regular distros (e.g. Arch, Ubuntu). From each of such boot I would be happy to see number of new dmesg warnings/errors plus number of probe deferrals.
Since the drivers could be switched to modules (and some distros might do it), they might be hit by surprise regressions in boot performance due to probe deferrals. This should be also checked on these platforms. Geert pointed out before that clocks in many cases are not optional - driver needs them and will wait defer.
Quoted 5 lines
Quoted 3 lines
Assuming of course that boot succeeds. Minor differences in boot speed should not be a problem, I think, because distro anyway chosen all-module approach so it accepts the penalty.
Do you have any suggestions in terms of devboards?
Minimal set:
1. Something with Exynos4 (e.g. Odroid U3 with Exynos4412).
2. Something with Exynos 5422/5800 (e.g. Odroid XU3/XU4/HC1/MC1 or Chromebook Peach Pi).
3. Exynos5433 - TM2 or TM2E. Boards are not widely available, so we need to rely on provided testing by Samsung.
What would be good is to also test Exynos3 boards, but this is also not widely available.
Best regards,
Krzysztof
Quoted 22 lines · Arnd Bergmann
Quoted 14 lines · Krzysztof Kozlowski
Quoted 5 lines · Will McVicker
Quoted 4 lines · Krzysztof Kozlowski
What is more, it seems you entirely ignored Geert's comments. I pointed attention to it last time and you just said you will send v2 instead of joining discussion.
Quoted 7 lines
Quoted 2 lines
It's a NAK for this reason - ignoring what Geert brought: you just broke distro configs for Exynos.
First off I did want to chime into the discussion from the previous patchset, but I felt that Lee and Saravana addressed all your concerns regarding the intent and feasibility. You also made it clear what the next steps were that I needed to take.
Quoted 5 lines
One of the steps was problem with distros using everything as modules. They should not receive these drivers as modules.
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Quoted 7 lines
Agreed. I absolutely love the work of the GKI developers to turn more drivers into loadable modules, but the "correctness-first" principle is not up for negotiation. If you are uncomfortable with the code or the amount of testing because you think it breaks something, you should reject the patches. Moving core platform functionality is fundamentally hard and it can go wrong in all possible ways where it used to work by accident because the init order was fixed.
Exactly.
And patches to change this that haven't been tested at all should be very clearly marked that way.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
You said that upstream supports a generic
kernel, but I argue that the upstream "generic" arm64 kernel can't be considered generic if it builds in SoC specific drivers that can be modules.
Quoted 8 lines
Quoted 2 lines
Good point, but since having them as modules was not tested, I consider it as theoretical topic.
I actually disagree strongly with labelling the kernel as "non-generic" just because it requires platform specific support to be built-in rather than a loadable module. This has never been possible on any platform I'm aware of, and likely never will, except for minor variations of an existing platform.
There seem to be two different meanings for a "generic" kernel:
1. To me, a generic kernel is a kernel that can boot on a variety of hardware platforms. and will operate correctly with all expected functionality. That is, it should include[1] drivers for all that hardware.
Examples of this are arm/multi_v7_defconfig and arm64/defconfig.
2. To GKI, a generic kernel is a minimal kernel that doesn't include any[2] hardware-specific drivers, and has loadable modules for all[2] hardware-specific drivers. Before loading these modules, it can run an application on the CPU, but it cannot do any I/O to real hardware.
[1] Developers usually want the drivers builtin, distros want them modular.
[2] On arm/arm64, this must rely on the GIC and architectured timer being built-in (AFAIK these cannot be modules yet), and thus precludes booting on Cortex A9 and older that lack GIC and/or architectured timer as their primary interrupt controller and timer. I'm wondering how this works on other architectures with less-standardized interrupt controllers and timers?
Quoted 7 lines
Look at x86 as an example: there are less than a dozen SoC platforms supported and they are incredibly similar hardware-wise, but the kernel is anything but "generic" in the sense that was mentioned above. Most of the platform specific drivers in
arch/x86/platformand the corresponding bits indrivers/{irqchip,clocksource,iommu}are always built-in, and a lot more is hardwired in architecture code as PCI quirks or conditional on cpuid or dmi firmware checks.
Indeed. X86 has less variability in the hardware than other architectures, and has all the critical drivers built-in or in firmware. Where there have been big differences (remember Voyager?), x86 never had the option to have a generic (in the meaning of 1 above) kernel (but James did try, cfr. https://lwn.net/Articles/328469/).
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
By insisting on drivers (most of which will not be executed in the vast majority of cases) to be built-in, you are insisting on a downstream kernel fork, which all of us would like to avoid [0].
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPM
Quoted 8 lines
We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared. Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Accepting changes based on the proviso that vendors upstream all of their work-in-progress solutions is an unfair position. We already discussed why upstreaming support for bleeding edge H/W and functionality is unrealistic in terms of competitive advantage.
Besides, we might not be talking about new silicon at all (I don't believe anyone alluded to that). The flexibility in configuration simply allows for generic upstream drivers to be swapped out for ones which may have more or slightly different functionality (that can't be publicly shared until release).
Quoted 7 lines
Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Everyone else are working like this. NXP, Renesas, Xilinx, TI, Rockchip, AllWinner. Samsung or Google is not special to receive an exception for this.
Exynos isn't special in this regard. This applies to any vendor who releases Android images and wishes to be solve all of the issues the GKI project addresses (please read the article above for more about this).
I truly hope this has helped to align my thoughts with yours.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Quoted 9 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 3 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Quoted 8 lines
Quoted 5 lines
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them.
Why?
Quoted 3 lines
For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
If the drivers are really essential/necessary/required, this precludes running the generic kernel image on the platform that requires them, making the kernel not sufficiently generic.
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
True, if "potentially". If not potentially, they must be included.
Quoted 5 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
The key here is if the driver is required or not to use the platform, and why it is required. If the requirement comes from some deficiency in the kernel code or config system, it should be fixed, if possible. And the fix should be tested.
If it cannot be fixed, the driver should be included, else it would preclude running the generic kernel on the affected platform.
Quoted 34 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
By insisting on drivers (most of which will not be executed in the vast majority of cases) to be built-in, you are insisting on a downstream kernel fork, which all of us would like to avoid [0].
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 8 lines
We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared. Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Accepting changes based on the proviso that vendors upstream all of their work-in-progress solutions is an unfair position. We already discussed why upstreaming support for bleeding edge H/W and functionality is unrealistic in terms of competitive advantage.
Quoted 5 lines
Besides, we might not be talking about new silicon at all (I don't believe anyone alluded to that). The flexibility in configuration simply allows for generic upstream drivers to be swapped out for ones which may have more or slightly different functionality (that can't be publicly shared until release).
The replacement drivers are already a downstream kernel fork, which you would like to avoid?
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 16 lines · Geert Uytterhoeven
Hi Lee,
Quoted 13 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 7 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them.
Why?
Because without them the image wouldn't functional on any level.
But you're right, there is still no requirement for it to be built-in.
Quoted 7 lines
Quoted 3 lines
For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
If the drivers are really essential/necessary/required, this precludes running the generic kernel image on the platform that requires them, making the kernel not sufficiently generic.
If they are not at all present, then yes. However that is not what is being suggested. The essential functionality will be provided. Just not built-in.
Quoted 8 lines
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
True, if "potentially". If not potentially, they must be included.
I'm not sure what you're trying to say here. Would you mind elaborating?
Quoted 6 lines
Quoted 5 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 6 lines
The key here is if the driver is required or not to use the platform, and why it is required. If the requirement comes from some deficiency in the kernel code or config system, it should be fixed, if possible. And the fix should be tested.
If it cannot be fixed, the driver should be included, else it would preclude running the generic kernel on the affected platform.
Sorry, I'm not following.
Quoted 34 lines
Quoted 34 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
By insisting on drivers (most of which will not be executed in the vast majority of cases) to be built-in, you are insisting on a downstream kernel fork, which all of us would like to avoid [0].
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 8 lines
We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared. Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Accepting changes based on the proviso that vendors upstream all of their work-in-progress solutions is an unfair position. We already discussed why upstreaming support for bleeding edge H/W and functionality is unrealistic in terms of competitive advantage.
Quoted 8 lines
Quoted 5 lines
Besides, we might not be talking about new silicon at all (I don't believe anyone alluded to that). The flexibility in configuration simply allows for generic upstream drivers to be swapped out for ones which may have more or slightly different functionality (that can't be publicly shared until release).
The replacement drivers are already a downstream kernel fork, which you would like to avoid?
Avoid, yes, absolutely.
However, in the real world of competitive tech, other than in extreme cases such as; fully open source, community driven, cute embedded hobby platforms, there will always be changes required on-top of upstream projects. Even as upstream kernel developers we need to understand and respect that.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Quoted 30 lines · Lee Jones
Quoted 14 lines · Geert Uytterhoeven
Quoted 13 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 7 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them.
Why?
Because without them the image wouldn't functional on any level.
But you're right, there is still no requirement for it to be built-in.
Quoted 4 lines
Quoted 3 lines
For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Quoted 7 lines
Quoted 3 lines
If the drivers are really essential/necessary/required, this precludes running the generic kernel image on the platform that requires them, making the kernel not sufficiently generic.
If they are not at all present, then yes. However that is not what is being suggested. The essential functionality will be provided. Just not built-in.
I really meant "essential/necessary/required to be built-in".
Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
True, if "potentially". If not potentially, they must be included.
I'm not sure what you're trying to say here. Would you mind elaborating?
It was a comment to your "*potentially* utilised on *some* platforms". It is clear they are not used on the other ("not *some*") platforms, but your sentence was unclear whether they are always or only sometimes used on "*some*" platforms.
"always" => "not potentially"
"sometimes" => "potentially".
I hope this makes it more clear.
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 8 lines
Quoted 6 lines
The key here is if the driver is required or not to use the platform, and why it is required. If the requirement comes from some deficiency in the kernel code or config system, it should be fixed, if possible. And the fix should be tested.
If it cannot be fixed, the driver should be included, else it would preclude running the generic kernel on the affected platform.Sorry, I'm not following.
It all depends on why the driver is "required to be built-in". Depending on the reason behind that requirement, the driver can be changed from built-in to modular without ill effects on functionality.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 37 lines · Geert Uytterhoeven
Hi Lee,
Quoted 34 lines · Lee Jones
Quoted 14 lines · Geert Uytterhoeven
Quoted 13 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 7 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them.
Why?
Because without them the image wouldn't functional on any level.
But you're right, there is still no requirement for it to be built-in.
Quoted 7 lines
Quoted 3 lines
For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
If the drivers are really essential/necessary/required, this precludes running the generic kernel image on the platform that requires them, making the kernel not sufficiently generic.
Quoted 5 lines
Quoted 3 lines
If they are not at all present, then yes. However that is not what is being suggested. The essential functionality will be provided. Just not built-in.
I really meant "essential/necessary/required to be built-in".
Then I agree with you. My position is that if they don't *have* to be built-in, then why force it?
Quoted 11 lines
Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
True, if "potentially". If not potentially, they must be included.
I'm not sure what you're trying to say here. Would you mind elaborating?
Quoted 8 lines
It was a comment to your "*potentially* utilised on *some* platforms". It is clear they are not used on the other ("not *some*") platforms, but your sentence was unclear whether they are always or only sometimes used on "*some*" platforms.
"always" => "not potentially"
"sometimes" => "potentially".I hope this makes it more clear.
Not really, but I'll try to clean mine up:
The aim is to have a single kernel (image + modules) that can be booted on a plethora of platforms. For the sake of argument say 10. Let's also say that each of the platforms are equal and will be booted the same amount of times.
Taking the example above, when I say that the H/W specific drivers will only be *potentially* utilised, I mean that they will only be bound and probed 1/10 times i.e. when booted on the associated platform. Which means that in the vast majority of boots (9/10) they will lie dormant, taking up unnecessary space.
Another way to say this would be; the kernel needs to have the capability to boot all of the supported platforms, but it will only ever be utilised on one at a time.
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
The key here is if the driver is required or not to use the platform, and why it is required. If the requirement comes from some deficiency in the kernel code or config system, it should be fixed, if possible. And the fix should be tested.
If it cannot be fixed, the driver should be included, else it would preclude running the generic kernel on the affected platform.
Quoted 5 lines
Quoted 1 line
Sorry, I'm not following.
It all depends on why the driver is "required to be built-in". Depending on the reason behind that requirement, the driver can be changed from built-in to modular without ill effects on functionality.
Absolutely.
There are cases where drivers simply can't be built as modules. These unavoidable situations are legitimate use-cases and the technology/ code-base will have to work around these as required.
The argument here is that if they can be separated and have been shown to work well in either use-case, then it is my opinion that placing an artificial barrier up based mostly on politics is not the correct approach.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Quoted 78 lines · Lee Jones
Quoted 35 lines · Geert Uytterhoeven
Quoted 34 lines · Lee Jones
Quoted 14 lines · Geert Uytterhoeven
Quoted 13 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 7 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them.
Why?
Because without them the image wouldn't functional on any level.
But you're right, there is still no requirement for it to be built-in.
Quoted 7 lines
Quoted 3 lines
For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
If the drivers are really essential/necessary/required, this precludes running the generic kernel image on the platform that requires them, making the kernel not sufficiently generic.
Quoted 5 lines
Quoted 3 lines
If they are not at all present, then yes. However that is not what is being suggested. The essential functionality will be provided. Just not built-in.
I really meant "essential/necessary/required to be built-in".
Then I agree with you. My position is that if they don't *have* to be built-in, then why force it?
Quoted 11 lines
Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
True, if "potentially". If not potentially, they must be included.
I'm not sure what you're trying to say here. Would you mind elaborating?
Quoted 8 lines
It was a comment to your "*potentially* utilised on *some* platforms". It is clear they are not used on the other ("not *some*") platforms, but your sentence was unclear whether they are always or only sometimes used on "*some*" platforms.
"always" => "not potentially"
"sometimes" => "potentially".I hope this makes it more clear.
Not really, but I'll try to clean mine up:
The aim is to have a single kernel (image + modules) that can be booted on a plethora of platforms. For the sake of argument say 10. Let's also say that each of the platforms are equal and will be booted the same amount of times.
Taking the example above, when I say that the H/W specific drivers will only be *potentially* utilised, I mean that they will only be bound and probed 1/10 times i.e. when booted on the associated platform. Which means that in the vast majority of boots (9/10) they will lie dormant, taking up unnecessary space.
Quoted 3 lines
Another way to say this would be; the kernel needs to have the capability to boot all of the supported platforms, but it will only ever be utilised on one at a time.
That's true even for drivers for "generic" hardware, right? E.g. arm64 selects ARM_GIC and ARM_GIC_V3, where most (all?) platforms have at most one of them.
Quoted 21 lines
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines
Quoted 5 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
The key here is if the driver is required or not to use the platform, and why it is required. If the requirement comes from some deficiency in the kernel code or config system, it should be fixed, if possible. And the fix should be tested.
If it cannot be fixed, the driver should be included, else it would preclude running the generic kernel on the affected platform.Quoted 5 lines
Quoted 1 line
Sorry, I'm not following.
It all depends on why the driver is "required to be built-in". Depending on the reason behind that requirement, the driver can be changed from built-in to modular without ill effects on functionality.
Absolutely.
Quoted 8 lines
There are cases where drivers simply can't be built as modules. These unavoidable situations are legitimate use-cases and the technology/ code-base will have to work around these as required.
The argument here is that if they can be separated and have been shown to work well in either use-case, then it is my opinion that placing an artificial barrier up based mostly on politics is not the correct approach.
Agreed.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 13 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 7 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
Quoted 7 lines
Quoted 1 line
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
By "essential", I meant drivers which are needed for supported platform to boot properly. Also without significant performance penalty due to probe deferrals.
Therefore if someone selects ARCH_EXYNOS in mainline kernel, it means he/she wants such devices to be working fine with generic kernel.
This is the difference with term "drivers have to be available".
Quoted 7 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Geert responded here. If drivers are essential for supported platforms to boot, having them built-in has technical sense.
For other drivers not and we do not discuss such cases.
Quoted 6 lines
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Geert responded here.
Quoted 10 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
By insisting on drivers (most of which will not be executed in the vast majority of cases) to be built-in, you are insisting on a downstream kernel fork, which all of us would like to avoid [0].
The same with all the DTS and hundreds of drivers you keep out of tree.
Quoted 17 lines
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 5 lines
We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared. Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable. Please upstream your drivers and DTS.
Quoted 5 lines
Quoted 2 lines
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Accepting changes based on the proviso that vendors upstream all of their work-in-progress solutions is an unfair position.
You twisted (or ignored) my words here. I said before - sacrificing any mainline platform (e.g. reliable boot for distro) for an out-of-tree vendor which does no want to upstream drivers is the unfair position. One of the incentives of upstreaming is to be able to shape kernel and be considered in kernel upstream decisions. That's the privileged for upstreamed platforms - they have an impact.
Vendor decides to stay out - fine, vendor's choice. You loose impact. Any sad words like "oh upstream does not want to change for me" should receive a message: "please join the upstream :), so we will change together".
Quoted 3 lines
We already
discussed why upstreaming support for bleeding edge H/W and functionality is unrealistic in terms of competitive advantage.
Nope, my last point was not responded to. You said that there is no point for vendor to upstream bleeding edge HW. Then you said that there is also little point to upstream older HW.
Basically following this approach you agree that vendor does not have to do anything because it is inconvenient for him.
However upstream has to adapt to downstream vendor, right?
No, this is *unfair to all the platforms we upstreamed*.
Quoted 11 lines
Besides, we might not be talking about new silicon at all (I don't believe anyone alluded to that). The flexibility in configuration simply allows for generic upstream drivers to be swapped out for ones which may have more or slightly different functionality (that can't be publicly shared until release).
Quoted 3 lines
Please explain why Exynos is special that it does not require essential drivers to be selected as built-in. For example why aren't same changes done for Renesas?
Quoted 8 lines
Quoted 3 lines
Everyone else are working like this. NXP, Renesas, Xilinx, TI, Rockchip, AllWinner. Samsung or Google is not special to receive an exception for this.
Exynos isn't special in this regard. This applies to any vendor who releases Android images and wishes to be solve all of the issues the GKI project addresses (please read the article above for more about this).
We have then slightly different goals, because you are driven by Android release images and this is why you are less interested in NXP and Renesas. Only some vendors should receive such changes? No, in upstream we are not focusing on any specific distro and there are other uses of Exynos (and NXP and Renesas) platforms. Therefore the choice/policy and overall design tries to match all vendors, not only some subset convenient to Android.
If Android (or some vendor) wants to have exception for a specific driver/platform, it must do it in upstream way, by contributing, not by changing to match downstream needs while ignoring other users.
Best regards,
Krzysztof
Quoted 16 lines · Krzysztof Kozlowski
Quoted 15 lines · Lee Jones
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
Quoted 8 lines
Quoted 5 lines
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
By "essential", I meant drivers which are needed for supported platform to boot properly.
Yes, I know what you meant by essential.
My comment still stands. As long as they are provided, it should work.
Quoted 2 lines
Also without significant performance penalty due to probe deferrals.
Agreed. We will try to find a way to test this.
Quoted 9 lines
Quoted 6 lines
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Geert responded here. If drivers are essential for supported platforms to boot, having them built-in has technical sense.
We're going to have to agree to disagree here.
None of us are currently aware of any technical reasons why these particular drivers have to be built-in. Hopefully we can prove this out with testing.
Quoted 5 lines
Quoted 3 lines
By insisting on drivers (most of which will not be executed in the vast majority of cases) to be built-in, you are insisting on a downstream kernel fork, which all of us would like to avoid [0].
The same with all the DTS and hundreds of drivers you keep out of tree.
I do not keep any drivers out of tree. :)
Quoted 18 lines
Quoted 10 lines
Quoted 7 lines
We don't even know what are these unsupported, downstream platforms you want customize kernel for. They cannot be audited, cannot be compared. Affecting upstream platforms just because vendor/downstream does not want to mainline some code is unacceptable.
Please upstream your drivers and DTS.
Accepting changes based on the proviso that vendors upstream all of their work-in-progress solutions is an unfair position.
You twisted (or ignored) my words here. I said before - sacrificing any mainline platform (e.g. reliable boot for distro) for an out-of-tree vendor which does no want to upstream drivers is the unfair position. One of the incentives of upstreaming is to be able to shape kernel and be considered in kernel upstream decisions. That's the privileged for upstreamed platforms - they have an impact.
Quoted 4 lines
Vendor decides to stay out - fine, vendor's choice. You loose impact. Any sad words like "oh upstream does not want to change for me" should receive a message: "please join the upstream :), so we will change together".
No one is proposing to sacrifice anything.
This change is believed to be benign.
Quoted 7 lines
Quoted 3 lines
We already
discussed why upstreaming support for bleeding edge H/W and functionality is unrealistic in terms of competitive advantage.Nope, my last point was not responded to. You said that there is no point for vendor to upstream bleeding edge HW. Then you said that there is also little point to upstream older HW.
The driver in question is already upstream.
It is my hope that the vendor will realign with Mainline and upstream the differences (providing the current competitive edge). Although I have no real influence in this regard, as I have no contact with them.
Quoted 6 lines
Basically following this approach you agree that vendor does not have to do anything because it is inconvenient for him.
However upstream has to adapt to downstream vendor, right?
No, this is *unfair to all the platforms we upstreamed*.
The point here is that this change should not be a determent to anyone.
Quoted 13 lines
Quoted 4 lines
Exynos isn't special in this regard. This applies to any vendor who releases Android images and wishes to be solve all of the issues the GKI project addresses (please read the article above for more about this).
We have then slightly different goals, because you are driven by Android release images and this is why you are less interested in NXP and Renesas. Only some vendors should receive such changes? No, in upstream we are not focusing on any specific distro and there are other uses of Exynos (and NXP and Renesas) platforms. Therefore the choice/policy and overall design tries to match all vendors, not only some subset convenient to Android.
Quoted 3 lines
If Android (or some vendor) wants to have exception for a specific driver/platform, it must do it in upstream way, by contributing, not by changing to match downstream needs while ignoring other users.
No users are being ignored. No damaging changes are being proposed.
This has little to do with Android and everything to do with the possibly of a more generic kernel. Moving drivers out of platform code and into /drivers along with enabling components as modules has been an on-going effort for many years now.
I'm struggling to see how this is different.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
2021年9月30日(木) 18:23 Lee Jones <[email protected]>:
Quoted 35 lines
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 5 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
[Chiming in as a clock driver sub-maintainer and someone who spent a non-insignificant part of his life on SoC driver bring-up - not as a Google employee.]
I'd argue that the proper way for them to achieve it would be to extend the upstream frameworks and/or existing drivers with appropriate APIs to allow their downstream modules to plug into what's already available upstream.
Best regards,
Tomasz
Hi Tomasz,
Quoted 41 lines · Tomasz Figa
2021年9月30日(木) 18:23 Lee Jones <[email protected]>:
Quoted 34 lines
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 5 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
Quoted 8 lines
[Chiming in as a clock driver sub-maintainer and someone who spent a non-insignificant part of his life on SoC driver bring-up - not as a Google employee.]
I'd argue that the proper way for them to achieve it would be to extend the upstream frameworks and/or existing drivers with appropriate APIs to allow their downstream modules to plug into what's already available upstream.
Yes, that's one possible solution.
We do have to be careful this would not just become some fishy API to touch deeply buried internals, and for which there are no valid use cases in upstream.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 42 lines · Tomasz Figa
2021年9月30日(木) 18:23 Lee Jones <[email protected]>:
Quoted 35 lines
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 5 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
Quoted 8 lines
[Chiming in as a clock driver sub-maintainer and someone who spent a non-insignificant part of his life on SoC driver bring-up - not as a Google employee.]
I'd argue that the proper way for them to achieve it would be to extend the upstream frameworks and/or existing drivers with appropriate APIs to allow their downstream modules to plug into what's already available upstream.
Is that the same as exporting symbols to framework APIs?
Since this is already a method GKI uses to allow external modules to interact with the core kernel/frameworks. However, it's not possible to upstream these without an upstream user for each one.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
2021年9月30日(木) 20:51 Lee Jones <[email protected]>:
Quoted 54 lines · Tomasz Figa
Quoted 43 lines
2021年9月30日(木) 18:23 Lee Jones <[email protected]>:
Quoted 35 lines
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 5 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
Quoted 8 lines
[Chiming in as a clock driver sub-maintainer and someone who spent a non-insignificant part of his life on SoC driver bring-up - not as a Google employee.]
I'd argue that the proper way for them to achieve it would be to extend the upstream frameworks and/or existing drivers with appropriate APIs to allow their downstream modules to plug into what's already available upstream.
Quoted 5 lines
Is that the same as exporting symbols to framework APIs?
Since this is already a method GKI uses to allow external modules to interact with the core kernel/frameworks. However, it's not possible to upstream these without an upstream user for each one.
Not necessary the core frameworks, could also be changing the ways the existing drivers register to allow additional drivers to extend the functionality rather than completely overwrite them. It's really hard to tell what the right way would be without knowing the exact things they find missing in the upstream drivers. As for upstream users, this is exactly the point - upstream is a bidirectional effort, one takes from it and should contribute things back.
Generally, the subsystems being mentioned here are so basic (clock, pinctrl, rtc), that I really can't imagine what kind of rocket science one might want to hide for competitive reasons... If it's for an entire SoC, I wonder why Intel and AMD don't have similar concerns and contribute support for their newest hardware far before the release.
Quoted 7 lines · Tomasz Figa
2021年9月30日(木) 20:51 Lee Jones <[email protected]>:
Quoted 6 lines · Tomasz Figa
Is that the same as exporting symbols to framework APIs?
Quoted 7 lines
Quoted 3 lines
Since this is already a method GKI uses to allow external modules to interact with the core kernel/frameworks. However, it's not possible to upstream these without an upstream user for each one.
Not necessary the core frameworks, could also be changing the ways the existing drivers register to allow additional drivers to extend the functionality rather than completely overwrite them.
Yes, the first user could be within the kernel after modifying some of the drivers.
Quoted 11 lines
It's really hard
to tell what the right way would be without knowing the exact things they find missing in the upstream drivers. As for upstream users, this is exactly the point - upstream is a bidirectional effort, one takes from it and should contribute things back.Generally, the subsystems being mentioned here are so basic (clock, pinctrl, rtc), that I really can't imagine what kind of rocket science one might want to hide for competitive reasons... If it's for an entire SoC, I wonder why Intel and AMD don't have similar concerns and contribute support for their newest hardware far before the release.
Lee used the argument of not-disclosing-edge-hw but I also don't see much of it in the case of few drivers needed to be overridden. Just bunch of registers for the same stuff we have sine 8 years. Rather the vendor does not want to commit effort towards upstreaming these...
Best regards,
Krzysztof
Quoted 69 lines · Tomasz Figa
2021年9月30日(木) 20:51 Lee Jones <[email protected]>:
Quoted 54 lines · Tomasz Figa
Quoted 43 lines
2021年9月30日(木) 18:23 Lee Jones <[email protected]>:
Quoted 35 lines
I've taken the liberty of cherry-picking some of the points you have reiteratted a few times. Hopefully I can help to address them adequently.
Quoted 9 lines · Krzysztof Kozlowski
Reminder: these are essential drivers and all Exynos platforms must have them as built-in (at least till someone really tests this on multiple setups).
Therefore I don't agree with calling it a "problem" that we select *necessary* drivers for supported platforms. It's by design - supported platforms should receive them without ability to remove.
The selected drivers are essential for supported platforms.
SoC specific drivers are only essential/necessary/required in images designed to execute solely on a platform that requires them. For a kernel image which is designed to be generic i.e. one that has the ability to boot on vast array of platforms, the drivers simply have to be *available*.
Forcing all H/W drivers that are only *potentially* utilised on *some* platforms as core binary built-ins doesn't make any technical sense. The two most important issues this causes are image size and a lack of configurability/flexibility relating to real-world application i.e. the one issue we already agreed upon; H/W or features that are too new (pre-release).
Bloating a generic kernel with potentially hundreds of unnecessary drivers that will never be executed in the vast majority of instances doesn't achieve anything. If we have a kernel image that has the ability to boot on 10's of architectures which have 10's of platforms each, that's a whole host of unused/wasted executable space.
Quoted 5 lines
In order for vendors to work more closely with upstream, they need the ability to over-ride a *few* drivers to supplement them with some functionality which they believe provides them with a competitive edge (I think you called this "value-add" before) prior to the release of a device. This is a requirement that cannot be worked around.
Quoted 8 lines
[Chiming in as a clock driver sub-maintainer and someone who spent a non-insignificant part of his life on SoC driver bring-up - not as a Google employee.]
I'd argue that the proper way for them to achieve it would be to extend the upstream frameworks and/or existing drivers with appropriate APIs to allow their downstream modules to plug into what's already available upstream.
Quoted 5 lines
Is that the same as exporting symbols to framework APIs?
Since this is already a method GKI uses to allow external modules to interact with the core kernel/frameworks. However, it's not possible to upstream these without an upstream user for each one.
Not necessary the core frameworks, could also be changing the ways the existing drivers register to allow additional drivers to extend the functionality rather than completely overwrite them. It's really hard to tell what the right way would be without knowing the exact things they find missing in the upstream drivers. As for upstream users, this is exactly the point - upstream is a bidirectional effort, one takes from it and should contribute things back.
Quoted 5 lines
Generally, the subsystems being mentioned here are so basic (clock, pinctrl, rtc), that I really can't imagine what kind of rocket science one might want to hide for competitive reasons... If it's for an entire SoC, I wonder why Intel and AMD don't have similar concerns and contribute support for their newest hardware far before the release.
I don't have visibility into the driver-overrides I'm afraid.
I do know that code-space can be a problem though. So any way we can make the core binary smaller (i.e. remove anything that can be built as a module) will have a positive effect.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 5 lines · Tomasz Figa
Generally, the subsystems being mentioned here are so basic (clock, pinctrl, rtc), that I really can't imagine what kind of rocket science one might want to hide for competitive reasons... If it's for an entire SoC, I wonder why Intel and AMD don't have similar concerns and contribute support for their newest hardware far before the release.
There is no reason at all, and to be honest this whole discussion with these bullshit arguments from the Google/Linaro/SoC vendor crowd just shows how on crack these people are, and shows a good example of why we should not support these models at all. There is no good reason to "overide" uptream functionality EVER. Stop digging yourselves into your ever bigger holes and just f***king contribute upstream NOW. Just as we always have we should not give you more rope to shoot yoursel while ausing us extra overhead.
Quoted 8 lines · Christoph Hellwig
Quoted 5 lines · Tomasz Figa
Generally, the subsystems being mentioned here are so basic (clock, pinctrl, rtc), that I really can't imagine what kind of rocket science one might want to hide for competitive reasons... If it's for an entire SoC, I wonder why Intel and AMD don't have similar concerns and contribute support for their newest hardware far before the release.
Quoted 8 lines
There is no reason at all, and to be honest this whole discussion with these bullshit arguments from the Google/Linaro/SoC vendor crowd just shows how on crack these people are, and shows a good example of why we should not support these models at all. There is no good reason to "overide" uptream functionality EVER. Stop digging yourselves into your ever bigger holes and just f***king contribute upstream NOW. Just as we always have we should not give you more rope to shoot yoursel while ausing us extra overhead.
Maybe you need to read up the code of conduct again.
-Saravana
Quoted 1 line · Saravana Kannan
Maybe you need to read up the code of conduct again.
*plonk*
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPM
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
Best regards,
Krzysztof
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPM
Quoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Haven't I explained this several times already? :)
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 21 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
Quoted 4 lines
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
Best regards,
Krzysztof
Quoted 25 lines · Krzysztof Kozlowski
Quoted 24 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Quoted 6 lines
Quoted 1 line
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
git reset --hard mainline/master # job done - tea break :)
Seriously though, we wish to encourage the use of GKI so all vendors can enjoy the benefits of more easily updateable/secure code-bases.
I can't see how pushing back on seamlessly benign changes would benefit them or anyone else.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Quoted 32 lines · Lee Jones
Quoted 24 lines · Krzysztof Kozlowski
Quoted 23 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Quoted 6 lines
Quoted 1 line
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
Quoted 7 lines
git reset --hard mainline/master # job done - tea break :)
Seriously though, we wish to encourage the use of GKI so all vendors can enjoy the benefits of more easily updateable/secure code-bases.
I can't see how pushing back on seamlessly benign changes would benefit them or anyone else.
I like your wording ;-)
Indeed, seamlessly benign changes, which are (1) not tested, and (2) some believed by the platform maintainer to break the platform. What can possibly go wrong? ;-)
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 40 lines · Geert Uytterhoeven
Hi Lee,
Quoted 37 lines · Lee Jones
Quoted 24 lines · Krzysztof Kozlowski
Quoted 23 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Quoted 6 lines
Quoted 1 line
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
git reset --hard mainline/master # job done - tea break :)
Seriously though, we wish to encourage the use of GKI so all vendors can enjoy the benefits of more easily updateable/secure code-bases.
Quoted 8 lines
Quoted 2 lines
I can't see how pushing back on seamlessly benign changes would benefit them or anyone else.
I like your wording ;-)
Indeed, seamlessly benign changes, which are (1) not tested, and (2) some believed by the platform maintainer to break the platform. What can possibly go wrong? ;-)
William has already shown a willingness to test the series.
There is already a downstream proof-of-concept of this working.
I am hopeful. :)
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Lee,
Quoted 42 lines · Lee Jones
Quoted 41 lines · Geert Uytterhoeven
Quoted 39 lines · Lee Jones
Quoted 24 lines · Krzysztof Kozlowski
Quoted 23 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Quoted 6 lines
Quoted 1 line
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
git reset --hard mainline/master # job done - tea break :)
Seriously though, we wish to encourage the use of GKI so all vendors can enjoy the benefits of more easily updateable/secure code-bases.
I can't see how pushing back on seamlessly benign changes would benefit them or anyone else.
Quoted 7 lines
Quoted 5 lines
I like your wording ;-)
Indeed, seamlessly benign changes, which are (1) not tested, and (2) some believed by the platform maintainer to break the platform. What can possibly go wrong? ;-)
William has already shown a willingness to test the series.
I'm looking forward to the results!
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 58 lines · Geert Uytterhoeven
Hi Lee,
Quoted 42 lines · Lee Jones
Quoted 41 lines · Geert Uytterhoeven
Quoted 39 lines · Lee Jones
Quoted 24 lines · Krzysztof Kozlowski
Quoted 23 lines · Lee Jones
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Lee Jones
[0] Full disclosure: part of my role at Linaro is to keep the Android kernel running as close to Mainline as possible and encourage/push the upstream-first mantra, hence my involvement with this and other sets. I assure you all intentions are good and honourable. If you haven't already seen it, please see Todd's most recent update on the goals and status of GKI:
Article: https://tinyurl.com/saaen3sp
Video: https://youtu.be/O_lCFGinFPMQuoted 6 lines
Side topic, why this patchset is in your scope or Will's/Google's scope? Just drop it from Android main kernel, it will not be your problem. I mean, really, you don't need this patchset in your tree at all. The only platform which needs it, the only platform which will loose something will be one specific vendor. Therefore this will be an incentive for them to join both discussions and upstream development. :)
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Quoted 6 lines
Quoted 1 line
Haven't I explained this several times already? :)
Either that way or the same as Will's doing but that's not my question. I understand you flush the queue of your GKI patches to be closer to upstream. Reduce the backlog/burden. you can achieve your goal by simply dropping such patch and making it not your problem. :)
git reset --hard mainline/master # job done - tea break :)
Seriously though, we wish to encourage the use of GKI so all vendors can enjoy the benefits of more easily updateable/secure code-bases.
I can't see how pushing back on seamlessly benign changes would benefit them or anyone else.
Quoted 7 lines
Quoted 5 lines
I like your wording ;-)
Indeed, seamlessly benign changes, which are (1) not tested, and (2) some believed by the platform maintainer to break the platform. What can possibly go wrong? ;-)
William has already shown a willingness to test the series.
I'm looking forward to the results!
Gr{oetje,eeting}s,
Quoted 8 lines
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Krzysztof and Geert,
Thanks for all the responses! There are a few things I want to respond to. Since this thread is 32 responses deep, I'm copying and pasting the parts I'm responding to. I hope that isn't an issue.
Quoted 3 lines
You are changing not default, but selectability which is part of the enforced configuration to make platforms working. The distros do not always choose defaults but rather all as modules.
I'm guessing you are referring to distros using the "allmodconfig". I agree that this would affect their platforms in the sense that they would get a module vs built-in, but obviously that is the choice they are making by using "allmodconfig". I also agree that my changes should not break any distros. I definitely don't want you or anyone else to accept these without them being tested and verified.
Quoted 4 lines
I don't understand the point (2) here. Anyone can use upstream kernel for supported and unsupported platforms. How upstream benefits from a change affecting supported platforms made for unsupported, downstream platforms.
We believe that if we make it easier for SoC vendors to directly use the upstream kernel during bring-up and during the development stages of their project, then that will decrease the friction of working with upstream (less downstream changes) and increase the upstream contributions. As mentioned in the LPC talk by Todd, the Android kernel team is constantly pushing to reduce the number of out-of-tree changes we carry so that we can ultimately use the upstream kernel directly.
Quoted 2 lines
You also mentioned downstream devices but without actually ever defining them. Please be more specific. What SoC, what hardware?
Saravana provided 2 platforms in his response. The first one is hikey960 devboard and the second one is the Pixel 5 which uses the Qualcomm sm7250 chipset. In particular the Pixel 5 is running a fully modular kernel. It has 320+ modules including interrupt controllers, timers, pinctrl and clocks.
To address your question regarding if Exynos is special? No, Exynos isn't special. There is a long road ahead of us to get to where we'd ultimately like to be and ARCH_EXYNOS happens to be along the way. Currently, we are working through the arm64 ARCH_XXX configs that are supported by Android to make sure that all drivers included by any ARCH_XXX are modularized if they can be which benefits all parties. From my understanding upstream does support modularizing drivers and we can help with that. I believe this also kind of addresses Geert's comments regarding what a generic kernel is. We look at it as a kernel that only includes the absolutely necessary SoC drivers as built-in. Obviously we can't go back and change older hardware like Cortex A9 to support this, but we can do our best to support future SoCs.
Lastly, you mentioned a couple of times that we should just disable ARCH_EXYNOS and move on. At the moment we are not able to do that because some SoC specific drivers actually do need to be built-in to the kernel (tainting it's generic-ness unfortunately) and are protected by these ARCH_XXX configs. One example that you or Geert pointed out is the early console serial driver which Greg did try to decouple from ARCH_EXYNOS but was turned down. There are likely other reasons as well that I don't know of off hand.
I hope Lee and I answered all your questions. Ultimately, I think we all understand what is being debated here and it sounds like we all agree that we are okay with modules as long as (1) they are proven to work as both built-in and as modules and (2) can't be disabled if they are required by the platform, but can be built-in or modular. Let me know if I misunderstood that.
Regarding the testing, I will look around for a devboard that I can test my clock driver changes on since those are the most contested ones. I obviously am not going to be able to find all the HW variants you mentioned, but I will try to find an ARM64 Exynos7 or Exynos5433 devboard since my patchset only modifies the clocks for these SoCs which seem to be the most contested. Once I provide some testing results, I'm hoping someone upstream can help verify that on the other affected hardware. Of course this will take some time though.
In the meantime, I will try to push some of these easier patches that don't need extensive testing, but are required before modularizing the drivers.
Thanks,
Will
Quoted 4 lines · Lee Jones
How would they fix this besides upstreaming support for unreleased work-in-progress H/W?
Haven't I explained this several times already? :)
No you haven't. Mostly likely because there is absolutely no good explanation. And if google/Linaro think they want to create hooks for stupid vendor modules we need to do whatever we can to make your life as miserable as possible.
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
Krzysztof
Quoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
-Olof
Quoted 57 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
Quoted 6 lines
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Oolf, Geert, Krzysztof, Arnd,
I skimmed through the emails and you all make a lot of good points. It looks like you all at least like the idea of being able to have a minimal generic kernel where everything that can be a module is a module. Please correct me if I'm wrong on that.
I was thinking about this patch series and I was wondering if it'd be good to come at it from the other end. Instead of taking the mostly builtin generic kernel and trying to rip out drivers as modules (and not having enough hardware to test them all) and hitting all these issues, we could come at it from the other end.
A config like ARM64_MINIMAL_GENERIC_KERNEL that's off by default. But if it's selected, all the "selects" done by the various ARCH_XXX are not done any more. Something like:
ARCH_XXX
select XXX_CLK1 if !ARM64_MINIMAL_GENERIC_KERNEL select XXX_PINCTRL1 if !ARM64_MINIMAL_GENERIC_KERNEL
ARCH_YYY
select YYY_CLK1 if !ARM64_MINIMAL_GENERIC_KERNEL select YYY_PINCTRL1 if !ARM64_MINIMAL_GENERIC_KERNEL
And ARM64_MINIMAL_GENERIC_KERNEL itself would select the absolutely mandatory stuff that can never be made into a module like the GIC, architectured timer (as Geert mentioned) and UART early console driver. I'm not sure if ARM32 has an equivalent to the standardized GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
This way, we'll keep the existing model working, while working on a fully modular kernel from the other end.
Thoughts?
-Saravana
Quoted 59 lines · Saravana Kannan
Quoted 57 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
Quoted 8 lines
Quoted 6 lines
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Oolf, Geert, Krzysztof, Arnd,
So close.
Quoted 1 line
I skimmed through the emails and you all make a lot of good points.
I skimmed through this email and I think it adds a lot of new complexity and fragility to solve a problem that doesn't really exist for upstream, adding yet more config parameter combinations to build and test for.
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
-Olof
Quoted 61 lines · Olof Johansson
Quoted 59 lines · Saravana Kannan
Quoted 57 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Are you saying that modularizing drivers is opening up a loophole? How is this different from Krysztof pushing changes to modularize the Exynos ChipId driver just last week [1]. I understand the push back on "these aren't tested yet" and I agree that we should not merge them until they are (I've re-iterated that multiple times and have requested for testing help multiple times since I can't get my hands on any Exynos arm64 hardware), but are you saying that if I gather the test data to prove that these drivers can actually be made into modules that you will still deny them out of the interest of the community?
[1] https://lore.kernel.org/linux-samsung-soc/[email protected]/T/#t
--Will
Quoted 19 lines
Quoted 3 lines
Oolf, Geert, Krzysztof, Arnd,
So close.
Quoted 1 line
I skimmed through the emails and you all make a lot of good points.
I skimmed through this email and I think it adds a lot of new complexity and fragility to solve a problem that doesn't really exist for upstream, adding yet more config parameter combinations to build and test for.
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
-Olof
Quoted 10 lines · Will McVicker
Quoted 9 lines · Olof Johansson
Quoted 9 lines
Quoted 9 lines
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Are you saying that modularizing drivers is opening up a loophole? How is this different from Krysztof pushing changes to modularize the Exynos ChipId driver just last week [1].
Modularizing drivers, which can work as modules or even can be disabled because they are not essential for platform boot, is not opening loophole and is helping upstream platforms. Modularizing everything, even essential drivers, because downstream does not want to contribute rest of its drivers, is not beneficial to the upstream project. Since downstream does want to contribute its platforms and drivers, it decides to change mainline project to fits its needs. Only its needs, not others.
I was repeating this multiple times - there is no point, no incentive for the mainline to allow disabling essential SoC drivers. It's only downstream interest without any benefit to the upstream.
Best regards,
Krzysztof
Quoted 65 lines · Olof Johansson
Quoted 63 lines · Saravana Kannan
Quoted 61 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Oolf, Geert, Krzysztof, Arnd,
So close.
I'm sorry, it's pretty late here and I'm sleepy and messed it up.
Quoted 7 lines
Quoted 2 lines
I skimmed through the emails and you all make a lot of good points.
I skimmed through this email and I think it adds a lot of new complexity and fragility to solve a problem that doesn't really exist for upstream, adding yet more config parameter combinations to build and test for.
How is this not an upstream problem? Having a minimal kernel with as many drivers as modules is of interest to upstream. And what's the complexity in having a config to easily disable a bunch of configs? The new config gives a clear config against which new platforms/systems should be developed against.
Quoted 4 lines
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Sure it would help memory after boot, but it won't help with size on "disk", kernel load time, etc. And some of the devices have very tight boot requirements. Think battery operated outdoor cameras for example.
-Saravana
Quoted 67 lines · Saravana Kannan
Quoted 65 lines · Olof Johansson
Quoted 63 lines · Saravana Kannan
Quoted 61 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Oolf, Geert, Krzysztof, Arnd,
So close.
I'm sorry, it's pretty late here and I'm sleepy and messed it up.
This email thread will be here in the morning too, this is the last reply I will make tonight myself.
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
I skimmed through the emails and you all make a lot of good points.
I skimmed through this email and I think it adds a lot of new complexity and fragility to solve a problem that doesn't really exist for upstream, adding yet more config parameter combinations to build and test for.
Quoted 5 lines
How is this not an upstream problem? Having a minimal kernel with as many drivers as modules is of interest to upstream. And what's the complexity in having a config to easily disable a bunch of configs? The new config gives a clear config against which new platforms/systems should be developed against.
The approach for general kernel upstream has been to have the built-in drivers needed to reach rootfs and runtime load the rest from there. For downstream embedded kernels, the right answer is to just exclude the drivers you don't need.
I agree, some of this is probably useful work but
1) It was posted as a burden on the maintainers of the legacy platforms to test and make sure it's not broken
2) It regresses distro configs, which we do care about
3) Based on the above, it's unclear whether the people pushing for this patchset cares about the existing platforms, they're looking to solve a different problem
I didn't go back and look at who and where, but the person who started justifying this work with making it easier for out-of-tree vendors to integrate their non-contributed code killed this patchset.
Would the out-of-tree argument on its own kill it? No, but the above complexities/cost, plus the actual intent behind the patchset did.
Quoted 7 lines
Quoted 3 lines
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Sure it would help memory after boot, but it won't help with size on "disk", kernel load time, etc. And some of the devices have very tight boot requirements. Think battery operated outdoor cameras for example.
I would question the choices made if someone ships a camera with a generic kernel (without a generic filesystem which at that point also needs sufficient storage such that the kernel image size becomes a secondary consideration). Once your storage is that constrained the balance shifts towards a custom minimal config.
-Olof
Quoted 88 lines · Olof Johansson
Quoted 67 lines · Saravana Kannan
Quoted 65 lines · Olof Johansson
Quoted 63 lines · Saravana Kannan
Quoted 61 lines · Olof Johansson
Quoted 41 lines · Will McVicker
Quoted 29 lines · Krzysztof Kozlowski
Quoted 20 lines · Will McVicker
This is v2 of the series of patches that modularizes a number of core
ARCH_EXYNOSdrivers. Based off of the feedback from the v1 series, I have modularized all of the drivers that are removed from theARCH_EXYNOSseries of "select XXX". This includes setting the following configs as tristate:*
COMMON_CLK_SAMSUNG
*EXYNOS_ARM64_COMMON_CLK
*PINCTRL_SAMSUNG
*PINCTRL_EXYNOS
*EXYNOS_PMU_ARM64
*EXYNOS_PM_DOMAINSAdditionally, it introduces the config
EXYNOS_PMU_ARM64andEXYNOS_PMU_ARMwhich was previouslyEXYNOS_PMUandEXYNOS_PMU_ARM_DRIVERSrespectively. The reason for these new configs is because we are not able to easily modularize the ARMv7 PMU driver due to built-in arch dependencies onpmu_base_addrunderarch/arm/mach-exynos/*. So the new configs split up the ARM and ARM64 portions into two separate configs.Quoted 6 lines
Overall, these drivers didn't require much refactoring and converted to modules relatively easily. However, due to my lack of exynos hardware, I was not able to boot test these changes. I'm mostly concerned about the
CLK_OF_DECLARE()changes having dependencies on early timers. So I'm requesting help for testing these changes on the respective hardware.Quoted 8 lines
These are all not tested at all? In such case, since these are not trivial changes, please mark the series as RFT.
I will not be able to test these for some days, so it must wait.
Best regards,
KrzysztofQuoted 8 lines
+Cc Arnd and Olof,
Hi Krzysztof,
To avoid the scrambled conversation from the first patchset, I'm going to address all your general questions here in the cover letter thread so that it's easier for everyone to follow and reference in the future.
This patchset shouldn't go in.
GKI is a fantastic effort, since it finally seems like Google has the backbone to put pressure on the vendors to upstream all their stuff.
This patcheset dilutes and undermines all of that by opening up a truck-size loophole, reducing the impact of GKI, and overall removes leverage to get vendors to do the right thing.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
It's against our interest as a community to have this happen, since there's no other reasonably justifiable reason to do this.
Oolf, Geert, Krzysztof, Arnd,
So close.
I'm sorry, it's pretty late here and I'm sleepy and messed it up.
This email thread will be here in the morning too, this is the last reply I will make tonight myself.
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
I skimmed through the emails and you all make a lot of good points.
I skimmed through this email and I think it adds a lot of new complexity and fragility to solve a problem that doesn't really exist for upstream, adding yet more config parameter combinations to build and test for.
Quoted 8 lines
Quoted 5 lines
How is this not an upstream problem? Having a minimal kernel with as many drivers as modules is of interest to upstream. And what's the complexity in having a config to easily disable a bunch of configs? The new config gives a clear config against which new platforms/systems should be developed against.
The approach for general kernel upstream has been to have the built-in drivers needed to reach rootfs and runtime load the rest from there.
Correction: The intent for our own multiconfigs have been this. If people want to go more granular and allow for modules of their core drivers, we have never pushed back (but the rest of the argument still stands).
Quoted 34 lines
For downstream embedded kernels, the right answer is to just exclude the drivers you don't need.
I agree, some of this is probably useful work but
1) It was posted as a burden on the maintainers of the legacy platforms to test and make sure it's not broken
2) It regresses distro configs, which we do care about
3) Based on the above, it's unclear whether the people pushing for this patchset cares about the existing platforms, they're looking to solve a different problemI didn't go back and look at who and where, but the person who started justifying this work with making it easier for out-of-tree vendors to integrate their non-contributed code killed this patchset.
Would the out-of-tree argument on its own kill it? No, but the above complexities/cost, plus the actual intent behind the patchset did.
Quoted 7 lines
Quoted 3 lines
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Sure it would help memory after boot, but it won't help with size on "disk", kernel load time, etc. And some of the devices have very tight boot requirements. Think battery operated outdoor cameras for example.
I would question the choices made if someone ships a camera with a generic kernel (without a generic filesystem which at that point also needs sufficient storage such that the kernel image size becomes a secondary consideration). Once your storage is that constrained the balance shifts towards a custom minimal config.
-Olof
Quoted 7 lines · Saravana Kannan
Quoted 3 lines
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Sure it would help memory after boot, but it won't help with size on "disk", kernel load time, etc. And some of the devices have very tight boot requirements. Think battery operated outdoor cameras for example.
I think we can draw a clear line (or several lines) between devices that boot from strictly constrained NOR flash and those that run a platform-independent kernel.
Also, when I look at a distro kernel, I see over 5000 kernel modules that need to be stored on disk, but only a small fraction of those are platform specific while most are for general-purpose pluggable devices, network features or file systems that could be used on any system.
The vmlinux file is clearly too big and includes too much stuff that should be in loadable modules, but I'm not really that worried about disk space for the platform specific code.
Arnd
Quoted 2 lines · Arnd Bergmann
The vmlinux file is clearly too big and includes too much stuff that should be in loadable modules
This for me is the crux of the matter.
The ability to replace modules was only brought to light as an "and also, this is possible". However in retrospect, given the attention this has received, it probably shouldn't have even mentioned, as it's not that important.
We should focus on the benefits of making parts of the kernel modular if technically possible. The most prominent of those is core binary size, since this has a direct impact on boot-time and RAM usage.
Reclaiming dead code after boot is certainly one way to tackle part of the problem. Ensuring that it's not even loaded into RAM in the first place is a better more encompassing solution to both issues IMHO.
Signature · 4 lines
Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 11 lines · Lee Jones
Quoted 2 lines · Arnd Bergmann
The vmlinux file is clearly too big and includes too much stuff that should be in loadable modules
This for me is the crux of the matter.
The ability to replace modules was only brought to light as an "and also, this is possible". However in retrospect, given the attention this has received, it probably shouldn't have even mentioned, as it's not that important.
Too late, unfortunately.
I would actually argue that given the benefit of needing more vendor engagement upstream for GKI to be smooth, it's in our interest to welcome those engagements and make the most of it and help vendors get there, and it's against those interests to make it easier to be out-of-tree if it comes at the expense of our in-tree users and maintainers which this does.
Quoted 3 lines
We should focus on the benefits of making parts of the kernel modular if technically possible. The most prominent of those is core binary size, since this has a direct impact on boot-time and RAM usage.
The way forward here should be to focus on the problem that needs to be solved (vmlinux size) and not overly fixate on whether this patchset is what needs to be merged to reach there, given the downsides observed. I'm not saying let's not improve vmlinux binary size, but this particular approach isn't appealing.
Quoted 3 lines
Reclaiming dead code after boot is certainly one way to tackle part of the problem. Ensuring that it's not even loaded into RAM in the first place is a better more encompassing solution to both issues IMHO.
See my reply to Arnd; the reason some of these drivers aren't modules today is because they are needed during early boot to bring the platform to a stable operating point. Work on fixing the binary size is terrific, but this approach seems to be shortsighted and it's been done in a way that rubs the maintainers the wrong way.
-Olof
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
This would be really neat.
Also the ages-old problem of discarding unreferenced strings which apparently still isn't possible would be great to have solved. E.g. to couple strings to the file/module it belongs with and discard them if the code is not probed.
Yours,
Linus Walleij
Hi Olof,
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
We used to have such a functionality in arch/ppc (not arch/powerpc!), where code/data could be tagged __prep, __chrp, or __pmac, to put it in a special section, and to be freed with initdata when unused. It was removed in v2.6.15[1], as the savings weren't worth the hassle. In a more fragmented space like arm the memory lost due to alignment of the sections would be even more substantial.
Another problem is to know when is the end of the boot, especially with deferred probing.
[1] 6c45ab992e4299c8 ("[PATCH] powerpc: Remove section free() and linker script bits")
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 8 lines · Geert Uytterhoeven
Hi Olof,
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Quoted 6 lines
We used to have such a functionality in
arch/ppc(notarch/powerpc!), where code/data could be tagged__prep,__chrp, or__pmac, to put it in a special section, and to be freed with initdata when unused. It was removed in v2.6.15[1], as the savings weren't worth the hassle. In a more fragmented space like arm the memory lost due to alignment of the sections would be even more substantial.
Yeah, the balance between per-platform code size and overall kernel code size shifted over time to a point where it wasn't as meaningful on ppc.
Quoted 2 lines
Another problem is to know when is the end of the boot, especially with deferred probing.
Most of this code either has a module_init() or an initcall that actually registers the drivers and/or probes for the platform and does the work.
This means you can have a late equivalent hook/initcall that determines whether this path ended up being probed/used. If it wasn't, you can then unregister and flag the corresponding memory to be freed at the end, and would take out the heuristics and guessing on needing to do it automatically from the code path that's doing said freeing.
-Olof
Quoted 28 lines · Olof Johansson
Quoted 8 lines · Geert Uytterhoeven
Hi Olof,
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Quoted 6 lines
We used to have such a functionality in
arch/ppc(notarch/powerpc!), where code/data could be tagged__prep,__chrp, or__pmac, to put it in a special section, and to be freed with initdata when unused. It was removed in v2.6.15[1], as the savings weren't worth the hassle. In a more fragmented space like arm the memory lost due to alignment of the sections would be even more substantial.Yeah, the balance between per-platform code size and overall kernel code size shifted over time to a point where it wasn't as meaningful on ppc.
Quoted 2 lines
Another problem is to know when is the end of the boot, especially with deferred probing.
Most of this code either has a
module_init()or an initcall that actually registers the drivers and/or probes for the platform and does the work.
Quoted 8 lines
This means you can have a late equivalent hook/initcall that determines whether this path ended up being probed/used. If it wasn't, you can then unregister and flag the corresponding memory to be freed at the end, and would take out the heuristics and guessing on needing to do it automatically from the code path that's doing said freeing.
-Olof
First off, I appreciate the constructive conversations and I understand the ask here. So I'd like to close the "we don't want this" and "this isn't possible" conversation. We have already proven downstream that it is in fact possible to modularize these drivers on other SoCs (mentioned earlier if you missed it) and I'd like to direct the conversation towards verifying/testing here instead of negatively arguing about how SoC vendors aren't upstreaming their drivers. I think everyone understands that, but unfortunately I have no control over that even though I would love everyone to work upstream directly.
I am fine with forcing these drivers to always be enabled in some form upstream even though it doesn't really make much sense for a generic kernel that will run on Qualcomm, Exynos, Mediatek, (you name it) SoC devices. I thought about how to do this yesterday and wasn't able to come up with a proper solution that didn't always force this driver to be a module when CONFIG_MODULES is enabled.
For example, if I do this below, then we will be forcing all builds to use CONFIG_XXX as a module if they want just one driver as a module.
config XXX
tristate "blah blah" if COMPILE_TEST
default m if (ARCH_XXX && MODULES)
default ARCH_XXX
The best I was able to come up with was this below which would allow the driver to be a module or built-in; however, obviously it lets you disable it in EXPERT mode.
config XXX
tristate "blah blah" if COMPILE_TEST || EXPERT
default ARCH_XXX
Let me know if you have a better solution that doesn't force the driver to be a module when CONFIG_MODULES=y. Saravana did propose a MINIMUM_ARM64_GENERIC_KERNEL config that could solve this, but that too was shot down.
Thanks,
Will
Quoted 49 lines · Will McVicker
Quoted 28 lines · Olof Johansson
Quoted 8 lines · Geert Uytterhoeven
Hi Olof,
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Quoted 6 lines
We used to have such a functionality in
arch/ppc(notarch/powerpc!), where code/data could be tagged__prep,__chrp, or__pmac, to put it in a special section, and to be freed with initdata when unused. It was removed in v2.6.15[1], as the savings weren't worth the hassle. In a more fragmented space like arm the memory lost due to alignment of the sections would be even more substantial.Yeah, the balance between per-platform code size and overall kernel code size shifted over time to a point where it wasn't as meaningful on ppc.
Quoted 2 lines
Another problem is to know when is the end of the boot, especially with deferred probing.
Most of this code either has a
module_init()or an initcall that actually registers the drivers and/or probes for the platform and does the work.Quoted 8 lines
This means you can have a late equivalent hook/initcall that determines whether this path ended up being probed/used. If it wasn't, you can then unregister and flag the corresponding memory to be freed at the end, and would take out the heuristics and guessing on needing to do it automatically from the code path that's doing said freeing.
-Olof
First off, I appreciate the constructive conversations and I understand the ask here. So I'd like to close the "we don't want this" and "this isn't possible" conversation. We have already proven downstream that it is in fact possible to modularize these drivers on other SoCs (mentioned earlier if you missed it) and I'd like to direct the conversation towards verifying/testing here instead of negatively arguing about how SoC vendors aren't upstreaming their drivers. I think everyone understands that, but unfortunately I have no control over that even though I would love everyone to work upstream directly.
Quoted 6 lines
I am fine with forcing these drivers to always be enabled in some form upstream even though it doesn't really make much sense for a generic kernel that will run on Qualcomm, Exynos, Mediatek, (you name it) SoC devices. I thought about how to do this yesterday and wasn't able to come up with a proper solution that didn't always force this driver to be a module when
CONFIG_MODULESis enabled.
This line of reasoning: "I couldn't think of a better option" made us merge a userspace ABI some time ago that within a few months was replaced with a better solution. In that case it was the kernel headers bundling with a build (extending the vmlinux size by a lot), that seemed to have no concerns about binary growth. Not all that far after that went in, the BPF folks came up with a solid solution for CO-RE by introducing BTF, etc.
So, the argument "I can't think of a better solution" is a local maxima that we shouldn't settle for if there's a likely better global maxima available with a bit more time/effort. If we say "this problem is worth solving but this doesn't seem to be the solution we want to go for" we might actually be better off long-term.
-Olof
Quoted 66 lines · Olof Johansson
Quoted 49 lines · Will McVicker
Quoted 28 lines · Olof Johansson
Quoted 8 lines · Geert Uytterhoeven
Hi Olof,
Quoted 3 lines · Olof Johansson
A much more valuable approach would be to work towards being able to free up memory by un-probed drivers at the end of boot. That would possibly benefit all platforms on all architectures.
Quoted 6 lines
We used to have such a functionality in
arch/ppc(notarch/powerpc!), where code/data could be tagged__prep,__chrp, or__pmac, to put it in a special section, and to be freed with initdata when unused. It was removed in v2.6.15[1], as the savings weren't worth the hassle. In a more fragmented space like arm the memory lost due to alignment of the sections would be even more substantial.Yeah, the balance between per-platform code size and overall kernel code size shifted over time to a point where it wasn't as meaningful on ppc.
Quoted 2 lines
Another problem is to know when is the end of the boot, especially with deferred probing.
Most of this code either has a
module_init()or an initcall that actually registers the drivers and/or probes for the platform and does the work.Quoted 8 lines
This means you can have a late equivalent hook/initcall that determines whether this path ended up being probed/used. If it wasn't, you can then unregister and flag the corresponding memory to be freed at the end, and would take out the heuristics and guessing on needing to do it automatically from the code path that's doing said freeing.
-Olof
First off, I appreciate the constructive conversations and I understand the ask here. So I'd like to close the "we don't want this" and "this isn't possible" conversation. We have already proven downstream that it is in fact possible to modularize these drivers on other SoCs (mentioned earlier if you missed it) and I'd like to direct the conversation towards verifying/testing here instead of negatively arguing about how SoC vendors aren't upstreaming their drivers. I think everyone understands that, but unfortunately I have no control over that even though I would love everyone to work upstream directly.
Quoted 6 lines
I am fine with forcing these drivers to always be enabled in some form upstream even though it doesn't really make much sense for a generic kernel that will run on Qualcomm, Exynos, Mediatek, (you name it) SoC devices. I thought about how to do this yesterday and wasn't able to come up with a proper solution that didn't always force this driver to be a module when
CONFIG_MODULESis enabled.This line of reasoning: "I couldn't think of a better option" made us merge a userspace ABI some time ago that within a few months was replaced with a better solution. In that case it was the kernel headers bundling with a build (extending the vmlinux size by a lot), that seemed to have no concerns about binary growth. Not all that far after that went in, the BPF folks came up with a solid solution for CO-RE by introducing BTF, etc.
Quoted 8 lines
So, the argument "I can't think of a better solution" is a local maxima that we shouldn't settle for if there's a likely better global maxima available with a bit more time/effort. If we say "this problem is worth solving but this doesn't seem to be the solution we want to go for" we might actually be better off long-term.
-Olof
If the answer is, "we don't have a solution for that" then that's fine. I'm not an expert at Kconfig configurations and am asking if there is already a way to handle this. To me it sounded like there might be a solution already due to this policy of "we don't allow disabling drivers that are essential." I'll wait for Krysztof to respond (or someone who has a potential solution) before I dig into this deeper.
--Will
Hi Saravana,
Quoted 19 lines · Saravana Kannan
I skimmed through the emails and you all make a lot of good points. It looks like you all at least like the idea of being able to have a minimal generic kernel where everything that can be a module is a module. Please correct me if I'm wrong on that.
I was thinking about this patch series and I was wondering if it'd be good to come at it from the other end. Instead of taking the mostly builtin generic kernel and trying to rip out drivers as modules (and not having enough hardware to test them all) and hitting all these issues, we could come at it from the other end.
A config like
ARM64_MINIMAL_GENERIC_KERNELthat's off by default. But if it's selected, all the "selects" done by the variousARCH_XXXare not done any more. Something like:
ARCH_XXX
selectXXX_CLK1if !ARM64_MINIMAL_GENERIC_KERNEL selectXXX_PINCTRL1if !ARM64_MINIMAL_GENERIC_KERNEL
Quoted 8 lines
ARCH_YYY
selectYYY_CLK1if !ARM64_MINIMAL_GENERIC_KERNEL selectYYY_PINCTRL1if !ARM64_MINIMAL_GENERIC_KERNELAnd
ARM64_MINIMAL_GENERIC_KERNELitself would select the absolutely mandatory stuff that can never be made into a module like the GIC, architectured timer (as Geert mentioned) and UART early console driver. I'm not sure if ARM32 has an equivalent to the standardized
While the UART early console can work (assuming the related hardware setup has been done by the boot loader), the actual serial driver usually cannot, as it relies on clocks, PM Domains, pin control, which won't be available until the corresponding modular drivers are loaded. Actually earlycon is a debug feature, so I'm wondering if you actually want that in your GKI kernel?
Quoted 6 lines
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoted 16 lines · Geert Uytterhoeven
Quoted 6 lines · Saravana Kannan
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Quoted 4 lines
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.
There are a number of well-known examples even with 64-bit chips or Cortex-A7/A15 based SoCs that can't use the architected timer, irqchip or iommu.
Apple M1, Broadcom BCM283x, Samsung Exynos5 and some Hisilicon server parts come to mind, I'm sure there are more.
Quoted 2 lines
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
There is some work in the more actively maintained MIPS platforms to make those behave more like Arm/powerpc/riscv/m68k platforms, using a single image and moving drivers into modules. Most MIPS platforms seem unlikely to get updated to this, and will continue to require a SoC specific kernel binary forever, similar to the renesas superh platforms. Most of the less common architectures (arc, csky, hexagon, nios2, xtensa, microblaze, nds32, openrisc, sparc/leon) are way behind that though, and generally don't work at all without out-of-tree code.
Arnd
Quoted 23 lines · Arnd Bergmann
Quoted 16 lines · Geert Uytterhoeven
Quoted 6 lines · Saravana Kannan
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Quoted 4 lines
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.
Quoted 7 lines
There are a number of well-known examples even with 64-bit chips or Cortex-A7/A15 based SoCs that can't use the architected timer, irqchip or iommu.
Apple M1, Broadcom BCM283x, Samsung Exynos5 and some Hisilicon server parts come to mind, I'm sure there are more.
There's also more and more movement towards having coprocessors with standardized interfaces dealing with this functionality. We're currently at the point where they have coprocessors with non-standardized interfaces, and it's useful to keep encouraging convergence in this area to everybody's benefit. I don't find it particularly useful to make life easier for the custom solutions at the expense of others like this patchset does, when that's (just beyond? on?) the horizon.
Quoted 13 lines
Quoted 2 lines
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
There is some work in the more actively maintained MIPS platforms to make those behave more like Arm/powerpc/riscv/m68k platforms, using a single image and moving drivers into modules. Most MIPS platforms seem unlikely to get updated to this, and will continue to require a SoC specific kernel binary forever, similar to the renesas superh platforms. Most of the less common architectures (arc, csky, hexagon, nios2, xtensa, microblaze, nds32, openrisc, sparc/leon) are way behind that though, and generally don't work at all without out-of-tree code.
One of the arguments for needing some of these core drivers in-kernel is that some platforms boot at very conservative DVFS operating points, to a degree that you really want to turn up the CPU clocks fairly early during boot.
If you don't have the drivers built-in, you can't do that and/or you create possible fragile or awkward inter-module dependencies with deferred probing, etc. We do care about boot time enough to prefer to just build them in for this reason.
If vmlinux binary size is a concern, maybe it's time to consider splitting the drivers into a bare-minimum piece that's not a module for early setup, and the rest that can be loaded post-boot.
-Olof
Quoted 61 lines · Olof Johansson
Quoted 23 lines · Arnd Bergmann
Quoted 16 lines · Geert Uytterhoeven
Quoted 6 lines · Saravana Kannan
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Quoted 4 lines
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.Quoted 7 lines
There are a number of well-known examples even with 64-bit chips or Cortex-A7/A15 based SoCs that can't use the architected timer, irqchip or iommu.
Apple M1, Broadcom BCM283x, Samsung Exynos5 and some Hisilicon server parts come to mind, I'm sure there are more.
There's also more and more movement towards having coprocessors with standardized interfaces dealing with this functionality. We're currently at the point where they have coprocessors with non-standardized interfaces, and it's useful to keep encouraging convergence in this area to everybody's benefit. I don't find it particularly useful to make life easier for the custom solutions at the expense of others like this patchset does, when that's (just beyond? on?) the horizon.
Quoted 13 lines
Quoted 2 lines
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
There is some work in the more actively maintained MIPS platforms to make those behave more like Arm/powerpc/riscv/m68k platforms, using a single image and moving drivers into modules. Most MIPS platforms seem unlikely to get updated to this, and will continue to require a SoC specific kernel binary forever, similar to the renesas superh platforms. Most of the less common architectures (arc, csky, hexagon, nios2, xtensa, microblaze, nds32, openrisc, sparc/leon) are way behind that though, and generally don't work at all without out-of-tree code.
One of the arguments for needing some of these core drivers in-kernel is that some platforms boot at very conservative DVFS operating points, to a degree that you really want to turn up the CPU clocks fairly early during boot.
Quoted 4 lines
If you don't have the drivers built-in, you can't do that and/or you create possible fragile or awkward inter-module dependencies with deferred probing, etc. We do care about boot time enough to prefer to just build them in for this reason.
Go look at a Pixel 5, we got this working just fine with all these drivers as modules and we definitely care about boot time. You just need to load your CPU freq driver and the other ones it needs early on. And with fw_devlink=on (default in upstream), there's hardly any deferred probing.
Quoted 3 lines
If vmlinux binary size is a concern, maybe it's time to consider splitting the drivers into a bare-minimum piece that's not a module for early setup, and the rest that can be loaded post-boot.
Isn't this literally what I was suggesting with my ARM64_MINIMAL_GENERIC_KERNEL suggestion? Build in all the bare minimum drivers that are needed before module loading can happen? You'd just select them all under that config. And any existing platform that wants to use it would break up their drivers into modules and switch to it.
-Saravana
2021年10月2日(土) 4:26 Saravana Kannan <[email protected]>:
Quoted 78 lines · Olof Johansson
Quoted 61 lines · Arnd Bergmann
Quoted 23 lines · Geert Uytterhoeven
Quoted 16 lines · Saravana Kannan
Quoted 6 lines
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Quoted 4 lines
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.Quoted 7 lines
There are a number of well-known examples even with 64-bit chips or Cortex-A7/A15 based SoCs that can't use the architected timer, irqchip or iommu.
Apple M1, Broadcom BCM283x, Samsung Exynos5 and some Hisilicon server parts come to mind, I'm sure there are more.
There's also more and more movement towards having coprocessors with standardized interfaces dealing with this functionality. We're currently at the point where they have coprocessors with non-standardized interfaces, and it's useful to keep encouraging convergence in this area to everybody's benefit. I don't find it particularly useful to make life easier for the custom solutions at the expense of others like this patchset does, when that's (just beyond? on?) the horizon.
Quoted 13 lines
Quoted 2 lines
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
There is some work in the more actively maintained MIPS platforms to make those behave more like Arm/powerpc/riscv/m68k platforms, using a single image and moving drivers into modules. Most MIPS platforms seem unlikely to get updated to this, and will continue to require a SoC specific kernel binary forever, similar to the renesas superh platforms. Most of the less common architectures (arc, csky, hexagon, nios2, xtensa, microblaze, nds32, openrisc, sparc/leon) are way behind that though, and generally don't work at all without out-of-tree code.
One of the arguments for needing some of these core drivers in-kernel is that some platforms boot at very conservative DVFS operating points, to a degree that you really want to turn up the CPU clocks fairly early during boot.
Quoted 4 lines
If you don't have the drivers built-in, you can't do that and/or you create possible fragile or awkward inter-module dependencies with deferred probing, etc. We do care about boot time enough to prefer to just build them in for this reason.
Go look at a Pixel 5, we got this working just fine with all these drivers as modules and we definitely care about boot time. You just need to load your CPU freq driver and the other ones it needs early on. And with fw_devlink=on (default in upstream), there's hardly any deferred probing.
Quoted 3 lines
If vmlinux binary size is a concern, maybe it's time to consider splitting the drivers into a bare-minimum piece that's not a module for early setup, and the rest that can be loaded post-boot.
Quoted 6 lines
Isn't this literally what I was suggesting with my
ARM64_MINIMAL_GENERIC_KERNELsuggestion? Build in all the bare minimum drivers that are needed before module loading can happen? You'd just select them all under that config. And any existing platform that wants to use it would break up their drivers into modules and switch to it.
I think the point here is that pinctrl and clk are considered a part of "bare minimum" for Exynos.
Quoted 68 lines · Saravana Kannan
Quoted 61 lines · Olof Johansson
Quoted 23 lines · Arnd Bergmann
Quoted 16 lines · Geert Uytterhoeven
Quoted 6 lines · Saravana Kannan
GIC and arch timer. Basically the minimal kernel would need a timer for the scheduler tick and IRQ controller to get the timer IRQ and the fixed clock driver if the archtimer uses one to get its frequency and the early UART console is pointless as a module (so build it in to allow debugging/development).
Quoted 8 lines
And then all new drivers, we should make sure are implemented as tristate drivers. And we can go back and slowly work on converting existing drivers to modules (community effort -- not one person or entity) -- at least the ones where the author has hardware or ones where the change is very likely to be correct and someone else is willing to test it. We'll never be able to support some/all ARM32 (do they even have a GIC/arch timer standard?), but at least for ARM64, this seems like a viable goal.
Quoted 4 lines
Cortex-A7/A15 and later have GIC and architectured timer, so it should work for contemporary systems.
Cortex-A9 systems may have GIC, and TWD and/or Global Timer (but I've seen SoCs where the interrupt for the latter was not wired :-(.Quoted 7 lines
There are a number of well-known examples even with 64-bit chips or Cortex-A7/A15 based SoCs that can't use the architected timer, irqchip or iommu.
Apple M1, Broadcom BCM283x, Samsung Exynos5 and some Hisilicon server parts come to mind, I'm sure there are more.
There's also more and more movement towards having coprocessors with standardized interfaces dealing with this functionality. We're currently at the point where they have coprocessors with non-standardized interfaces, and it's useful to keep encouraging convergence in this area to everybody's benefit. I don't find it particularly useful to make life easier for the custom solutions at the expense of others like this patchset does, when that's (just beyond? on?) the horizon.
Quoted 13 lines
Quoted 2 lines
What are the plans for other architectures? I've seen similar patches being applied for e.g. MIPS.
There is some work in the more actively maintained MIPS platforms to make those behave more like Arm/powerpc/riscv/m68k platforms, using a single image and moving drivers into modules. Most MIPS platforms seem unlikely to get updated to this, and will continue to require a SoC specific kernel binary forever, similar to the renesas superh platforms. Most of the less common architectures (arc, csky, hexagon, nios2, xtensa, microblaze, nds32, openrisc, sparc/leon) are way behind that though, and generally don't work at all without out-of-tree code.
One of the arguments for needing some of these core drivers in-kernel is that some platforms boot at very conservative DVFS operating points, to a degree that you really want to turn up the CPU clocks fairly early during boot.
Quoted 4 lines
If you don't have the drivers built-in, you can't do that and/or you create possible fragile or awkward inter-module dependencies with deferred probing, etc. We do care about boot time enough to prefer to just build them in for this reason.
Quoted 5 lines
Go look at a Pixel 5, we got this working just fine with all these drivers as modules and we definitely care about boot time. You just need to load your CPU freq driver and the other ones it needs early on. And with fw_devlink=on (default in upstream), there's hardly any deferred probing.
Unfortunately these problems are usually easier to fix on new platforms, especially during new product development. The hard part is making sure you haven't regressed any of the legacy platforms when you're changing the implementation for them as well.
Quoted 10 lines
Quoted 3 lines
If vmlinux binary size is a concern, maybe it's time to consider splitting the drivers into a bare-minimum piece that's not a module for early setup, and the rest that can be loaded post-boot.
Isn't this literally what I was suggesting with my
ARM64_MINIMAL_GENERIC_KERNELsuggestion? Build in all the bare minimum drivers that are needed before module loading can happen? You'd just select them all under that config. And any existing platform that wants to use it would break up their drivers into modules and switch to it.
Do you understand the implications of your proposal on the complexity for those who care about making sure different config combinations keep building and working, i.e. both the minimal-generic-kernel and the regular generic version? You've doubled the testing workload for all of those folks. It's a different mindset from when you mostly need to care about your one config and platform.
-Olof