RE: [RESEND PATCH v4 2/2] ARM: EXYNOS: Refactor the pm code to use DT based lookup
Bartlomiej Zolnierkiewicz wrote: > > Hi, > Hi, > On Tuesday, August 19, 2014 01:26:49 PM Vikas Sajjan wrote: > > Hi Kukjin, > > > > On Tue, Aug 19, 2014 at 12:22 PM, Vikas Sajjan > > wrote: > > > Refactoring the pm.c to avoid using "soc_is_exynos" checks, > > > instead use the DT based lookup. > > > > > > While at it, consolidate the common code across SoCs > > > and create static helper functions. > > > > > > Signed-off-by: Vikas Sajjan > > > Reviewed-by: Tomasz Figa > > > > Can you pick this series... > > I think that my PM_SLEEP=n build fixes should go in before this cleanup > series: > > http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg35430.html > Bart, your patch has a problem due to dependency, Firmware-assisted suspend/ resume patch from Tomasz as I emailed today morning in my time. So I will apply this series firstly. Please respin the fixes once the build breakage resolved. If you have any idea, please let me know. Thanks, Kukjin > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > > > > --- > > > arch/arm/mach-exynos/pm.c | 167 > > > +-- > > > arch/arm/mach-exynos/regs-pmu.h |1 + > > > 2 files changed, 126 insertions(+), 42 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 2/6] clk: samsung: add cpu clock configuration data and instantiate cpu clock
With the addition of the new Samsung specific cpu-clock type, the arm clock can be represented as a cpu-clock type. Add the CPU clock configuration data and instantiate the CPU clock type for Exynos4210, Exynos5250 and Exynos5420. Cc: Tomasz Figa Signed-off-by: Thomas Abraham Acked-by: Mike Turquette Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- drivers/clk/samsung/clk-exynos4.c | 15 ++ drivers/clk/samsung/clk-exynos5250.c | 25 + drivers/clk/samsung/clk-exynos5420.c | 45 include/dt-bindings/clock/exynos5250.h |1 + include/dt-bindings/clock/exynos5420.h |2 + 5 files changed, 88 insertions(+), 0 deletions(-) diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index 8617f49..101f549 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -19,6 +19,7 @@ #include #include "clk.h" +#include "clk-cpu.h" /* Exynos4 clock controller register offsets */ #define SRC_LEFTBUS0x4200 @@ -1355,6 +1356,16 @@ static struct samsung_pll_clock exynos4x12_plls[nr_plls] __initdata = { VPLL_LOCK, VPLL_CON0, NULL), }; +static const struct exynos_cpuclk_cfg_data e4210_armclk_d[] __initconst = { + { 120, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 5), }, + { 100, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 4), }, + { 80, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), }, + { 50, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), }, + { 40, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), }, + { 20, E4210_CPU_DIV0(0, 1, 1, 1, 3, 1), E4210_CPU_DIV1(0, 3), }, + { 0 }, +}; + static void __init exynos4_core_down_clock(enum exynos4_soc soc) { unsigned int tmp; @@ -1458,6 +1469,10 @@ static void __init exynos4_clk_init(struct device_node *np, samsung_clk_register_fixed_factor(ctx, exynos4210_fixed_factor_clks, ARRAY_SIZE(exynos4210_fixed_factor_clks)); + exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk", + mout_core_p4210[0], mout_core_p4210[1], 0x14200, + e4210_armclk_d, ARRAY_SIZE(e4210_armclk_d), + CLK_CPU_NEEDS_DEBUG_ALT_DIV | CLK_CPU_HAS_DIV1); } else { samsung_clk_register_mux(ctx, exynos4x12_mux_clks, ARRAY_SIZE(exynos4x12_mux_clks)); diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c index 70ec3d2..e19e365 100644 --- a/drivers/clk/samsung/clk-exynos5250.c +++ b/drivers/clk/samsung/clk-exynos5250.c @@ -19,6 +19,7 @@ #include #include "clk.h" +#include "clk-cpu.h" #define APLL_LOCK 0x0 #define APLL_CON0 0x100 @@ -748,6 +749,26 @@ static struct samsung_pll_clock exynos5250_plls[nr_plls] __initdata = { VPLL_LOCK, VPLL_CON0, NULL), }; +static const struct exynos_cpuclk_cfg_data exynos5250_armclk_d[] __initconst = { + { 170, E5250_CPU_DIV0(5, 3, 7, 7, 7, 3), E5250_CPU_DIV1(2, 0), }, + { 160, E5250_CPU_DIV0(4, 1, 7, 7, 7, 3), E5250_CPU_DIV1(2, 0), }, + { 150, E5250_CPU_DIV0(4, 1, 7, 7, 7, 2), E5250_CPU_DIV1(2, 0), }, + { 140, E5250_CPU_DIV0(4, 1, 6, 7, 7, 2), E5250_CPU_DIV1(2, 0), }, + { 130, E5250_CPU_DIV0(3, 1, 6, 7, 7, 2), E5250_CPU_DIV1(2, 0), }, + { 120, E5250_CPU_DIV0(3, 1, 5, 7, 7, 2), E5250_CPU_DIV1(2, 0), }, + { 110, E5250_CPU_DIV0(3, 1, 5, 7, 7, 3), E5250_CPU_DIV1(2, 0), }, + { 100, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 90, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 80, E5250_CPU_DIV0(2, 1, 4, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 70, E5250_CPU_DIV0(1, 1, 3, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 60, E5250_CPU_DIV0(1, 1, 3, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 50, E5250_CPU_DIV0(1, 1, 2, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 40, E5250_CPU_DIV0(1, 1, 2, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 30, E5250_CPU_DIV0(1, 1, 1, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 20, E5250_CPU_DIV0(1, 1, 1, 7, 7, 1), E5250_CPU_DIV1(2, 0), }, + { 0 }, +}; + static const struct of_device_id ext_clk_match[] __initconst = { { .compatible = "samsung,clock-xxti", .data = (void *)0, }, { }, @@ -797,6 +818,10 @@ static void __init exynos5250_clk_init(struct device_node *np) ARRAY_SIZE(exynos5250_div_clks)); samsung_clk_register_gate(ctx, exynos5250_gate_clks, ARRAY_SIZE(exynos5250_gate_clks)); + exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk", + mout_cpu_p[0], mout_cpu_p[1], 0x200, + exyno
[PATCH v10 4/6] ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420
The new CPU clock type allows the use of generic CPUfreq drivers. So for Exynos4210/5250, switch to using generic cpufreq driver. For Exynos5420, which did not have CPUfreq driver support, enable the use of generic CPUfreq driver. Suggested-by: Tomasz Figa Cc: Kukjin Kim Signed-off-by: Thomas Abraham Reviewed-by: Tomasz Figa Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- arch/arm/mach-exynos/exynos.c | 24 +++- 1 files changed, 23 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index b89e5f3..c3b6b5d 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -280,6 +280,28 @@ static void __init exynos_init_irq(void) exynos_map_pmu(); } +static const struct of_device_id exynos_cpufreq_matches[] = { + { .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" }, + { .compatible = "samsung,exynos5250", .data = "cpufreq-cpu0" }, + { .compatible = "samsung,exynos4210", .data = "cpufreq-cpu0" }, + { .compatible = "samsung,exynos5440", .data = "exynos5440-cpufreq" }, + { /* sentinel */ } +}; + +static void __init exynos_cpufreq_init(void) +{ + struct device_node *root = of_find_node_by_path("/"); + const struct of_device_id *match; + + match = of_match_node(exynos_cpufreq_matches, root); + if (!match) { + platform_device_register_simple("exynos-cpufreq", -1, NULL, 0); + return; + } + + platform_device_register_simple(match->data, -1, NULL, 0); +} + static void __init exynos_dt_machine_init(void) { struct device_node *i2c_np; @@ -319,7 +341,7 @@ static void __init exynos_dt_machine_init(void) of_machine_is_compatible("samsung,exynos5250")) platform_device_register(&exynos_cpuidle); - platform_device_register_simple("exynos-cpufreq", -1, NULL, 0); + exynos_cpufreq_init(); of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); } -- 1.6.6.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 3/6] ARM: dts: Exynos: add CPU OPP and regulator supply property
For Exynos 4210/5250/5420 based platforms, add CPU operating points and CPU regulator supply properties for migrating from Exynos specific cpufreq driver to using generic cpufreq drivers. Cc: Kukjin Kim Cc: Doug Anderson Cc: Javier Martinez Canillas Cc: Andreas Faerber Cc: Sachin Kamat Signed-off-by: Thomas Abraham Reviewed-by: Andreas Farber Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- arch/arm/boot/dts/exynos4210-origen.dts |4 ++ arch/arm/boot/dts/exynos4210-trats.dts |4 ++ arch/arm/boot/dts/exynos4210-universal_c210.dts |4 ++ arch/arm/boot/dts/exynos4210.dtsi | 14 - arch/arm/boot/dts/exynos5250-arndale.dts|4 ++ arch/arm/boot/dts/exynos5250-smdk5250.dts |4 ++ arch/arm/boot/dts/exynos5250-snow.dts |4 ++ arch/arm/boot/dts/exynos5250.dtsi | 25 ++- arch/arm/boot/dts/exynos5420.dtsi | 38 +++ 9 files changed, 99 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts index f767c42..887dded 100644 --- a/arch/arm/boot/dts/exynos4210-origen.dts +++ b/arch/arm/boot/dts/exynos4210-origen.dts @@ -334,3 +334,7 @@ }; }; }; + +&cpu0 { + cpu0-supply = <&buck1_reg>; +}; diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index f516da9..66119dd 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -446,3 +446,7 @@ }; }; }; + +&cpu0 { + cpu0-supply = <&varm_breg>; +}; diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts index d50eb3a..bf0a39c 100644 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts @@ -492,3 +492,7 @@ &mdma1 { reg = <0x1284 0x1000>; }; + +&cpu0 { + cpu0-supply = <&vdd_arm_reg>; +}; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index bcc9e63..69bac07 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -35,10 +35,22 @@ #address-cells = <1>; #size-cells = <0>; - cpu@900 { + cpu0: cpu@900 { device_type = "cpu"; compatible = "arm,cortex-a9"; reg = <0x900>; + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; + clock-latency = <16>; + + operating-points = < + 120 125 + 100 115 + 80 1075000 + 50 975000 + 40 975000 + 20 95 + >; }; cpu@901 { diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts index 3acd97e..da2b3e1 100644 --- a/arch/arm/boot/dts/exynos5250-arndale.dts +++ b/arch/arm/boot/dts/exynos5250-arndale.dts @@ -563,3 +563,7 @@ }; }; }; + +&cpu0 { + cpu0-supply = <&buck2_reg>; +}; diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts index 6a0f4c0..0eedb88 100644 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -406,3 +406,7 @@ }; }; }; + +&cpu0 { + cpu0-supply = <&buck2_reg>; +}; diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index e51fcef..f954e82 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -624,4 +624,8 @@ num-cs = <1>; }; +&cpu0 { + cpu0-supply = <&buck2_reg>; +}; + #include "cros-ec-keyboard.dtsi" diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index f21b9aa..d4b418e 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -58,11 +58,34 @@ #address-cells = <1>; #size-cells = <0>; - cpu@0 { + cpu0: cpu@0 { device_type = "cpu"; compatible = "arm,cortex-a15"; reg = <0>; clock-frequency = <17>; + + clocks = <&clock CLK_ARM_CLK>; + clock-names = "cpu"; + clock-latency = <14>; + + operating-points = < + 170 130 + 160 125 + 150 1225000 +
[PATCH v10 6/6] clk: samsung: remove unused clock aliases and update clock flags
With some of the Exynos SoCs switched over to use the generic CPUfreq drivers, the unused clock aliases can be removed. In addition to this, the individual clock blocks which are now encapsulated with the consolidate CPU clock type can now be marked with read-only flags. Cc: Tomasz Figa Signed-off-by: Thomas Abraham Acked-by: Viresh Kumar Acked-by: Mike Turquette Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- drivers/clk/samsung/clk-exynos4.c| 48 +- drivers/clk/samsung/clk-exynos5250.c | 19 - drivers/clk/samsung/clk-exynos5420.c | 27 -- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index 101f549..04619a1 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -578,7 +578,8 @@ static struct samsung_mux_clock exynos4210_mux_clks[] __initdata = { MUX(0, "mout_fimd1", group1_p4210, E4210_SRC_LCD1, 0, 4), MUX(0, "mout_mipi1", group1_p4210, E4210_SRC_LCD1, 12, 4), MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_CPU, 8, 1), - MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1), + MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1, 0, + CLK_MUX_READ_ONLY), MUX(0, "mout_hpm", mout_core_p4210, SRC_CPU, 20, 1), MUX(CLK_SCLK_VPLL, "sclk_vpll", sclk_vpll_p4210, SRC_TOP0, 8, 1), MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4210, SRC_CAM, 0, 4), @@ -714,15 +715,24 @@ static struct samsung_div_clock exynos4_div_clks[] __initdata = { DIV(0, "div_clkout_rightbus", "mout_clkout_rightbus", CLKOUT_CMU_RIGHTBUS, 8, 6), - DIV(0, "div_core", "mout_core", DIV_CPU0, 0, 3), - DIV(0, "div_corem0", "div_core2", DIV_CPU0, 4, 3), - DIV(0, "div_corem1", "div_core2", DIV_CPU0, 8, 3), - DIV(0, "div_periph", "div_core2", DIV_CPU0, 12, 3), - DIV(0, "div_atb", "mout_core", DIV_CPU0, 16, 3), - DIV(0, "div_pclk_dbg", "div_atb", DIV_CPU0, 20, 3), - DIV(CLK_ARM_CLK, "div_core2", "div_core", DIV_CPU0, 28, 3), - DIV(0, "div_copy", "mout_hpm", DIV_CPU1, 0, 3), - DIV(0, "div_hpm", "div_copy", DIV_CPU1, 4, 3), + DIV_F(0, "div_core", "mout_core", DIV_CPU0, 0, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_corem0", "div_core2", DIV_CPU0, 4, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_corem1", "div_core2", DIV_CPU0, 8, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_periph", "div_core2", DIV_CPU0, 12, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_atb", "mout_core", DIV_CPU0, 16, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_pclk_dbg", "div_atb", DIV_CPU0, 20, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(CLK_ARM_CLK, "div_core2", "div_core", DIV_CPU0, 28, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_copy", "mout_hpm", DIV_CPU1, 0, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), + DIV_F(0, "div_hpm", "div_copy", DIV_CPU1, 4, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), DIV(0, "div_clkout_cpu", "mout_clkout_cpu", CLKOUT_CMU_CPU, 8, 6), DIV(0, "div_fimc0", "mout_fimc0", DIV_CAM, 0, 4), @@ -770,7 +780,8 @@ static struct samsung_div_clock exynos4_div_clks[] __initdata = { DIV(0, "div_spi_pre2", "div_spi2", DIV_PERIL2, 8, 8), DIV(0, "div_audio1", "mout_audio1", DIV_PERIL4, 0, 4), DIV(0, "div_audio2", "mout_audio2", DIV_PERIL4, 16, 4), - DIV(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3), + DIV_F(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3, + CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY), DIV_F(0, "div_mipi_pre0", "div_mipi0", DIV_LCD0, 20, 4, CLK_SET_RATE_PARENT, 0), DIV_F(0, "div_mmc_pre0", "div_mmc0", DIV_FSYS1, 8, 8, @@ -1187,17 +1198,10 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = { 0), }; -static struct samsung_clock_alias exynos4_aliases[] __initdata = { +static struct samsung_clock_alias exynos4x12_aliases[] __initdata = { ALIAS(CLK_MOUT_CORE, NULL, "moutcore"), ALIAS(CLK_ARM_CLK, NULL, "armclk"), ALIAS(CLK_SCLK_APLL, NULL, "mout_apll"), -}; - -static struct samsung_clock_alias exynos4210_aliases[] __initdata = { - ALIAS(CLK_SCLK_MPLL, NULL, "mout_mpll"), -}; - -static struct samsung_clock_alias exynos4x12_aliases[] __initdata = { ALIAS(CLK_MOUT_MPLL_USER_C, NULL, "mout_mpll"), }; @@ -1464,8 +1468,6 @@ static void __init e
[PATCH v10 5/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support
Exynos4210 and Exynos5250 based platforms have switched over to use generic cpufreq drivers for cpufreq functionality. So the Exynos specific cpufreq drivers for these platforms can be removed. Cc: Bartlomiej Zolnierkiewicz Signed-off-by: Thomas Abraham Acked-by: Viresh Kumar Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- drivers/cpufreq/Kconfig.arm | 22 -- drivers/cpufreq/Makefile|2 -- 2 files changed, 0 insertions(+), 24 deletions(-) diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 193a137..78df4e6 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -28,17 +28,6 @@ config ARM_VEXPRESS_SPC_CPUFREQ config ARM_EXYNOS_CPUFREQ bool -config ARM_EXYNOS4210_CPUFREQ - bool "SAMSUNG EXYNOS4210" - depends on CPU_EXYNOS4210 - default y - select ARM_EXYNOS_CPUFREQ - help - This adds the CPUFreq driver for Samsung EXYNOS4210 - SoC (S5PV310 or S5PC210). - - If in doubt, say N. - config ARM_EXYNOS4X12_CPUFREQ bool "SAMSUNG EXYNOS4x12" depends on SOC_EXYNOS4212 || SOC_EXYNOS4412 @@ -50,17 +39,6 @@ config ARM_EXYNOS4X12_CPUFREQ If in doubt, say N. -config ARM_EXYNOS5250_CPUFREQ - bool "SAMSUNG EXYNOS5250" - depends on SOC_EXYNOS5250 - default y - select ARM_EXYNOS_CPUFREQ - help - This adds the CPUFreq driver for Samsung EXYNOS5250 - SoC. - - If in doubt, say N. - config ARM_EXYNOS5440_CPUFREQ bool "SAMSUNG EXYNOS5440" depends on SOC_EXYNOS5440 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 40c53dc..74e55f9 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -52,9 +52,7 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o -obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o -obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ) += exynos5440-cpufreq.o obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)+= imx6q-cpufreq.o -- 1.6.6.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 0/6] cpufreq: use generic cpufreq drivers for exynos platforms
This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq drivers and enables the use of cpufreq-cpu0 driver for these platforms. This series also enables cpufreq support for Exynos5420 using arm_big_little cpufreq driver. This patch series is dependent on two other patches 1. ARM: dts: add CPU nodes for Exynos4 SoCs - https://lkml.org/lkml/2014/7/21/315 2. clk: exynos4: remove duplicate div_core2 divider clock instantiation - http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg34859.html Thomas Abraham (6): clk: samsung: add infrastructure to register cpu clocks clk: samsung: add cpu clock configuration data and instantiate cpu clock ARM: dts: Exynos: add CPU OPP and regulator supply property ARM: Exynos: switch to using generic cpufreq driver for Exynos4210/5250/5420 cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support clk: samsung: remove unused clock aliases and update clock flags arch/arm/boot/dts/exynos4210-origen.dts |4 + arch/arm/boot/dts/exynos4210-trats.dts |4 + arch/arm/boot/dts/exynos4210-universal_c210.dts |4 + arch/arm/boot/dts/exynos4210.dtsi | 14 +- arch/arm/boot/dts/exynos5250-arndale.dts|4 + arch/arm/boot/dts/exynos5250-smdk5250.dts |4 + arch/arm/boot/dts/exynos5250-snow.dts |4 + arch/arm/boot/dts/exynos5250.dtsi | 25 ++- arch/arm/boot/dts/exynos5420.dtsi | 38 +++ arch/arm/mach-exynos/exynos.c | 24 ++- drivers/clk/samsung/Makefile|2 +- drivers/clk/samsung/clk-cpu.c | 335 +++ drivers/clk/samsung/clk-cpu.h | 91 ++ drivers/clk/samsung/clk-exynos4.c | 63 +++-- drivers/clk/samsung/clk-exynos5250.c| 44 +++- drivers/clk/samsung/clk-exynos5420.c| 72 +- drivers/cpufreq/Kconfig.arm | 22 -- drivers/cpufreq/Makefile|2 - include/dt-bindings/clock/exynos5250.h |1 + include/dt-bindings/clock/exynos5420.h |2 + -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 1/6] clk: samsung: add infrastructure to register cpu clocks
The CPU clock provider supplies the clock to the CPU clock domain. The composition and organization of the CPU clock provider could vary among Exynos SoCs. A CPU clock provider can be composed of clock mux, dividers and gates. This patch defines a new clock type for CPU clock provider and adds infrastructure to register the CPU clock providers for Samsung platforms. Signed-off-by: Thomas Abraham Reviewed-by: Tomasz Figa Acked-by: Mike Turquette Tested-by: Javier Martinez Canillas Tested-by: Chander Kashyap --- drivers/clk/samsung/Makefile |2 +- drivers/clk/samsung/clk-cpu.c | 335 + drivers/clk/samsung/clk-cpu.h | 91 +++ 3 files changed, 427 insertions(+), 1 deletions(-) create mode 100644 drivers/clk/samsung/clk-cpu.c create mode 100644 drivers/clk/samsung/clk-cpu.h diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 6fb4bc6..8909c93 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile @@ -2,7 +2,7 @@ # Samsung Clock specific Makefile # -obj-$(CONFIG_COMMON_CLK) += clk.o clk-pll.o +obj-$(CONFIG_COMMON_CLK) += clk.o clk-pll.o clk-cpu.o obj-$(CONFIG_SOC_EXYNOS3250) += clk-exynos3250.o obj-$(CONFIG_ARCH_EXYNOS4) += clk-exynos4.o obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c new file mode 100644 index 000..009a21b --- /dev/null +++ b/drivers/clk/samsung/clk-cpu.c @@ -0,0 +1,335 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * Author: Thomas Abraham + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This file contains the utility function to register CPU clock for Samsung + * Exynos platforms. A CPU clock is defined as a clock supplied to a CPU or a + * group of CPUs. The CPU clock is typically derived from a hierarchy of clock + * blocks which includes mux and divider blocks. There are a number of other + * auxiliary clocks supplied to the CPU domain such as the debug blocks and AXI + * clock for CPU domain. The rates of these auxiliary clocks are related to the + * CPU clock rate and this relation is usually specified in the hardware manual + * of the SoC or supplied after the SoC characterization. + * + * The below implementation of the CPU clock allows the rate changes of the CPU + * clock and the corresponding rate changes of the auxillary clocks of the CPU + * domain. The platform clock driver provides a clock register configuration + * for each configurable rate which is then used to program the clock hardware + * registers to acheive a fast co-oridinated rate change for all the CPU domain + * clocks. + * + * On a rate change request for the CPU clock, the rate change is propagated + * upto the PLL supplying the clock to the CPU domain clock blocks. While the + * CPU domain PLL is reconfigured, the CPU domain clocks are driven using an + * alternate clock source. If required, the alternate clock source is divided + * down in order to keep the output clock rate within the previous OPP limits. +*/ + +#include +#include "clk-cpu.h" + +#define E4210_SRC_CPU 0x0 +#define E4210_STAT_CPU 0x200 +#define E4210_DIV_CPU0 0x300 +#define E4210_DIV_CPU1 0x304 +#define E4210_DIV_STAT_CPU00x400 +#define E4210_DIV_STAT_CPU10x404 + +#define E4210_DIV0_RATIO0_MASK 0x7 +#define E4210_DIV1_HPM_MASK(0x7 << 4) +#define E4210_DIV1_COPY_MASK (0x7 << 0) +#define E4210_MUX_HPM_MASK (1 << 20) +#define E4210_DIV0_ATB_SHIFT 16 +#define E4210_DIV0_ATB_MASK(DIV_MASK << E4210_DIV0_ATB_SHIFT) + +#define MAX_DIV8 +#define DIV_MASK 7 +#define DIV_MASK_ALL 0x +#define MUX_MASK 7 + +/* + * Helper function to wait until divider(s) have stabilized after the divider + * value has changed. + */ +static void wait_until_divider_stable(void __iomem *div_reg, unsigned long mask) +{ + unsigned long timeout = jiffies + msecs_to_jiffies(10); + + do { + if (!(readl(div_reg) & mask)) + return; + } while (time_before(jiffies, timeout)); + + pr_err("%s: timeout in divider stablization\n", __func__); +} + +/* + * Helper function to wait until mux has stabilized after the mux selection + * value was changed. + */ +static void wait_until_mux_stable(void __iomem *mux_reg, u32 mux_pos, + unsigned long mux_value) +{ + unsigned long timeout = jiffies + msecs_to_jiffies(10); + + do { + if (((readl(mux_reg) >> mux_pos) & MUX_MASK) == mux_value) + return; + } while (time_before(jiffies, timeout)); + + pr_err("%s: re-parenting mux timed-out\n", __func__); +} + +/* common round rate callback useabl
[PATCH] drm/exynos: fimd: fix screen shaking issue on i80 mode
This patch resolves a issue that screen is shaked after resumed. The issue could be incurred when overlay registers are updated to new buffer while fimd is still transmitting video data. So this patch make sure to wait for the completion of the transmission if fimd is transmitting video data before updating overlay registers. Signed-off-by: Inki Dae --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 2f896df..8a45a70 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -665,6 +665,25 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos) } /* +* Wait for the completion of current transmission if fimd is still +* transmitting video data. +* +* Below codes resolve following issue: +* when resumed, fimd_win_commit() could be called to update overlay +* relevent registers and then pae flip could be performed by userspace +* request. The problem is that te interrupt could occur and +* fimd_trigger() could be called before fimd_win_commit is called by +* page flip function, exynos_drm_crtc_page_flip. +* +* In this case, there is a problem that fimd_win_commit() is called by +* the page flip request while fimd is still transmitting video data +* so overlay registers are updated to new buffer. That makes screen +* to be shaked. +*/ + if (ctx->i80_if && atomic_read(&ctx->triggering)) + fimd_wait_for_vblank(mgr); + + /* * SHADOWCON/PRTCON register is used for enabling timing. * * for example, once only width value of a register is set, -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] pinctrl: samsung: Separate per-bank init and runtime data
Currently the driver mixes constant init data with runtime data, which is far from being elegant and can invite potential hard to track issues. This patch intends to solve this by introducing a new samsung_pin_bank_data structure to hold only constant data known at compile time, which can be copied to main samsung_pin_bank struct used at runtime. In addition, thanks to this change, all per-bank initdata can be marked with const and __initconst keywords and dropped after init completes. Signed-off-by: Tomasz Figa --- drivers/pinctrl/samsung/pinctrl-exynos.c | 44 +++ drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 8 +++--- drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 2 +- drivers/pinctrl/samsung/pinctrl-samsung.c | 18 +++-- drivers/pinctrl/samsung/pinctrl-samsung.h | 33 --- 5 files changed, 72 insertions(+), 33 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 552f7c75243d..b4490cb2a439 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -633,7 +633,7 @@ static void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) } /* pin banks of s5pv210 pin-controller */ -static struct samsung_pin_bank s5pv210_pin_bank[] = { +static const struct samsung_pin_bank_data s5pv210_pin_bank[] __initconst = { EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpa0", 0x00), EXYNOS_PIN_BANK_EINTG(4, 0x020, "gpa1", 0x04), EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpb", 0x08), @@ -683,7 +683,7 @@ const struct samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { }; /* pin banks of exynos3250 pin-controller 0 */ -static struct samsung_pin_bank exynos3250_pin_banks0[] = { +static const struct samsung_pin_bank_data exynos3250_pin_banks0[] __initconst = { EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpa0", 0x00), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpb", 0x08), @@ -694,7 +694,7 @@ static struct samsung_pin_bank exynos3250_pin_banks0[] = { }; /* pin banks of exynos3250 pin-controller 1 */ -static struct samsung_pin_bank exynos3250_pin_banks1[] = { +static const struct samsung_pin_bank_data exynos3250_pin_banks1[] __initconst = { EXYNOS_PIN_BANK_EINTN(8, 0x120, "gpe0"), EXYNOS_PIN_BANK_EINTN(8, 0x140, "gpe1"), EXYNOS_PIN_BANK_EINTN(3, 0x180, "gpe2"), @@ -737,7 +737,7 @@ const struct samsung_pin_ctrl exynos3250_pin_ctrl[] __initconst = { }; /* pin banks of exynos4210 pin-controller 0 */ -static struct samsung_pin_bank exynos4210_pin_banks0[] = { +static const struct samsung_pin_bank_data exynos4210_pin_banks0[] __initconst = { EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpa0", 0x00), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpb", 0x08), @@ -757,7 +757,7 @@ static struct samsung_pin_bank exynos4210_pin_banks0[] = { }; /* pin banks of exynos4210 pin-controller 1 */ -static struct samsung_pin_bank exynos4210_pin_banks1[] = { +static const struct samsung_pin_bank_data exynos4210_pin_banks1[] __initconst = { EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpj0", 0x00), EXYNOS_PIN_BANK_EINTG(5, 0x020, "gpj1", 0x04), EXYNOS_PIN_BANK_EINTG(7, 0x040, "gpk0", 0x08), @@ -781,7 +781,7 @@ static struct samsung_pin_bank exynos4210_pin_banks1[] = { }; /* pin banks of exynos4210 pin-controller 2 */ -static struct samsung_pin_bank exynos4210_pin_banks2[] = { +static const struct samsung_pin_bank_data exynos4210_pin_banks2[] __initconst = { EXYNOS_PIN_BANK_EINTN(7, 0x000, "gpz"), }; @@ -813,7 +813,7 @@ const struct samsung_pin_ctrl exynos4210_pin_ctrl[] __initconst = { }; /* pin banks of exynos4x12 pin-controller 0 */ -static struct samsung_pin_bank exynos4x12_pin_banks0[] = { +static const struct samsung_pin_bank_data exynos4x12_pin_banks0[] __initconst = { EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpa0", 0x00), EXYNOS_PIN_BANK_EINTG(6, 0x020, "gpa1", 0x04), EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpb", 0x08), @@ -830,7 +830,7 @@ static struct samsung_pin_bank exynos4x12_pin_banks0[] = { }; /* pin banks of exynos4x12 pin-controller 1 */ -static struct samsung_pin_bank exynos4x12_pin_banks1[] = { +static const struct samsung_pin_bank_data exynos4x12_pin_banks1[] __initconst = { EXYNOS_PIN_BANK_EINTG(7, 0x040, "gpk0", 0x08), EXYNOS_PIN_BANK_EINTG(7, 0x060, "gpk1", 0x0c), EXYNOS_PIN_BANK_EINTG(7, 0x080, "gpk2", 0x10), @@ -857,12 +857,12 @@ static struct samsung_pin_bank exynos4x12_pin_banks1[] = { }; /* pin banks of exynos4x12 pin-controller 2 */ -static struct samsung_pin_bank exynos4x12_pin_banks2[] = { +static const struct samsung_pin_bank_data exynos4x12_pin_banks2[] __initconst = { EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpz", 0x00), }; /* pin banks of exynos4x12 pin-controller 3 */ -static struct samsung_pin_bank exynos4x12_pin_ban
[PATCH 4/5] pinctrl: samsung: Constify samsung_pin_ctrl struct
In order to separate initialization constants from runtime data, this patch modifies the driver to store only constant data in samsung_pin_ctrl struct and copy data required at runtime to samsung_pinctrl_drv_data struct. This makes it possible to mark all existing instances of samsung_pin_ctrl struct as const and __initconst. Signed-off-by: Tomasz Figa --- drivers/pinctrl/samsung/pinctrl-exynos.c | 39 +++--- drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 12 ++--- drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 14 ++--- drivers/pinctrl/samsung/pinctrl-samsung.c | 86 +++ drivers/pinctrl/samsung/pinctrl-samsung.h | 40 +++--- 5 files changed, 96 insertions(+), 95 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 0202e0016233..552f7c75243d 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -277,8 +277,7 @@ static const struct irq_domain_ops exynos_gpio_irqd_ops = { static irqreturn_t exynos_eint_gpio_irq(int irq, void *data) { struct samsung_pinctrl_drv_data *d = data; - struct samsung_pin_ctrl *ctrl = d->ctrl; - struct samsung_pin_bank *bank = ctrl->pin_banks; + struct samsung_pin_bank *bank = d->pin_banks; unsigned int svc, group, pin, virq; svc = readl(d->virt_base + EXYNOS_SVC_OFFSET); @@ -325,8 +324,8 @@ static int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) return -ENXIO; } - bank = d->ctrl->pin_banks; - for (i = 0; i < d->ctrl->nr_banks; ++i, ++bank) { + bank = d->pin_banks; + for (i = 0; i < d->nr_banks; ++i, ++bank) { if (bank->eint_type != EINT_TYPE_GPIO) continue; bank->irq_domain = irq_domain_add_linear(bank->of_node, @@ -498,8 +497,8 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) if (!wkup_np) return -ENODEV; - bank = d->ctrl->pin_banks; - for (i = 0; i < d->ctrl->nr_banks; ++i, ++bank) { + bank = d->pin_banks; + for (i = 0; i < d->nr_banks; ++i, ++bank) { if (bank->eint_type != EINT_TYPE_WKUP) continue; @@ -556,9 +555,9 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) irq_set_chained_handler(irq, exynos_irq_demux_eint16_31); irq_set_handler_data(irq, muxed_data); - bank = d->ctrl->pin_banks; + bank = d->pin_banks; idx = 0; - for (i = 0; i < d->ctrl->nr_banks; ++i, ++bank) { + for (i = 0; i < d->nr_banks; ++i, ++bank) { if (bank->eint_type != EINT_TYPE_WKUP_MUX) continue; @@ -590,11 +589,10 @@ static void exynos_pinctrl_suspend_bank( static void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) { - struct samsung_pin_ctrl *ctrl = drvdata->ctrl; - struct samsung_pin_bank *bank = ctrl->pin_banks; + struct samsung_pin_bank *bank = drvdata->pin_banks; int i; - for (i = 0; i < ctrl->nr_banks; ++i, ++bank) + for (i = 0; i < drvdata->nr_banks; ++i, ++bank) if (bank->eint_type == EINT_TYPE_GPIO) exynos_pinctrl_suspend_bank(drvdata, bank); } @@ -626,11 +624,10 @@ static void exynos_pinctrl_resume_bank( static void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) { - struct samsung_pin_ctrl *ctrl = drvdata->ctrl; - struct samsung_pin_bank *bank = ctrl->pin_banks; + struct samsung_pin_bank *bank = drvdata->pin_banks; int i; - for (i = 0; i < ctrl->nr_banks; ++i, ++bank) + for (i = 0; i < drvdata->nr_banks; ++i, ++bank) if (bank->eint_type == EINT_TYPE_GPIO) exynos_pinctrl_resume_bank(drvdata, bank); } @@ -673,7 +670,7 @@ static struct samsung_pin_bank s5pv210_pin_bank[] = { EXYNOS_PIN_BANK_EINTW(8, 0xc60, "gph3", 0x0c), }; -struct samsung_pin_ctrl s5pv210_pin_ctrl[] = { +const struct samsung_pin_ctrl s5pv210_pin_ctrl[] __initconst = { { /* pin-controller instance 0 data */ .pin_banks = s5pv210_pin_bank, @@ -720,7 +717,7 @@ static struct samsung_pin_bank exynos3250_pin_banks1[] = { * Samsung pinctrl driver data for Exynos3250 SoC. Exynos3250 SoC includes * two gpio/pin-mux/pinconfig controllers. */ -struct samsung_pin_ctrl exynos3250_pin_ctrl[] = { +const struct samsung_pin_ctrl exynos3250_pin_ctrl[] __initconst = { { /* pin-controller instance 0 data */ .pin_banks = exynos3250_pin_banks0, @@ -792,7 +789,7 @@ static struct samsung_pin_bank exynos4210_pin_banks2[] = { * Samsung pinctrl driver data for Exynos4210 SoC. Exynos4210 SoC includes * three gpio/pin-mux/pinconfig controllers. */ -struct samsung_pin_ctrl exynos4210_pin_ctrl[] = { +const str
[PATCH 1/5] pinctrl: samsung: Make samsung_pinctrl_get_soc_data use ERR_PTR()
Currently the function returns a valid pointer on success and NULL on error, so exact error code is lost. This patch changes return convention of the function to use ERR_PTR() on error instead. Signed-off-by: Tomasz Figa --- drivers/pinctrl/samsung/pinctrl-samsung.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 4a47691c32b1..0504f0b75de8 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -988,7 +988,7 @@ static struct samsung_pin_ctrl *samsung_pinctrl_get_soc_data( id = of_alias_get_id(node, "pinctrl"); if (id < 0) { dev_err(&pdev->dev, "failed to get alias id\n"); - return NULL; + return ERR_PTR(-ENOENT); } match = of_match_node(samsung_pinctrl_dt_match, node); ctrl = (struct samsung_pin_ctrl *)match->data + id; @@ -1040,9 +1040,9 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) } ctrl = samsung_pinctrl_get_soc_data(drvdata, pdev); - if (!ctrl) { + if (IS_ERR(ctrl)) { dev_err(&pdev->dev, "driver data not available\n"); - return -EINVAL; + return PTR_ERR(ctrl); } drvdata->ctrl = ctrl; drvdata->dev = dev; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] pinctrl: samsung: Data structure clean-up
This series intends to clean up data structures used by pinctrl-samsung driver. More specifically, it separates initial compile time constants from data used at runtime, allowing unused variant data to be dropped and selected structures constified to improve safety. As a side effect, size of vmlinux built from multi_v7_defconfig was reduced from: text databssdec hexfilename 10296708 1227100 313544 11837352 b49fa8 vmlinux to: text databssdec hexfilename 10296740 1176860 313544 11787144 b3db88 vmlinux and quite a bit of data were moved from normal data sections to .init.data: pre: Idx Name Size VMA LMA File off Algn 3 .rodata 0026c080 c0881000 c0881000 00681000 2**6 23 .init.data0003ff7c c0bdb830 c0bdb830 009e3830 2**3 24 .data..percpu 2100 c0c1c000 c0c1c000 00a24000 2**6 25 .data 000e98e0 c0c2 c0c2 00a28000 2**6 post: Idx Name Size VMA LMA File off Algn 3 .rodata 0026bf20 c0881000 c0881000 00681000 2**6 23 .init.data00041bbc c0bdb830 c0bdb830 009e3830 2**3 24 .data..percpu 2100 c0c1e000 c0c1e000 00a26000 2**6 25 .data 000db860 c0c22000 c0c22000 00a2a000 2**6 This series should not introduce any functional changes. Tested on S3C6410-based Mini6410 board, booting with device tree. Marek, Bart, could you do some testing on Exynos-based boards, just to make sure? Tomasz Figa (5): pinctrl: samsung: Make samsung_pinctrl_get_soc_data use ERR_PTR() pinctrl: samsung: Drop unused label field in samsung_pin_ctrl struct pinctrl: samsung: Constify samsung_pin_bank_type struct pinctrl: samsung: Constify samsung_pin_ctrl struct pinctrl: samsung: Separate per-bank init and runtime data drivers/pinctrl/samsung/pinctrl-exynos.c | 113 +++ drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 30 +++ drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 31 drivers/pinctrl/samsung/pinctrl-samsung.c | 126 -- drivers/pinctrl/samsung/pinctrl-samsung.h | 78 -- 5 files changed, 193 insertions(+), 185 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] pinctrl: samsung: Constify samsung_pin_bank_type struct
This structure is not intended to be modified at runtime and functions as constant data shared between multiple pin banks. This patch makes all instances of it constant across the driver. Signed-off-by: Tomasz Figa --- drivers/pinctrl/samsung/pinctrl-exynos.c | 8 drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 6 +++--- drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 14 +++--- drivers/pinctrl/samsung/pinctrl-samsung.c | 20 +--- drivers/pinctrl/samsung/pinctrl-samsung.h | 2 +- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 7e6463f970ff..0202e0016233 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -46,12 +46,12 @@ static inline struct exynos_irq_chip *to_exynos_irq_chip(struct irq_chip *chip) return container_of(chip, struct exynos_irq_chip, chip); } -static struct samsung_pin_bank_type bank_type_off = { +static const struct samsung_pin_bank_type bank_type_off = { .fld_width = { 4, 1, 2, 2, 2, 2, }, .reg_offset = { 0x00, 0x04, 0x08, 0x0c, 0x10, 0x14, }, }; -static struct samsung_pin_bank_type bank_type_alive = { +static const struct samsung_pin_bank_type bank_type_alive = { .fld_width = { 4, 1, 2, 2, }, .reg_offset = { 0x00, 0x04, 0x08, 0x0c, }, }; @@ -171,7 +171,7 @@ static int exynos_irq_request_resources(struct irq_data *irqd) struct irq_chip *chip = irq_data_get_irq_chip(irqd); struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip); struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); - struct samsung_pin_bank_type *bank_type = bank->type; + const struct samsung_pin_bank_type *bank_type = bank->type; struct samsung_pinctrl_drv_data *d = bank->drvdata; unsigned int shift = EXYNOS_EINT_CON_LEN * irqd->hwirq; unsigned long reg_con = our_chip->eint_con + bank->eint_offset; @@ -210,7 +210,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd) struct irq_chip *chip = irq_data_get_irq_chip(irqd); struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip); struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); - struct samsung_pin_bank_type *bank_type = bank->type; + const struct samsung_pin_bank_type *bank_type = bank->type; struct samsung_pinctrl_drv_data *d = bank->drvdata; unsigned int shift = EXYNOS_EINT_CON_LEN * irqd->hwirq; unsigned long reg_con = our_chip->eint_con + bank->eint_offset; diff --git a/drivers/pinctrl/samsung/pinctrl-s3c24xx.c b/drivers/pinctrl/samsung/pinctrl-s3c24xx.c index e38925906bd3..9db6cf5c8823 100644 --- a/drivers/pinctrl/samsung/pinctrl-s3c24xx.c +++ b/drivers/pinctrl/samsung/pinctrl-s3c24xx.c @@ -44,12 +44,12 @@ #define EINT_EDGE_BOTH 6 #define EINT_MASK 0xf -static struct samsung_pin_bank_type bank_type_1bit = { +static const struct samsung_pin_bank_type bank_type_1bit = { .fld_width = { 1, 1, }, .reg_offset = { 0x00, 0x04, }, }; -static struct samsung_pin_bank_type bank_type_2bit = { +static const struct samsung_pin_bank_type bank_type_2bit = { .fld_width = { 2, 1, 2, }, .reg_offset = { 0x00, 0x04, 0x08, }, }; @@ -143,7 +143,7 @@ static void s3c24xx_eint_set_handler(unsigned int irq, unsigned int type) static void s3c24xx_eint_set_function(struct samsung_pinctrl_drv_data *d, struct samsung_pin_bank *bank, int pin) { - struct samsung_pin_bank_type *bank_type = bank->type; + const struct samsung_pin_bank_type *bank_type = bank->type; unsigned long flags; void __iomem *reg; u8 shift; diff --git a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c index fcf8c36e727e..2a14db2826d8 100644 --- a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c +++ b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c @@ -68,32 +68,32 @@ #define EINT_CON_MASK 0xF #define EINT_CON_LEN 4 -static struct samsung_pin_bank_type bank_type_4bit_off = { +static const struct samsung_pin_bank_type bank_type_4bit_off = { .fld_width = { 4, 1, 2, 0, 2, 2, }, .reg_offset = { 0x00, 0x04, 0x08, 0, 0x0c, 0x10, }, }; -static struct samsung_pin_bank_type bank_type_4bit_alive = { +static const struct samsung_pin_bank_type bank_type_4bit_alive = { .fld_width = { 4, 1, 2, }, .reg_offset = { 0x00, 0x04, 0x08, }, }; -static struct samsung_pin_bank_type bank_type_4bit2_off = { +static const struct samsung_pin_bank_type bank_type_4bit2_off = { .fld_width = { 4, 1, 2, 0, 2, 2, }, .reg_offset = { 0x00, 0x08, 0x0c, 0, 0x10, 0x14, }, }; -static struct samsung_pin_bank_type bank_type_4bit2_alive = { +static const struct samsung_pin_bank_type bank_type_4bit2_alive = { .fld_width = { 4
[PATCH 2/5] pinctrl: samsung: Drop unused label field in samsung_pin_ctrl struct
There is no code using it and in fact there are pin controller variants that do not even have this field initialized in their init data. This patch removes it completely. Signed-off-by: Tomasz Figa --- drivers/pinctrl/samsung/pinctrl-exynos.c | 22 -- drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 4 drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 1 - drivers/pinctrl/samsung/pinctrl-samsung.h | 3 --- 4 files changed, 30 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index d7154ed0b0eb..7e6463f970ff 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -682,7 +682,6 @@ struct samsung_pin_ctrl s5pv210_pin_ctrl[] = { .eint_wkup_init = exynos_eint_wkup_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "s5pv210-gpio-ctrl0", }, }; @@ -729,7 +728,6 @@ struct samsung_pin_ctrl exynos3250_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos3250-gpio-ctrl0", }, { /* pin-controller instance 1 data */ .pin_banks = exynos3250_pin_banks1, @@ -738,7 +736,6 @@ struct samsung_pin_ctrl exynos3250_pin_ctrl[] = { .eint_wkup_init = exynos_eint_wkup_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos3250-gpio-ctrl1", }, }; @@ -803,7 +800,6 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4210-gpio-ctrl0", }, { /* pin-controller instance 1 data */ .pin_banks = exynos4210_pin_banks1, @@ -812,12 +808,10 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = { .eint_wkup_init = exynos_eint_wkup_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4210-gpio-ctrl1", }, { /* pin-controller instance 2 data */ .pin_banks = exynos4210_pin_banks2, .nr_banks = ARRAY_SIZE(exynos4210_pin_banks2), - .label = "exynos4210-gpio-ctrl2", }, }; @@ -891,7 +885,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4x12-gpio-ctrl0", }, { /* pin-controller instance 1 data */ .pin_banks = exynos4x12_pin_banks1, @@ -900,7 +893,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = { .eint_wkup_init = exynos_eint_wkup_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4x12-gpio-ctrl1", }, { /* pin-controller instance 2 data */ .pin_banks = exynos4x12_pin_banks2, @@ -908,7 +900,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4x12-gpio-ctrl2", }, { /* pin-controller instance 3 data */ .pin_banks = exynos4x12_pin_banks3, @@ -916,7 +907,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos4x12-gpio-ctrl3", }, }; @@ -989,7 +979,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = { .eint_wkup_init = exynos_eint_wkup_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resume, - .label = "exynos5250-gpio-ctrl0", }, { /* pin-controller instance 1 data */ .pin_banks = exynos5250_pin_banks1, @@ -997,7 +986,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = { .eint_gpio_init = exynos_eint_gpio_init, .suspend= exynos_pinctrl_suspend, .resume = exynos_pinctrl_resum
Re: [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices
On 22 September 2014 06:40, Pankaj Dubey wrote: > Currently a syscon entity can be only registered directly through a > platform device that binds to a dedicated syscon driver. However in > certain use cases it is desirable to make a device used with another > driver a syscon interface provider. > > For example, certain SoCs (e.g. Exynos) contain system controller > blocks which perform various functions such as power domain control, > CPU power management, low power mode control, but in addition contain > certain IP integration glue, such as various signal masks, > coprocessor power control, etc. In such case, there is a need to have > a dedicated driver for such system controller but also share registers > with other drivers. The latter is where the syscon interface is helpful. > > In case of DT based platforms, this patch decouples syscon object from > syscon platform driver, and allows to create syscon objects first time > when it is required by calling of syscon_regmap_lookup_by APIs and keep > a list of such syscon objects along with syscon provider device_nodes > and regmap handles. > > For non-DT based platforms, this patch keeps syscon platform driver > structure where is can be probed and such non-DT based drivers can use > syscon_regmap_lookup_by_pdev API and get access to regmap handles. > Once all users of "syscon_regmap_lookup_by_pdev" migrated to DT based, > we can completly remove platform driver of syscon, and keep only helper > functions to get regmap handles. > > Suggested-by: Arnd Bergmann > Suggested-by: Tomasz Figa > Tested-by: Vivek Gautam > Tested-by: Javier Martinez Canillas > Signed-off-by: Pankaj Dubey Hi Pankaj, I wrote a clk driver using syscon and your patch. clk driver uses CLK_OF_DECLARE, btw. It works but I get a '(null): Failed to create debugfs directory' message in the boot log. Tested-by: Joachim Eastwood regards, Joachim Eastwood -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] ARM: mm: Fix ifdef around cpu_*_do_[suspend, resume] ops
On 08/13/14 00:11, Bartlomiej Zolnierkiewicz wrote: Ifdef around cpu_\name\()_do_suspend and cpu_\name\()_do_resume ops in proc-macros.S should check for CONFIG_ARM_CPU_SUSPEND and not CONFIG_PM_SLEEP. Fix it. [ Please note that cpu_v7_do_[suspend,resume] code in proc-v7.S already correctly checks for CONFIG_ARM_CPU_SUSPEND, same is true for functions for other architectures. ] This fix is needed for decoupling suspend/resume and advanced cpuidle support on Exynos platform (next patch fixes build for config with CONFIG_PM_SLEEP=n and CONFIG_ARM_EXYNOS_CPUIDLE=y). If this fix is not present then the following OOPS happens on the first attempt to go into advanced cpuidle mode (AFTR): [ 22.244143] Unable to handle kernel NULL pointer dereference at virtual address [ 22.250759] pgd = c0004000 [ 22.253445] [] *pgd= [ 22.257012] Internal error: Oops: 8007 [#1] PREEMPT SMP ARM [ 22.262906] Modules linked in: [ 22.265949] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-next-20140811-dirty #730 [ 22.273757] task: c05dce68 ti: c05d2000 task.ti: c05d2000 [ 22.279139] PC is at 0x0 [ 22.281661] LR is at __cpu_suspend_save+0x4c/0xa8 [ 22.286344] pc : [<>]lr : []psr: a093 [ 22.286344] sp : c05d3ef4 ip : c05da414 fp : 0001 [ 22.297799] r10: c05da414 r9 : c0609cb0 r8 : 000f [ 22.303008] r7 : c05da444 r6 : 0038 r5 : ea802c00 r4 : c05d3f14 [ 22.309517] r3 : r2 : c05d3f4c r1 : 0038 r0 : c05d3f20 [ 22.316029] Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [ 22.323406] Control: 10c5387d Table: 69d5404a DAC: 0015 [ 22.329135] Process swapper/0 (pid: 0, stack limit = 0xc05d2240) [ 22.335124] Stack: (0xc05d3ef4 to 0xc05d4000) [ 22.339466] 3ee0: ea802c00 0038 c05d3f4c [ 22.347626] 3f00: 0007 c00123bc c001d468 6a888000 c05d3f4c 8000 [ 22.355785] 3f20: 0007 c003d3a0 193d eaf9dde4 eaf9dde4 c02ef0c8 c000969c fffe [ 22.363944] 3f40: c0037b54 eaf9dbb8 e9d1a380 c001d468 c0609cb0 [ 22.372103] 3f60: c0609cb0 c061649e 0001 c001250c eaf9dbb8 0001 c0609cb0 c001d618 [ 22.380262] 3f80: c001d5d0 c02ef56c 2d9d2e1e 0005 eaf9dbb8 c02edcc4 2d9d2e1e 0005 [ 22.388421] 3fa0: c040446c c05da4ec c040446c eaf9dbb8 c05cfbb0 c004c580 c05dce68 c05b3ae8 [ 22.396580] 3fc0: c058bb24 c058b5e4 c05b3ae8 [ 22.404740] 3fe0: c0616994 c05da47c c05b3ae4 c05ddeec 4000406a 40008074 [ 22.412909] [] (__cpu_suspend_save) from [] (__cpu_suspend+0x5c/0x70) [ 22.421074] [] (__cpu_suspend) from [] (init_thread_union+0x1f4c/0x2000) [ 22.429479] Code: bad PC value [ 22.432518] ---[ end trace fb90ebf4217d0ad9 ]--- [ 22.437116] Kernel panic - not syncing: Attempted to kill the idle task! [ 22.443800] Rebooting in 5 seconds.. This patch has been tested on Exynos4210 based Origen board. Signed-off-by: Bartlomiej Zolnierkiewicz Acked-by: Kyungmin Park --- arch/arm/mm/proc-macros.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S index ee1d805..ba1196c 100644 --- a/arch/arm/mm/proc-macros.S +++ b/arch/arm/mm/proc-macros.S @@ -279,7 +279,7 @@ ENTRY(\name\()_processor_functions) .if \suspend .word cpu_\name\()_suspend_size -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND .word cpu_\name\()_do_suspend .word cpu_\name\()_do_resume #else I'm fine on this but need to get ack from Russell. Russell, how do you think about this? Note, I already applied this into samsung tree but if you mind, I'll revert it. - Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] ARM: dts: exynos3250 add MFC codec device node
On 09/15/14 20:44, Jacek Anaszewski wrote: Signed-off-by: Jacek Anaszewski Signed-off-by: Kyungmin Park Cc: Kukjin Kim --- arch/arm/boot/dts/exynos3250.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi index 351871a..68c9e7c 100644 --- a/arch/arm/boot/dts/exynos3250.dtsi +++ b/arch/arm/boot/dts/exynos3250.dtsi @@ -283,6 +283,16 @@ status = "disabled"; }; + mfc: codec@1340 { + compatible = "samsung,mfc-v7"; + reg =<0x1340 0x1>; + interrupts =<0 102 0>; + clock-names = "mfc", "sclk_mfc"; + clocks =<&cmu CLK_MFC>,<&cmu CLK_SCLK_MFC>; + samsung,power-domain =<&pd_mfc>; + status = "disabled"; + }; + serial_0: serial@1380 { compatible = "samsung,exynos4210-uart"; reg =<0x1380 0x100>; Applied, thanks. - Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] ARM: firmware: Introduce suspend and resume operations
On 08/26/14 23:10, Tomasz Figa wrote: This patch extends the firmware_ops structure with two new callbacks: .suspend() and .resume(). The former is intended to ask the firmware to save all its volatile state and suspend the system, without returning back to the kernel in between. The latter is to be called early by very low level platform suspend code after waking up to restore low level hardware state, which can't be restored in non-secure mode. While at it, outdated version of the structure is removed from the documentation and replaced with a reference to the header file. Signed-off-by: Tomasz Figa Acked-by: Alexandre Courbot --- Documentation/arm/firmware.txt | 28 +--- arch/arm/include/asm/firmware.h | 8 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/Documentation/arm/firmware.txt b/Documentation/arm/firmware.txt index c2e468f..da6713a 100644 --- a/Documentation/arm/firmware.txt +++ b/Documentation/arm/firmware.txt @@ -7,32 +7,14 @@ world, which changes the way some things have to be initialized. This makes a need to provide an interface for such platforms to specify available firmware operations and call them when needed. -Firmware operations can be specified using struct firmware_ops - - struct firmware_ops { - /* - * Enters CPU idle mode - */ - int (*do_idle)(void); - /* - * Sets boot address of specified physical CPU - */ - int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); - /* - * Boots specified physical CPU - */ - int (*cpu_boot)(int cpu); - /* - * Initializes L2 cache - */ - int (*l2x0_init)(void); - }; - -and then registered with register_firmware_ops function +Firmware operations can be specified by filling in a struct firmware_ops +with appropriate callbacks and then registering it with register_firmware_ops() +function. void register_firmware_ops(const struct firmware_ops *ops) -the ops pointer must be non-NULL. +The ops pointer must be non-NULL. More information about struct firmware_ops +and its members can be found in arch/arm/include/asm/firmware.h header. There is a default, empty set of operations provided, so there is no need to set anything if platform does not require firmware operations. diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h index 2c9f10d..5904f59 100644 --- a/arch/arm/include/asm/firmware.h +++ b/arch/arm/include/asm/firmware.h @@ -41,6 +41,14 @@ struct firmware_ops { * Initializes L2 cache */ int (*l2x0_init)(void); + /* +* Enter system-wide suspend. +*/ + int (*suspend)(void); + /* +* Restore state of privileged hardware after system-wide suspend. +*/ + int (*resume)(void); }; /* Global pointer for current firmware_ops structure, can't be NULL. */ Hi Russell, I've applied this in samsung tree for exynos stuff, if you have any objection, please kindly let me know. Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: dts: Add Maxim 77693 PMIC to the Trats2 board
On 09/16/14 00:06, Krzysztof Kozlowski wrote: The MAX77693 is a companion power management IC for smart phones and tablets. The MAX77693 contains input over-voltage protection (OVP), a fully-integrated 2.5A switching charger for Lithium Ion battery with integrated battery disconnect, OTG/accessory 5V output power, a high-current white LED driver for camera flash, two safeout LDOs, a haptic motor driver, Model Gauge m3 battery fuel gauge and MicroUSB Interface Controller (MUIC). I2C serial interface is used for communicating. Add MAX77693 node to the Trats2 board. This allows using: - charger regulator, - 2 safeout LDO regulators (for USB OTG), - extcon. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Javier Martinez Canillas --- Changes since v1: == 1. Use GPIO_ACTIVE_HIGH for gpio flags (suggested by Javier Martinez Canillas). 2. Use regulator node name instead of deprecated regulator-compatible property (suggested by Javier Martinez Canillas). 3. Add Javier Martinez Canillas' reviewed-by tag. --- arch/arm/boot/dts/exynos4412-trats2.dts | 32 1 file changed, 32 insertions(+) Looks good to me, applied. Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos5250-snow
Hello Kukjin, On 09/23/2014 06:00 PM, Kukjin Kim wrote: > On 09/23/14 15:17, Kukjin Kim wrote: > > I've applied above and this series and please double-check the commits > in my tree. If no problems, I will send the branch out for v3.18 soon... > > Thanks, > Kukjin > I've looked the RTC source clock patches and look good to me. Thanks a lot for your help! Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3 v3] Fix Exynos USB on kernels with USB Gadget support
On 09/19/14 16:43, Sjoerd Simons wrote: Hey Kukjin, Hi, Sorry for late response... It's been almost a month since I posted the first iteration of this patchset on the list, with only trivial cosmetic changes and an addition of a similar fix for Arndale Octa boards. Do you feel it needs more review from specific folks before pulling it in or ? Seems a bit of a shame if this would fail to make it for 3.18 (one could even argue the dt patches are 3.17 RC material!) as it fixes USB functionality being broken depending on the kernel configuration used. On Mon, 2014-09-15 at 12:52 +0200, Sjoerd Simons wrote: When building a kernel with support for both USB host and USB Gadget support on the dwc3 controller on the Exynos5 soc will go into USB OTG mode unless otherwise specified in the dtb, which is unhelpful for boards hooked up to run as USB host. First patch in this set explicitely set the dual-role mode for the dwc3 controller on Peach pi and Peach pit boards to host mode. Second patch adds similar updates for Arndale Octa as reported by Andreas Färber. Last patch enables gadget mode in the default exynos config to more easily catch/trigger issues like these. I suspect the Samsung SMDK5420 DTS might need similar changes, so it would be great if users of those board could verify this. Changes in v2: alphabetically sort the dts entries Changes in v3: Add DTS updates for arndale octa Sjoerd Simons (3): ARM: dts: exynos: Explicitly set dr_mode on peach-pit and peach-pi ARM: dts: exynos: Explicitly set dr_mode on arndale-octa ARM: exynos_defconfig: enable USB gadget support arch/arm/boot/dts/exynos5420-arndale-octa.dts | 4 arch/arm/boot/dts/exynos5420-peach-pit.dts| 8 arch/arm/boot/dts/exynos5420.dtsi | 4 ++-- arch/arm/boot/dts/exynos5800-peach-pi.dts | 8 arch/arm/configs/exynos_defconfig | 1 + 5 files changed, 23 insertions(+), 2 deletions(-) I have no objection on this series but needs to be rebased on my tree, can you please respin on top of my tree? Thanks, Kukjin -- Thanks. Best regards, Kgene. -- Kukjin Kim , Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos5250-snow
On 09/23/14 15:17, Kukjin Kim wrote: Kukjin Kim wrote: Andreas Färber wrote: [...] Kukjin: Andreas's patch series was Reviewed long ago I think and by now I'd imagine it's got some conflicts due to it not having been applied in a timely fashion. Perhaps you could fix it up for Andreas (since he's already rebased it several times) and land it? If I had to guess, outstanding from Andreas's series (patchwork IDs listed): 4751131 New [v7] ARM: dts: Prepare node labels for exynos5250 4664801 New [v6,04/10] ARM: dts: Clean up exynos5250-snow 4664771 New [v6,05/10] ARM: dts: Fill in bootargs for exynos5250-snow 4664731 New [v6,06/10] ARM: dts: Clean up exynos5250-smdk5250 4664751 New [v6,07/10] ARM: dts: Clean up exynos5250-arndale 4664711 New [v6,08/10] ARM: dts: Fix apparent GPIO typo in exynos5250-arndale 4664681 New [v6,09/10] ARM: dts: Simplify USB3503 on exynos5250-arndale 4664691 New [v6,10/10] ARM: dts: Add exynos5250-spring device tree Yes, Kukjin told me not to resend anything, and I haven't heard back from him since his Kernel Summit trip. Sorry about that and I thought I took them :( I'll sort them out tonight including this series together. Hmm...since some conflicts with dp and mmc_slot related patches, I couldn't, but Andreas' patches will be handled by tonight :( - Kukjin I've applied above and this series and please double-check the commits in my tree. If no problems, I will send the branch out for v3.18 soon... Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 17:38, Thierry Reding wrote: > On Tue, Sep 23, 2014 at 03:09:44PM +0300, Tomi Valkeinen wrote: >> On 23/09/14 13:01, Thierry Reding wrote: >>> On Tue, Sep 23, 2014 at 12:40:24PM +0300, Tomi Valkeinen wrote: > [...] What exactly is a bridge and what is an encoder? Those are DRM constructs, aren't they? >>> >>> Yes. I think bridges are mostly a superset of encoders. >> >> Superset in what way? If I have a SiI9022 RGB-to-HDMI encoder embedded >> into my SoC (i.e. a DRM encoder), or I have a board with a SiI9022 >> encoder on the board (i.e. a DRM bridge), what is different? > > Superset in what they represent from a software point of view. As in: > an encoder /is a/ bridge. Though they aren't currently implemented that > way. So they are equal, not superset? Or what does an encoder do that a bridge doesn't? As I see it, a video pipeline consist of a video source (display controller usually), a chain of encoders (all of which may not be strictly "encoders", they could be level shifters, muxes, ESD protection devices or such), and either a display device like a panel or a connector to outside world. >>> >>> Well, the panel itself is attached to a connector. The connector is >>> really what userspace uses to control the output and indirectly the >>> panel. >> >> Yep. That's also something that should be fixed. A connector SW entity >> is fine when connecting an external monitor, but a panel directly >> connected to the SoC does not have a connector, and it's the panel that >> should be controlled from the userspace. > > I disagree. A panel is usually also attached using some sort of > connector. Maybe not an HDMI, VGA or DP connector, but still some sort > of connector where often it can even be removed. Yes, but a normal panel connector is really just an extension of wires. There is no difference if you wire the panel directly to the video source, or if there's a connector. I think usually connectors to the external world are not quite as simple, as there may be some protection components or such, but even if there's nothing extra, they are still a clear component visible to outside world. For HDMI you (sometimes) even need properties for the connector, like source for +5V, i2c bus, hotplug pin. But even if there are no extra properties like that, I still think it's good to have a connector entity for a connector to outside world. Whereas I don't see any need for such when the connector is internal. That said, I don't see any reason to prevent that either. > Panels are theoretically hot-pluggable, too, much in the same way that > monitors are. So are encoders, in the same way. I have a main board, with a video connector. That goes to an encoder on display board, and that again has a connector, to which the panel is connected. I also have a panel that can be conneted directly to the main board's video output. >> But again, the legacy... > > You've got to make the abstraction somewhere, and I'm quite surprised > actually how well the DRM abstractions are working out. I mean we can > support anything from HDMI to DP to eDP, DSI and LVDS with just the > connector abstraction and it all just works. That makes it a pretty > good abstraction in my book. Yes, I agree it's good in the sense that it works. And much better than fbdev, which has nothing. But it's not good in the sense that it's not what I'd design if I could start from scratch. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 17:29, Thierry Reding wrote: >>> Trying to do this within the bridge's node directly has two flaws: >>> >>> 1) It violates the DT principle of describing hardware. The >>>device itself does not know anything about multiple streams >>>and deals only with a single input and output. You'd be >>>describing a board specific aspect in the device binding. >> >> We _are_ describing board specific aspects in the board.dts file. That's >> what it is about. >> >> If the board's hardware is such that the bridge's voltage level needs to >> be higher when using panel0, that's very much describing hardware. > > You misunderstood. It's describing a use-case rather than the device > itself. You describe, *within the device node*, that it can be muxed > whereas the device itself doesn't know anything about muxing. That's > where the violation is. You require a board-integration issue to be > described in the binding of a specific device. The .dts files are not only about hard HW features. The .dts files are full of configuration properties and such. So while the aim should be to describe only the HW, I think it's well within the limits to describe things like whether to enable/disable HW features for a particular output. I guess there could be an external mux node, and that mux node could contain HW specific properties for both the inputs and outputs of the mux. The input and output drivers could fetch their features from that mux's node. But that sounds much worse, than having ports/endpoints and HW specific properties there. (or actually, does it... it would simplify some things, but how would the bridge driver get its properties...). And I don't quite see your point. Presuming we have a property in the bridge for, say, "higher-voltage-level", which with simple phandles would be present in the main bridge node, and presuming that's ok. (Is it in your opinion? That is describing configuration, not HW.) If so, I don't think it's different than having two endpoints, each with their own property. In that case we just have the wires from the single output going into to destinations, and so we need to different places for the property. >>> We have a common helper already. It's called of_parse_phandle(). >> >> Yes, but what is the name of the property, and where does it go? Does >> the panel have a phandle to the bridge? Does the bridge have a phandle >> to the panel? > > My point was that we don't need a common helper library if we have a way > of getting at the phandle and resolve the phandle to a bridge or a > panel. While I think standard names can be advantageous, using the > regular of_parse_phandle() and a lookup function we don't even need > them. Well, we have also cases where the more complex video graphs are needed (or, at least, are not too complex, so they could well be used). If so, it'd be nice to have the driver manage the connections via a single API, which would underneath manage both the video graphs and simple phandles, depending on what is used in the .dts file. - study the connections before the drivers are loaded >>> >>> Why would you need that? >> >> I think this was discussed earlier, maybe Laurent remembers the use cases. >> >> I think one possibility here is to have an understanding of the >> pipelines even if the modules have not been loaded or a single device >> has failed to probe. > > I don't see how that would be useful. Ok. But it's still something that can be done with standardized bindings, and cannot be done with non-standardized. - handle the connections anywhere else than the specific driver for the component >>> >>> Why should we do that? >> >> We could, for example, have a single component (common or SoC specific) >> that manages the video pipelines. The drivers for the >> encoders/bridges/panels would just probe the devices, without doing >> anything else. The master component would then parse the links and >> connect the devices in whatever way it sees best. > > You don't need any of that. The SoC driver simply obtains a phandle to > the first bridge connected to it. That bridge will already have done the > same for any chained bridges. There's no need to connect them in any way > anymore because they are already properly set up. Yes, there are different ways to do this in the SW side. The point is, with standard bindings we have many different options how to do it. With non-standard bindings we don't. If there are no strong reasons to use non-standard bindings, I don't see why we would not standardize them. >>> Having a standard representation only matters if you want to be able to >>> parse the pipeline without knowing about the device specifics. But I'm >>> not sure I understand why you would want to do that. This sounds like >>> you'd want some generic code to be able to construct a pipeline. But at >>> the same time you can't do anything meaningful with that pipeline >>> because the generic code doesn't know ho
Re: [PATCH v2 1/5] pinctrl: exynos: Generalize the eint16_31 demux code
On Tue, Sep 23, 2014 at 10:16 AM, Abhilash Kesavan wrote: > The function exynos_irq_demux_eint16_31 uses pre-defined offsets for external > interrupt pending status and mask registers. So this function is not > extensible > for Exynos7 SoC which have these registers at different offsets. So generalize > the exynos_irq_demux_eint16_31 function by using the pending/mask register > offset values from the exynos_irq_chip structure. > > Signed-off-by: Abhilash Kesavan > Reviewed-by: Thomas Abraham > Tested-by: Thomas Abraham > Cc: Thomas Abraham > Cc: Tomasz Figa > Cc: Linus Walleij I'm waiting for the maintainers to ACK these patches before I take them. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 03:00:31PM +0300, Tomi Valkeinen wrote: > On 23/09/14 12:53, Thierry Reding wrote: > > >> Yes, but in this case we know of existing boards that have complex > >> setups. It's not theoretical. > > > > Complex setups involving /this particular/ bridge and binding are > > theoretical at this point, however. > > Right, but this discussion, at least from my part, has not so much been > about this particular bridge, but bridges in general. > > So I don't have any requirements for this bridge driver/bindings to > support complex use cases at the time being. > > But I do want us to have an option to use the bridge in the future in > such complex case. And yes, we can't guarantee that we'd hit the > bindings right whatever we do, but we should think about it and at least > try. > > >>> phandles should simply point to the next element in the pipeline and the > >>> OS abstractions should be good enough to handle the details about how to > >>> chain the elements. > >> > >> I, on the other hand, would rather see the links the other way around. > >> Panel having a link to the video source, etc. > > > > Why? It seems much more natural to describe this using the natural flow > > of data. The device furthest away from the CPU should be the target of > > the phandle. That way the DT can be used to trace the flow of data down > > the pipeline. > > Because I see video components "using" their video sources. A panel uses > an encoder to produce video data for the panel. An encoder uses its > video source to produce video data. A bit like a device uses a gpio, or > a pwm. > > Especially with more complex encoders/panels having the panel/encoder in > control of its video source is often the easiest (only?) way to manage > the job. This has worked well on OMAP, whereas the earlier model where > the video source controlled the video sink was full of problems. Not > that that exactly proves anything, but that's my experience, and I > didn't find any easy solutions when the video source was in control. > > So while the video data flows from SoC to the panel, the control goes > the other way. Whether the DT links should model the video data or > control, I don't know. Both feel natural to me. > > But if a panel driver controls its video source, it makes sense for the > panel driver to get its video source in its probe, and that happens > easiest if the panel has a link to the video source. That's an orthogonal problem. You speak about the link in software here, whereas the phandles only represent the link in the description of hardware. Now DT describes hardware from the perspective of the CPU, so it's sort of like a cave that you're trying to explore. You start at the top with the address bus, from where a couple of tunnels lead to the various peripherals on the address bus. A display controller might have another tunnel to a panel, etc. If you go the other way around, how do you detect how things connect? Where do you get the information about the panel so you can trace back to the origin? Thierry pgpr3k8lde8_S.pgp Description: PGP signature
Re: [PATCH 2/3] kbuild: remove unnecessary variable initializaions
On Tue, Sep 9, 2014 at 12:26 PM, Masahiro Yamada wrote: > Clearing obj-y, obj-m, obj-n, obj- in each Makefile is > a useless habit. > > They are non-exported variables; therefore they are always empty > whenever descending into each subdirectory. > (Moreorver, obj-y and obj-m are also set to empty at the beginning > of scripts/Makefile.build) > > Signed-off-by: Masahiro Yamada Acked-by: Linus Walleij Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts
On 23.09.2014 10:16, Abhilash Kesavan wrote: [snip] > @@ -383,9 +377,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data > *irqd, unsigned int on) > /* > * irq_chip for wakeup interrupts > */ > -static struct exynos_irq_chip exynos_wkup_irq_chip = { > +static struct exynos_irq_chip *exynos_wkup_irq_chip; > + [snip] > @@ -459,7 +480,7 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, > struct irq_desc *desc) > static int exynos_wkup_irq_map(struct irq_domain *h, unsigned int virq, > irq_hw_number_t hw) > { > - irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip.chip, > + irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip->chip, > handle_level_irq); > irq_set_chip_data(virq, h->host_data); > set_irq_flags(virq, IRQF_VALID); > @@ -491,7 +512,12 @@ static int exynos_eint_wkup_init(struct > samsung_pinctrl_drv_data *d) > int idx, irq; > > for_each_child_of_node(dev->of_node, np) { > - if (of_match_node(exynos_wkup_irq_ids, np)) { > + const struct of_device_id *match; > + > + match = of_match_node(exynos_wkup_irq_ids, np); > + if (match) { > + exynos_wkup_irq_chip = kmemdup(match->data, > + sizeof(struct exynos_irq_chip), GFP_KERNEL); That's not exactly what I had in my mind. You just changed the code to write to another global variable. This is not the best practice and might cause hard to track issues in future, when extending the driver. >From what I can see, the best solution would be to add .irq_chip field to samsung_pin_bank struct. Then exynos_wkup_irq_map() could access it through h->host_data pointer and exynos_irq_demux_eint16_31() could also retrieve the irq chip through bank struct without the need too add chip field to exynos_muxed_weint_data struct. As a side effect, it would be possible to consolidate exynos_wkup_irqd_ops with exynos_gpio_irqd_ops, which currently only differ in irq chip passed as argument to irq_set_chip_and_handler() in .map() callback. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 02:12:52PM +0300, Laurent Pinchart wrote: > On Tuesday 23 September 2014 11:53:15 Thierry Reding wrote: > > On Tue, Sep 23, 2014 at 12:30:20PM +0300, Tomi Valkeinen wrote: > > > On 23/09/14 09:21, Thierry Reding wrote: > > > >> Well, I can write almost any kind of bindings, and then evidently my > > > >> device would work. For me, on my board. > > > > > > > > Well, that's the whole problem with DT. For many devices we only have a > > > > single setup to test against. And even when we have several they often > > > > are derived from each other. But the alternative would be to defer > > > > (possibly indefinitely) merging support for a device until a second, > > > > wildly different setup shows up. That's completely unreasonable and we > > > > need to start somewhere. > > > > > > Yes, but in this case we know of existing boards that have complex > > > setups. It's not theoretical. > > > > Complex setups involving /this particular/ bridge and binding are > > theoretical at this point, however. > > > > > > phandles should simply point to the next element in the pipeline and the > > > > OS abstractions should be good enough to handle the details about how to > > > > chain the elements. > > > > > > I, on the other hand, would rather see the links the other way around. > > > Panel having a link to the video source, etc. > > > > Why? It seems much more natural to describe this using the natural flow > > of data. The device furthest away from the CPU should be the target of > > the phandle. That way the DT can be used to trace the flow of data down > > the pipeline. > > > > > The video graphs have two-way links, which of course is the safest > > > options, but also more verbose and redundant. > > > > Right. If we take that line of thinking to the extreme we end up listing > > individual registers in DT so that we can safely assume that we can > > control all possible aspects of the device. > > And the other extreme would be to have a single DT node for the logical video > device with all information pertaining to it stored in C code. Extremes are > never good, we need to find a reasonable middle-ground here. I believe OF > graph fulfills that requirement, it might be slightly more verbose than a > single phandle, but it's also much more versatile. Oh well, yet another one of these things where we'll never reach an agreement I guess. Thierry pgpUc2rfeHK9K.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 02:52:24PM +0300, Laurent Pinchart wrote: > On Tuesday 23 September 2014 13:47:40 Andrzej Hajda wrote: > > On 09/23/2014 01:23 PM, Laurent Pinchart wrote: [...] > > > This becomes an issue even on Linux when considering video-related devices > > > that can be part of either a capture pipeline or a display pipeline. If > > > the link always goes in the data flow direction, then it will be easy to > > > locate the downstream device (bridge or panel) from the display controller > > > driver, but it would be much more difficult to locate the same device from > > > a camera driver as all of a sudden the device would become an upstream > > > device. > > > > Why? > > > > If you have graph: > > sensor --> camera > > > > Then camera register itself in some framework as a destination device > > and sensor looks in this framework for the device identified by remote > > endpoint. > > Then sensor tells camera it is connected to it and voila. > > Except that both kernelspace and userspace deal with cameras the other way > around, the master device is the camera receiver, not the camera sensor. DRM > is architected the same way, with the component that performs DMA operations > being the master device. I don't see what's wrong with having the camera reference the sensor by phandle instead. That's much more natural in my opinion. Thierry pgppqsIzozOOf.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 02:31:35PM +0300, Tomi Valkeinen wrote: > On 23/09/14 12:39, Thierry Reding wrote: > > > My point is that if you use plain phandles you usually have the > > meta-data already. Referring to the above example, bridge0 knows that it > > should look for a bridge with phandle &bridge1, whereas bridge1 knows > > that the device it is connected to is a panel. > > The bridge should not care what kind of device is there on the other > end. The bridge just has an output, going to another video component. You'll need to know at some point that one of the devices in a panel, otherwise you can't treat it like a panel. > >> Well, I can't say about this particular bridge, but afaik you can > >> connect a parallel RGB signal to multiple panels just like that, without > >> any muxing. > > > > Right, but in that case you're not reconfiguring the signal in any way > > for each of the panels. You send one single signal to all of them. For > > Yes, that's one use case, cloning. But I was not talking about that. > > > all intents and purposes there is only one panel. Well, I guess you > > could have separate backlights for the panels. In that case though it > > seems better to represent it at least as a virtual mux or bridge, or > > perhaps a "mux panel". > > I was talking about the case where you have two totally different > devices, let's say panels, connected to the same output. One could take > a 16-bit parallel RGB signal, the other 24-bit. Only one of them can be > enabled at a time (from HW perspective both can be enabled at the same > time, but then the other one shows garbage). So we're essentially back to a mux, albeit maybe a virtual one. Thierry pgpNsId53T6BL.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 12:34:54PM +0200, Andrzej Hajda wrote: > On 09/23/2014 12:10 PM, Thierry Reding wrote: > > On Tue, Sep 23, 2014 at 11:43:47AM +0200, Andrzej Hajda wrote: > >> On 09/23/2014 10:35 AM, Thierry Reding wrote: > > [...] > >>> But I agree that it would be nice to unify bridges and encoders more. It > >>> should be possible to make encoder always a bridge (or perhaps even > >>> replace encoders with bridges altogether). Then once you're out of the > >>> DRM device everything would be a bridge until you get to a panel. > >> I agree that some sort of unification of bridges, (slave) encoders is a > >> good > >> thing, this way we stay only with bridges and panels as receivers. > >> But we will still have to deal with the code like: > >> if (on the other end of the link is panel) > >> do panel framework specific things > >> else > >> do bridge framework specific things > >> > >> The code in both branches usually does similar things but due to framework > >> differences it is difficult to merge. > > That's because they are inherently different entities. You can perform > > operations on a panel that don't make sense for a bridge and vice-versa. > > > >> Ideally it would be best to get rid of such constructs. For example by > >> trying to > >> make frameworks per interface type exposed by device (eg. video_sink) and > >> not by device type (drm_bridge, drm_panel). > > But then you loose all information about the real type of device. > Driver knows type of its device anyway. Why should it know device > type of devices it is connected to? It is enough it knows how to talk to > other device. > Like in real world, why desktop PC should know it is connected to DVI > monitor or to > DVI/HDMI converter which is connected to TV? TVs are much more standardized. There are things like DDC/CI which can be used to control the device. Or there's additional circuitry or control paths to change things like brightness. The same isn't true of panels. > > Or you > > have to create a common base class. And then you're still back to > > dealing with the two specific cases in many places, so the gain seems > > rather minimal. > > For example RGB panel and RGB/??? bridge have the same hardware input > interface. > Why shall they have different representation in kernel? Because panels have different requirements than bridges. Panels for example require the backlight to be adjustable, bridges don't. Thierry pgpXZaI4D9qDQ.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 03:09:44PM +0300, Tomi Valkeinen wrote: > On 23/09/14 13:01, Thierry Reding wrote: > > On Tue, Sep 23, 2014 at 12:40:24PM +0300, Tomi Valkeinen wrote: [...] > >> What exactly is a bridge and what is an encoder? Those are DRM > >> constructs, aren't they? > > > > Yes. I think bridges are mostly a superset of encoders. > > Superset in what way? If I have a SiI9022 RGB-to-HDMI encoder embedded > into my SoC (i.e. a DRM encoder), or I have a board with a SiI9022 > encoder on the board (i.e. a DRM bridge), what is different? Superset in what they represent from a software point of view. As in: an encoder /is a/ bridge. Though they aren't currently implemented that way. > >> As I see it, a video pipeline consist of a video source (display > >> controller usually), a chain of encoders (all of which may not be > >> strictly "encoders", they could be level shifters, muxes, ESD protection > >> devices or such), and either a display device like a panel or a > >> connector to outside world. > > > > Well, the panel itself is attached to a connector. The connector is > > really what userspace uses to control the output and indirectly the > > panel. > > Yep. That's also something that should be fixed. A connector SW entity > is fine when connecting an external monitor, but a panel directly > connected to the SoC does not have a connector, and it's the panel that > should be controlled from the userspace. I disagree. A panel is usually also attached using some sort of connector. Maybe not an HDMI, VGA or DP connector, but still some sort of connector where often it can even be removed. Panels are theoretically hot-pluggable, too, much in the same way that monitors are. > But again, the legacy... You've got to make the abstraction somewhere, and I'm quite surprised actually how well the DRM abstractions are working out. I mean we can support anything from HDMI to DP to eDP, DSI and LVDS with just the connector abstraction and it all just works. That makes it a pretty good abstraction in my book. Thierry pgphirLfreb0H.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 02:15:54PM +0300, Tomi Valkeinen wrote: > On 23/09/14 12:28, Thierry Reding wrote: > > >> But, for example, let's say that the board is designed in a way that for > >> panel0 the bridge needs to output a bit higher level voltages than for > >> panel1. That's not a property of the panel, so it can't be queried from > >> the panel. > >> > >> That feature (varying the voltage level) is specific to the bridge, and > >> thus the properties should be in the bridge's DT data. So we need two > >> different configurations in the bridge, one for panel0 and one for > >> panel1. This is what endpoints give us. > > > > You could get the same by moving the mux in front of the bridge instead. > > I didn't understand. Moving the mux between the SoC and the bridge? How > would that solve the problem. Or what do you mean with "in the front of > the bridge"? Because then you'd have multiple instances of the bridge that could describe which parameters it wants. But writing it out loud this is equally bad. More below. > > Trying to do this within the bridge's node directly has two flaws: > > > > 1) It violates the DT principle of describing hardware. The > >device itself does not know anything about multiple streams > >and deals only with a single input and output. You'd be > >describing a board specific aspect in the device binding. > > We _are_ describing board specific aspects in the board.dts file. That's > what it is about. > > If the board's hardware is such that the bridge's voltage level needs to > be higher when using panel0, that's very much describing hardware. You misunderstood. It's describing a use-case rather than the device itself. You describe, *within the device node*, that it can be muxed whereas the device itself doesn't know anything about muxing. That's where the violation is. You require a board-integration issue to be described in the binding of a specific device. > > 2) Such a solution would have to be implemented for all bridge > >devices that can potentially be muxed (really all of them). > >If you describe it accurately in a separate mux node you get > >genericity for free and it will work for all bridges. > > I do agree that a generic mux is better if there's nothing special to be > configured. But how do you use a generic mux node for bridge device > specific features? Good question. I don't have an immediate answer to that and would have to think about it for some more. > >> I disagree. If we don't have a common way to describe the connections > >> between video devices, we cannot: > >> > >> - have a common helper library to parse the connections > > > > We have a common helper already. It's called of_parse_phandle(). > > Yes, but what is the name of the property, and where does it go? Does > the panel have a phandle to the bridge? Does the bridge have a phandle > to the panel? My point was that we don't need a common helper library if we have a way of getting at the phandle and resolve the phandle to a bridge or a panel. While I think standard names can be advantageous, using the regular of_parse_phandle() and a lookup function we don't even need them. > >> - study the connections before the drivers are loaded > > > > Why would you need that? > > I think this was discussed earlier, maybe Laurent remembers the use cases. > > I think one possibility here is to have an understanding of the > pipelines even if the modules have not been loaded or a single device > has failed to probe. I don't see how that would be useful. > >> - handle the connections anywhere else than the specific driver for the > >> component > > > > Why should we do that? > > We could, for example, have a single component (common or SoC specific) > that manages the video pipelines. The drivers for the > encoders/bridges/panels would just probe the devices, without doing > anything else. The master component would then parse the links and > connect the devices in whatever way it sees best. You don't need any of that. The SoC driver simply obtains a phandle to the first bridge connected to it. That bridge will already have done the same for any chained bridges. There's no need to connect them in any way anymore because they are already properly set up. > > Having a standard representation only matters if you want to be able to > > parse the pipeline without knowing about the device specifics. But I'm > > not sure I understand why you would want to do that. This sounds like > > you'd want some generic code to be able to construct a pipeline. But at > > the same time you can't do anything meaningful with that pipeline > > because the generic code doesn't know how to program the pipeline. The > > only thing you'll get is a list of devices in the pipeline, but you can > > do that by looking at the bindings and the DT content, no matter what > > the properties are named > > You can, but then you need to know all the possible variat
Re: [PATCH] [media] s5p-mfc: Use decode status instead of display status on MFCv5
Hey Kamil, On Tue, 2014-09-23 at 15:58 +0200, Kamil Debski wrote: > > Commit 90c0ae50097 changed how the frame_type of a decoded frame > > gets determined, by switching from the get_dec_frame_type to > > get_disp_frame_type operation. Unfortunately it seems that on MFC v5 > > the > > result of get_disp_frame_type is always 0 (no display) when decoding > > (tested with H264), resulting in no frame ever being output from the > > decoder. > > Could you tell me which firmware version do you use (date)? I'm using the firmware version as included in http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/ Unfortunately there is no specific version information included about the firmware. The commit that added it is fb5cda9c70277f6 dated Nov 28 17:43:06 2012 +0530, so that at least gives some information about the time-frame. -- Sjoerd Simons Collabora Ltd. smime.p7s Description: S/MIME cryptographic signature
RE: [PATCH] [media] s5p-mfc: Use decode status instead of display status on MFCv5
Hi Sjoerd, > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Sjoerd Simons > Sent: Monday, September 22, 2014 2:52 PM > To: Kyungmin Park; Kamil Debski; Arun Kumar K > Cc: Mauro Carvalho Chehab; linux-arm-ker...@lists.infradead.org; linux- > me...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-samsung- > s...@vger.kernel.org; Daniel Drake; Sjoerd Simons > Subject: [PATCH] [media] s5p-mfc: Use decode status instead of display > status on MFCv5 > > Commit 90c0ae50097 changed how the frame_type of a decoded frame > gets determined, by switching from the get_dec_frame_type to > get_disp_frame_type operation. Unfortunately it seems that on MFC v5 > the > result of get_disp_frame_type is always 0 (no display) when decoding > (tested with H264), resulting in no frame ever being output from the > decoder. Could you tell me which firmware version do you use (date)? Best wishes, -- Kamil Debski Samsung R&D Institute Poland > This patch reverts MFC v5 to the previous behaviour while keeping the > new behaviour for v6 and up. > > Signed-off-by: Sjoerd Simons > --- > drivers/media/platform/s5p-mfc/s5p_mfc.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c > b/drivers/media/platform/s5p-mfc/s5p_mfc.c > index d35b041..27ca9d0 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c > @@ -264,7 +264,12 @@ static void s5p_mfc_handle_frame_new(struct > s5p_mfc_ctx *ctx, unsigned int err) > unsigned int frame_type; > > dspl_y_addr = s5p_mfc_hw_call(dev->mfc_ops, get_dspl_y_adr, dev); > - frame_type = s5p_mfc_hw_call(dev->mfc_ops, get_disp_frame_type, > ctx); > + if (IS_MFCV6_PLUS(dev)) > + frame_type = s5p_mfc_hw_call(dev->mfc_ops, > + get_disp_frame_type, ctx); > + else > + frame_type = s5p_mfc_hw_call(dev->mfc_ops, > + get_dec_frame_type, dev); > > /* If frame is same as previous then skip and do not dequeue */ > if (frame_type == S5P_FIMV_DECODE_FRAME_SKIPPED) { > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/2] serial: samsung: add support for early console
On 23.09.2014 14:53, Alim Akhtar wrote: > Hi Marek, > > On Mon, Sep 22, 2014 at 6:44 PM, Marek Szyprowski > wrote: >> Hello, >> >> This patchset adds support for early console defined in device tree. As >> an example, DTS files for all Exynos4 based machines are updated with >> the correct value for common chosen/sdtout property. >> >> To get it fully functional additional improvements (support for >> early_ioremap) are needed in early console code. >> > Is these really tested? > So that mean we need to wait till __ioremap__ support available for ARM. This was originally tested by creating a fixed mapping for all UART ports on respective platforms in .map_io() callback of machine descriptor. This might be the way to support this on 32-bit ARM platforms until early ioremap is available. However if no mapping is available, the earlycon will simply not register and won't break anything, so it should be safe to keep. > How to take this forward then? Here is a platform (exynos7) which > needs earlycon support for debugging some of the early core things and > ARM64 has ioremap support as well. There is no reason why we should > hold earlycon support for exyons7. > I see two solution here: > 1> How about re-spin patch-1 which will just add ealrycon support for > __exynos4210__ serial type only? And rest of the types can be added as > incremental patches, as and when ioremap for ARM available. > Atleast this approch will solve exynos7 problem of not having earlycon > support. > And This patch can be tested on exynos7 (I can do that). I don't really think there is a need to remove support for other variants. As I mentioned, they can be supported without early_ioremap() by static IO mapping. > 2> Take my patch which is working and tested for exynos7 and can be > easily extended and generalized when ioremap for ARM is available. > > My preference would be the the first one. > Let me know your thoughts please. > In case you are ok with 1st approach, can you please re-spin patch1 > with needed changes? I'd say this patch could be merged as is. For 32-bit platforms additional patches might be posted separately to add early static mapping or just wait until early ioremap becomes availabl.e Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/2] serial: samsung: add support for early console
Hi Marek, On Tue, Sep 23, 2014 at 6:38 PM, Marek Szyprowski wrote: > Hello, > > On 2014-09-23 14:53, Alim Akhtar wrote: >> >> Hi Marek, >> >> On Mon, Sep 22, 2014 at 6:44 PM, Marek Szyprowski >> wrote: >>> >>> Hello, >>> >>> This patchset adds support for early console defined in device tree. As >>> an example, DTS files for all Exynos4 based machines are updated with >>> the correct value for common chosen/sdtout property. >>> >>> To get it fully functional additional improvements (support for >>> early_ioremap) are needed in early console code. >>> >> Is these really tested? > > > I've tested it on Exynos4412 based OdroidU3 board. It works fine, although > early console is not initialized as early as it might be. That's the only > drawback, besides that it works fine. > >> So that mean we need to wait till __ioremap__ support available for ARM. >> How to take this forward then? Here is a platform (exynos7) which >> needs earlycon support for debugging some of the early core things and >> ARM64 has ioremap support as well. There is no reason why we should >> hold earlycon support for exyons7. > > > I prefer to simply merge the patch as is. Once early con gets full > functionality, all required samsung code will be already in then. > Thats good then, I was thinking you (or someone else, might be) are going to hold this till __ioremap__ gets added for ARM. > >> I see two solution here: >> 1> How about re-spin patch-1 which will just add ealrycon support for >> __exynos4210__ serial type only? And rest of the types can be added as >> incremental patches, as and when ioremap for ARM available. >> Atleast this approch will solve exynos7 problem of not having earlycon >> support. >> And This patch can be tested on exynos7 (I can do that). >> 2> Take my patch which is working and tested for exynos7 and can be >> easily extended and generalized when ioremap for ARM is available. >> >> My preference would be the the first one. >> Let me know your thoughts please. >> In case you are ok with 1st approach, can you please re-spin patch1 >> with needed changes? > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland > -- Regards, Alim -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/2] serial: samsung: add support for early console
Hello, On 2014-09-23 14:53, Alim Akhtar wrote: Hi Marek, On Mon, Sep 22, 2014 at 6:44 PM, Marek Szyprowski wrote: Hello, This patchset adds support for early console defined in device tree. As an example, DTS files for all Exynos4 based machines are updated with the correct value for common chosen/sdtout property. To get it fully functional additional improvements (support for early_ioremap) are needed in early console code. Is these really tested? I've tested it on Exynos4412 based OdroidU3 board. It works fine, although early console is not initialized as early as it might be. That's the only drawback, besides that it works fine. So that mean we need to wait till __ioremap__ support available for ARM. How to take this forward then? Here is a platform (exynos7) which needs earlycon support for debugging some of the early core things and ARM64 has ioremap support as well. There is no reason why we should hold earlycon support for exyons7. I prefer to simply merge the patch as is. Once early con gets full functionality, all required samsung code will be already in then. I see two solution here: 1> How about re-spin patch-1 which will just add ealrycon support for __exynos4210__ serial type only? And rest of the types can be added as incremental patches, as and when ioremap for ARM available. Atleast this approch will solve exynos7 problem of not having earlycon support. And This patch can be tested on exynos7 (I can do that). 2> Take my patch which is working and tested for exynos7 and can be easily extended and generalized when ioremap for ARM is available. My preference would be the the first one. Let me know your thoughts please. In case you are ok with 1st approach, can you please re-spin patch1 with needed changes? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/2] serial: samsung: add support for early console
Hi Marek, On Mon, Sep 22, 2014 at 6:44 PM, Marek Szyprowski wrote: > Hello, > > This patchset adds support for early console defined in device tree. As > an example, DTS files for all Exynos4 based machines are updated with > the correct value for common chosen/sdtout property. > > To get it fully functional additional improvements (support for > early_ioremap) are needed in early console code. > Is these really tested? So that mean we need to wait till __ioremap__ support available for ARM. How to take this forward then? Here is a platform (exynos7) which needs earlycon support for debugging some of the early core things and ARM64 has ioremap support as well. There is no reason why we should hold earlycon support for exyons7. I see two solution here: 1> How about re-spin patch-1 which will just add ealrycon support for __exynos4210__ serial type only? And rest of the types can be added as incremental patches, as and when ioremap for ARM available. Atleast this approch will solve exynos7 problem of not having earlycon support. And This patch can be tested on exynos7 (I can do that). 2> Take my patch which is working and tested for exynos7 and can be easily extended and generalized when ioremap for ARM is available. My preference would be the the first one. Let me know your thoughts please. In case you are ok with 1st approach, can you please re-spin patch1 with needed changes? > Best regards > Marek Szyprowski > Samsung R&D Institute Poland > > > Patch summary: > > Tomasz Figa (2): > serial: samsung: Add support for of_earlycon > ARM: dts: exynos4: Add stdout-path properties > > arch/arm/boot/dts/exynos4210-origen.dts | 1 + > arch/arm/boot/dts/exynos4210-smdkv310.dts | 1 + > arch/arm/boot/dts/exynos4210-trats.dts | 1 + > arch/arm/boot/dts/exynos4210-universal_c210.dts | 1 + > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 + > arch/arm/boot/dts/exynos4412-origen.dts | 1 + > arch/arm/boot/dts/exynos4412-smdk4412.dts | 1 + > arch/arm/boot/dts/exynos4412-tiny4412.dts | 4 + > arch/arm/boot/dts/exynos4412-trats2.dts | 1 + > drivers/tty/serial/Kconfig | 1 + > drivers/tty/serial/samsung.c| 97 > + > 11 files changed, 113 insertions(+) > > -- > 1.9.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Regards, Alim -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 01:52 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 13:47:40 Andrzej Hajda wrote: >> On 09/23/2014 01:23 PM, Laurent Pinchart wrote: >>> On Tuesday 23 September 2014 13:18:30 Andrzej Hajda wrote: On 09/23/2014 01:10 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: >> On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: >>> On 23/09/14 09:21, Thierry Reding wrote: > Well, I can write almost any kind of bindings, and then evidently my > device would work. For me, on my board. Well, that's the whole problem with DT. For many devices we only have a single setup to test against. And even when we have several they often are derived from each other. But the alternative would be to defer (possibly indefinitely) merging support for a device until a second, wildly different setup shows up. That's completely unreasonable and we need to start somewhere. >>> >>> Yes, but in this case we know of existing boards that have complex >>> setups. It's not theoretical. >>> >>> I'm not saying we should stop everything until we have a 100% solution >>> for the rare complex cases. But we should keep them in mind and, when >>> possible, solve problems in a way that will work for the complex cases >>> also. >>> > I guess non-video devices haven't had need for those. I have had > lots of boards with video setup that cannot be represented with > simple phandles. I'm not sure if I have just been unlucky or what, > but my understand is that other people have encountered such boards > also. Usually the problems encountered there have been circumvented > with some hacky video driver for that specific board, or maybe a > static configuration handled by the boot loader. I have yet to encounter such a setup. Can you point me at a DTS for one such setup? I do remember a couple of hypothetical cases being discussed at one time or another, but I haven't seen any actual DTS content where this was needed. >>> >>> No, I can't point to them as they are not in the mainline (at least >>> the ones I've been working on), for obvious reasons. >>> >>> With a quick glance, I have the following devices in my cabinet that >>> have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx >>> EVM. Many Nokia devices used to have such setups, usually so that the >>> LCD and tv-out were connected to the same video source. >>> > Do we have a standard way of representing the video pipeline with > simple phandles? Or does everyone just do their own version? If > there's no standard way, it sounds it'll be a mess to support in the > future. It doesn't matter all that much whether the representation is standard. >>> >>> Again, I disagree. >>> phandles should simply point to the next element in the pipeline and the OS abstractions should be good enough to handle the details about how to chain the elements. >>> >>> I, on the other hand, would rather see the links the other way around. >>> Panel having a link to the video source, etc. >>> >>> The video graphs have two-way links, which of course is the safest >>> options, but also more verbose and redundant. >>> >>> When this was discussed earlier, it was unclear which way the links >>> should be. It's true that only links to one direction are strictly >>> needed, but the question raised was that if in the drivers we end up >>> always going the links the other way, the performance penalty may be >>> somewhat big. (If I recall right). >> >> I do not see why performance may drop significantly? >> If the link is one-way it should probably work as below: >> - the destination registers itself in some framework, >> - the source looks for the destination in this framework using phandle, >> - the source starts to communicate with the destination - since now >> full two way link can be established dynamically. >> >> Where do you see here big performance penalty? > > The performance-related problems arise when you need to locate the > remote device in the direction opposite to the phandle link direction. > Traversing a link forward just involves a phandle lookup, but traversing > it backwards isn't possible the same way. But you do not need to traverse backwards. You just wait when the source start to communicate with the destination, at this moment destination can build back-link dynamically. >>> >>> Your driver might not need it today for your use cases, but can you be >>> certain that no driver on any OS would need to ? >> >> I have just showed how to create back-link dynamica
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 01:52 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 13:47:40 Andrzej Hajda wrote: >> On 09/23/2014 01:23 PM, Laurent Pinchart wrote: >>> On Tuesday 23 September 2014 13:18:30 Andrzej Hajda wrote: On 09/23/2014 01:10 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: >> On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: >>> On 23/09/14 09:21, Thierry Reding wrote: > Well, I can write almost any kind of bindings, and then evidently my > device would work. For me, on my board. Well, that's the whole problem with DT. For many devices we only have a single setup to test against. And even when we have several they often are derived from each other. But the alternative would be to defer (possibly indefinitely) merging support for a device until a second, wildly different setup shows up. That's completely unreasonable and we need to start somewhere. >>> >>> Yes, but in this case we know of existing boards that have complex >>> setups. It's not theoretical. >>> >>> I'm not saying we should stop everything until we have a 100% solution >>> for the rare complex cases. But we should keep them in mind and, when >>> possible, solve problems in a way that will work for the complex cases >>> also. >>> > I guess non-video devices haven't had need for those. I have had > lots of boards with video setup that cannot be represented with > simple phandles. I'm not sure if I have just been unlucky or what, > but my understand is that other people have encountered such boards > also. Usually the problems encountered there have been circumvented > with some hacky video driver for that specific board, or maybe a > static configuration handled by the boot loader. I have yet to encounter such a setup. Can you point me at a DTS for one such setup? I do remember a couple of hypothetical cases being discussed at one time or another, but I haven't seen any actual DTS content where this was needed. >>> >>> No, I can't point to them as they are not in the mainline (at least >>> the ones I've been working on), for obvious reasons. >>> >>> With a quick glance, I have the following devices in my cabinet that >>> have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx >>> EVM. Many Nokia devices used to have such setups, usually so that the >>> LCD and tv-out were connected to the same video source. >>> > Do we have a standard way of representing the video pipeline with > simple phandles? Or does everyone just do their own version? If > there's no standard way, it sounds it'll be a mess to support in the > future. It doesn't matter all that much whether the representation is standard. >>> >>> Again, I disagree. >>> phandles should simply point to the next element in the pipeline and the OS abstractions should be good enough to handle the details about how to chain the elements. >>> >>> I, on the other hand, would rather see the links the other way around. >>> Panel having a link to the video source, etc. >>> >>> The video graphs have two-way links, which of course is the safest >>> options, but also more verbose and redundant. >>> >>> When this was discussed earlier, it was unclear which way the links >>> should be. It's true that only links to one direction are strictly >>> needed, but the question raised was that if in the drivers we end up >>> always going the links the other way, the performance penalty may be >>> somewhat big. (If I recall right). >> >> I do not see why performance may drop significantly? >> If the link is one-way it should probably work as below: >> - the destination registers itself in some framework, >> - the source looks for the destination in this framework using phandle, >> - the source starts to communicate with the destination - since now >> full two way link can be established dynamically. >> >> Where do you see here big performance penalty? > > The performance-related problems arise when you need to locate the > remote device in the direction opposite to the phandle link direction. > Traversing a link forward just involves a phandle lookup, but traversing > it backwards isn't possible the same way. But you do not need to traverse backwards. You just wait when the source start to communicate with the destination, at this moment destination can build back-link dynamically. >>> >>> Your driver might not need it today for your use cases, but can you be >>> certain that no driver on any OS would need to ? >> >> I have just showed how to create back-link dynamica
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 13:01, Thierry Reding wrote: > On Tue, Sep 23, 2014 at 12:40:24PM +0300, Tomi Valkeinen wrote: >> On 23/09/14 11:35, Thierry Reding wrote: >> >>> Well, a display controller is never going to attach to a panel directly. >> >> With parallel RGB, that (almost) happens. There's voltage level shifting >> probably in the middle, but I don't see anything else there. > > The level shifting could be considered an encoder. Anyway, I didn't mean > physically attach to a panel but rather in DRM. You'll always need an > encoder and connector before you go to the panel. "For now", you mean. I'd rather see much more freedom there. If a display controller is connected to a panel directly, with only non-controllable level shifters in between (I don't even think those are strictly needed. why couldn't there be a low-end soc that works with higher voltages?), I think we should model it in the SW side by just having a device for the display controller and a device for the panel. No artificial encoders/bridges/connectors needed in between. But I understand that for DRM legacy reasons that may never happen. >>> But I agree that it would be nice to unify bridges and encoders more. It >>> should be possible to make encoder always a bridge (or perhaps even >>> replace encoders with bridges altogether). Then once you're out of the >>> DRM device everything would be a bridge until you get to a panel. >> >> What exactly is a bridge and what is an encoder? Those are DRM >> constructs, aren't they? > > Yes. I think bridges are mostly a superset of encoders. Superset in what way? If I have a SiI9022 RGB-to-HDMI encoder embedded into my SoC (i.e. a DRM encoder), or I have a board with a SiI9022 encoder on the board (i.e. a DRM bridge), what is different? >> As I see it, a video pipeline consist of a video source (display >> controller usually), a chain of encoders (all of which may not be >> strictly "encoders", they could be level shifters, muxes, ESD protection >> devices or such), and either a display device like a panel or a >> connector to outside world. > > Well, the panel itself is attached to a connector. The connector is > really what userspace uses to control the output and indirectly the > panel. Yep. That's also something that should be fixed. A connector SW entity is fine when connecting an external monitor, but a panel directly connected to the SoC does not have a connector, and it's the panel that should be controlled from the userspace. But again, the legacy... >> Am I right that in DRM world the encoder is the first device in the >> display chain after the display controller, > > Yes. > >> and the next is a bridge? > > A bridge or a connector. Typically it would be a connector, but that's > only if you don't have bridges in between. > >> That sounds totally artificial, and I hope we don't reflect that in the >> DT side in any way. > > Yes, they are software concepts and I'm not aware of any of it being > exposed in DT. A display controller is usually implemented as DRM CRTC > object and outputs (DSI, eDP, HDMI) as encoder (often with a connector > tied to it). Ok. Thanks for the clarifications. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 12:53, Thierry Reding wrote: >> Yes, but in this case we know of existing boards that have complex >> setups. It's not theoretical. > > Complex setups involving /this particular/ bridge and binding are > theoretical at this point, however. Right, but this discussion, at least from my part, has not so much been about this particular bridge, but bridges in general. So I don't have any requirements for this bridge driver/bindings to support complex use cases at the time being. But I do want us to have an option to use the bridge in the future in such complex case. And yes, we can't guarantee that we'd hit the bindings right whatever we do, but we should think about it and at least try. >>> phandles should simply point to the next element in the pipeline and the >>> OS abstractions should be good enough to handle the details about how to >>> chain the elements. >> >> I, on the other hand, would rather see the links the other way around. >> Panel having a link to the video source, etc. > > Why? It seems much more natural to describe this using the natural flow > of data. The device furthest away from the CPU should be the target of > the phandle. That way the DT can be used to trace the flow of data down > the pipeline. Because I see video components "using" their video sources. A panel uses an encoder to produce video data for the panel. An encoder uses its video source to produce video data. A bit like a device uses a gpio, or a pwm. Especially with more complex encoders/panels having the panel/encoder in control of its video source is often the easiest (only?) way to manage the job. This has worked well on OMAP, whereas the earlier model where the video source controlled the video sink was full of problems. Not that that exactly proves anything, but that's my experience, and I didn't find any easy solutions when the video source was in control. So while the video data flows from SoC to the panel, the control goes the other way. Whether the DT links should model the video data or control, I don't know. Both feel natural to me. But if a panel driver controls its video source, it makes sense for the panel driver to get its video source in its probe, and that happens easiest if the panel has a link to the video source. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tuesday 23 September 2014 13:47:40 Andrzej Hajda wrote: > On 09/23/2014 01:23 PM, Laurent Pinchart wrote: > > On Tuesday 23 September 2014 13:18:30 Andrzej Hajda wrote: > >> On 09/23/2014 01:10 PM, Laurent Pinchart wrote: > >>> On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: > On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > > On 23/09/14 09:21, Thierry Reding wrote: > >>> Well, I can write almost any kind of bindings, and then evidently my > >>> device would work. For me, on my board. > >> > >> Well, that's the whole problem with DT. For many devices we only have > >> a single setup to test against. And even when we have several they > >> often are derived from each other. But the alternative would be to > >> defer (possibly indefinitely) merging support for a device until a > >> second, wildly different setup shows up. That's completely > >> unreasonable and we need to start somewhere. > > > > Yes, but in this case we know of existing boards that have complex > > setups. It's not theoretical. > > > > I'm not saying we should stop everything until we have a 100% solution > > for the rare complex cases. But we should keep them in mind and, when > > possible, solve problems in a way that will work for the complex cases > > also. > > > >>> I guess non-video devices haven't had need for those. I have had > >>> lots of boards with video setup that cannot be represented with > >>> simple phandles. I'm not sure if I have just been unlucky or what, > >>> but my understand is that other people have encountered such boards > >>> also. Usually the problems encountered there have been circumvented > >>> with some hacky video driver for that specific board, or maybe a > >>> static configuration handled by the boot loader. > >> > >> I have yet to encounter such a setup. Can you point me at a DTS for > >> one such setup? I do remember a couple of hypothetical cases being > >> discussed at one time or another, but I haven't seen any actual DTS > >> content where this was needed. > > > > No, I can't point to them as they are not in the mainline (at least > > the ones I've been working on), for obvious reasons. > > > > With a quick glance, I have the following devices in my cabinet that > > have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > > EVM. Many Nokia devices used to have such setups, usually so that the > > LCD and tv-out were connected to the same video source. > > > >>> Do we have a standard way of representing the video pipeline with > >>> simple phandles? Or does everyone just do their own version? If > >>> there's no standard way, it sounds it'll be a mess to support in the > >>> future. > >> > >> It doesn't matter all that much whether the representation is > >> standard. > > > > Again, I disagree. > > > >> phandles should simply point to the next element in the pipeline and > >> the OS abstractions should be good enough to handle the details about > >> how to chain the elements. > > > > I, on the other hand, would rather see the links the other way around. > > Panel having a link to the video source, etc. > > > > The video graphs have two-way links, which of course is the safest > > options, but also more verbose and redundant. > > > > When this was discussed earlier, it was unclear which way the links > > should be. It's true that only links to one direction are strictly > > needed, but the question raised was that if in the drivers we end up > > always going the links the other way, the performance penalty may be > > somewhat big. (If I recall right). > > I do not see why performance may drop significantly? > If the link is one-way it should probably work as below: > - the destination registers itself in some framework, > - the source looks for the destination in this framework using phandle, > - the source starts to communicate with the destination - since now > full two way link can be established dynamically. > > Where do you see here big performance penalty? > >>> > >>> The performance-related problems arise when you need to locate the > >>> remote device in the direction opposite to the phandle link direction. > >>> Traversing a link forward just involves a phandle lookup, but traversing > >>> it backwards isn't possible the same way. > >> > >> But you do not need to traverse backwards. You just wait when the source > >> start to communicate with the destination, at this moment destination can > >> build back-link dynamically. > > > > Your driver might not need it today for your use cases, but can you be > > certain that no driver on any OS would need to ? > > I have just showed how to create back-link dynamically if we have only > forward-lin
DT property to selectively disable device features (was [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties)
Hi Thierry, On Tuesday 23 September 2014 07:55:34 Thierry Reding wrote: > On Tue, Sep 23, 2014 at 03:00:37AM +0300, Laurent Pinchart wrote: > > On Monday 22 September 2014 13:35:15 Thierry Reding wrote: > >> On Mon, Sep 22, 2014 at 04:53:22PM +0530, Ajay kumar wrote: > >>> On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding wrote: > On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: [snip] > I think you misunderstood what I said, or maybe I didn't explain > clearly what I meant. If the PS8622 provides a backlight there's > nothing wrong with always exposing it. The bridge itself isn't going > to be using the backlight anyway. Rather the panel itself should be > using the backlight device exposed by PS8622 or some separate > backlight device. > > To illustrate by an example: > ps8622: ... { > compatible = "parade,ps8622"; > ... > }; > > panel { > ... > backlight = <&ps8622>; > ... > }; > >>> > >>> No, if ps8622 backlight control is used, we need not specify the > >>> backlight phandle for the panel driver. Somehow, ps8622 internal > >>> circuitry keeps the bootup glitch free :) > >>> Backlight control and panel controls can be separate then. > >> > >> But they shouldn't. Your panel driver should always be the one to > >> control backlight. How else is the bridge supposed to know when to turn > >> backlight on or off? > >> > What you did in v6 of this series was look up a backlight device and > then not use it. That seemed unnecessary. Looking at v6 again the > reason for getting a phandle to the backlight was so that the device > itself did not expose its own backlight controlling circuitry if an > external one was being used. But since the bridge has no business > controlling the backlight, having the backlight phandle in the > bridge is not correct. > > So I think what you could do in the driver instead is always expose > the backlight device. If the panel used a different backlight, the > PS8622's internal on simply wouldn't be accessed. It would still be > possible to control the backlight in sysfs, but that shouldn't be a > problem (only root can access it) > >>> > >>> That would be like simple exposing a feature which cannot be used > >>> by the user, ideally which "should not be" used by the user. > >> > >> And it won't be used unless they access the sysfs files directly. There > >> are a lot of cases where we expose functionality that cannot be > >> meaningfully used by the user. For example, a GPIO may not be routed to > >> anything on a board, yet we don't explicitly hide any specific GPIOs > >> from users. > >> > That said, I have no strong objections to the boolean property if > you feel like it's really necessary. > >>> > >>> Won't you think having a boolean property for an optional > >>> feature of the device, is better than all these? > >> > >> Like I said, I'm indifferent on the matter. I don't think it's necessary > >> to hide the backlight device, but if you want to, please feel free to do > >> so. > > > > DT typically use > > > > status = "disabled" > > > > to disable devices. In this case we don't want to disable the ps8622 > > completely, but just one of its functions. Maybe a "backlight-status" > > property could be used for that ? If that's considered too verbose, I > > would be fine with a "disable-" boolean property too. > > Another alternative would be to make the backlight a subnode: > > ps8622: bridge@... { > compatible = "parade,ps8622"; > ... > > backlight { > ... > status = "disabled"; > ... > }; > }; I've thought about that as well. I feel it's getting a bit complex for little added benefit, but I don't have a very strong opinion. Devices that expose several functions are not the odd case, and the need to selectively mark some of them as disabled in DT seems pretty common to me. I wonder if we should try to decide on standard bindings for that. To recap the proposals, we could have - a boolean "disable-" property - a text "-status" property - a "" subnode with a "status" text property Anything else ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 01:23 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 13:18:30 Andrzej Hajda wrote: >> On 09/23/2014 01:10 PM, Laurent Pinchart wrote: >>> On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > On 23/09/14 09:21, Thierry Reding wrote: >>> Well, I can write almost any kind of bindings, and then evidently my >>> device would work. For me, on my board. >> Well, that's the whole problem with DT. For many devices we only have a >> single setup to test against. And even when we have several they often >> are derived from each other. But the alternative would be to defer >> (possibly indefinitely) merging support for a device until a second, >> wildly different setup shows up. That's completely unreasonable and we >> need to start somewhere. > Yes, but in this case we know of existing boards that have complex > setups. It's not theoretical. > > I'm not saying we should stop everything until we have a 100% solution > for the rare complex cases. But we should keep them in mind and, when > possible, solve problems in a way that will work for the complex cases > also. > >>> I guess non-video devices haven't had need for those. I have had lots >>> of boards with video setup that cannot be represented with simple >>> phandles. I'm not sure if I have just been unlucky or what, but my >>> understand is that other people have encountered such boards also. >>> Usually the problems encountered there have been circumvented with >>> some hacky video driver for that specific board, or maybe a static >>> configuration handled by the boot loader. >> I have yet to encounter such a setup. Can you point me at a DTS for one >> such setup? I do remember a couple of hypothetical cases being >> discussed at one time or another, but I haven't seen any actual DTS >> content where this was needed. > No, I can't point to them as they are not in the mainline (at least the > ones I've been working on), for obvious reasons. > > With a quick glance, I have the following devices in my cabinet that > have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > EVM. Many Nokia devices used to have such setups, usually so that the > LCD and tv-out were connected to the same video source. > >>> Do we have a standard way of representing the video pipeline with >>> simple phandles? Or does everyone just do their own version? If >>> there's no standard way, it sounds it'll be a mess to support in the >>> future. >> It doesn't matter all that much whether the representation is standard. > Again, I disagree. > >> phandles should simply point to the next element in the pipeline and >> the OS abstractions should be good enough to handle the details about >> how to chain the elements. > I, on the other hand, would rather see the links the other way around. > Panel having a link to the video source, etc. > > The video graphs have two-way links, which of course is the safest > options, but also more verbose and redundant. > > When this was discussed earlier, it was unclear which way the links > should be. It's true that only links to one direction are strictly > needed, but the question raised was that if in the drivers we end up > always going the links the other way, the performance penalty may be > somewhat big. (If I recall right). I do not see why performance may drop significantly? If the link is one-way it should probably work as below: - the destination registers itself in some framework, - the source looks for the destination in this framework using phandle, - the source starts to communicate with the destination - since now full two way link can be established dynamically. Where do you see here big performance penalty? >>> The performance-related problems arise when you need to locate the remote >>> device in the direction opposite to the phandle link direction. Traversing >>> a link forward just involves a phandle lookup, but traversing it >>> backwards isn't possible the same way. >> But you do not need to traverse backwards. You just wait when the source >> start to communicate with the destination, at this moment destination can >> build back-link dynamically. > Your driver might not need it today for your use cases, but can you be > certain > that no driver on any OS would need to ? I have just showed how to create back-link dynamically if we have only forward-link in DT. Ie it is a trivial 'proof' that the direction is not so important. So I do not understand why do you pose such question? > > This becomes an issue even on Linux when considering video-related devices > that can be part of either a capture pipeline or a display pipeline. If the > link always goes in the data flow dir
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
Hi Thierry, On Tuesday 23 September 2014 12:10:33 Thierry Reding wrote: > On Tue, Sep 23, 2014 at 11:43:47AM +0200, Andrzej Hajda wrote: > > On 09/23/2014 10:35 AM, Thierry Reding wrote: > [...] > > > > But I agree that it would be nice to unify bridges and encoders more. It > > > should be possible to make encoder always a bridge (or perhaps even > > > replace encoders with bridges altogether). Then once you're out of the > > > DRM device everything would be a bridge until you get to a panel. > > > > I agree that some sort of unification of bridges, (slave) encoders is a > > good thing, this way we stay only with bridges and panels as receivers.> > > But we will still have to deal with the code like: > > > > if (on the other end of the link is panel) > > do panel framework specific things > > else > > do bridge framework specific things > > > > The code in both branches usually does similar things but due to framework > > differences it is difficult to merge. > > That's because they are inherently different entities. You can perform > operations on a panel that don't make sense for a bridge and vice-versa. But a subset of the operations are common, so it's annoying to have to explicitly deal with both objects in code that only cares about the common subset of operations. > > Ideally it would be best to get rid of such constructs. For example by > > trying to make frameworks per interface type exposed by device (eg. > > video_sink) and not by device type (drm_bridge, drm_panel). > > But then you loose all information about the real type of device. Or you > have to create a common base class. And then you're still back to dealing > with the two specific cases in many places, so the gain seems rather > minimal. We would need to experiment with the concept, but a single abstract class works surprisingly well in V4L. Devices as diverse as camera sensors, HDMI receivers or video scalers expose a single API, and drivers for the logical capture devices (roughly equivalent to the DRM display controller drivers) turned out not to need much knowledge about what the connected devices really are in order to use them. This was implemented with a superset of operations, with many of them being optional to implement for component drivers. Of course knowing the exact type of a component can still be sometimes useful, but that would be pretty easy to implement through a query operation exposed by the components. We're digressing here, but my point is that the encoder/bridge merge that we seem to agree about should, in my opinion, be followed by a merge with panels. I am, however, more interested at this point by the encoder/bridge merge, as having two separate frameworks there is the biggest pain point for my use cases. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 12:39, Thierry Reding wrote: > My point is that if you use plain phandles you usually have the > meta-data already. Referring to the above example, bridge0 knows that it > should look for a bridge with phandle &bridge1, whereas bridge1 knows > that the device it is connected to is a panel. The bridge should not care what kind of device is there on the other end. The bridge just has an output, going to another video component. >> Well, I can't say about this particular bridge, but afaik you can >> connect a parallel RGB signal to multiple panels just like that, without >> any muxing. > > Right, but in that case you're not reconfiguring the signal in any way > for each of the panels. You send one single signal to all of them. For Yes, that's one use case, cloning. But I was not talking about that. > all intents and purposes there is only one panel. Well, I guess you > could have separate backlights for the panels. In that case though it > seems better to represent it at least as a virtual mux or bridge, or > perhaps a "mux panel". I was talking about the case where you have two totally different devices, let's say panels, connected to the same output. One could take a 16-bit parallel RGB signal, the other 24-bit. Only one of them can be enabled at a time (from HW perspective both can be enabled at the same time, but then the other one shows garbage). Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tuesday 23 September 2014 13:18:30 Andrzej Hajda wrote: > On 09/23/2014 01:10 PM, Laurent Pinchart wrote: > > On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: > >> On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > >>> On 23/09/14 09:21, Thierry Reding wrote: > > Well, I can write almost any kind of bindings, and then evidently my > > device would work. For me, on my board. > > Well, that's the whole problem with DT. For many devices we only have a > single setup to test against. And even when we have several they often > are derived from each other. But the alternative would be to defer > (possibly indefinitely) merging support for a device until a second, > wildly different setup shows up. That's completely unreasonable and we > need to start somewhere. > >>> > >>> Yes, but in this case we know of existing boards that have complex > >>> setups. It's not theoretical. > >>> > >>> I'm not saying we should stop everything until we have a 100% solution > >>> for the rare complex cases. But we should keep them in mind and, when > >>> possible, solve problems in a way that will work for the complex cases > >>> also. > >>> > > I guess non-video devices haven't had need for those. I have had lots > > of boards with video setup that cannot be represented with simple > > phandles. I'm not sure if I have just been unlucky or what, but my > > understand is that other people have encountered such boards also. > > Usually the problems encountered there have been circumvented with > > some hacky video driver for that specific board, or maybe a static > > configuration handled by the boot loader. > > I have yet to encounter such a setup. Can you point me at a DTS for one > such setup? I do remember a couple of hypothetical cases being > discussed at one time or another, but I haven't seen any actual DTS > content where this was needed. > >>> > >>> No, I can't point to them as they are not in the mainline (at least the > >>> ones I've been working on), for obvious reasons. > >>> > >>> With a quick glance, I have the following devices in my cabinet that > >>> have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > >>> EVM. Many Nokia devices used to have such setups, usually so that the > >>> LCD and tv-out were connected to the same video source. > >>> > > Do we have a standard way of representing the video pipeline with > > simple phandles? Or does everyone just do their own version? If > > there's no standard way, it sounds it'll be a mess to support in the > > future. > > It doesn't matter all that much whether the representation is standard. > >>> > >>> Again, I disagree. > >>> > phandles should simply point to the next element in the pipeline and > the OS abstractions should be good enough to handle the details about > how to chain the elements. > >>> > >>> I, on the other hand, would rather see the links the other way around. > >>> Panel having a link to the video source, etc. > >>> > >>> The video graphs have two-way links, which of course is the safest > >>> options, but also more verbose and redundant. > >>> > >>> When this was discussed earlier, it was unclear which way the links > >>> should be. It's true that only links to one direction are strictly > >>> needed, but the question raised was that if in the drivers we end up > >>> always going the links the other way, the performance penalty may be > >>> somewhat big. (If I recall right). > >> > >> I do not see why performance may drop significantly? > >> If the link is one-way it should probably work as below: > >> - the destination registers itself in some framework, > >> - the source looks for the destination in this framework using phandle, > >> - the source starts to communicate with the destination - since now full > >> two way link can be established dynamically. > >> > >> Where do you see here big performance penalty? > > > > The performance-related problems arise when you need to locate the remote > > device in the direction opposite to the phandle link direction. Traversing > > a link forward just involves a phandle lookup, but traversing it > > backwards isn't possible the same way. > > But you do not need to traverse backwards. You just wait when the source > start to communicate with the destination, at this moment destination can > build back-link dynamically. Your driver might not need it today for your use cases, but can you be certain that no driver on any OS would need to ? This becomes an issue even on Linux when considering video-related devices that can be part of either a capture pipeline or a display pipeline. If the link always goes in the data flow direction, then it will be easy to locate the downstream device (bridge or panel) from the display controller driver, but it would be much more difficult to locate the same device from a camera driver as al
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 01:10 PM, Laurent Pinchart wrote: > On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: >> On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: >>> On 23/09/14 09:21, Thierry Reding wrote: > Well, I can write almost any kind of bindings, and then evidently my > device would work. For me, on my board. Well, that's the whole problem with DT. For many devices we only have a single setup to test against. And even when we have several they often are derived from each other. But the alternative would be to defer (possibly indefinitely) merging support for a device until a second, wildly different setup shows up. That's completely unreasonable and we need to start somewhere. >>> Yes, but in this case we know of existing boards that have complex >>> setups. It's not theoretical. >>> >>> I'm not saying we should stop everything until we have a 100% solution >>> for the rare complex cases. But we should keep them in mind and, when >>> possible, solve problems in a way that will work for the complex cases >>> also.> > I guess non-video devices haven't had need for those. I have had lots of > boards with video setup that cannot be represented with simple phandles. > I'm not sure if I have just been unlucky or what, but my understand is > that other people have encountered such boards also. Usually the > problems encountered there have been circumvented with some hacky video > driver for that specific board, or maybe a static configuration handled > by the boot loader. I have yet to encounter such a setup. Can you point me at a DTS for one such setup? I do remember a couple of hypothetical cases being discussed at one time or another, but I haven't seen any actual DTS content where this was needed. >>> No, I can't point to them as they are not in the mainline (at least the >>> ones I've been working on), for obvious reasons. >>> >>> With a quick glance, I have the following devices in my cabinet that >>> have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx >>> EVM. Many Nokia devices used to have such setups, usually so that the >>> LCD and tv-out were connected to the same video source. >>> > Do we have a standard way of representing the video pipeline with simple > phandles? Or does everyone just do their own version? If there's no > standard way, it sounds it'll be a mess to support in the future. It doesn't matter all that much whether the representation is standard. >>> Again, I disagree. >>> phandles should simply point to the next element in the pipeline and the OS abstractions should be good enough to handle the details about how to chain the elements. >>> I, on the other hand, would rather see the links the other way around. >>> Panel having a link to the video source, etc. >>> >>> The video graphs have two-way links, which of course is the safest >>> options, but also more verbose and redundant. >>> >>> When this was discussed earlier, it was unclear which way the links >>> should be. It's true that only links to one direction are strictly >>> needed, but the question raised was that if in the drivers we end up >>> always going the links the other way, the performance penalty may be >>> somewhat big. (If I recall right). >> I do not see why performance may drop significantly? >> If the link is one-way it should probably work as below: >> - the destination registers itself in some framework, >> - the source looks for the destination in this framework using phandle, >> - the source starts to communicate with the destination - since now full >> two way link can be established dynamically. >> >> Where do you see here big performance penalty? > The performance-related problems arise when you need to locate the remote > device in the direction opposite to the phandle link direction. Traversing a > link forward just involves a phandle lookup, but traversing it backwards > isn't > possible the same way. > But you do not need to traverse backwards. You just wait when the source start to communicate with the destination, at this moment destination can build back-link dynamically. Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 12:28, Thierry Reding wrote: >> But, for example, let's say that the board is designed in a way that for >> panel0 the bridge needs to output a bit higher level voltages than for >> panel1. That's not a property of the panel, so it can't be queried from >> the panel. >> >> That feature (varying the voltage level) is specific to the bridge, and >> thus the properties should be in the bridge's DT data. So we need two >> different configurations in the bridge, one for panel0 and one for >> panel1. This is what endpoints give us. > > You could get the same by moving the mux in front of the bridge instead. I didn't understand. Moving the mux between the SoC and the bridge? How would that solve the problem. Or what do you mean with "in the front of the bridge"? > Trying to do this within the bridge's node directly has two flaws: > > 1) It violates the DT principle of describing hardware. The > device itself does not know anything about multiple streams > and deals only with a single input and output. You'd be > describing a board specific aspect in the device binding. We _are_ describing board specific aspects in the board.dts file. That's what it is about. If the board's hardware is such that the bridge's voltage level needs to be higher when using panel0, that's very much describing hardware. > 2) Such a solution would have to be implemented for all bridge > devices that can potentially be muxed (really all of them). > If you describe it accurately in a separate mux node you get > genericity for free and it will work for all bridges. I do agree that a generic mux is better if there's nothing special to be configured. But how do you use a generic mux node for bridge device specific features? Well, I think it'd be great to have all bindings use ports/endpoints (or a standard simpler representation), and have good helpers for those. Then, the bridge driver would not need to know or care about endpoints as such, it would just be told that this port & endpoint should be enabled, and the bridge driver would get its configuration for that endpoint, which it would program to the HW. And the same would happen with or without complex video graph. But that's not strictly required, the bridge driver could as well support only single configuration. When someone uses the bridge in a setup that requires multiple endpoints, he can then extend the driver to support multiple endpoints. >> I disagree. If we don't have a common way to describe the connections >> between video devices, we cannot: >> >> - have a common helper library to parse the connections > > We have a common helper already. It's called of_parse_phandle(). Yes, but what is the name of the property, and where does it go? Does the panel have a phandle to the bridge? Does the bridge have a phandle to the panel? >> - study the connections before the drivers are loaded > > Why would you need that? I think this was discussed earlier, maybe Laurent remembers the use cases. I think one possibility here is to have an understanding of the pipelines even if the modules have not been loaded or a single device has failed to probe. >> - handle the connections anywhere else than the specific driver for the >> component > > Why should we do that? We could, for example, have a single component (common or SoC specific) that manages the video pipelines. The drivers for the encoders/bridges/panels would just probe the devices, without doing anything else. The master component would then parse the links and connect the devices in whatever way it sees best. >>> While there are probably legitimate cases where the video graph is >>> useful and makes sense, in many cases terms like ports and endpoints are >>> simply confusing. >> >> I again agree that I'd like to have a simpler representation than video >> graphs for the simpler use cases. But again, I think it's important to >> have a standard representation for those connections. >> >> Why do you think it wouldn't be good to have a standard for the simpler >> connections (in addition to the video graphs)? What kind of device >> specific variations do you see that would be needed? > > What do you mean by standard? I agree that it would make sense to give > properties standard names, but I don't think we need to go any further. Standard names and standard direction. I don't think there's anything else to simple phandles. > I mean, in the case of a simple phandle there's not much more to it > anyway. But I fail to understand why standard properties should be a > hard requirement. And I fail to see why they should not be a hard requirement =). > Having a standard representation only matters if you want to be able to > parse the pipeline without knowing about the device specifics. But I'm > not sure I understand why you would want to do that. This sounds like > you'd want some generic code to be able to construct a pipeline. But at > th
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tuesday 23 September 2014 11:53:15 Thierry Reding wrote: > On Tue, Sep 23, 2014 at 12:30:20PM +0300, Tomi Valkeinen wrote: > > On 23/09/14 09:21, Thierry Reding wrote: > > >> Well, I can write almost any kind of bindings, and then evidently my > > >> device would work. For me, on my board. > > > > > > Well, that's the whole problem with DT. For many devices we only have a > > > single setup to test against. And even when we have several they often > > > are derived from each other. But the alternative would be to defer > > > (possibly indefinitely) merging support for a device until a second, > > > wildly different setup shows up. That's completely unreasonable and we > > > need to start somewhere. > > > > Yes, but in this case we know of existing boards that have complex > > setups. It's not theoretical. > > Complex setups involving /this particular/ bridge and binding are > theoretical at this point, however. > > > > phandles should simply point to the next element in the pipeline and the > > > OS abstractions should be good enough to handle the details about how to > > > chain the elements. > > > > I, on the other hand, would rather see the links the other way around. > > Panel having a link to the video source, etc. > > Why? It seems much more natural to describe this using the natural flow > of data. The device furthest away from the CPU should be the target of > the phandle. That way the DT can be used to trace the flow of data down > the pipeline. > > > The video graphs have two-way links, which of course is the safest > > options, but also more verbose and redundant. > > Right. If we take that line of thinking to the extreme we end up listing > individual registers in DT so that we can safely assume that we can > control all possible aspects of the device. And the other extreme would be to have a single DT node for the logical video device with all information pertaining to it stored in C code. Extremes are never good, we need to find a reasonable middle-ground here. I believe OF graph fulfills that requirement, it might be slightly more verbose than a single phandle, but it's also much more versatile. > Like I said, this seems to be the latest response to DT ABI stability > requirement. Add as much data to a DT node as possible so that it has > higher chances of being complete. The result is often overly complex DT > content that doesn't add any real value. > > > When this was discussed earlier, it was unclear which way the links > > should be. It's true that only links to one direction are strictly > > needed, but the question raised was that if in the drivers we end up > > always going the links the other way, the performance penalty may be > > somewhat big. (If I recall right). > > I doubt that graphs will become so complex that walking it would become > a performance bottleneck. In fact if we're constantly walking the graph > we're already doing something wrong. It should only be necessary when > the pipeline configuration changes, which should hopefully not be very > often (i.e. not several times per second). -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tuesday 23 September 2014 12:02:45 Andrzej Hajda wrote: > On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > > On 23/09/14 09:21, Thierry Reding wrote: > >>> Well, I can write almost any kind of bindings, and then evidently my > >>> device would work. For me, on my board. > >> > >> Well, that's the whole problem with DT. For many devices we only have a > >> single setup to test against. And even when we have several they often > >> are derived from each other. But the alternative would be to defer > >> (possibly indefinitely) merging support for a device until a second, > >> wildly different setup shows up. That's completely unreasonable and we > >> need to start somewhere. > > > > Yes, but in this case we know of existing boards that have complex > > setups. It's not theoretical. > > > > I'm not saying we should stop everything until we have a 100% solution > > for the rare complex cases. But we should keep them in mind and, when > > possible, solve problems in a way that will work for the complex cases > > also.> > >>> I guess non-video devices haven't had need for those. I have had lots of > >>> boards with video setup that cannot be represented with simple phandles. > >>> I'm not sure if I have just been unlucky or what, but my understand is > >>> that other people have encountered such boards also. Usually the > >>> problems encountered there have been circumvented with some hacky video > >>> driver for that specific board, or maybe a static configuration handled > >>> by the boot loader. > >> > >> I have yet to encounter such a setup. Can you point me at a DTS for one > >> such setup? I do remember a couple of hypothetical cases being discussed > >> at one time or another, but I haven't seen any actual DTS content where > >> this was needed. > > > > No, I can't point to them as they are not in the mainline (at least the > > ones I've been working on), for obvious reasons. > > > > With a quick glance, I have the following devices in my cabinet that > > have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > > EVM. Many Nokia devices used to have such setups, usually so that the > > LCD and tv-out were connected to the same video source. > > > >>> Do we have a standard way of representing the video pipeline with simple > >>> phandles? Or does everyone just do their own version? If there's no > >>> standard way, it sounds it'll be a mess to support in the future. > >> > >> It doesn't matter all that much whether the representation is standard. > > > > Again, I disagree. > > > >> phandles should simply point to the next element in the pipeline and the > >> OS abstractions should be good enough to handle the details about how to > >> chain the elements. > > > > I, on the other hand, would rather see the links the other way around. > > Panel having a link to the video source, etc. > > > > The video graphs have two-way links, which of course is the safest > > options, but also more verbose and redundant. > > > > When this was discussed earlier, it was unclear which way the links > > should be. It's true that only links to one direction are strictly > > needed, but the question raised was that if in the drivers we end up > > always going the links the other way, the performance penalty may be > > somewhat big. (If I recall right). > > I do not see why performance may drop significantly? > If the link is one-way it should probably work as below: > - the destination registers itself in some framework, > - the source looks for the destination in this framework using phandle, > - the source starts to communicate with the destination - since now full > two way link can be established dynamically. > > Where do you see here big performance penalty? The performance-related problems arise when you need to locate the remote device in the direction opposite to the phandle link direction. Traversing a link forward just involves a phandle lookup, but traversing it backwards isn't possible the same way. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 12:10 PM, Thierry Reding wrote: > On Tue, Sep 23, 2014 at 11:43:47AM +0200, Andrzej Hajda wrote: >> On 09/23/2014 10:35 AM, Thierry Reding wrote: > [...] >>> But I agree that it would be nice to unify bridges and encoders more. It >>> should be possible to make encoder always a bridge (or perhaps even >>> replace encoders with bridges altogether). Then once you're out of the >>> DRM device everything would be a bridge until you get to a panel. >> I agree that some sort of unification of bridges, (slave) encoders is a good >> thing, this way we stay only with bridges and panels as receivers. >> But we will still have to deal with the code like: >> if (on the other end of the link is panel) >> do panel framework specific things >> else >> do bridge framework specific things >> >> The code in both branches usually does similar things but due to framework >> differences it is difficult to merge. > That's because they are inherently different entities. You can perform > operations on a panel that don't make sense for a bridge and vice-versa. > >> Ideally it would be best to get rid of such constructs. For example by >> trying to >> make frameworks per interface type exposed by device (eg. video_sink) and >> not by device type (drm_bridge, drm_panel). > But then you loose all information about the real type of device. Driver knows type of its device anyway. Why should it know device type of devices it is connected to? It is enough it knows how to talk to other device. Like in real world, why desktop PC should know it is connected to DVI monitor or to DVI/HDMI converter which is connected to TV? > Or you > have to create a common base class. And then you're still back to > dealing with the two specific cases in many places, so the gain seems > rather minimal. For example RGB panel and RGB/??? bridge have the same hardware input interface. Why shall they have different representation in kernel? Regards Andrzej > > Thierry -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices
Hi Dong, On Monday, September 22, 2014, Dong Aisheng wrote, > On Mon, Sep 22, 2014 at 10:10:07AM +0530, Pankaj Dubey wrote: [snip] > > -static int syscon_match_node(struct device *dev, void *data) > > +static struct syscon *of_syscon_register(struct device_node *np) > > { > > - struct device_node *dn = data; > > + struct platform_device *pdev = NULL; > > + struct syscon *syscon; > > + struct regmap *regmap; > > + void __iomem *base; > > + int ret; > > + > > + if (!of_device_is_compatible(np, "syscon")) > > + return ERR_PTR(-EINVAL); > > + > > + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL); > > + if (!syscon) > > + return ERR_PTR(-ENOMEM); > > + > > + base = of_iomap(np, 0); > > + if (!base) { > > + ret = -ENOMEM; > > + goto err_map; > > + } > > + > > + /* if device is already populated and available then use it */ > > + pdev = of_find_device_by_node(np); > > + if (!pdev) { > > + /* for early users create dummy syscon device and use it */ > > + pdev = platform_device_alloc("dummy-syscon", -1); > > Can we create specific devices for each syscon device? > That looks more reasonable to me. > Then we can dump registers in regmap debugfs more clearly, just like other devices. > And i think it's better to follow device tree way to generate devices from node. > If DT core find a device is already created, it can ignore that device to avoid creating > duplicated device during of_platform_populate. > If you are referring to use "of_platform_device_create", then I am afraid that it can't be used in very early stage. For example, current patch I tested by calling syscon_lookup_by_phandle from "init_irq" hook of machine_desc, and it worked well, but If I use "of_platform_device_create" instead of dummy device, it fails to create pdev and this I verified on Exynos platform. The reason I selected "init_irq" is because this comes just before smp init and where once we tried to get regmap handle, but could not do so. Also if it was just a matter of creating platform_device at early stage then early initialization of syscon could have been solved till now. So I would suggest if we really want this patch to cover early initialization of syscon (which is not it's main purpose) current patch is sufficient. @Arnd, since I have addressed crash issue as reported by Dong Aisheng, I would like to take your ack, but since there are some more code changes other than what you suggested I request you to check this and if you are ok, then I would like to your ack so that I can request to maintainer for taking this patch. Thanks, Pankaj Dubey > Regards > Dong Aisheng > > > + if (!pdev) { > > + ret = -ENOMEM; > > + goto err_pdev; > > + } > > + pdev->dev.of_node = of_node_get(np); > > + } > > + > > + regmap = devm_regmap_init_mmio(&pdev->dev, base, > &syscon_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&pdev->dev, "regmap init failed\n"); > > + ret = PTR_ERR(regmap); > > + goto err_regmap; > > + } > > + > > + syscon->regmap = regmap; > > + syscon->np = np; > > > > - return (dev->of_node == dn) ? 1 : 0; > > + spin_lock(&syscon_list_slock); > > + list_add_tail(&syscon->list, &syscon_list); > > + spin_unlock(&syscon_list_slock); > > + > > + return syscon; > > + > > +err_regmap: > > + if (!strcmp(pdev->name, "dummy-syscon")) { > > + of_node_put(np); > > + platform_device_put(pdev); > > + } > > +err_pdev: > > + iounmap(base); > > +err_map: > > + kfree(syscon); > > + return ERR_PTR(ret); > > } > > > > struct regmap *syscon_node_to_regmap(struct device_node *np) { > > - struct syscon *syscon; > > - struct device *dev; > > + struct syscon *entry, *syscon = NULL; > > > > - dev = driver_find_device(&syscon_driver.driver, NULL, np, > > -syscon_match_node); > > - if (!dev) > > - return ERR_PTR(-EPROBE_DEFER); > > + spin_lock(&syscon_list_slock); > > > > - syscon = dev_get_drvdata(dev); > > + list_for_each_entry(entry, &syscon_list, list) > > + if (entry->np == np) { > > + syscon = entry; > > + break; > > + } > > + > > + spin_unlock(&syscon_list_slock); > > + > > + if (!syscon) > > + syscon = of_syscon_register(np); > > + > > + if (IS_ERR(syscon)) > > + return ERR_CAST(syscon); > > > > return syscon->regmap; > > } > > @@ -110,17 +185,6 @@ struct regmap > > *syscon_regmap_lookup_by_phandle(struct device_node *np, } > > EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle); > > > > -static const struct of_device_id of_syscon_match[] = { > > - { .compatible = "syscon", }, > > - { }, > > -}; > > - > > -static struct regmap_config syscon_regmap_config = { > > - .reg_bits = 32, > > - .val_bits = 32, > > - .reg_stride = 4, > > -}; > > - > > static
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 11:43:47AM +0200, Andrzej Hajda wrote: > On 09/23/2014 10:35 AM, Thierry Reding wrote: [...] > > But I agree that it would be nice to unify bridges and encoders more. It > > should be possible to make encoder always a bridge (or perhaps even > > replace encoders with bridges altogether). Then once you're out of the > > DRM device everything would be a bridge until you get to a panel. > > I agree that some sort of unification of bridges, (slave) encoders is a good > thing, this way we stay only with bridges and panels as receivers. > But we will still have to deal with the code like: > if (on the other end of the link is panel) > do panel framework specific things > else > do bridge framework specific things > > The code in both branches usually does similar things but due to framework > differences it is difficult to merge. That's because they are inherently different entities. You can perform operations on a panel that don't make sense for a bridge and vice-versa. > Ideally it would be best to get rid of such constructs. For example by > trying to > make frameworks per interface type exposed by device (eg. video_sink) and > not by device type (drm_bridge, drm_panel). But then you loose all information about the real type of device. Or you have to create a common base class. And then you're still back to dealing with the two specific cases in many places, so the gain seems rather minimal. Thierry pgpQXCfg46tQu.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > On 23/09/14 09:21, Thierry Reding wrote: > >>> Well, I can write almost any kind of bindings, and then evidently my >>> device would work. For me, on my board. >> >> Well, that's the whole problem with DT. For many devices we only have a >> single setup to test against. And even when we have several they often >> are derived from each other. But the alternative would be to defer >> (possibly indefinitely) merging support for a device until a second, >> wildly different setup shows up. That's completely unreasonable and we >> need to start somewhere. > > Yes, but in this case we know of existing boards that have complex > setups. It's not theoretical. > > I'm not saying we should stop everything until we have a 100% solution > for the rare complex cases. But we should keep them in mind and, when > possible, solve problems in a way that will work for the complex cases also. > >>> I guess non-video devices haven't had need for those. I have had lots of >>> boards with video setup that cannot be represented with simple phandles. >>> I'm not sure if I have just been unlucky or what, but my understand is >>> that other people have encountered such boards also. Usually the >>> problems encountered there have been circumvented with some hacky video >>> driver for that specific board, or maybe a static configuration handled >>> by the boot loader. >> >> I have yet to encounter such a setup. Can you point me at a DTS for one >> such setup? I do remember a couple of hypothetical cases being discussed >> at one time or another, but I haven't seen any actual DTS content where >> this was needed. > > No, I can't point to them as they are not in the mainline (at least the > ones I've been working on), for obvious reasons. > > With a quick glance, I have the following devices in my cabinet that > have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > EVM. Many Nokia devices used to have such setups, usually so that the > LCD and tv-out were connected to the same video source. > >>> Do we have a standard way of representing the video pipeline with simple >>> phandles? Or does everyone just do their own version? If there's no >>> standard way, it sounds it'll be a mess to support in the future. >> >> It doesn't matter all that much whether the representation is standard. > > Again, I disagree. > >> phandles should simply point to the next element in the pipeline and the >> OS abstractions should be good enough to handle the details about how to >> chain the elements. > > I, on the other hand, would rather see the links the other way around. > Panel having a link to the video source, etc. > > The video graphs have two-way links, which of course is the safest > options, but also more verbose and redundant. > > When this was discussed earlier, it was unclear which way the links > should be. It's true that only links to one direction are strictly > needed, but the question raised was that if in the drivers we end up > always going the links the other way, the performance penalty may be > somewhat big. (If I recall right). I do not see why performance may drop significantly? If the link is one-way it should probably work as below: - the destination registers itself in some framework, - the source looks for the destination in this framework using phandle, - the source starts to communicate with the destination - since now full two way link can be established dynamically. Where do you see here big performance penalty? Regards Andrzej > > Tomi > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 11:30 AM, Tomi Valkeinen wrote: > On 23/09/14 09:21, Thierry Reding wrote: > >>> Well, I can write almost any kind of bindings, and then evidently my >>> device would work. For me, on my board. >> >> Well, that's the whole problem with DT. For many devices we only have a >> single setup to test against. And even when we have several they often >> are derived from each other. But the alternative would be to defer >> (possibly indefinitely) merging support for a device until a second, >> wildly different setup shows up. That's completely unreasonable and we >> need to start somewhere. > > Yes, but in this case we know of existing boards that have complex > setups. It's not theoretical. > > I'm not saying we should stop everything until we have a 100% solution > for the rare complex cases. But we should keep them in mind and, when > possible, solve problems in a way that will work for the complex cases also. > >>> I guess non-video devices haven't had need for those. I have had lots of >>> boards with video setup that cannot be represented with simple phandles. >>> I'm not sure if I have just been unlucky or what, but my understand is >>> that other people have encountered such boards also. Usually the >>> problems encountered there have been circumvented with some hacky video >>> driver for that specific board, or maybe a static configuration handled >>> by the boot loader. >> >> I have yet to encounter such a setup. Can you point me at a DTS for one >> such setup? I do remember a couple of hypothetical cases being discussed >> at one time or another, but I haven't seen any actual DTS content where >> this was needed. > > No, I can't point to them as they are not in the mainline (at least the > ones I've been working on), for obvious reasons. > > With a quick glance, I have the following devices in my cabinet that > have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx > EVM. Many Nokia devices used to have such setups, usually so that the > LCD and tv-out were connected to the same video source. > >>> Do we have a standard way of representing the video pipeline with simple >>> phandles? Or does everyone just do their own version? If there's no >>> standard way, it sounds it'll be a mess to support in the future. >> >> It doesn't matter all that much whether the representation is standard. > > Again, I disagree. > >> phandles should simply point to the next element in the pipeline and the >> OS abstractions should be good enough to handle the details about how to >> chain the elements. > > I, on the other hand, would rather see the links the other way around. > Panel having a link to the video source, etc. > > The video graphs have two-way links, which of course is the safest > options, but also more verbose and redundant. > > When this was discussed earlier, it was unclear which way the links > should be. It's true that only links to one direction are strictly > needed, but the question raised was that if in the drivers we end up > always going the links the other way, the performance penalty may be > somewhat big. (If I recall right). I do not see why performance may drop significantly? If the link is one-way it should probably work as below: - the destination registers itself in some framework, - the source looks for the destination in this framework using phandle, - the source starts to communicate with the destination - since now full two way link can be established dynamically. Where do you see here big performance penalty? Regards Andrzej > > Tomi > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 12:40:24PM +0300, Tomi Valkeinen wrote: > On 23/09/14 11:35, Thierry Reding wrote: > > > Well, a display controller is never going to attach to a panel directly. > > With parallel RGB, that (almost) happens. There's voltage level shifting > probably in the middle, but I don't see anything else there. The level shifting could be considered an encoder. Anyway, I didn't mean physically attach to a panel but rather in DRM. You'll always need an encoder and connector before you go to the panel. > > But I agree that it would be nice to unify bridges and encoders more. It > > should be possible to make encoder always a bridge (or perhaps even > > replace encoders with bridges altogether). Then once you're out of the > > DRM device everything would be a bridge until you get to a panel. > > What exactly is a bridge and what is an encoder? Those are DRM > constructs, aren't they? Yes. I think bridges are mostly a superset of encoders. > As I see it, a video pipeline consist of a video source (display > controller usually), a chain of encoders (all of which may not be > strictly "encoders", they could be level shifters, muxes, ESD protection > devices or such), and either a display device like a panel or a > connector to outside world. Well, the panel itself is attached to a connector. The connector is really what userspace uses to control the output and indirectly the panel. > Am I right that in DRM world the encoder is the first device in the > display chain after the display controller, Yes. > and the next is a bridge? A bridge or a connector. Typically it would be a connector, but that's only if you don't have bridges in between. > That sounds totally artificial, and I hope we don't reflect that in the > DT side in any way. Yes, they are software concepts and I'm not aware of any of it being exposed in DT. A display controller is usually implemented as DRM CRTC object and outputs (DSI, eDP, HDMI) as encoder (often with a connector tied to it). Thierry pgp7W8l2H20js.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 12:30:20PM +0300, Tomi Valkeinen wrote: > On 23/09/14 09:21, Thierry Reding wrote: > > >> Well, I can write almost any kind of bindings, and then evidently my > >> device would work. For me, on my board. > > > > Well, that's the whole problem with DT. For many devices we only have a > > single setup to test against. And even when we have several they often > > are derived from each other. But the alternative would be to defer > > (possibly indefinitely) merging support for a device until a second, > > wildly different setup shows up. That's completely unreasonable and we > > need to start somewhere. > > Yes, but in this case we know of existing boards that have complex > setups. It's not theoretical. Complex setups involving /this particular/ bridge and binding are theoretical at this point, however. > > phandles should simply point to the next element in the pipeline and the > > OS abstractions should be good enough to handle the details about how to > > chain the elements. > > I, on the other hand, would rather see the links the other way around. > Panel having a link to the video source, etc. Why? It seems much more natural to describe this using the natural flow of data. The device furthest away from the CPU should be the target of the phandle. That way the DT can be used to trace the flow of data down the pipeline. > The video graphs have two-way links, which of course is the safest > options, but also more verbose and redundant. Right. If we take that line of thinking to the extreme we end up listing individual registers in DT so that we can safely assume that we can control all possible aspects of the device. Like I said, this seems to be the latest response to DT ABI stability requirement. Add as much data to a DT node as possible so that it has higher chances of being complete. The result is often overly complex DT content that doesn't add any real value. > When this was discussed earlier, it was unclear which way the links > should be. It's true that only links to one direction are strictly > needed, but the question raised was that if in the drivers we end up > always going the links the other way, the performance penalty may be > somewhat big. (If I recall right). I doubt that graphs will become so complex that walking it would become a performance bottleneck. In fact if we're constantly walking the graph we're already doing something wrong. It should only be necessary when the pipeline configuration changes, which should hopefully not be very often (i.e. not several times per second). Thierry pgp2ekOBx8Dal.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 09/23/2014 10:35 AM, Thierry Reding wrote: > On Tue, Sep 23, 2014 at 09:24:12AM +0200, Andrzej Hajda wrote: >> Hi Thierry, Tomi, >> >> On 09/23/2014 08:04 AM, Thierry Reding wrote: >>> On Mon, Sep 22, 2014 at 05:23:25PM +0300, Tomi Valkeinen wrote: On 22/09/14 11:06, Thierry Reding wrote: >>> Why do we need a complex graph when it can be handled using a simple >>> phandle? >> Maybe in your case you can handle it with simple phandle. Can you >> guarantee that it's enough for everyone, on all platforms? > Nobody can guarantee that. An interesting effect that DT ABI stability > has had is that people now try to come up with overly generic concepts > to describe devices in an attempt to make them more future-proof. I > don't think we're doing ourselves a favour with that approach. I kind of agree. However, there are boards that need a more complex approach than just a simple phandle. They are nothing new. So while it may be true that this specific encoder has never been used in such a complex way, and there's a chance that it never will, my comments are not really about this particular encoder. What I want is a standard way to describe the video components. If we have a standard complex one (video graphs) and a standard simple one (simple phandles), it's ok for me. >>> I certainly agree that it's useful to have standard ways to describe at >>> least various aspects. For example I think it would be useful to add >>> standard properties for a bridge's connections, such as "bridge" or >>> "panel" to allow bridge chaining and attaching them to panels. >>> >>> But I think that should be the end of it. Mandating anything other than >>> that will just complicate things and limit what people can do in the >>> binding. >>> >>> One of the disadvantages of the video graph bindings is that they are >>> overly vague in that they don't carry information about what type a >>> device is. Bindings then have to require additional meta-data, at which >>> point it's become far easier to describe things with a custom property >>> that already provides context. >> I think it is opposite, graph bindings should describe only the link and >> certainly not what type of device is behind the endpoint. The fact we >> may need that information is a disadvantage of Linux frameworks and >> should be corrected in Linux, not by adding Linux specific properties to >> DT. For example display controller should not care where its output goes >> to: panel, encoder, bridge, whatever. It should care only if the >> parameters of the link are agreed with the receiver. > Well, a display controller is never going to attach to a panel directly. Yes it is. For example Exynos Display Controller uses parallel RGB as its output format, so in case of DPI panels it is connected directly to the panel. I guess it is similar with other SoCs. > But I agree that it would be nice to unify bridges and encoders more. It > should be possible to make encoder always a bridge (or perhaps even > replace encoders with bridges altogether). Then once you're out of the > DRM device everything would be a bridge until you get to a panel. I agree that some sort of unification of bridges, (slave) encoders is a good thing, this way we stay only with bridges and panels as receivers. But we will still have to deal with the code like: if (on the other end of the link is panel) do panel framework specific things else do bridge framework specific things The code in both branches usually does similar things but due to framework differences it is difficult to merge. Ideally it would be best to get rid of such constructs. For example by trying to make frameworks per interface type exposed by device (eg. video_sink) and not by device type (drm_bridge, drm_panel). I have proposed it is possible by (ab)using drm_panel framework for DSI/LVDS bridge [1]. [1]: http://lists.freedesktop.org/archives/dri-devel/2014-February/053705.html Regards Andrzej > > I think we discussed this a while back in the context of bridge > chaining. > > Thierry -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 11:35, Thierry Reding wrote: > Well, a display controller is never going to attach to a panel directly. With parallel RGB, that (almost) happens. There's voltage level shifting probably in the middle, but I don't see anything else there. > But I agree that it would be nice to unify bridges and encoders more. It > should be possible to make encoder always a bridge (or perhaps even > replace encoders with bridges altogether). Then once you're out of the > DRM device everything would be a bridge until you get to a panel. What exactly is a bridge and what is an encoder? Those are DRM constructs, aren't they? As I see it, a video pipeline consist of a video source (display controller usually), a chain of encoders (all of which may not be strictly "encoders", they could be level shifters, muxes, ESD protection devices or such), and either a display device like a panel or a connector to outside world. Am I right that in DRM world the encoder is the first device in the display chain after the display controller, and the next is a bridge? That sounds totally artificial, and I hope we don't reflect that in the DT side in any way. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 11:54:27AM +0300, Tomi Valkeinen wrote: > On 23/09/14 09:04, Thierry Reding wrote: > > > I certainly agree that it's useful to have standard ways to describe at > > least various aspects. For example I think it would be useful to add > > standard properties for a bridge's connections, such as "bridge" or > > "panel" to allow bridge chaining and attaching them to panels. > > I don't see a need for such properties. Do you have examples where they > would be needed? > > The driver for the respective device does know if it's a bridge or a > panel, so that information is there as soon as the driver has loaded. These are used for chaining not identifying the device. For example: bridge0: ... { ... bridge = <&bridge1>; ... }; bridge1: ... { ... panel = <&panel>; ... }; panel: ... { ... }; > > But I think that should be the end of it. Mandating anything other than > > that will just complicate things and limit what people can do in the > > binding. > > > > One of the disadvantages of the video graph bindings is that they are > > overly vague in that they don't carry information about what type a > > device is. Bindings then have to require additional meta-data, at which > > point it's become far easier to describe things with a custom property > > that already provides context. > > I don't see why the graphs and such metadata are connected in any way. > They are separate issues. If we need such metadata, it needs to be added > in any case. That is not related to the graphs. My point is that if you use plain phandles you usually have the meta-data already. Referring to the above example, bridge0 knows that it should look for a bridge with phandle &bridge1, whereas bridge1 knows that the device it is connected to is a panel. > >> Yes, there's always one active input and one output for this bridge. > >> What the video graphs would bring is to have the possibility to have > >> multiple inputs and outputs, of which a single ones could be active at a > >> time. The different inputs and outputs could even have different > >> settings required (say, first output requires this bit set, but when > >> using second output that bit must be cleared). > > > > As discussed elsewhere this should be handled at a different level then. > > DT should describe the hardware and this particular bridge device simply > > doesn't have a means to connect more than a single input or more than a > > single output. > > Well, I can't say about this particular bridge, but afaik you can > connect a parallel RGB signal to multiple panels just like that, without > any muxing. Right, but in that case you're not reconfiguring the signal in any way for each of the panels. You send one single signal to all of them. For all intents and purposes there is only one panel. Well, I guess you could have separate backlights for the panels. In that case though it seems better to represent it at least as a virtual mux or bridge, or perhaps a "mux panel". Thierry pgp_yRoktfrfR.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 09:21, Thierry Reding wrote: >> Well, I can write almost any kind of bindings, and then evidently my >> device would work. For me, on my board. > > Well, that's the whole problem with DT. For many devices we only have a > single setup to test against. And even when we have several they often > are derived from each other. But the alternative would be to defer > (possibly indefinitely) merging support for a device until a second, > wildly different setup shows up. That's completely unreasonable and we > need to start somewhere. Yes, but in this case we know of existing boards that have complex setups. It's not theoretical. I'm not saying we should stop everything until we have a 100% solution for the rare complex cases. But we should keep them in mind and, when possible, solve problems in a way that will work for the complex cases also. >> I guess non-video devices haven't had need for those. I have had lots of >> boards with video setup that cannot be represented with simple phandles. >> I'm not sure if I have just been unlucky or what, but my understand is >> that other people have encountered such boards also. Usually the >> problems encountered there have been circumvented with some hacky video >> driver for that specific board, or maybe a static configuration handled >> by the boot loader. > > I have yet to encounter such a setup. Can you point me at a DTS for one > such setup? I do remember a couple of hypothetical cases being discussed > at one time or another, but I haven't seen any actual DTS content where > this was needed. No, I can't point to them as they are not in the mainline (at least the ones I've been working on), for obvious reasons. With a quick glance, I have the following devices in my cabinet that have more complex setups: OMAP 4430 SDP, BeagleBoneBlack + LCD, AM43xx EVM. Many Nokia devices used to have such setups, usually so that the LCD and tv-out were connected to the same video source. >> Do we have a standard way of representing the video pipeline with simple >> phandles? Or does everyone just do their own version? If there's no >> standard way, it sounds it'll be a mess to support in the future. > > It doesn't matter all that much whether the representation is standard. Again, I disagree. > phandles should simply point to the next element in the pipeline and the > OS abstractions should be good enough to handle the details about how to > chain the elements. I, on the other hand, would rather see the links the other way around. Panel having a link to the video source, etc. The video graphs have two-way links, which of course is the safest options, but also more verbose and redundant. When this was discussed earlier, it was unclear which way the links should be. It's true that only links to one direction are strictly needed, but the question raised was that if in the drivers we end up always going the links the other way, the performance penalty may be somewhat big. (If I recall right). Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 11:41:52AM +0300, Tomi Valkeinen wrote: > On 23/09/14 08:53, Thierry Reding wrote: > > >> Yes, it's true we need a mux there. But we still have the complication > >> that for panel0 we may need different ps8622 settings than for panel1. > > > > Yes, and that's why the bridge should be querying the panel for the > > information it needs to determine the settings. > > That only works if the settings to be queried are standard ones. > > But, for example, let's say that the board is designed in a way that for > panel0 the bridge needs to output a bit higher level voltages than for > panel1. That's not a property of the panel, so it can't be queried from > the panel. > > That feature (varying the voltage level) is specific to the bridge, and > thus the properties should be in the bridge's DT data. So we need two > different configurations in the bridge, one for panel0 and one for > panel1. This is what endpoints give us. You could get the same by moving the mux in front of the bridge instead. Trying to do this within the bridge's node directly has two flaws: 1) It violates the DT principle of describing hardware. The device itself does not know anything about multiple streams and deals only with a single input and output. You'd be describing a board specific aspect in the device binding. 2) Such a solution would have to be implemented for all bridge devices that can potentially be muxed (really all of them). If you describe it accurately in a separate mux node you get genericity for free and it will work for all bridges. > >> If that's the case, then I think we'd need to have two output endpoints > >> in ps8622, both going to the mux, and each having the settings for the > >> respective panel. > > > > But we'd be lying in DT. It no longer describes the hardware properly. > > The device only has a single input and a single output with no means to > > mux anything. Hence the device tree shouldn't be faking multiple inputs > > or outputs. > > No, a port is a single physical output. An endpoint is a logical entity > on that single physical output. > > So a bridge with a single output has one output port, but it may have as > many endpoints on that port as needed. Only one endpoint of a port can > be active at a time. As I mentioned above, this seems to me to be the wrong point of abstraction. > > I don't think we need to have a common way to describe video devices. In > > my opinion DT bindings are much better if they are specifically tailored > > towards the device that they describe. We'll provide a driver for that > > device anyway, so we should be creating appropriate abstractions at the > > OS level to properly handle them. > > I disagree. If we don't have a common way to describe the connections > between video devices, we cannot: > > - have a common helper library to parse the connections We have a common helper already. It's called of_parse_phandle(). > - study the connections before the drivers are loaded Why would you need that? > - handle the connections anywhere else than the specific driver for the > component Why should we do that? > > To stay with the example of the board/display, I'd think that the final > > component of the board DT would implement a bridge. The first component > > of the display DT would similarly implement a bridge. Now if we have a > > way of chaining bridges and controlling a chain of bridges, then there > > is no need for anything more complex than a plain phandle in a property > > from the board bridge to the display bridge. > > Right, that works for many cases. Of course, nothing says there's a > bridge on the main board, it could be connected directly to the SoC. In either case the SoC driver would probably know how to talk to a bridge anyway, whether it be one on the main board or on the display board. > >>> Whether this is described using a single phandle to the bridge or a more > >>> complicated graph is irrelevant. What matters is that you get a phandle > >>> to the bridge. The job of the operating system is to give drivers a way > >>> to resolve that phandle to some object and provide an API to access that > >>> object. > >> > >> I agree it's not relevant whether we use a simple phandle or complex > >> graph. What matter is that we have a standard way to express the video > >> paths, which everybody uses. > > > > Not necessarily. Consistency is always good, but I think simplicity > > trumps consistency. What matters isn't how the phandle is referenced in > > the device tree, what matters is that it is referenced and that it makes > > sense in the context of the specific device. Anything else is the job of > > the OS. > > > > While there are probably legitimate cases where the video graph is > > useful and makes sense, in many cases terms like ports and endpoints are > > simply confusing. > > I again agree that I'd like to have a simpler
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 09:04, Thierry Reding wrote: > I certainly agree that it's useful to have standard ways to describe at > least various aspects. For example I think it would be useful to add > standard properties for a bridge's connections, such as "bridge" or > "panel" to allow bridge chaining and attaching them to panels. I don't see a need for such properties. Do you have examples where they would be needed? The driver for the respective device does know if it's a bridge or a panel, so that information is there as soon as the driver has loaded. > But I think that should be the end of it. Mandating anything other than > that will just complicate things and limit what people can do in the > binding. > > One of the disadvantages of the video graph bindings is that they are > overly vague in that they don't carry information about what type a > device is. Bindings then have to require additional meta-data, at which > point it's become far easier to describe things with a custom property > that already provides context. I don't see why the graphs and such metadata are connected in any way. They are separate issues. If we need such metadata, it needs to be added in any case. That is not related to the graphs. >> Yes, there's always one active input and one output for this bridge. >> What the video graphs would bring is to have the possibility to have >> multiple inputs and outputs, of which a single ones could be active at a >> time. The different inputs and outputs could even have different >> settings required (say, first output requires this bit set, but when >> using second output that bit must be cleared). > > As discussed elsewhere this should be handled at a different level then. > DT should describe the hardware and this particular bridge device simply > doesn't have a means to connect more than a single input or more than a > single output. Well, I can't say about this particular bridge, but afaik you can connect a parallel RGB signal to multiple panels just like that, without any muxing. If a mux is needed, I agree that there should be a device for that mux. But we still need a way to have multiple different "configuration sets" for the bridge, as I explained in the earlier mail. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 8/8] arm64: dts: add symlink
Hi Chanho, On Tue, Sep 23, 2014 at 1:16 PM, Chanho Park wrote: > Hi, > >> -Original Message- >> From: linux-arm-kernel [mailto:linux-arm-kernel- >> boun...@lists.infradead.org] On Behalf Of Abhilash Kesavan >> Sent: Monday, September 22, 2014 1:47 PM >> To: linux-samsung-soc@vger.kernel.org; linux-arm- >> ker...@lists.infradead.org; devicet...@vger.kernel.org; >> catalin.mari...@arm.com >> Cc: naveenkrishna...@gmail.com; kesavan.abhil...@gmail.com; >> tomasz.f...@gmail.com >> Subject: [PATCH v5 8/8] arm64: dts: add symlink >> >> From: Pankaj Dubey >> >> Add symlink to include/dt-bindings from arch/arm64/boot/dts/include/ to >> match the ones in ARM architectures so that preprocessed device >> tree files can include various useful constant definitions. >> >> See commit c58299aa8754 ("kbuild: create an "include chroot" for DT >> bindings") >> merged in v3.10-rc1 for details. >> >> CC: Catalin Marinas >> Signed-off-by: Pankaj Dubey >> Signed-off-by: Abhilash Kesavan >> Reviewed-by: Thomas Abraham >> Tested-by: Thomas Abraham >> --- >> arch/arm64/boot/dts/include/dt-bindings | 1 + >> 1 file changed, 1 insertion(+) >> create mode 12 arch/arm64/boot/dts/include/dt-bindings >> >> diff --git a/arch/arm64/boot/dts/include/dt-bindings >> b/arch/arm64/boot/dts/include/dt-bindings >> new file mode 12 >> index 000..1e89bce >> --- /dev/null >> +++ b/arch/arm64/boot/dts/include/dt-bindings >> @@ -0,0 +1 @@ >> +../../../../../include/dt-bindings/ >> \ No newline at end of file > ^ > It generates incorrect symlink. Please remove last line like below: > > diff --git a/arch/arm64/boot/dts/include/dt-bindings > b/arch/arm64/boot/dts/include/dt-bindings > new file mode 12 > index 000..499472b > --- /dev/null > +++ b/arch/arm64/boot/dts/include/dt-bindings > @@ -0,0 +1 @@ > +../../../../../include/dt-bindings/ I just re-checked this patch and it seems to be working fine. I did find a thread where patchwork appeared to be changing the patch causing issues [1]. Are you downloading this via patchwork ? [1] http://www.spinics.net/lists/linux-kbuild/msg08656.html Regards, Abhilash > -- > > Best Regards, > Chanho Park > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
Hi Chanho, On Tue, Sep 23, 2014 at 1:20 PM, Chanho Park wrote: > Hi, > >> -Original Message- >> From: linux-arm-kernel [mailto:linux-arm-kernel- >> boun...@lists.infradead.org] On Behalf Of Abhilash Kesavan >> Sent: Monday, September 22, 2014 1:47 PM >> To: linux-samsung-soc@vger.kernel.org; linux-arm- >> ker...@lists.infradead.org; devicet...@vger.kernel.org; >> catalin.mari...@arm.com >> Cc: naveenkrishna...@gmail.com; kesavan.abhil...@gmail.com; >> tomasz.f...@gmail.com >> Subject: [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 >> SoC >> >> Changes since v4: >> - Fixed comments from Tomasz Figa: >> - Changed the namespace prefix from exynos to samsung >> - Defined bindings to take all input clocks >> - Sorted the Kconfig entries alphabetically in clock Makefile >> - Used consistent 1 tab line breaks across the clock file >> - Statically initialized the samsung_cmu_info struct >> - Enabled exynos7 in the arm64 defconfig as per Catalin Marinas' comment. >> - Added Kukjin Kim's ack along with Thomas Abraham's tested and reviewed >> tags. >> >> Changes since v3: >> - Removed aliases for serial controllers from dtsi file and moved it >> into board specific dts file as suggested by Arnd. >> - Based this series on Robert Richter's patches for adding vendor >> device tree sub-directories for arm64. >> http://comments.gmane.org/gmane.linux.kbuild.devel/12131 >> >> This patchset supports the new Exynos7 Samsung SoC based on Cortex-A57. >> Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC >> processor. >> >> The following patches are tested based on linux-next tree (20140919). >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/ >> >> Following patches are required for this series: >> 1- "tty/serial: fix config dependencies for samsung serial" >>https://www.mail-archive.com/linux-samsung-soc >> vger.kernel.org/msg36208.html >> 2- "dts, kbuild: Implement support for dtb vendor subdirs" patchset >>http://comments.gmane.org/gmane.linux.kbuild.devel/12131 Thanks for your comments. I have mentioned Robert's patchset as being a dependency for my series here. Regards, Abhilash > > Maybe you make this patch on top of the Robert's patch. > As I know, Robert's patch is not yet merged in mainline. > You should specify this dependency in mail thread. > > [1]: https://lkml.org/lkml/2014/9/5/64 > > Best Regards, > Chanho Park > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 23/09/14 08:53, Thierry Reding wrote: >> Yes, it's true we need a mux there. But we still have the complication >> that for panel0 we may need different ps8622 settings than for panel1. > > Yes, and that's why the bridge should be querying the panel for the > information it needs to determine the settings. That only works if the settings to be queried are standard ones. But, for example, let's say that the board is designed in a way that for panel0 the bridge needs to output a bit higher level voltages than for panel1. That's not a property of the panel, so it can't be queried from the panel. That feature (varying the voltage level) is specific to the bridge, and thus the properties should be in the bridge's DT data. So we need two different configurations in the bridge, one for panel0 and one for panel1. This is what endpoints give us. >> If that's the case, then I think we'd need to have two output endpoints >> in ps8622, both going to the mux, and each having the settings for the >> respective panel. > > But we'd be lying in DT. It no longer describes the hardware properly. > The device only has a single input and a single output with no means to > mux anything. Hence the device tree shouldn't be faking multiple inputs > or outputs. No, a port is a single physical output. An endpoint is a logical entity on that single physical output. So a bridge with a single output has one output port, but it may have as many endpoints on that port as needed. Only one endpoint of a port can be active at a time. > I don't think we need to have a common way to describe video devices. In > my opinion DT bindings are much better if they are specifically tailored > towards the device that they describe. We'll provide a driver for that > device anyway, so we should be creating appropriate abstractions at the > OS level to properly handle them. I disagree. If we don't have a common way to describe the connections between video devices, we cannot: - have a common helper library to parse the connections - study the connections before the drivers are loaded - handle the connections anywhere else than the specific driver for the component > To stay with the example of the board/display, I'd think that the final > component of the board DT would implement a bridge. The first component > of the display DT would similarly implement a bridge. Now if we have a > way of chaining bridges and controlling a chain of bridges, then there > is no need for anything more complex than a plain phandle in a property > from the board bridge to the display bridge. Right, that works for many cases. Of course, nothing says there's a bridge on the main board, it could be connected directly to the SoC. >>> Whether this is described using a single phandle to the bridge or a more >>> complicated graph is irrelevant. What matters is that you get a phandle >>> to the bridge. The job of the operating system is to give drivers a way >>> to resolve that phandle to some object and provide an API to access that >>> object. >> >> I agree it's not relevant whether we use a simple phandle or complex >> graph. What matter is that we have a standard way to express the video >> paths, which everybody uses. > > Not necessarily. Consistency is always good, but I think simplicity > trumps consistency. What matters isn't how the phandle is referenced in > the device tree, what matters is that it is referenced and that it makes > sense in the context of the specific device. Anything else is the job of > the OS. > > While there are probably legitimate cases where the video graph is > useful and makes sense, in many cases terms like ports and endpoints are > simply confusing. I again agree that I'd like to have a simpler representation than video graphs for the simpler use cases. But again, I think it's important to have a standard representation for those connections. Why do you think it wouldn't be good to have a standard for the simpler connections (in addition to the video graphs)? What kind of device specific variations do you see that would be needed? Even if the points I gave about why a common way to describe connections is important would not be relevant today, it sounds much safer to have a standard representation in the DT for the connections. I'd only go for device specific custom descriptions if there's a very important reason for that. And I can't come up with any reason. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 09:24:12AM +0200, Andrzej Hajda wrote: > Hi Thierry, Tomi, > > On 09/23/2014 08:04 AM, Thierry Reding wrote: > > On Mon, Sep 22, 2014 at 05:23:25PM +0300, Tomi Valkeinen wrote: > >> On 22/09/14 11:06, Thierry Reding wrote: > >> > > Why do we need a complex graph when it can be handled using a simple > > phandle? > > Maybe in your case you can handle it with simple phandle. Can you > guarantee that it's enough for everyone, on all platforms? > >>> > >>> Nobody can guarantee that. An interesting effect that DT ABI stability > >>> has had is that people now try to come up with overly generic concepts > >>> to describe devices in an attempt to make them more future-proof. I > >>> don't think we're doing ourselves a favour with that approach. > >> > >> I kind of agree. However, there are boards that need a more complex > >> approach than just a simple phandle. They are nothing new. > >> > >> So while it may be true that this specific encoder has never been used > >> in such a complex way, and there's a chance that it never will, my > >> comments are not really about this particular encoder. > >> > >> What I want is a standard way to describe the video components. If we > >> have a standard complex one (video graphs) and a standard simple one > >> (simple phandles), it's ok for me. > > > > I certainly agree that it's useful to have standard ways to describe at > > least various aspects. For example I think it would be useful to add > > standard properties for a bridge's connections, such as "bridge" or > > "panel" to allow bridge chaining and attaching them to panels. > > > > But I think that should be the end of it. Mandating anything other than > > that will just complicate things and limit what people can do in the > > binding. > > > > One of the disadvantages of the video graph bindings is that they are > > overly vague in that they don't carry information about what type a > > device is. Bindings then have to require additional meta-data, at which > > point it's become far easier to describe things with a custom property > > that already provides context. > > I think it is opposite, graph bindings should describe only the link and > certainly not what type of device is behind the endpoint. The fact we > may need that information is a disadvantage of Linux frameworks and > should be corrected in Linux, not by adding Linux specific properties to > DT. For example display controller should not care where its output goes > to: panel, encoder, bridge, whatever. It should care only if the > parameters of the link are agreed with the receiver. Well, a display controller is never going to attach to a panel directly. But I agree that it would be nice to unify bridges and encoders more. It should be possible to make encoder always a bridge (or perhaps even replace encoders with bridges altogether). Then once you're out of the DRM device everything would be a bridge until you get to a panel. I think we discussed this a while back in the context of bridge chaining. Thierry pgpor5We3x5KM.pgp Description: PGP signature
[PATCH v2 0/5] Add initial support for pinctrl on Exynos7
Changes since v1: - Marked the newly created irq_chip instances as __initdata - Used kmemdup to keep a copy of the irq_chip - Change the pinctrl name from sd0_rdqs to sd0_ds as per UM - Moved the pinctrl enablement for exynos7 into a separate patch - Added tested-by and reviewed-by tags from Thomas Abraham Following patches have been tested on linux-next (20140922). https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/ Following patches are required for this series: 1) "tty/serial: fix config dependencies for samsung serial" https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36208.html 2) "dts, kbuild: Implement support for dtb vendor subdirs" patchset http://comments.gmane.org/gmane.linux.kbuild.devel/12131 3) "arch: arm64: enable support for Samsung Exynos7 SoC" patchset (v5) http://www.spinics.net/lists/arm-kernel/msg364014.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts
Exynos7 uses different offsets for wakeup interrupt configuration registers. So a new irq_chip instance for Exynos7 wakeup interrupts is added. The irq_chip selection is now based on the wakeup interrupt controller compatible string. Signed-off-by: Abhilash Kesavan Reviewed-by: Thomas Abraham Tested-by: Thomas Abraham Cc: Thomas Abraham Cc: Tomasz Figa Cc: Linus Walleij --- .../bindings/pinctrl/samsung-pinctrl.txt |2 + drivers/pinctrl/samsung/pinctrl-exynos.c | 48 +++- drivers/pinctrl/samsung/pinctrl-exynos.h |3 ++ 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index e82aaf4..f80519a 100644 --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt @@ -136,6 +136,8 @@ B. External Wakeup Interrupts: For supporting external wakeup interrupts, a found on Samsung S3C64xx SoCs, - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs. + - samsung,exynos7-wakeup-eint: represents wakeup interrupt controller + found on Samsung Exynos7 SoC. - interrupt-parent: phandle of the interrupt parent to which the external wakeup interrupts are forwarded to. - interrupts: interrupt used by multiplexed wakeup interrupts. diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index b5e1cd4..7072bfa 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -56,12 +56,6 @@ static struct samsung_pin_bank_type bank_type_alive = { .reg_offset = { 0x00, 0x04, 0x08, 0x0c, }, }; -/* list of external wakeup controllers supported */ -static const struct of_device_id exynos_wkup_irq_ids[] = { - { .compatible = "samsung,exynos4210-wakeup-eint", }, - { } -}; - static void exynos_irq_mask(struct irq_data *irqd) { struct irq_chip *chip = irq_data_get_irq_chip(irqd); @@ -383,9 +377,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) /* * irq_chip for wakeup interrupts */ -static struct exynos_irq_chip exynos_wkup_irq_chip = { +static struct exynos_irq_chip *exynos_wkup_irq_chip; + +static struct exynos_irq_chip exynos4210_wkup_irq_chip __initdata = { .chip = { - .name = "exynos_wkup_irq_chip", + .name = "exynos4210_wkup_irq_chip", .irq_unmask = exynos_irq_unmask, .irq_mask = exynos_irq_mask, .irq_ack = exynos_irq_ack, @@ -399,6 +395,31 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = { .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, }; +static struct exynos_irq_chip exynos7_wkup_irq_chip __initdata = { + .chip = { + .name = "exynos7_wkup_irq_chip", + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + .irq_request_resources = exynos_irq_request_resources, + .irq_release_resources = exynos_irq_release_resources, + }, + .eint_con = EXYNOS7_WKUP_ECON_OFFSET, + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, +}; + +/* list of external wakeup controllers supported */ +static const struct of_device_id exynos_wkup_irq_ids[] = { + { .compatible = "samsung,exynos4210-wakeup-eint", + .data = &exynos4210_wkup_irq_chip }, + { .compatible = "samsung,exynos7-wakeup-eint", + .data = &exynos7_wkup_irq_chip }, + { } +}; + /* interrupt handler for wakeup interrupts 0..15 */ static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc) { @@ -459,7 +480,7 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc) static int exynos_wkup_irq_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw) { - irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip.chip, + irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip->chip, handle_level_irq); irq_set_chip_data(virq, h->host_data); set_irq_flags(virq, IRQF_VALID); @@ -491,7 +512,12 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) int idx, irq; for_each_child_of_node(dev->of_node, np) { - if (of_match_node(exynos_wkup_irq_ids, np)) { + const struct of_device_id *match; + + match = of_match_node(exynos_wkup_irq_ids, np); + if (match) { + exynos_wkup
[PATCH v2 4/5] arm64: dts: Add initial pinctrl support to EXYNOS7
From: Naveen Krishna Ch Add intial pin configuration nodes for EXYNOS7. Signed-off-by: Naveen Krishna Ch Signed-off-by: Abhilash Kesavan Reviewed-by: Thomas Abraham Tested-by: Thomas Abraham Cc: Rob Herring Cc: Catalin Marinas Cc: Tomasz Figa Cc: Linus Walleij Cc: Thomas Abraham --- arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi | 560 +++ arch/arm64/boot/dts/exynos/exynos7.dtsi | 66 +++ 2 files changed, 626 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi diff --git a/arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi b/arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi new file mode 100644 index 000..c7c41c1 --- /dev/null +++ b/arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi @@ -0,0 +1,560 @@ +/* + * Samsung's Exynos7 SoC pin-mux and pin-config device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung's Exynos7 SoC pin-mux and pin-config options are listed as + * device tree nodes in this file. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +&pinctrl_alive { + gpa0: gpa0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + interrupt-parent = <&gic>; + #interrupt-cells = <2>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, +<0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>; + }; + + gpa1: gpa1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + interrupt-parent = <&gic>; + #interrupt-cells = <2>; + interrupts = <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, +<0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>; + }; + + gpa2: gpa2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpa3: gpa3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; +}; + +&pinctrl_bus0 { + gpb0: gpb0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc0: gpc0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc1: gpc1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc2: gpc2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc3: gpc3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd0: gpd0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd1: gpd1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd2: gpd2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd4: gpd4 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd5: gpd5 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd6: gpd6 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd7: gpd7 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd8: gpd8 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg0: gpg0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg3: gpg3 { + gpio-controller; +
[PATCH v2 5/5] arm64: exynos: Enable pinctrl support for Exynos7
From: Naveen Krishna Ch Enable pinctrl support for exynos7 SoCs. Signed-off-by: Naveen Krishna Ch Signed-off-by: Abhilash Kesavan Reviewed-by: Thomas Abraham Tested-by: Thomas Abraham Cc: Rob Herring Cc: Catalin Marinas Cc: Tomasz Figa Cc: Linus Walleij Cc: Thomas Abraham --- arch/arm64/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1874e1a..4ee1250 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -146,6 +146,8 @@ config ARCH_EXYNOS7 bool "ARMv8 based Samsung Exynos7" select ARCH_EXYNOS select COMMON_CLK_SAMSUNG + select PINCTRL + select PINCTRL_EXYNOS help This enables support for Samsung Exynos7 SoC family -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] pinctrl: exynos: Add initial driver data for Exynos7
From: Naveen Krishna Ch This patch adds initial driver data for Exynos7 pinctrl support. Signed-off-by: Naveen Krishna Ch Signed-off-by: Abhilash Kesavan Reviewed-by: Thomas Abraham Tested-by: Thomas Abraham Cc: Thomas Abraham Cc: Tomasz Figa Cc: Linus Walleij --- .../bindings/pinctrl/samsung-pinctrl.txt |1 + drivers/pinctrl/samsung/pinctrl-exynos.c | 113 drivers/pinctrl/samsung/pinctrl-samsung.c |2 + drivers/pinctrl/samsung/pinctrl-samsung.h |1 + 4 files changed, 117 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index f80519a..8425838 100644 --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt @@ -18,6 +18,7 @@ Required Properties: - "samsung,exynos5250-pinctrl": for Exynos5250 compatible pin-controller. - "samsung,exynos5260-pinctrl": for Exynos5260 compatible pin-controller. - "samsung,exynos5420-pinctrl": for Exynos5420 compatible pin-controller. + - "samsung,exynos7-pinctrl": for Exynos7 compatible pin-controller. - reg: Base address of the pin controller hardware module and length of the address space it occupies. diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 7072bfa..25ca8e8 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -1206,3 +1206,116 @@ struct samsung_pin_ctrl exynos5420_pin_ctrl[] = { .label = "exynos5420-gpio-ctrl4", }, }; + +/* pin banks of exynos7 pin-controller - ALIVE */ +static struct samsung_pin_bank exynos7_pin_banks0[] = { + EXYNOS_PIN_BANK_EINTW(8, 0x000, "gpa0", 0x00), + EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04), + EXYNOS_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08), + EXYNOS_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c), +}; + +/* pin banks of exynos7 pin-controller - BUS0 */ +static struct samsung_pin_bank exynos7_pin_banks1[] = { + EXYNOS_PIN_BANK_EINTG(5, 0x000, "gpb0", 0x00), + EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpc0", 0x04), + EXYNOS_PIN_BANK_EINTG(2, 0x040, "gpc1", 0x08), + EXYNOS_PIN_BANK_EINTG(6, 0x060, "gpc2", 0x0c), + EXYNOS_PIN_BANK_EINTG(8, 0x080, "gpc3", 0x10), + EXYNOS_PIN_BANK_EINTG(4, 0x0a0, "gpd0", 0x14), + EXYNOS_PIN_BANK_EINTG(6, 0x0c0, "gpd1", 0x18), + EXYNOS_PIN_BANK_EINTG(8, 0x0e0, "gpd2", 0x1c), + EXYNOS_PIN_BANK_EINTG(5, 0x100, "gpd4", 0x20), + EXYNOS_PIN_BANK_EINTG(4, 0x120, "gpd5", 0x24), + EXYNOS_PIN_BANK_EINTG(6, 0x140, "gpd6", 0x28), + EXYNOS_PIN_BANK_EINTG(3, 0x160, "gpd7", 0x2c), + EXYNOS_PIN_BANK_EINTG(2, 0x180, "gpd8", 0x30), + EXYNOS_PIN_BANK_EINTG(2, 0x1a0, "gpg0", 0x34), + EXYNOS_PIN_BANK_EINTG(4, 0x1c0, "gpg3", 0x38), +}; + +/* pin banks of exynos7 pin-controller - NFC */ +static struct samsung_pin_bank exynos7_pin_banks2[] = { + EXYNOS_PIN_BANK_EINTG(3, 0x000, "gpj0", 0x00), +}; + +/* pin banks of exynos7 pin-controller - TOUCH */ +static struct samsung_pin_bank exynos7_pin_banks3[] = { + EXYNOS_PIN_BANK_EINTG(3, 0x000, "gpj1", 0x00), +}; + +/* pin banks of exynos7 pin-controller - FF */ +static struct samsung_pin_bank exynos7_pin_banks4[] = { + EXYNOS_PIN_BANK_EINTG(4, 0x000, "gpg4", 0x00), +}; + +/* pin banks of exynos7 pin-controller - ESE */ +static struct samsung_pin_bank exynos7_pin_banks5[] = { + EXYNOS_PIN_BANK_EINTG(5, 0x000, "gpv7", 0x00), +}; + +/* pin banks of exynos7 pin-controller - FSYS0 */ +static struct samsung_pin_bank exynos7_pin_banks6[] = { + EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpr4", 0x00), +}; + +/* pin banks of exynos7 pin-controller - FSYS1 */ +static struct samsung_pin_bank exynos7_pin_banks7[] = { + EXYNOS_PIN_BANK_EINTG(4, 0x000, "gpr0", 0x00), + EXYNOS_PIN_BANK_EINTG(8, 0x020, "gpr1", 0x04), + EXYNOS_PIN_BANK_EINTG(5, 0x040, "gpr2", 0x08), + EXYNOS_PIN_BANK_EINTG(8, 0x060, "gpr3", 0x0c), +}; + +struct samsung_pin_ctrl exynos7_pin_ctrl[] = { + { + /* pin-controller instance 0 Alive data */ + .pin_banks = exynos7_pin_banks0, + .nr_banks = ARRAY_SIZE(exynos7_pin_banks0), + .eint_gpio_init = exynos_eint_gpio_init, + .eint_wkup_init = exynos_eint_wkup_init, + .label = "exynos7-gpio-ctrl0", + }, { + /* pin-controller instance 1 BUS0 data */ + .pin_banks = exynos7_pin_banks1, + .nr_banks = ARRAY_SIZE(exynos7_pin_banks1), + .eint_gpio_init = exynos_eint_gpio_init, + .label = "exynos7-gpio-ctrl1", + }, { + /* pin-controller instance 2 NFC data */ + .pin_banks = exynos7_pin_banks2, + .
[PATCH v2 1/5] pinctrl: exynos: Generalize the eint16_31 demux code
The function exynos_irq_demux_eint16_31 uses pre-defined offsets for external interrupt pending status and mask registers. So this function is not extensible for Exynos7 SoC which have these registers at different offsets. So generalize the exynos_irq_demux_eint16_31 function by using the pending/mask register offset values from the exynos_irq_chip structure. Signed-off-by: Abhilash Kesavan Reviewed-by: Thomas Abraham Tested-by: Thomas Abraham Cc: Thomas Abraham Cc: Tomasz Figa Cc: Linus Walleij --- drivers/pinctrl/samsung/pinctrl-exynos.c |6 -- drivers/pinctrl/samsung/pinctrl-exynos.h |1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index d7154ed..b5e1cd4 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -444,10 +444,11 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc) chained_irq_enter(chip, desc); for (i = 0; i < eintd->nr_banks; ++i) { + struct exynos_irq_chip *our_chip = eintd->chip; struct samsung_pin_bank *b = eintd->banks[i]; - pend = readl(d->virt_base + EXYNOS_WKUP_EPEND_OFFSET + pend = readl(d->virt_base + our_chip->eint_pend + b->eint_offset); - mask = readl(d->virt_base + EXYNOS_WKUP_EMASK_OFFSET + mask = readl(d->virt_base + our_chip->eint_mask + b->eint_offset); exynos_irq_demux_eint(pend & ~mask, b->irq_domain); } @@ -565,6 +566,7 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) muxed_data->banks[idx++] = bank; } muxed_data->nr_banks = muxed_banks; + muxed_data->chip = &exynos_wkup_irq_chip; return 0; } diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 3c91c35..e060722 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -94,6 +94,7 @@ struct exynos_weint_data { * @banks: array of banks being part of the mux */ struct exynos_muxed_weint_data { + struct exynos_irq_chip *chip; unsigned int nr_banks; struct samsung_pin_bank *banks[]; }; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty/serial: fix config dependencies for samsung serial
On Tuesday 23 September 2014 12:34:26 Abhilash Kesavan wrote: > >> > >> The SERIAL_SAMSUNG_UARTS_4 and SERIAL_SAMSUNG_UARTS config options are > >> meaningful only if SERIAL_SAMSUNG is enabled. Hence the dependency > >> rules were changed. I will repost this patch with better description. > > > > My point is that the options are used by both the uart driver and > > the platform code, e.g. for the purpose of PM debugging which can > > be enabled even when the serial-samsung driver is turned off. > > I looked through the code and could find two references to these > symbols in the platform-specific code: arch/arm/plat-samsung/init.c > and arch/arm/mach-s3c64xx/irq-pm.c. The first one is being used to > initialize the uart platform device in non-dt way while the second one > saves and restores the uart irq masks for all channels across a > suspend/resume cycle. Also, the save/restore will get bypassed in case > CONFIG_SERIAL_SAMSUNG_UARTS is not defined. It does not look like > either of these cases would cause any issue with the current patch, > can you please help me understand the problem ? It's possible that it works fine now, I just pointed out that we had bugs in this area in the past, so it would be good if you can add to the changelog a statement that you have checked that the new patch does not cause problems here. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
Hi, > -Original Message- > From: linux-arm-kernel [mailto:linux-arm-kernel- > boun...@lists.infradead.org] On Behalf Of Abhilash Kesavan > Sent: Monday, September 22, 2014 1:47 PM > To: linux-samsung-soc@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; devicet...@vger.kernel.org; > catalin.mari...@arm.com > Cc: naveenkrishna...@gmail.com; kesavan.abhil...@gmail.com; > tomasz.f...@gmail.com > Subject: [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 > SoC > > Changes since v4: > - Fixed comments from Tomasz Figa: > - Changed the namespace prefix from exynos to samsung > - Defined bindings to take all input clocks > - Sorted the Kconfig entries alphabetically in clock Makefile > - Used consistent 1 tab line breaks across the clock file > - Statically initialized the samsung_cmu_info struct > - Enabled exynos7 in the arm64 defconfig as per Catalin Marinas' comment. > - Added Kukjin Kim's ack along with Thomas Abraham's tested and reviewed > tags. > > Changes since v3: > - Removed aliases for serial controllers from dtsi file and moved it > into board specific dts file as suggested by Arnd. > - Based this series on Robert Richter's patches for adding vendor > device tree sub-directories for arm64. > http://comments.gmane.org/gmane.linux.kbuild.devel/12131 > > This patchset supports the new Exynos7 Samsung SoC based on Cortex-A57. > Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC > processor. > > The following patches are tested based on linux-next tree (20140919). > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/ > > Following patches are required for this series: > 1- "tty/serial: fix config dependencies for samsung serial" >https://www.mail-archive.com/linux-samsung-soc > vger.kernel.org/msg36208.html > 2- "dts, kbuild: Implement support for dtb vendor subdirs" patchset >http://comments.gmane.org/gmane.linux.kbuild.devel/12131 Maybe you make this patch on top of the Robert's patch. As I know, Robert's patch is not yet merged in mainline. You should specify this dependency in mail thread. [1]: https://lkml.org/lkml/2014/9/5/64 Best Regards, Chanho Park -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 8/8] arm64: dts: add symlink
Hi, > -Original Message- > From: linux-arm-kernel [mailto:linux-arm-kernel- > boun...@lists.infradead.org] On Behalf Of Abhilash Kesavan > Sent: Monday, September 22, 2014 1:47 PM > To: linux-samsung-soc@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; devicet...@vger.kernel.org; > catalin.mari...@arm.com > Cc: naveenkrishna...@gmail.com; kesavan.abhil...@gmail.com; > tomasz.f...@gmail.com > Subject: [PATCH v5 8/8] arm64: dts: add symlink > > From: Pankaj Dubey > > Add symlink to include/dt-bindings from arch/arm64/boot/dts/include/ to > match the ones in ARM architectures so that preprocessed device > tree files can include various useful constant definitions. > > See commit c58299aa8754 ("kbuild: create an "include chroot" for DT > bindings") > merged in v3.10-rc1 for details. > > CC: Catalin Marinas > Signed-off-by: Pankaj Dubey > Signed-off-by: Abhilash Kesavan > Reviewed-by: Thomas Abraham > Tested-by: Thomas Abraham > --- > arch/arm64/boot/dts/include/dt-bindings | 1 + > 1 file changed, 1 insertion(+) > create mode 12 arch/arm64/boot/dts/include/dt-bindings > > diff --git a/arch/arm64/boot/dts/include/dt-bindings > b/arch/arm64/boot/dts/include/dt-bindings > new file mode 12 > index 000..1e89bce > --- /dev/null > +++ b/arch/arm64/boot/dts/include/dt-bindings > @@ -0,0 +1 @@ > +../../../../../include/dt-bindings/ > \ No newline at end of file ^ It generates incorrect symlink. Please remove last line like below: diff --git a/arch/arm64/boot/dts/include/dt-bindings b/arch/arm64/boot/dts/include/dt-bindings new file mode 12 index 000..499472b --- /dev/null +++ b/arch/arm64/boot/dts/include/dt-bindings @@ -0,0 +1 @@ +../../../../../include/dt-bindings/ -- Best Regards, Chanho Park -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
Hi Thierry, Tomi, On 09/23/2014 08:04 AM, Thierry Reding wrote: > On Mon, Sep 22, 2014 at 05:23:25PM +0300, Tomi Valkeinen wrote: >> On 22/09/14 11:06, Thierry Reding wrote: >> > Why do we need a complex graph when it can be handled using a simple > phandle? Maybe in your case you can handle it with simple phandle. Can you guarantee that it's enough for everyone, on all platforms? >>> >>> Nobody can guarantee that. An interesting effect that DT ABI stability >>> has had is that people now try to come up with overly generic concepts >>> to describe devices in an attempt to make them more future-proof. I >>> don't think we're doing ourselves a favour with that approach. >> >> I kind of agree. However, there are boards that need a more complex >> approach than just a simple phandle. They are nothing new. >> >> So while it may be true that this specific encoder has never been used >> in such a complex way, and there's a chance that it never will, my >> comments are not really about this particular encoder. >> >> What I want is a standard way to describe the video components. If we >> have a standard complex one (video graphs) and a standard simple one >> (simple phandles), it's ok for me. > > I certainly agree that it's useful to have standard ways to describe at > least various aspects. For example I think it would be useful to add > standard properties for a bridge's connections, such as "bridge" or > "panel" to allow bridge chaining and attaching them to panels. > > But I think that should be the end of it. Mandating anything other than > that will just complicate things and limit what people can do in the > binding. > > One of the disadvantages of the video graph bindings is that they are > overly vague in that they don't carry information about what type a > device is. Bindings then have to require additional meta-data, at which > point it's become far easier to describe things with a custom property > that already provides context. I think it is opposite, graph bindings should describe only the link and certainly not what type of device is behind the endpoint. The fact we may need that information is a disadvantage of Linux frameworks and should be corrected in Linux, not by adding Linux specific properties to DT. For example display controller should not care where its output goes to: panel, encoder, bridge, whatever. It should care only if the parameters of the link are agreed with the receiver. On the other side I agree graph bindings are bloated and it should be possible to simplify them to single phandle if we do not need extra parameters. Regards Andrzej > The point of the ports/endpoint graph is to also support more complicated scenarios. >>> >>> But the scenario will always remain the same for this bridge. There will >>> always be an input signal and a translation to some output signal along >>> with some parameters to control how that translation happens. More >>> complicated scenarios will likely need to be represented differently at >>> a higher level than the bridge. >> >> Yes, there's always one active input and one output for this bridge. >> What the video graphs would bring is to have the possibility to have >> multiple inputs and outputs, of which a single ones could be active at a >> time. The different inputs and outputs could even have different >> settings required (say, first output requires this bit set, but when >> using second output that bit must be cleared). > > As discussed elsewhere this should be handled at a different level then. > DT should describe the hardware and this particular bridge device simply > doesn't have a means to connect more than a single input or more than a > single output. > > Thierry > > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts
Hi Tomasz, On Mon, Sep 22, 2014 at 1:25 PM, Tomasz Figa wrote: > On 22.09.2014 08:17, Abhilash Kesavan wrote: >> Hi Tomasz, >> >> On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa wrote: >>> Hi Abhilash, >>> >>> Please see my comments inline. >>> >>> On 13.09.2014 10:50, Abhilash Kesavan wrote: Exynos7 uses different offsets for wakeup interrupt configuration registers. So a new irq_chip instance for Exynos7 wakeup interrupts is added. The irq_chip selection is now based on the wakeup interrupt controller compatible string. >>> >>> [snip] >>> @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) /* * irq_chip for wakeup interrupts */ -static struct exynos_irq_chip exynos_wkup_irq_chip = { +static struct exynos_irq_chip exynos_wkup_irq_chip; + >>> >>> Why do you still need this, if you have both variants below? >> >> After adding __initdata to the two variants, I will require to have a >> copy of one of them. >> >>> +static struct exynos_irq_chip exynos4210_wkup_irq_chip = { .chip = { - .name = "exynos_wkup_irq_chip", + .name = "exynos4210_wkup_irq_chip", .irq_unmask = exynos_irq_unmask, .irq_mask = exynos_irq_mask, .irq_ack = exynos_irq_ack, @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = { .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, }; +static struct exynos_irq_chip exynos7_wkup_irq_chip = { + .chip = { + .name = "exynos7_wkup_irq_chip", + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + }, + .eint_con = EXYNOS7_WKUP_ECON_OFFSET, + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, +}; + +/* list of external wakeup controllers supported */ +static const struct of_device_id exynos_wkup_irq_ids[] = { + { .compatible = "samsung,exynos4210-wakeup-eint", + .data = &exynos4210_wkup_irq_chip }, + { .compatible = "samsung,exynos7-wakeup-eint", + .data = &exynos7_wkup_irq_chip }, + { } +}; + /* interrupt handler for wakeup interrupts 0..15 */ static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc) { @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) int idx, irq; for_each_child_of_node(dev->of_node, np) { - if (of_match_node(exynos_wkup_irq_ids, np)) { + const struct of_device_id *match; + + match = of_match_node(exynos_wkup_irq_ids, np); + if (match) { + memcpy(&exynos_wkup_irq_chip, match->data, + sizeof(struct exynos_irq_chip)); >>> >>> Hmm, this doesn't look correct to me. You are modifying a static struct >>> here. Why couldn't you simply use the exynos irq chip pointed by >>> match->data in further registration code? >> >> That will not be available later once I use __initdata. >> > > Then either __initdata shouldn't be necessary or kmemdup() should be > used to allocate a copy. Will fix this and send out a new version soon. Regards, Abhilash > >>> wkup_np = np; break; } diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index e060722..0db1e52 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -25,6 +25,9 @@ #define EXYNOS_WKUP_ECON_OFFSET 0xE00 #define EXYNOS_WKUP_EMASK_OFFSET 0xF00 #define EXYNOS_WKUP_EPEND_OFFSET 0xF40 +#define EXYNOS7_WKUP_ECON_OFFSET 0x700 +#define EXYNOS7_WKUP_EMASK_OFFSET0x900 +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00 >>> >>> Interestingly enough, the offsets look just like the normal GPIO >>> interrupt controller of previous Exynos SoCs. Are you sure those are >>> correct? Also if somehow the controller now resembles the normal one, >>> doesn't it have the SVC register making it possible to reuse the non >>> wake-up code instead? >> >> The wakeup interrupt register offsets are the same as the GPIO >> interrupt offsets in earlier Exynos SoCs. There is no SVC register for >> the wakeup interrupt block. > > OK, your code is fine then. > > Best regards, > Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordo
Re: [PATCH] tty/serial: fix config dependencies for samsung serial
Hi Arnd, I will be taking this forward as Naveen is no longer with Samsung. On Wed, Sep 3, 2014 at 4:26 PM, Arnd Bergmann wrote: > On Wednesday 03 September 2014 13:51:56 Naveen Krishna Ch wrote: >> On 2 September 2014 21:16, Arnd Bergmann wrote: >> > On Tuesday 02 September 2014 20:52:00 Naveen Krishna Chatradhi wrote: >> >> Make the config symbols SERIAL_SAMSUNG_UARTS_4 and >> >> SERIAL_SAMSUNG_UARTS depend on SERIAL_SAMSUNG rather than >> >> PLAT_SAMSUNG. >> > >> > Please always describe why you are doing a change. This patch >> > seems really pointless. I will re-post the patch with a better commit message once we sort the below issue. >> >> The SERIAL_SAMSUNG_UARTS_4 and SERIAL_SAMSUNG_UARTS config options are >> meaningful only if SERIAL_SAMSUNG is enabled. Hence the dependency >> rules were changed. I will repost this patch with better description. > > My point is that the options are used by both the uart driver and > the platform code, e.g. for the purpose of PM debugging which can > be enabled even when the serial-samsung driver is turned off. I looked through the code and could find two references to these symbols in the platform-specific code: arch/arm/plat-samsung/init.c and arch/arm/mach-s3c64xx/irq-pm.c. The first one is being used to initialize the uart platform device in non-dt way while the second one saves and restores the uart irq masks for all channels across a suspend/resume cycle. Also, the save/restore will get bypassed in case CONFIG_SERIAL_SAMSUNG_UARTS is not defined. It does not look like either of these cases would cause any issue with the current patch, can you please help me understand the problem ? Regards, Abhilash > >> >> config SERIAL_SAMSUNG_UARTS_4 >> >> bool >> >> - depends on PLAT_SAMSUNG >> >> + depends on SERIAL_SAMSUNG >> >> default y if !(CPU_S3C2410 || CPU_S3C2412 || CPU_S3C2440 || >> >> CPU_S3C2442) >> >> help >> >> Internal node for the common case of 4 Samsung compatible UARTs >> >> >> >> config SERIAL_SAMSUNG_UARTS >> >> int >> >> - depends on PLAT_SAMSUNG >> >> + depends on SERIAL_SAMSUNG >> >> default 4 if SERIAL_SAMSUNG_UARTS_4 || CPU_S3C2416 >> >> default 3 >> >> help >> >> >> > >> > Have you checked that it still builds on all samsung platforms when >> > SERIAL_SAMSUNG is disabled? We have had build errors in this area >> > in the past. >> >> Yes, it builds for other Samsung platforms. > > Ok. > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html