Re: [PATCH v2 5/7] clk: exynos: use cpu-clock provider type to represent arm clock
On Mon, Jan 20, 2014 at 1:17 PM, Lukasz Majewski wrote:
> Hi Thomas,
>
>> From: Thomas Abraham
>>
>> With the addition of the new Samsung specific cpu-clock type, the
>> arm clock can be represented as a cpu-clock type and the independent
>> clock blocks that made up the arm clock can be removed.
>>
>> Signed-off-by: Thomas Abraham
>> ---
>> .../devicetree/bindings/clock/exynos5250-clock.txt |1 +
>> drivers/clk/samsung/clk-exynos4.c | 11 +--
>> drivers/clk/samsung/clk-exynos5250.c |8
>> include/dt-bindings/clock/exynos5250.h |1 +
>> 4 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt
>> b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt index
>> 99eae9c..acf867a 100644 ---
>> a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt +++
>> b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt @@
>> -38,6 +38,7 @@ clock which they consume.
>>fin_pll1
>> + armclk 12
>>
>>[Clock Gate for Special Clocks]
>>
>> diff --git a/drivers/clk/samsung/clk-exynos4.c
>> b/drivers/clk/samsung/clk-exynos4.c index 010f071..efcf4a3 100644
>> --- a/drivers/clk/samsung/clk-exynos4.c
>> +++ b/drivers/clk/samsung/clk-exynos4.c
>> @@ -437,8 +437,6 @@ static struct samsung_mux_clock
>> exynos4x12_mux_clks[] __initdata = {
>> /* list of divider clocks supported in all exynos4 soc's */
>> static struct samsung_div_clock exynos4_div_clks[] __initdata = {
>> - DIV(0, "div_core", "mout_core", DIV_CPU0, 0, 3),
>> - DIV(0, "div_core2", "div_core", DIV_CPU0, 28, 3),
>> DIV(0, "div_fimc0", "mout_fimc0", DIV_CAM, 0, 4),
>> DIV(0, "div_fimc1", "mout_fimc1", DIV_CAM, 4, 4),
>> DIV(0, "div_fimc2", "mout_fimc2", DIV_CAM, 8, 4),
>> @@ -484,8 +482,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_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3),
>> - 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, 0),
>> 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,
>> @@ -870,7 +868,6 @@ static struct samsung_gate_clock
>> exynos4x12_gate_clks[] __initdata = {
>> static struct samsung_clock_alias exynos4_aliases[] __initdata = {
>> ALIAS(CLK_MOUT_CORE, NULL, "moutcore"),
>> - ALIAS(CLK_ARM_CLK, NULL, "armclk"),
>> ALIAS(CLK_SCLK_APLL, NULL, "mout_apll"),
>> };
>>
>> @@ -1125,12 +1122,14 @@ static void __init exynos4_clk_init(struct
>> device_node *np, samsung_clk_register_alias(exynos4_aliases,
>> ARRAY_SIZE(exynos4_aliases));
>>
>> + samsung_register_arm_clock(np, CLK_ARM_CLK, "mout_apll",
>> reg_base); +
>
> I've got some doubts about allowing only the "mout_apll" clock to be the
> only parent for armclk Samsung clock.
>
> For the Exynos4412 it is also valid to have SCLK_MPLL_USER_C [*] as a
> parent for this clock.
>
> The problem is that you are reparenting the armclk to [*] with the
> register modification - no CCF involved.
The MUX_CORE mux is part of the larger cpu clock type and not
registered as a separate mux clock with CCF. So I would like to know
if there are any potential issues you see if this mux clock is
internally managed within the set_rate of the larger cpu clock type.
>
> I just would like to know if this is yours design decision or
> something, that we have overlooked in the v1 of this patch series.
I did overlook this one. This will be fixed in the next version by
ensuring that the dividers for SCLK_HPM clock will be updated only if
mout_apll is the parent of the MUX_HPM clock mux.
Thanks,
Thomas.
>
>> pr_info("%s clocks: sclk_apll = %ld, sclk_mpll = %ld\n"
>> "\tsclk_epll = %ld, sclk_vpll = %ld, arm_clk =
>> %ld\n", exynos4_soc == EXYNOS4210 ? "Exynos4210" : "Exynos4x12",
>> _get_rate("sclk_apll"),
>> _get_rate("sclk_mpll"), _get_rate("sclk_epll"),
>> _get_rate("sclk_vpll"),
>> - _get_rate("arm_clk"));
>> + _get_rate("armclk"));
>> }
>>
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5250.c
>> b/drivers/clk/samsung/clk-exynos5250.c index ff4beeb..487be36 100644
>> --- a/drivers/clk/samsung/clk-exynos5250.c
>> +++ b/drivers/clk/samsung/clk-exynos5250.c
>> @@ -298,9 +298,8 @@ static struct samsung_div_clock
>> exynos5250_div_clks[] __initdata = { /*
>>* CMU_CPU
>>*/
>> - DIV(0, "div_arm", "mout_cpu", DIV_CPU0, 0, 3),
>> - DIV(0, "div_apll", "mout_apll", DIV_C
Re: [PATCH v3] phy: Add new Exynos5 USB 3.0 PHY driver
On Mon, Jan 20, 2014 at 7:12 PM, Vivek Gautam wrote:
> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
> The new driver uses the generic PHY framework and will interact
> with DWC3 controller present on Exynos5 series of SoCs.
> Thereby, removing old phy-samsung-usb3 driver and related code
> used untill now which was based on usb/phy framework.
>
> Signed-off-by: Vivek Gautam
Sorry, forgot to add a Reviewed-by tag from Felipe. :-(
Will update in the next version of patch after getting a feedback on this patch.
> ---
>
> Changes from v2:
> 1) Added support for multiple PHYs (UTMI+ and PIPE3) and
>related changes in the driver structuring.
> 2) Added a xlate function to get the required phy out of
>number of PHYs in mutiple PHY scenerio.
> 3) Changed the names of few structures and variables to
>have a clearer meaning.
> 4) Added 'usb3phy_config' structure to take care of mutiple
>phys for a SoC having 'exynos5_usb3phy_drv_data' driver data.
> 5) Not deleting support for old driver 'phy-samsung-usb3' until
>required support for generic phy is added to DWC3.
>
> .../devicetree/bindings/phy/samsung-phy.txt| 49 ++
> drivers/phy/Kconfig|8 +
> drivers/phy/Makefile |1 +
> drivers/phy/phy-exynos5-usb3.c | 621
>
> 4 files changed, 679 insertions(+)
> create mode 100644 drivers/phy/phy-exynos5-usb3.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index c0fccaa..57079f8 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -20,3 +20,52 @@ Required properties:
> - compatible : should be "samsung,exynos5250-dp-video-phy";
> - reg : offset and length of the Display Port PHY register set;
> - #phy-cells : from the generic PHY bindings, must be 0;
> +
> +Samsung Exynos5 SoC series USB 3.0 PHY controller
> +--
> +
> +Required properties:
> +- compatible : Should be set to one of the following supported values:
> + - "samsung,exynos5250-usb3phy" - for exynos5250 SoC,
> + - "samsung,exynos5420-usb3phy" - for exynos5420 SoC.
> +- reg : Register offset and length of USB 3.0 PHY register set;
> +- clocks: Clock IDs array as required by the controller
> +- clock-names: names of clocks correseponding to IDs in the clock property;
> + Required clocks:
> + - phy: main PHY clock (same as USB 3.0 controller IP clock),
> + used for register access.
> + - usb3phy_refclk: PHY's reference clock (usually crystal clock),
> + associated by phy name, used to determine bit values for
> + clock settings register.
> + Additional clock required for Exynos5420:
> + - usb30_sclk_100m: Additional special clock used for PHY operation
> + depicted as 'sclk_usbphy30' in CMU of Exynos5420.
> +- samsung,syscon-phandle: phandle for syscon interface, which is used to
> + control pmu registers for power isolation.
> +- #phy-cells : from the generic PHY bindings, must be 1;
> +
> +For "samsung,exynos5250-usb3phy" and "samsung,exynos5420-usb3phy" compatible
> +PHYs, the second cell in the PHY specifier identifies the PHY id, which is
> +interpreted as follows:
> + 0 - UTMI+ type phy,
> + 1 - PIPE3 type phy,
> +
> +Example:
> + usb3_phy: usbphy@1210 {
> + compatible = "samsung,exynos5250-usb3phy";
> + reg = <0x1210 0x100>;
> + clocks = <&clock 286>, <&clock 1>;
> + clock-names = "phy", "usb3phy_refclk";
> + samsung,syscon-phandle = <&pmu_syscon>;
> + #phy-cells = <1>;
> + };
> +
> +- aliases: For SoCs like Exynos5420 having multiple USB PHY controllers,
> + 'usb3_phy' nodes should have numbered alias in the aliases node,
> + in the form of usb3phyN, N = 0, 1... (depending on number of
> + controllers).
> +Example:
> + aliases {
> + usb3phy0 = &usb3_phy0;
> + usb3phy1 = &usb3_phy1;
> + };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 330ef2d..32f9f38 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -51,4 +51,12 @@ config PHY_EXYNOS_DP_VIDEO
> help
> Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_EXYNOS5_USB3
> + tristate "Exynos5 SoC series USB 3.0 PHY driver"
> + depends on ARCH_EXYNOS5
> + select GENERIC_PHY
> + select MFD_SYSCON
> + help
> + Enable USB 3.0 PHY support for Exynos 5 SoC series
> +
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..9c06a61 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,4 @@ obj-$
Re: [PATCH v2 4/7] ARM: dts: Exynos: add cpu nodes, opp and cpu clock frequency table
Hi Lukasz,
On Mon, Jan 20, 2014 at 1:02 PM, Lukasz Majewski wrote:
> Hi Thomas,
>
>> From: Thomas Abraham
>>
>> For all Exynos based platforms, add CPU nodes, operating points and
>> cpu clock frequency table for migrating from Exynos specific cpufreq
>> driver to using generic cpufreq-cpu0 driver.
>>
>> Signed-off-by: Thomas Abraham
>> ---
>> arch/arm/boot/dts/exynos4210-origen.dts |6 +++
>> arch/arm/boot/dts/exynos4210-trats.dts |6 +++
>> arch/arm/boot/dts/exynos4210-universal_c210.dts |6 +++
>> arch/arm/boot/dts/exynos4210.dtsi | 35
>> ++ arch/arm/boot/dts/exynos4212.dtsi
>> | 17 + arch/arm/boot/dts/exynos4412-odroidx.dts|
>> 6 +++ arch/arm/boot/dts/exynos4412-origen.dts |6 +++
>> arch/arm/boot/dts/exynos4412-trats2.dts |6 +++
>> arch/arm/boot/dts/exynos4412.dtsi | 30
>> arch/arm/boot/dts/exynos4x12.dtsi |
>> 35 ++ arch/arm/boot/dts/exynos5250-arndale.dts
>> |6 +++ arch/arm/boot/dts/exynos5250-cros-common.dtsi |6 +++
>> arch/arm/boot/dts/exynos5250-smdk5250.dts |6 +++
>> arch/arm/boot/dts/exynos5250.dtsi | 43
>> ++- 14 files changed, 213 insertions(+), 1
>> deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts
>> b/arch/arm/boot/dts/exynos4210-origen.dts index 2aa13cb..dd17e93
>> 100644 --- a/arch/arm/boot/dts/exynos4210-origen.dts
>> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
>> @@ -32,6 +32,12 @@
>> bootargs ="root=/dev/ram0 rw ramdisk=8192
>> initrd=0x4100,8M console=ttySAC2,115200 init=/linuxrc"; };
>>
>> + cpus {
>> + cpu@0 {
>> + cpu0-supply = <&buck1_reg>;
>> + };
>> + };
>> +
>> regulators {
>> compatible = "simple-bus";
>> #address-cells = <1>;
>> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts
>> b/arch/arm/boot/dts/exynos4210-trats.dts index 63cc571..25487d7 100644
>> --- a/arch/arm/boot/dts/exynos4210-trats.dts
>> +++ b/arch/arm/boot/dts/exynos4210-trats.dts
>> @@ -30,6 +30,12 @@
>> bootargs = "console=ttySAC2,115200N8
>> root=/dev/mmcblk0p5 rootwait earlyprintk panic=5"; };
>>
>> + cpus {
>> + cpu: cpu@0 {
>> + cpu0-supply = <&varm_breg>;
>> + };
>> + };
>> +
>> regulators {
>> compatible = "simple-bus";
>>
>> diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts
>> b/arch/arm/boot/dts/exynos4210-universal_c210.dts index
>> d2e3f5f..74d5a70 100644 ---
>> a/arch/arm/boot/dts/exynos4210-universal_c210.dts +++
>> b/arch/arm/boot/dts/exynos4210-universal_c210.dts @@ -28,6 +28,12 @@
>> bootargs = "console=ttySAC2,115200N8
>> root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1"; };
>>
>> + cpus {
>> + cpu: cpu@0 {
>> + cpu0-supply = <&vdd_arm_reg>;
>> + };
>> + };
>> +
>> mct@1005 {
>> compatible = "none";
>> };
>> diff --git a/arch/arm/boot/dts/exynos4210.dtsi
>> b/arch/arm/boot/dts/exynos4210.dtsi index 48ecd7a..40cd663 100644
>> --- a/arch/arm/boot/dts/exynos4210.dtsi
>> +++ b/arch/arm/boot/dts/exynos4210.dtsi
>> @@ -36,6 +36,34 @@
>> reg = <0x10023CA0 0x20>;
>> };
>>
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + cpu@0 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a9";
>> + reg = <0>;
>> + clocks = <&clock 12>;
>> + clock-names = "cpu";
>> +
>> + operating-points = <
>> + 20 95
>> + 40 975000
>> + 50 975000
>> + 80 1075000
>> + 100 115
>> + 120 125
>
> Please be consistent with "operating-points" definition. Here you use
> increasing order, when below you use decreasing one.
Okay, missed this one. Will fix in the next version.
Thanks,
Thomas.
>
>> + >;
>> + safe-opp = <80 1075000>;
>> + };
>> +
>> + cpu@1 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a9";
>> + reg = <1>;
>> + };
>> + };
>> +
>> gic: interrupt-controller@1049 {
>> cpu-offset = <0x8000>;
>> };
>> @@ -73,6 +101,13 @@
>> compatible = "samsung,exynos4210-clock";
>> reg = <0x1003 0x2>;
>> #clock-cells = <1>;
>> +
>> + arm-frequency-table = <120 120 0 3 7 3 4 1 7
>> 0 5 0 0>,
>> +
Re: [PATCH v2 3/7] devicetree: bindings: add cpu clock configuration data binding for Exynos4/5
Hi Rob, On Sat, Jan 18, 2014 at 8:52 PM, Rob Herring wrote: > On Sat, Jan 18, 2014 at 6:10 AM, Thomas Abraham wrote: >> From: Thomas Abraham >> >> The clk ops of the new Samsung cpu clock provider type requires configuration >> data that will be programmed in the multiple clock blocks encapsulated within >> the cpu clock provider type. This configuration data is held in the clock >> controller node. Update clock binding documentation about this configuration >> data format for Samsung Exynos4 and Exynos5 platforms. >> >> Cc: Rob Herring > > Please copy all maintainers. Okay. > >> Cc: Tomasz Figa >> Cc: >> Signed-off-by: Thomas Abraham >> --- >> .../devicetree/bindings/clock/exynos4-clock.txt| 30 >> >> .../devicetree/bindings/clock/exynos5250-clock.txt | 21 ++ >> 2 files changed, 51 insertions(+), 0 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt >> b/Documentation/devicetree/bindings/clock/exynos4-clock.txt >> index a2ac2d9..c28aabd 100644 >> --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt >> +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt >> @@ -15,6 +15,29 @@ Required Properties: >> >> - #clock-cells: should be 1. >> >> +- arm-frequency-table: defines the list of arm clock speeds supported and > > Seems like a Samsung specific property and nothing to do with ARM, so > it should be named accordingly. Okay. > >> + the associated configuration values required to setup the clock controller >> + for generating those speeds. The format of each entry included in the >> + arm-frequency-table should be as defined below (#cells per entry = 13) >> + >> + - for Exynos4210 and Exynos4212 based platforms: >> + cell #1: arm clock frequency >> + cell #2: expected arm clock parent frequency >> + cell #3 ~ cell 12#: value of clock divider in the following order >> + core_ratio, corem0_ratio, corem1_ratio, periph_ratio, >> + atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio, >> + copy_ratio, hpm_ratio. >> + cell #13: reserved (should be zero). >> + >> + - for Exynos4412 based platforms: >> + cell #1: arm clock frequency >> + cell #2: expected arm clock parent frequency >> + cell #3 ~ cell #13: value of clock divider in the following order >> + core_ratio, corem0_ratio, corem1_ratio, periph_ratio, >> + atb_ratio, pclk_dbg_ratio, apll_ratio, core2_ratio, >> + copy_ratio, hpm_ratio, cores_ratio > > You don't need voltages? Are the h/w limitations really ratios or each > clock has a max frequency that must be maintained? I would expect the > latter and think it would be better to specify maximum frequencies of > each clock. Then you can calculate the dividers to keep frequencies in > range. Voltage is not needed here since the cpu clock speed is managed by the cpufreq driver which will take care of setting appropriate voltage. Any other users apart from cpufreq will have to setup the voltage appropriately. About the ratios, yes the user manual lists the optimal clock frequency for these clocks. As suggested, I will remove the ratio list for these clocks. > > How will this scale to multi-cluster chips with different frequency ranges? Usually, each cluster would have its own independent clock input. So there will be two or more cpu clocks in multi-cluster chips. So the clocks can be independently programmed as required by the cluster. > > Rob > Thanks Rob for your review. Thomas. >> + >> + >> The following is the list of clocks generated by the controller. Each clock >> is >> assigned an identifier and client nodes use this identifier to specify the >> clock which they consume. Some of the clocks are available only on a >> particular >> @@ -275,6 +298,13 @@ Example 1: An example of a clock controller node is >> listed below. >> compatible = "samsung,exynos4210-clock"; >> reg = <0x1003 0x2>; >> #clock-cells = <1>; >> + >> + arm-frequency-table = <120 120 0 3 7 3 4 1 7 0 5 0>, >> + <100 100 0 3 7 3 4 1 7 0 4 0>, >> + < 80 80 0 3 7 3 3 1 7 0 3 0>, >> + < 50 50 0 3 7 3 3 1 7 0 3 0>, >> + < 40 40 0 3 7 3 3 1 7 0 3 0>, >> + < 20 20 0 1 3 1 1 1 0 0 3 0>; >> }; >> >> Example 2: UART controller node that consumes the clock generated by the >> clock >> diff --git a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt >> b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt >> index 72ce617..99eae9c 100644 >> --- a/Documentation/devicetree/bindings/clock/exynos5250-clock.txt >> +++ b/Documentation/devicetree/bindings/clock/exynos5250-clock.txt >> @@ -13,
Re: [patch] drm/exynos: potential use after free in exynos_drm_open()
On Tuesday, January 21, 2014 3:58 PM, Dan Carpenter wrote:
>
> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>
> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
> Signed-off-by: Dan Carpenter
Reviewed-by: Jingoo Han
Yes, right.
The freed 'file_priv' should not be re-used.
Best regards,
Jingoo Han
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0c5f8d..3c845292845a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
> drm_file *file)
> if (ret) {
> kfree(file_priv);
> file->driver_priv = NULL;
> + return ret;
> }
>
> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> @@ -186,7 +187,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
> drm_file *file)
> anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
> file_priv->anon_filp = anon_filp;
>
> - return ret;
> + return 0;
> }
>
> static void exynos_drm_preclose(struct drm_device *dev,
--
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] drm/exynos: potential use after free in exynos_drm_open()
Hi,
This issue had already been reported, and fixed by Sachin. And also
that patch has been merged to exynos-drm-next. Please see below link,
http://www.spinics.net/lists/dri-devel/msg51889.html
Thanks,
Inki Dae
2014/1/21 Dan Carpenter :
> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>
> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0c5f8d..3c845292845a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
> drm_file *file)
> if (ret) {
> kfree(file_priv);
> file->driver_priv = NULL;
> + return ret;
> }
>
> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> @@ -186,7 +187,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
> drm_file *file)
> anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
> file_priv->anon_filp = anon_filp;
>
> - return ret;
> + return 0;
> }
>
> static void exynos_drm_preclose(struct drm_device *dev,
> ___
> 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
[patch] drm/exynos: potential use after free in exynos_drm_open()
If exynos_drm_subdrv_open() fails then we re-use "file_priv".
Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 9d096a0c5f8d..3c845292845a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
drm_file *file)
if (ret) {
kfree(file_priv);
file->driver_priv = NULL;
+ return ret;
}
anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
@@ -186,7 +187,7 @@ static int exynos_drm_open(struct drm_device *dev, struct
drm_file *file)
anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
file_priv->anon_filp = anon_filp;
- return ret;
+ return 0;
}
static void exynos_drm_preclose(struct drm_device *dev,
--
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 V12 1/3] ARM: dts: Add pmu sysreg node to exynos5250 and exynos5420 dtsi files
Leela Krishna Amudala wrote: > > On 1/10/14, Wim Van Sebroeck wrote: > > Hi Leela, > > > >> This patch adds pmusysreg node to exynos5250 and exynos5420 dtsi files > to > >> handle PMU register accesses in a centralized way using syscon driver > >> > >> Signed-off-by: Leela Krishna Amudala > >> Reviewed-by: Tomasz Figa > >> Reviewed-by: Doug Anderson > >> Tested-by: Doug Anderson > >> --- > > ... > >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi > >> b/arch/arm/boot/dts/exynos5420.dtsi > >> index b1fa334..cd47db0 100644 > >> --- a/arch/arm/boot/dts/exynos5420.dtsi > >> +++ b/arch/arm/boot/dts/exynos5420.dtsi > >> @@ -402,4 +402,9 @@ > >>clock-names = "gscl"; > >>samsung,power-domain = <&gsc_pd>; > >>}; > > > > I can't add this patch since there is no "gscl" in the current linux- > tree. > > Is this depending on another patch or are we going to fix this in > another > > way? > > > > Hi Kgene, > Hi Leela, > Driver changes has been merged to Wim's tree, so can you please pick > this patch into your tree. > OK, let me take this and 3rd patch into Samsung tree for next time. 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] drm/exynos: Fix multiplatform breakage for ipp/gsc
Applied. Thanks, Inki Dae 2014/1/16 Tushar Behera : > There is no need to include "plat/map-base.h" in ipp driver. Remove > this and enable this driver for multi-platform. > > However gsc driver is not multiplatform compliant yet, so make the > compilation conditional upon !ARCH_MULTIPLATFORM. > > Signed-off-by: Tushar Behera > --- > Changes for V2: > * Made compilation of exynos_drm_gsc.c conditional on > !ARCH_MULTIPLATFORM. > > drivers/gpu/drm/exynos/Kconfig |4 ++-- > drivers/gpu/drm/exynos/exynos_drm_ipp.c |1 - > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig > index f227f54..6e1a1a2 100644 > --- a/drivers/gpu/drm/exynos/Kconfig > +++ b/drivers/gpu/drm/exynos/Kconfig > @@ -51,7 +51,7 @@ config DRM_EXYNOS_G2D > > config DRM_EXYNOS_IPP > bool "Exynos DRM IPP" > - depends on DRM_EXYNOS && !ARCH_MULTIPLATFORM > + depends on DRM_EXYNOS > help > Choose this option if you want to use IPP feature for DRM. > > @@ -69,6 +69,6 @@ config DRM_EXYNOS_ROTATOR > > config DRM_EXYNOS_GSC > bool "Exynos DRM GSC" > - depends on DRM_EXYNOS_IPP && ARCH_EXYNOS5 > + depends on DRM_EXYNOS_IPP && ARCH_EXYNOS5 && !ARCH_MULTIPLATFORM > help > Choose this option if you want to use Exynos GSC for DRM. > diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c > b/drivers/gpu/drm/exynos/exynos_drm_ipp.c > index d519a4e..eefb429 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c > @@ -16,7 +16,6 @@ > #include > #include > #include > -#include > > #include > #include > -- > 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 -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 04:26:23PM -0800, Greg KH wrote:
> On Tue, Jan 21, 2014 at 12:07:06AM +, Russell King - ARM Linux wrote:
> > On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote:
> > > On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote:
> > > > I don't believe the driver model has any locking to prevent a drivers
> > > > ->probe function running concurrently with it's ->remove function for
> > > > two (or more) devices.
> > >
> > > The bus prevents this from happening.
> > >
> > > > The locking against this is done on a per-device basis, not a per-driver
> > > > basis.
> > >
> > > No, on a per-bus basis.
> >
> > I don't see it.
> >
> > Let's start from driver_register().
>
> Which happens from module probing, which is single-threaded, right?
Yes, to _some_ extent - the driver is added to the bus list of drivers
before existing drivers are probed, so it's always worth bearing in
mind that if a new device comes along, it's possible for that device
to be offered to even a driver which hasn't finished returning from
its module_init().
> > If you think there's a per-driver lock that's held over probes or removes,
> > please point it out. I'm fairly certain that there isn't, because we have
> > to be able to deal with recursive probes (yes, we've had to deal with
> > those in the past.)
>
> Hm, you are right, I think that's why we had to remove the locks. The
> klist stuff handles us getting the needed locks for managing our
> internal lists of devices and drivers, and those should be fine.
>
> So, let's go back to your original worry, what are you concerned about?
> A device being removed while probe() is called?
My concern is that we're turning something which should be simple into
something unnecessarily complex. By that, I mean something along the
lines of:
static DEFINE_MUTEX(foo_mutex);
static unsigned foo_devices;
static int foo_probe(struct platform_device *pdev)
{
int ret;
mutex_lock(&foo_mutex);
if (foo_devices++ == 0)
uart_register_driver(&driver);
ret = foo_really_probe_device(pdev);
if (ret) {
if (--foo_devices == 0)
uart_unregister_driver(&driver);
}
mutex_unlock(&foo_mutex);
return ret;
}
static int foo_remove(struct platform_device *pdev)
{
mutex_lock(&foo_mutex);
foo_really_remove(pdev);
if (--foo_devices == 0)
uart_unregister_driver(&driver);
mutex_unlock(&foo_mutex);
return 0;
}
in every single serial driver we have... Wouldn't it just be better to
fix the major/minor number problem rather than have to add all that code
repetitively to all those drivers?
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
--
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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Tue, Jan 21, 2014 at 12:07:06AM +, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote: > > > I don't believe the driver model has any locking to prevent a drivers > > > ->probe function running concurrently with it's ->remove function for > > > two (or more) devices. > > > > The bus prevents this from happening. > > > > > The locking against this is done on a per-device basis, not a per-driver > > > basis. > > > > No, on a per-bus basis. > > I don't see it. > > Let's start from driver_register(). Which happens from module probing, which is single-threaded, right? Or from module_init callbacks, which is single-threaded. Normally, busses never add devices (which is what drivers bind to), except in a single-at-a-time fashion, unless they really know what they are doing (i.e. PCI had multi-threaded device probing for a while, don't remember if it still does...) > This takes no locks and calls bus_add_driver(). > This also takes no locks and calls driver_attach(). > This walks the list of devices calling __driver_attach() for each. > __driver_attach() tries to match the device against the driver, > locks the parent device if one exists, and the device which is about > to be probed. It then calls driver_probe_device(). > driver_probe_device() inserts a runtime barrier and calls really_probe(). > really_probe() ultimately calls either the bus ->probe method or the > driver ->probe method. > > At no point in that sequence do I see anything which does any locking > on a per-driver basis. Let's look at device_add(). > > device_add() calls bus_probe_device(), which then calls device_attach(). > device_attach() takes the device's lock, and walks the list of drivers > calling __device_attach() on each driver. This then calls down into > driver_probe_device(), and the path is the same as the above. > > I don't see any per-driver locking here either. > > I've checked the klist stuff, don't see anything there. Ditto for > bus_for_each_drv(). > > If you think there's a per-driver lock that's held over probes or removes, > please point it out. I'm fairly certain that there isn't, because we have > to be able to deal with recursive probes (yes, we've had to deal with > those in the past.) Hm, you are right, I think that's why we had to remove the locks. The klist stuff handles us getting the needed locks for managing our internal lists of devices and drivers, and those should be fine. So, let's go back to your original worry, what are you concerned about? A device being removed while probe() is called? thanks, greg k-h -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 11:47:34PM +, Alan Cox wrote: > But yes I agree about the idiom, but a definite NAK to any attempts to > plaster over this grand screwup by crapping in the tty core. Your turd, > deal with it locally in the ARM code if you can't apply common sense and > just go dynamic. I believe at the time there was no one maintaining the device list to _do_ that allocation - AMBA PL011 came along in 2005 after (I believe) hpa stopped looking after that list. So, please tell me how a number could be allocated properly without the device numbers list being maintained? I've no problem with going dynamic, and I suggest that you get a sense of perspective rather than just spouting rubbish from on high. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 03:51:28PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote: > > I don't believe the driver model has any locking to prevent a drivers > > ->probe function running concurrently with it's ->remove function for > > two (or more) devices. > > The bus prevents this from happening. > > > The locking against this is done on a per-device basis, not a per-driver > > basis. > > No, on a per-bus basis. I don't see it. Let's start from driver_register(). This takes no locks and calls bus_add_driver(). This also takes no locks and calls driver_attach(). This walks the list of devices calling __driver_attach() for each. __driver_attach() tries to match the device against the driver, locks the parent device if one exists, and the device which is about to be probed. It then calls driver_probe_device(). driver_probe_device() inserts a runtime barrier and calls really_probe(). really_probe() ultimately calls either the bus ->probe method or the driver ->probe method. At no point in that sequence do I see anything which does any locking on a per-driver basis. Let's look at device_add(). device_add() calls bus_probe_device(), which then calls device_attach(). device_attach() takes the device's lock, and walks the list of drivers calling __device_attach() on each driver. This then calls down into driver_probe_device(), and the path is the same as the above. I don't see any per-driver locking here either. I've checked the klist stuff, don't see anything there. Ditto for bus_for_each_drv(). If you think there's a per-driver lock that's held over probes or removes, please point it out. I'm fairly certain that there isn't, because we have to be able to deal with recursive probes (yes, we've had to deal with those in the past.) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 11:35:41PM +, Alan Cox wrote: > > The first bit is easy... but we need to add locks to every serial > > driver to prevent two probes operating concurrently... > > The bus probe should already be serializing surely ? Yes, it better be, otherwise that bus is badly broken. greg k-h -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 11:16:03PM +, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 03:11:41PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 09:32:06PM +, Russell King - ARM Linux wrote: > > > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > > > On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux > > > > wrote: > > > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > > > uart_register_driver call binds the driver to a specific device > > > > > > node through tty_register_driver call. This should typically happen > > > > > > during device probe call. > > > > > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > > > drivers are part of the kernel. Currently the driver registration > > > > > > fails > > > > > > if multiple serial drivers with same default major/minor numbers are > > > > > > included in the kernel. > > > > > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > > > > > The samsung-uart driver is at fault here - the major/minor numbers > > > > > were > > > > > officially registered to amba-pl011. Samsung needs to be fixed > > > > > properly. > > > > > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > > > these two drivers can't register with the tty layer _after_ the hardware > > > > is detected, and not before. > > > > > > > > That saves resources on systems that build the drivers in, yet do not > > > > have the hardware present, which is always a good thing. > > > > > > Great, so what you're saying is that we need to wait until the first > > > device calls into the probe function. What about removal... how does > > > a driver know when it's last device has been removed to de-register > > > that? > > > > The "bus" that the device is on handles that, right? > > > > > I guess it needs the driver model to provide some way to know when a > > > driver is completely unbound - but isn't that racy? > > > > How is it racy? That's how the driver model works... > > Think about what happens when the last device unregisters, but a new > device comes along and is probed. > > I don't believe the driver model has any locking to prevent a drivers > ->probe function running concurrently with it's ->remove function for > two (or more) devices. The bus prevents this from happening. > The locking against this is done on a per-device basis, not a per-driver > basis. No, on a per-bus basis. thanks, greg k-h -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, 20 Jan 2014 23:14:57 +
Mark Brown wrote:
> On Mon, Jan 20, 2014 at 09:43:05PM +, Alan Cox wrote:
>
> > The dynamic major/minor is the right patch. If the userspace breaks then
> > the userspace was broken, but I see no evidence in the discussion that
> > the userspace broke.
>
> The userspace breakage is that if someone has a static /dev that doesn't
> handle any dynamic devices then renumbering the device will cause that
> static /dev to stop matching the kernel.
Diddums and you've only provided theoretical cases not real world ones.
They should have followed proper practice and reserved their minors. The
device number belongs to the Altix. The driver should just move.
> > Thats what the list says. Samsung should have followed the rules, they
> > didn't so they get to pick up the pieces. The Amba driver wants moving as
> > well. It's easy. If you want something to be ABI then make sure you get
> > it upstream first, if not you get to own all the pain down the line.
>
> This stuff is all upstream already, a quick check suggests both drivers
> predate git - it's been noticed because the ARM multiplatform work has
> caused people to try booting kernels with both built in.
So ARM people didn't follow the policy on allocating device minors even
within their own community and got burned by it. That's despite having
previously been burned by abusing the ttyS0 8250 major/minor in the same
way, for the same purposes, and creating the same mess.
{facepalm...}
> > If the hardware isn't present then the driver shouldn't even register
> > with the tty layer in the first place so it doesn't make any resource
> > differeneces either for properly written code.
>
> Right, that's not the idiom that has been followed by any of serial
> drivers though so needs fixing too.
Actally some drivers do get this right but not many. ehv-bc for example
does.
But yes I agree about the idiom, but a definite NAK to any attempts to
plaster over this grand screwup by crapping in the tty core. Your turd,
deal with it locally in the ARM code if you can't apply common sense and
just go dynamic.
And please, after screwing this up twice - *learn* from the mess.
Alan
--
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 1/2] serial: samsung: Move uart_register_driver call to device probe
> The first bit is easy... but we need to add locks to every serial > driver to prevent two probes operating concurrently... The bus probe should already be serializing surely ? -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 11:14:57PM +, Mark Brown wrote: > On Mon, Jan 20, 2014 at 09:43:05PM +, Alan Cox wrote: > > If the hardware isn't present then the driver shouldn't even register > > with the tty layer in the first place so it doesn't make any resource > > differeneces either for properly written code. > > Right, that's not the idiom that has been followed by any of serial > drivers though so needs fixing too. It's not followed by serial drivers because it gets f*scking complicated to do it that way. In order to do it that way, what we need to do is: 1. On the first device probe, register the UART driver. 2. On subsequent device probes, don't register the UART driver because it's already registered. 3. When devices are removed, do nothing until the last device. 4. When the last device is removed, unregister the UART driver. The first bit is easy... but we need to add locks to every serial driver to prevent two probes operating concurrently... -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 03:11:41PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 09:32:06PM +, Russell King - ARM Linux wrote: > > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > > On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote: > > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > > uart_register_driver call binds the driver to a specific device > > > > > node through tty_register_driver call. This should typically happen > > > > > during device probe call. > > > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > > drivers are part of the kernel. Currently the driver registration > > > > > fails > > > > > if multiple serial drivers with same default major/minor numbers are > > > > > included in the kernel. > > > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > > > officially registered to amba-pl011. Samsung needs to be fixed > > > > properly. > > > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > > these two drivers can't register with the tty layer _after_ the hardware > > > is detected, and not before. > > > > > > That saves resources on systems that build the drivers in, yet do not > > > have the hardware present, which is always a good thing. > > > > Great, so what you're saying is that we need to wait until the first > > device calls into the probe function. What about removal... how does > > a driver know when it's last device has been removed to de-register > > that? > > The "bus" that the device is on handles that, right? > > > I guess it needs the driver model to provide some way to know when a > > driver is completely unbound - but isn't that racy? > > How is it racy? That's how the driver model works... Think about what happens when the last device unregisters, but a new device comes along and is probed. I don't believe the driver model has any locking to prevent a drivers ->probe function running concurrently with it's ->remove function for two (or more) devices. The locking against this is done on a per-device basis, not a per-driver basis. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 09:43:05PM +, Alan Cox wrote: > The dynamic major/minor is the right patch. If the userspace breaks then > the userspace was broken, but I see no evidence in the discussion that > the userspace broke. The userspace breakage is that if someone has a static /dev that doesn't handle any dynamic devices then renumbering the device will cause that static /dev to stop matching the kernel. > Thats what the list says. Samsung should have followed the rules, they > didn't so they get to pick up the pieces. The Amba driver wants moving as > well. It's easy. If you want something to be ABI then make sure you get > it upstream first, if not you get to own all the pain down the line. This stuff is all upstream already, a quick check suggests both drivers predate git - it's been noticed because the ARM multiplatform work has caused people to try booting kernels with both built in. > If the hardware isn't present then the driver shouldn't even register > with the tty layer in the first place so it doesn't make any resource > differeneces either for properly written code. Right, that's not the idiom that has been followed by any of serial drivers though so needs fixing too. signature.asc Description: Digital signature
Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 09:32:06PM +, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > > On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote: > > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > > uart_register_driver call binds the driver to a specific device > > > > node through tty_register_driver call. This should typically happen > > > > during device probe call. > > > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > > drivers are part of the kernel. Currently the driver registration fails > > > > if multiple serial drivers with same default major/minor numbers are > > > > included in the kernel. > > > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > I agree, the Samsung driver is "broken" here, but that's no reason why > > these two drivers can't register with the tty layer _after_ the hardware > > is detected, and not before. > > > > That saves resources on systems that build the drivers in, yet do not > > have the hardware present, which is always a good thing. > > Great, so what you're saying is that we need to wait until the first > device calls into the probe function. What about removal... how does > a driver know when it's last device has been removed to de-register > that? The "bus" that the device is on handles that, right? > I guess it needs the driver model to provide some way to know when a > driver is completely unbound - but isn't that racy? How is it racy? That's how the driver model works... thanks, greg k-h -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
> I had earlier submitted a patch [1] to remove the hard coded > major/minor number for Samsung UART driver, but that was rejected > because of userspace breakage. Without this patch, Samsung UART driver > can't bind to the hard-coded device node. Changing the default > major/minor will also not help to fix the objections raised in [1]. > > Would you please suggest a way forward? > > [1] https://lkml.org/lkml/2013/12/27/2 The dynamic major/minor is the right patch. If the userspace breaks then the userspace was broken, but I see no evidence in the discussion that the userspace broke. 204,64 belongs to Altix so neither of the clashing drivers should use it. We had a table to stop messes like this, and we have dynamic numbering to stop the table being needed for the future 50 = /dev/ttyIOC0 Altix serial card ... 81 = /dev/ttyIOC31 Altix serial card Thats what the list says. Samsung should have followed the rules, they didn't so they get to pick up the pieces. The Amba driver wants moving as well. It's easy. If you want something to be ABI then make sure you get it upstream first, if not you get to own all the pain down the line. Hacking up the core code to paper over this crap is not on. We've been insisting firmly and robustly for years that people don't keep borrowing names and numbers they are not allocated, and use dynamic values whenever possible. If the hardware isn't present then the driver shouldn't even register with the tty layer in the first place so it doesn't make any resource differeneces either for properly written code. Alan -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 01:16:01PM -0800, Greg KH wrote: > On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote: > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > > uart_register_driver call binds the driver to a specific device > > > node through tty_register_driver call. This should typically happen > > > during device probe call. > > > > > > In a multiplatform scenario, it is possible that multiple serial > > > drivers are part of the kernel. Currently the driver registration fails > > > if multiple serial drivers with same default major/minor numbers are > > > included in the kernel. > > > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > I agree, the Samsung driver is "broken" here, but that's no reason why > these two drivers can't register with the tty layer _after_ the hardware > is detected, and not before. > > That saves resources on systems that build the drivers in, yet do not > have the hardware present, which is always a good thing. Great, so what you're saying is that we need to wait until the first device calls into the probe function. What about removal... how does a driver know when it's last device has been removed to de-register that? I guess it needs the driver model to provide some way to know when a driver is completely unbound - but isn't that racy? -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 10:05:30AM +, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > > uart_register_driver call binds the driver to a specific device > > node through tty_register_driver call. This should typically happen > > during device probe call. > > > > In a multiplatform scenario, it is possible that multiple serial > > drivers are part of the kernel. Currently the driver registration fails > > if multiple serial drivers with same default major/minor numbers are > > included in the kernel. > > > > A typical case is observed with amba-pl011 and samsung-uart drivers. > > The samsung-uart driver is at fault here - the major/minor numbers were > officially registered to amba-pl011. Samsung needs to be fixed properly. I agree, the Samsung driver is "broken" here, but that's no reason why these two drivers can't register with the tty layer _after_ the hardware is detected, and not before. That saves resources on systems that build the drivers in, yet do not have the hardware present, which is always a good thing. thanks, greg k-h -- 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 04/10] base: power: Add generic OF-based power domain look-up
Hi Lorenzo,
On 16.01.2014 17:34, Lorenzo Pieralisi wrote:
Hi Tomasz,
thank you for posting this series. I would like to use the DT bindings
for power domains in the bindings for C-states on ARM:
http://comments.gmane.org/gmane.linux.power-management.general/41012
and in particular link a given C-state to a given power domain so that the
kernel will have a way to actually check what devices are lost upon C-state
entry (and for devices I also mean CPU peripheral like PMUs, GIC CPU IF,
caches and possibly cpus, all of them already represented with DT nodes).
I have a remark:
- Can we group device nodes under a single power-domain-parent so that
all devices defined under that parent won't have to re-define a
power-domain property (a property like interrupt-parent, so to speak)
What do you think ?
Hmm, I can see potential benefits of such construct on platforms with
clear hierarchy of devices, but to make sure I'm getting it correctly,
is the following what you have in mind?
soc-domain-x@1234 {
compatible = "...";
reg = <...>;
power-domain-parent = <&power_domains DOMAIN_X>;
device@1000 {
compatible = "...";
// inherits power-domain = <&power_domains DOMAIN_X>
};
device@2000 {
compatible = "...";
// inherits power-domain = <&power_domains DOMAIN_X>
};
};
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 04/10] base: power: Add generic OF-based power domain look-up
Hi Kevin, On 14.01.2014 16:42, Kevin Hilman wrote: Tomasz Figa writes: This patch introduces generic code to perform power domain look-up using device tree and automatically bind devices to their power domains. Generic device tree binding is introduced to specify power domains of devices in their device tree nodes. Backwards compatibility with legacy Samsung-specific power domain bindings is provided, but for now the new code is not compiled when CONFIG_ARCH_EXYNOS is selected to avoid collision with legacy code. This will change as soon as Exynos power domain code gets converted to use the generic framework in further patch. Signed-off-by: Tomasz Figa I haven't read through this in detail yet, but wanted to make sure that the DT representation can handle nested power domains. At least SH-mobile has a hierarchy of power domains and the genpd code can handle that, so wanted to make sure that the DT representation can handle it as well. The representation of power domains themselves as implied by this patch is fully platform-specific. The only generic part is the #power-domain-cells property, which defines the number of cells needed to identify the power domain of given provider. You are free to have any platform-specific properties (or even generic ones, added on top of this patch) to let you specify the hierarchy in DT. 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 v5 4/4] [media] exynos-scaler: Add DT bindings for SCALER driver
Hi Bartlomiej,
Thanks for the review.
Yes you are right. I didn't add the users for this driver.
Once the driver gets merged, I will send more patches with the users.
Already this driver merge is pending on DT maintainers ack and I
don't want to complex it more by adding DT patches :)
Definitely, I will send the users patches once the driver gets merged.
And I will address all your comments in next version of patch series.
Regards,
Shaik Ameer Basha
On Thu, Jan 9, 2014 at 6:20 PM, Bartlomiej Zolnierkiewicz
wrote:
>
> Hi,
>
> On Thursday, January 09, 2014 08:58:14 AM Shaik Ameer Basha wrote:
>> This patch adds the DT binding documentation for the
>> Exynos5420/5410 based SCALER device driver.
>>
>> Signed-off-by: Shaik Ameer Basha
>> Reviewed-by: Sylwester Nawrocki
>> ---
>> .../devicetree/bindings/media/exynos5-scaler.txt | 22
>>
>> 1 file changed, 22 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/exynos5-scaler.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> new file mode 100644
>> index 000..9328e7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> @@ -0,0 +1,22 @@
>> +* Samsung Exynos5 SCALER device
>> +
>> +SCALER is used for scaling, blending, color fill and color space
>> +conversion on EXYNOS[5420/5410] SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5420-scaler" or
>> + "samsung,exynos5410-scaler"
>> +- reg: should contain SCALER physical address location and length
>> +- interrupts: should contain SCALER interrupt number
>> +- clocks: should contain the SCALER clock specifier, from the
>> + common clock bindings
>> +- clock-names: should be "scaler"
>> +
>> +Example:
>> + scaler_0: scaler@1280 {
>> + compatible = "samsung,exynos5420-scaler";
>> + reg = <0x1280 0x1000>;
>> + interrupts = <0 220 0>;
>> + clocks = <&clock 381>;
>> + clock-names = "scaler";
>> + };
>
> Your patchset adds support for EXYNOS5 SCALER but doesn't add any real
> users of it yet. Could you please explain why?
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> --
> 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 1/4] phy: Add new Exynos5 USB 3.0 PHY driver
Hi Kishon, [...] Right. While 3.0 block(PIPE3) can be used for Super Speed, 2.0 block(UTMI+) can be used for High speed. >>> >>> >>> It should then come under *single IP muliple PHY* category similar >>> to what >>> Sylwester has done. [...] >> >> The idea is to model the driver as close to the hardware though I understand >> there won't be any advantages w.r.t power or performance. maybe in later >> versions of the IP we'll have separate bits to control usb3 and usb2. > > Ok, i will prepare the next patchset for separating out the possible > code based on > the UTMI+ or PIPE3 phys. Though when experimenting with the PHY > settings i can see > there's little of such code :-) > >> >> I think for power control we should have both usb3 and usb2 power-on callback >> calling a single function that controls the power bit. > Right. I will do that. Have posted the next version of patch with functionality to support multiple PHYs as suggested. Please review the same. Thanks !! -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- 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 v3] phy: Add new Exynos5 USB 3.0 PHY driver
Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
The new driver uses the generic PHY framework and will interact
with DWC3 controller present on Exynos5 series of SoCs.
Thereby, removing old phy-samsung-usb3 driver and related code
used untill now which was based on usb/phy framework.
Signed-off-by: Vivek Gautam
---
Changes from v2:
1) Added support for multiple PHYs (UTMI+ and PIPE3) and
related changes in the driver structuring.
2) Added a xlate function to get the required phy out of
number of PHYs in mutiple PHY scenerio.
3) Changed the names of few structures and variables to
have a clearer meaning.
4) Added 'usb3phy_config' structure to take care of mutiple
phys for a SoC having 'exynos5_usb3phy_drv_data' driver data.
5) Not deleting support for old driver 'phy-samsung-usb3' until
required support for generic phy is added to DWC3.
.../devicetree/bindings/phy/samsung-phy.txt| 49 ++
drivers/phy/Kconfig|8 +
drivers/phy/Makefile |1 +
drivers/phy/phy-exynos5-usb3.c | 621
4 files changed, 679 insertions(+)
create mode 100644 drivers/phy/phy-exynos5-usb3.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index c0fccaa..57079f8 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -20,3 +20,52 @@ Required properties:
- compatible : should be "samsung,exynos5250-dp-video-phy";
- reg : offset and length of the Display Port PHY register set;
- #phy-cells : from the generic PHY bindings, must be 0;
+
+Samsung Exynos5 SoC series USB 3.0 PHY controller
+--
+
+Required properties:
+- compatible : Should be set to one of the following supported values:
+ - "samsung,exynos5250-usb3phy" - for exynos5250 SoC,
+ - "samsung,exynos5420-usb3phy" - for exynos5420 SoC.
+- reg : Register offset and length of USB 3.0 PHY register set;
+- clocks: Clock IDs array as required by the controller
+- clock-names: names of clocks correseponding to IDs in the clock property;
+ Required clocks:
+ - phy: main PHY clock (same as USB 3.0 controller IP clock),
+ used for register access.
+ - usb3phy_refclk: PHY's reference clock (usually crystal clock),
+ associated by phy name, used to determine bit values for
+ clock settings register.
+ Additional clock required for Exynos5420:
+ - usb30_sclk_100m: Additional special clock used for PHY operation
+ depicted as 'sclk_usbphy30' in CMU of Exynos5420.
+- samsung,syscon-phandle: phandle for syscon interface, which is used to
+ control pmu registers for power isolation.
+- #phy-cells : from the generic PHY bindings, must be 1;
+
+For "samsung,exynos5250-usb3phy" and "samsung,exynos5420-usb3phy" compatible
+PHYs, the second cell in the PHY specifier identifies the PHY id, which is
+interpreted as follows:
+ 0 - UTMI+ type phy,
+ 1 - PIPE3 type phy,
+
+Example:
+ usb3_phy: usbphy@1210 {
+ compatible = "samsung,exynos5250-usb3phy";
+ reg = <0x1210 0x100>;
+ clocks = <&clock 286>, <&clock 1>;
+ clock-names = "phy", "usb3phy_refclk";
+ samsung,syscon-phandle = <&pmu_syscon>;
+ #phy-cells = <1>;
+ };
+
+- aliases: For SoCs like Exynos5420 having multiple USB PHY controllers,
+ 'usb3_phy' nodes should have numbered alias in the aliases node,
+ in the form of usb3phyN, N = 0, 1... (depending on number of
+ controllers).
+Example:
+ aliases {
+ usb3phy0 = &usb3_phy0;
+ usb3phy1 = &usb3_phy1;
+ };
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 330ef2d..32f9f38 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -51,4 +51,12 @@ config PHY_EXYNOS_DP_VIDEO
help
Support for Display Port PHY found on Samsung EXYNOS SoCs.
+config PHY_EXYNOS5_USB3
+ tristate "Exynos5 SoC series USB 3.0 PHY driver"
+ depends on ARCH_EXYNOS5
+ select GENERIC_PHY
+ select MFD_SYSCON
+ help
+ Enable USB 3.0 PHY support for Exynos 5 SoC series
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d0caae9..9c06a61 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
obj-$(CONFIG_OMAP_USB2)+= phy-omap-usb2.o
obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
+obj-$(CONFIG_PHY_EXYNOS5_USB3) += phy-exynos5-usb3.o
diff --git a/drivers/phy/phy-exynos5-usb3.c b/drivers/phy/phy-exynos5-usb3.c
new file mod
Re: [PATCH 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 05:23:21PM +0530, Tushar Behera wrote: > On 20 January 2014 15:35, Russell King - ARM Linux > wrote: > > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > >> uart_register_driver call binds the driver to a specific device > >> node through tty_register_driver call. This should typically happen > >> during device probe call. > >> > >> In a multiplatform scenario, it is possible that multiple serial > >> drivers are part of the kernel. Currently the driver registration fails > >> if multiple serial drivers with same default major/minor numbers are > >> included in the kernel. > >> > >> A typical case is observed with amba-pl011 and samsung-uart drivers. > > > > The samsung-uart driver is at fault here - the major/minor numbers were > > officially registered to amba-pl011. Samsung needs to be fixed properly. > > > > I had earlier submitted a patch [1] to remove the hard coded > major/minor number for Samsung UART driver, but that was rejected > because of userspace breakage. Without this patch, Samsung UART driver > can't bind to the hard-coded device node. Changing the default > major/minor will also not help to fix the objections raised in [1]. > > Would you please suggest a way forward? I still assert that it's for Samsung people to sort out their abuse of the major/minor numbers - we have a procedure in place to allocate these, I followed it for the AMBA PL011 driver, they didn't. Their problem - and they have to suffer with the consequences of that bad decision on their part, which is that they have to deal with the inevitable breakage caused by having to renumber. > [1] https://lkml.org/lkml/2013/12/27/2 lkml.org is dead at the moment - tried from two different ISPs in the UK, it's nameservers are unreachable, and its https port refuses connections. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On 20 January 2014 15:35, Russell King - ARM Linux wrote: > On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: >> uart_register_driver call binds the driver to a specific device >> node through tty_register_driver call. This should typically happen >> during device probe call. >> >> In a multiplatform scenario, it is possible that multiple serial >> drivers are part of the kernel. Currently the driver registration fails >> if multiple serial drivers with same default major/minor numbers are >> included in the kernel. >> >> A typical case is observed with amba-pl011 and samsung-uart drivers. > > The samsung-uart driver is at fault here - the major/minor numbers were > officially registered to amba-pl011. Samsung needs to be fixed properly. > I had earlier submitted a patch [1] to remove the hard coded major/minor number for Samsung UART driver, but that was rejected because of userspace breakage. Without this patch, Samsung UART driver can't bind to the hard-coded device node. Changing the default major/minor will also not help to fix the objections raised in [1]. Would you please suggest a way forward? [1] https://lkml.org/lkml/2013/12/27/2 -- Tushar Behera -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote: > uart_register_driver call binds the driver to a specific device > node through tty_register_driver call. This should typically happen > during device probe call. > > In a multiplatform scenario, it is possible that multiple serial > drivers are part of the kernel. Currently the driver registration fails > if multiple serial drivers with same default major/minor numbers are > included in the kernel. > > A typical case is observed with amba-pl011 and samsung-uart drivers. The samsung-uart driver is at fault here - the major/minor numbers were officially registered to amba-pl011. Samsung needs to be fixed properly. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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] serial: pl011: Move uart_register_driver call to device probe
On Mon, Jan 20, 2014 at 02:32:35PM +0530, Tushar Behera wrote: > uart_register_driver call binds the driver to a specific device > node through tty_register_driver call. This should typically happen > during device probe call. > > In a multiplatform scenario, it is possible that multiple serial > drivers are part of the kernel. Currently the driver registration fails > if multiple serial drivers with same default major/minor numbers are > included in the kernel. > > A typical case is observed with amba-pl011 and samsung-uart drivers. NAK. There should not be any other driver using amba-pl011's device numbers. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit". -- 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 1/2] serial: samsung: Move uart_register_driver call to device probe
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically happen
during device probe call.
In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with same default major/minor numbers are
included in the kernel.
A typical case is observed with amba-pl011 and samsung-uart drivers.
Signed-off-by: Tushar Behera
---
drivers/tty/serial/samsung.c | 40 +++-
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index c1af04d..f8f41dc 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1283,6 +1283,14 @@ static int s3c24xx_serial_probe(struct platform_device
*pdev)
if (ret < 0)
goto probe_err;
+ if (!s3c24xx_uart_drv.state) {
+ ret = uart_register_driver(&s3c24xx_uart_drv);
+ if (ret < 0) {
+ pr_err("Failed to register Samsung UART driver\n");
+ return ret;
+ }
+ }
+
dbg("%s: adding port\n", __func__);
uart_add_one_port(&s3c24xx_uart_drv, &ourport->port);
platform_set_drvdata(pdev, &ourport->port);
@@ -1315,6 +1323,8 @@ static int s3c24xx_serial_remove(struct platform_device
*dev)
uart_remove_one_port(&s3c24xx_uart_drv, port);
}
+ uart_unregister_driver(&s3c24xx_uart_drv);
+
return 0;
}
@@ -1814,35 +1824,7 @@ static struct platform_driver samsung_serial_driver = {
},
};
-/* module initialisation code */
-
-static int __init s3c24xx_serial_modinit(void)
-{
- int ret;
-
- ret = uart_register_driver(&s3c24xx_uart_drv);
- if (ret < 0) {
- pr_err("Failed to register Samsung UART driver\n");
- return ret;
- }
-
- ret = platform_driver_register(&samsung_serial_driver);
- if (ret < 0) {
- pr_err("Failed to register platform driver\n");
- uart_unregister_driver(&s3c24xx_uart_drv);
- }
-
- return ret;
-}
-
-static void __exit s3c24xx_serial_modexit(void)
-{
- platform_driver_unregister(&samsung_serial_driver);
- uart_unregister_driver(&s3c24xx_uart_drv);
-}
-
-module_init(s3c24xx_serial_modinit);
-module_exit(s3c24xx_serial_modexit);
+module_platform_driver(samsung_serial_driver);
MODULE_ALIAS("platform:samsung-uart");
MODULE_DESCRIPTION("Samsung SoC Serial port driver");
--
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 2/2] serial: pl011: Move uart_register_driver call to device probe
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically happen
during device probe call.
In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with same default major/minor numbers are
included in the kernel.
A typical case is observed with amba-pl011 and samsung-uart drivers.
Signed-off-by: Tushar Behera
---
drivers/tty/serial/amba-pl011.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index d58783d..d4eda24 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2154,9 +2154,19 @@ static int pl011_probe(struct amba_device *dev, const
struct amba_id *id)
amba_ports[i] = uap;
amba_set_drvdata(dev, uap);
+
+ if (!amba_reg.state) {
+ ret = uart_register_driver(&amba_reg);
+ if (ret < 0) {
+ pr_err("Failed to register AMBA-PL011 driver\n");
+ return ret;
+ }
+ }
+
ret = uart_add_one_port(&amba_reg, &uap->port);
if (ret) {
amba_ports[i] = NULL;
+ uart_unregister_driver(&amba_reg);
pl011_dma_remove(uap);
}
out:
@@ -2175,6 +2185,7 @@ static int pl011_remove(struct amba_device *dev)
amba_ports[i] = NULL;
pl011_dma_remove(uap);
+ uart_unregister_driver(&amba_reg);
return 0;
}
@@ -2230,22 +2241,14 @@ static struct amba_driver pl011_driver = {
static int __init pl011_init(void)
{
- int ret;
printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
- ret = uart_register_driver(&amba_reg);
- if (ret == 0) {
- ret = amba_driver_register(&pl011_driver);
- if (ret)
- uart_unregister_driver(&amba_reg);
- }
- return ret;
+ return amba_driver_register(&pl011_driver);
}
static void __exit pl011_exit(void)
{
amba_driver_unregister(&pl011_driver);
- uart_unregister_driver(&amba_reg);
}
/*
--
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 0/2] serial: Move uart_register_driver call to device probe
In a multiplatform scenario, it is possible that multiple serial drivers are part of the kernel. Currently the driver registration fails if multiple serial drivers with same default major/minor numbers are included in the kernel. A typical case is observed with amba-pl011 and samsung-uart drivers. This is the 3rd iteration to fix above problem. Iteration 1: [PATCH] serial: samsung: Remove hard-coded major/minor numbers https://lkml.org/lkml/2013/12/27/2 Iteration 2: [PATCH] tty: Fallback to use dynamic major number https://lkml.org/lkml/2014/1/16/2 Both these approaches were rejected because they were breaking userspace interface. Iteration 3: uart_register_driver call binds the driver to a specific device node through tty_driver_register call. This is moved to device probe call. Tushar Behera (2): serial: samsung: Move uart_register_driver call to device probe serial: pl011: Move uart_register_driver call to device probe drivers/tty/serial/amba-pl011.c | 21 +++- drivers/tty/serial/samsung.c| 40 +++ 2 files changed, 23 insertions(+), 38 deletions(-) -- 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 v2 2/7] clk: samsung: add infrastructure to register cpu clocks
Hi Thomas,
> From: Thomas Abraham
>
> 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.
>
> In addition to this, the arm cpu clock provider for Exynos4210 and
> compatible SoCs is instantiated using the new cpu clock type. The
> clock frequency table and the clock configuration data for this clock
> is obtained from device tree. This implementation is reusable for
> Exynos4x12 and Exynos5250 SoCs as well.
>
> Cc: Tomasz Figa
> Cc: Lukasz Majewski
> Signed-off-by: Thomas Abraham
> ---
> drivers/clk/samsung/Makefile |2 +-
> drivers/clk/samsung/clk-cpu.c | 345
> +
> drivers/clk/samsung/clk.h |3 + 3 files changed, 349
> insertions(+), 1 deletions(-) create mode 100644
> drivers/clk/samsung/clk-cpu.c
>
> diff --git a/drivers/clk/samsung/Makefile
> b/drivers/clk/samsung/Makefile index 8eb4799..e2b453f 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_ARCH_EXYNOS4) += clk-exynos4.o
> obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o
> obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o
> diff --git a/drivers/clk/samsung/clk-cpu.c
> b/drivers/clk/samsung/clk-cpu.c new file mode 100644
> index 000..92fba45
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -0,0 +1,345 @@
> +/*
> + * 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 functions to register the cpu
> clocks
> + * for samsung platforms.
> +*/
> +
> +#include
> +#include "clk.h"
> +
> +#define SRC_CPU 0x0
> +#define STAT_CPU 0x200
> +#define DIV_CPU0 0x300
> +#define DIV_CPU1 0x304
> +#define DIV_STAT_CPU00x400
> +#define DIV_STAT_CPU10x404
> +
> +/**
> + * struct samsung_cpuclk_freq_table: table of frequency supported by
> + * a cpu clock and associated data if any.
> + * @freq: points to a table of supported frequencies (in KHz)
> + * @freq_count: number of entries in the frequency table
> + * @data: cpu clock specific data, if any
> + *
> + * This structure holds the frequency options supported by the cpu
> clock in
> + * which this structure is contained. The data pointer is an
> optional data
> + * that can provide any additional configuration options for the
> supported
> + * frequencies. This structure is intended to be reusable for all
> cpu clocks
> + * in Samsung SoC based platforms
> + */
> +struct samsung_cpuclk_freq_table {
> + const unsigned long *freq; /* in KHz */
> + unsigned long freq_count;
> + const void *data;
> +};
> +
> +/**
> + * struct exynos4210_freq_data: format of auxillary data associated
> with
> + * each frequency supported by the cpu clock for exynos4210.
> + * @parent_freq: The frequency of the parent clock required to
> generate the
> + * supported cpu clock speed.
> + * @div0: value to be programmed in the div_cpu0 register.
> + * @div1: value to be programmed in the div_cpu1 register.
> + *
> + * This structure holds the auxillary configuration data for each
> supported
> + * cpu clock frequency on Exynos4210 and compatible SoCs.
> + */
> +struct exynos4210_freq_data {
> + unsigned long parent_freq;
> + unsigned intdiv0;
> + unsigned intdiv1;
> +};
> +
> +/**
> + * struct samsung_cpuclk: information about clock supplied to a CPU
> core.
> + * @hw: handle between ccf and cpu clock.
> + * @ctrl_base: base address of the clock controller.
> + * @offset: offset from the ctrl_base address where the cpu clock
> div/mux
> + * registers can be accessed.
> + * @parent: clock handle representing the clock output of the parent
> clock.
> + * @freq_table: the frequency table supported by this cpu clock.
> + */
> +struct samsung_cpuclk {
> + struct clk_hw hw;
> + void __iomem*ctrl_base;
> + unsigned long offset;
> + struct clk *parent;
> + const struct samsung_cpuclk_freq_table *freq_table;
> +};
> +
> +#define to_samsung_cpuclk(hw)container_of(hw, struct
> samsung_cpuclk, hw) +
> +/**
> + * struct samsung_cpuclk_match_data: soc specific data for cpu
> clocks.
> + * @parser: pointer to a function that can pars
Re: [PATCH v2 1/7] cpufreq: cpufreq-cpu0: allow optional safe voltage during frequency transitions
Hi Thomas,
> From: Thomas Abraham
>
> On some platforms such as the Samsung Exynos, changing the frequency
> of the CPU clock requires changing the frequency of the PLL that is
> supplying the CPU clock. To change the frequency of the PLL, the CPU
> clock is temporarily reparented to another parent clock.
>
> The clock frequency of this temporary parent clock could be much
> higher than the clock frequency of the PLL at the time of
> reparenting. Due to the temporary increase in the CPU clock speed,
> the CPU (and any other components in the CPU clock domain such as
> dividers, mux, etc.) have to to be operated at a higher voltage
> level, called the safe voltage level. This patch adds optional
> support to temporarily switch to a safe voltage level during CPU
> frequency transitions.
>
> Cc: Shawn Guo
> Signed-off-by: Thomas Abraham
> ---
> .../devicetree/bindings/cpufreq/cpufreq-cpu0.txt |7
> drivers/cpufreq/cpufreq-cpu0.c | 37
> +-- 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt
> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt index
> f055515..37453ab 100644 ---
> a/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt +++
> b/Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt @@ -19,6
> +19,12 @@ Optional properties:
> - cooling-min-level:
> - cooling-max-level:
> Please refer to
> Documentation/devicetree/bindings/thermal/thermal.txt. +- safe-opp:
> Certain platforms require that during a opp transition,
> + a system should not go below a particular opp level. For such
> systems,
> + this property specifies the minimum opp to be maintained during the
> + opp transitions. The safe-opp value is a tuple with first element
> + representing the safe frequency and the second element
> representing the
> + safe voltage.
>
> Examples:
>
> @@ -36,6 +42,7 @@ cpus {
> 396000 95
> 198000 85
> >;
> + safe-opp = <396000 95>
> clock-latency = <61036>; /* two CLK32 periods */
> #cooling-cells = <2>;
> cooling-min-level = <0>;
> diff --git a/drivers/cpufreq/cpufreq-cpu0.c
> b/drivers/cpufreq/cpufreq-cpu0.c index 0c12ffc..075d3d1 100644
> --- a/drivers/cpufreq/cpufreq-cpu0.c
> +++ b/drivers/cpufreq/cpufreq-cpu0.c
> @@ -27,6 +27,8 @@
>
> static unsigned int transition_latency;
> static unsigned int voltage_tolerance; /* in percentage */
> +static unsigned long safe_frequency;
> +static unsigned long safe_voltage;
>
> static struct device *cpu_dev;
> static struct clk *cpu_clk;
> @@ -64,17 +66,30 @@ static int cpu0_set_target(struct cpufreq_policy
> *policy, unsigned int index) volt_old =
> regulator_get_voltage(cpu_reg); }
>
> - pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> + pr_debug("\n\n%u MHz, %ld mV --> %u MHz, %ld mV\n",
>old_freq / 1000, volt_old ? volt_old / 1000 : -1,
>new_freq / 1000, volt ? volt / 1000 : -1);
>
> /* scaling up? scale voltage before frequency */
> - if (!IS_ERR(cpu_reg) && new_freq > old_freq) {
> + if (!IS_ERR(cpu_reg) && new_freq > old_freq &&
> + new_freq >= safe_frequency) {
> ret = regulator_set_voltage_tol(cpu_reg, volt, tol);
> if (ret) {
> pr_err("failed to scale voltage up: %d\n",
> ret); return ret;
> }
> + } else if (!IS_ERR(cpu_reg) && old_freq < safe_frequency) {
> + /*
> + * the scaled up voltage level for the new_freq is
> lower
> + * than the safe voltage level. so set safe_voltage
> + * as the intermediate voltage level and revert it
> + * back after the frequency has been changed.
> + */
> + ret = regulator_set_voltage_tol(cpu_reg,
> safe_voltage, tol);
> + if (ret) {
> + pr_err("failed to set safe voltage: %d\n",
> ret);
> + return ret;
> + }
> }
>
> ret = clk_set_rate(cpu_clk, freq_exact);
> @@ -86,7 +101,8 @@ static int cpu0_set_target(struct cpufreq_policy
> *policy, unsigned int index) }
>
> /* scaling down? scale voltage after frequency */
> - if (!IS_ERR(cpu_reg) && new_freq < old_freq) {
> + if (!IS_ERR(cpu_reg) &&
> + (new_freq < old_freq || new_freq <
> safe_frequency)) { ret = regulator_set_voltage_tol(cpu_reg, volt,
> tol); if (ret) {
> pr_err("failed to scale voltage down: %d\n",
> ret); @@ -116,6 +132,8 @@ static struct cpufreq_driver
> cpu0_cpufreq_driver = {
> static int cpu0_cpufreq_probe(struct platform_device *pdev)
> {
> + const struct property *prop;
> + struct dev_pm_opp *opp;
> struct device_node *np;
> int ret;
>
> @@ -165,13 +183,24 @@
Re: [PATCH v2 7/7] cpufreq: exynos: remove all exynos specific cpufreq driver support
Hi Thomas,
> From: Thomas Abraham
>
> Exynos4210, Exynos4x12 and Exynos5250 based platforms have switched
> over to use cpufreq-cpu0 driver for cpufreq functionality. So the
> Exynos specific cpufreq drivers for these platforms can be removed.
>
> Cc: Kukjin Kim
> Cc: Jaecheol Lee
> Signed-off-by: Thomas Abraham
> ---
> drivers/cpufreq/Kconfig.arm | 36
> drivers/cpufreq/Makefile |4 -
> drivers/cpufreq/exynos-cpufreq.c | 292
> --
> drivers/cpufreq/exynos-cpufreq.h | 91 ---
> drivers/cpufreq/exynos4210-cpufreq.c | 157 --
> drivers/cpufreq/exynos4x12-cpufreq.c | 211
> drivers/cpufreq/exynos5250-cpufreq.c | 183 - 7
> files changed, 0 insertions(+), 974 deletions(-) delete mode 100644
> drivers/cpufreq/exynos-cpufreq.c delete mode 100644
> drivers/cpufreq/exynos-cpufreq.h delete mode 100644
> drivers/cpufreq/exynos4210-cpufreq.c delete mode 100644
> drivers/cpufreq/exynos4x12-cpufreq.c delete mode 100644
> drivers/cpufreq/exynos5250-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 3129749..704656d 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -16,42 +16,6 @@ config ARM_DT_BL_CPUFREQ
> This enables probing via DT for Generic CPUfreq driver for
> ARM big.LITTLE platform. This gets frequency tables from DT.
>
> -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)
> - default y
> - select ARM_EXYNOS_CPUFREQ
> - help
> - This adds the CPUFreq driver for Samsung EXYNOS4X12
> - SoC (EXYNOS4212 or EXYNOS4412).
> -
> - 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 7494565..3abfe05 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -49,10 +49,6 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ)+=
> arm_big_little_dt.o
> obj-$(CONFIG_ARCH_DAVINCI_DA850) += 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
> diff --git a/drivers/cpufreq/exynos-cpufreq.c
> b/drivers/cpufreq/exynos-cpufreq.c deleted file mode 100644
> index fcd2914..000
> --- a/drivers/cpufreq/exynos-cpufreq.c
> +++ /dev/null
> @@ -1,292 +0,0 @@
> -/*
> - * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> - * http://www.samsung.com
> - *
> - * EXYNOS - CPU frequency scaling support for EXYNOS series
> - *
> - * 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.
> -*/
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -#include
> -
> -#include "exynos-cpufreq.h"
> -
> -static struct exynos_dvfs_info *exynos_info;
> -
> -static struct regulator *arm_regulator;
> -
> -static unsigned int locking_frequency;
> -static bool frequency_locked;
> -static DEFINE_MUTEX(cpufreq_lock);
> -
> -static int exynos_cpufreq_get_index(unsigned int freq)
> -{
> - struct cpufreq_frequency_table *freq_table =
> exynos_info->freq_table;
> - int index;
> -
> - for (index = 0;
> - freq_table[index].frequency != CPUFREQ_TABLE_END;
> index++)
> - if (freq_table[index].frequency == freq)
> - break;
> -
> - if (freq_table[index].frequency == CPUFREQ_TABLE_END)
> - return -EINVAL;
> -
> - return index;
> -}
> -
> -static int exynos_cpufreq_scale(unsigned int target_freq)
> -{
> - struct cpufreq_frequency_table *freq_table =
> exynos_info->freq_table;
> - unsigned i
