* [PATCH v3 0/8] Add Allwinner A20 GMAC ethernet support
From: Chen-Yu Tsai @ 2014-02-03 3:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This is the remaining part of v3 of the Allwinner A20 GMAC glue layer for
stmmac. The stmmac driver changes have been merged through net-next. The
remaining bits are clock and DT patches. The patches should be applied
over my clock renaming patches.
The Allwinner A20 SoC integrates an early version of dwmac
IP from Synopsys. On top of that is a hardware glue layer.
This layer needs to be configured before the dwmac can be
used.
Part of the glue layer is a clock mux, which controls the
source and direction of the TX clock used by GMAC.
Changes since v2:
* Added more comments on GMAC clock driver
* Drop CLK_SET_PARENT_GATE in GMAC clock driver
* Use macro for max clock parents
* Line wrapping
Changes since v1:
* Added optional reset control to stmmac driver core
* Added non CONFIG_RESET_CONROLLER routines for the above change
* Extended callback API, as discussed with Srinivas
* Used new stmmac_of_data to pass features and callbacks,
instead of platform data, as discussed
* Seperated clock module glue layer into clock driver
Cheers,
ChenYu
Chen-Yu Tsai (8):
clk: sunxi: Add Allwinner A20/A31 GMAC clock unit
ARM: dts: sun7i: Add GMAC clock node to sun7i DTSI
ARM: dts: sun7i: Add GMAC controller node to sun7i DTSI
ARM: dts: sun7i: Add pin muxing options for the GMAC
ARM: dts: sun7i: cubietruck: Enable the GMAC
ARM: dts: sun7i: cubieboard2: Enable GMAC instead of EMAC
ARM: dts: sun7i: olinuxino-micro: Enable GMAC instead of EMAC
ARM: dts: sun7i: Add ethernet alias for GMAC
Documentation/devicetree/bindings/clock/sunxi.txt | 26 +++++++
arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 27 ++++----
arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 12 ++++
arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 27 ++++----
arch/arm/boot/dts/sun7i-a20.dtsi | 71 ++++++++++++++++++-
drivers/clk/sunxi/clk-sunxi.c | 83 +++++++++++++++++++++++
6 files changed, 215 insertions(+), 31 deletions(-)
--
1.9.rc1
^ permalink raw reply
* [PATCH v4 8/8] ARM: dts: sun7i: rename clock node names to clk@N
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Device tree naming conventions state that node names should match
node function. Change fully functioning clock nodes to match and
add clock-output-names to all sunxi clock nodes.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun7i-a20.dtsi | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 119f066..1595e9a 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -46,11 +46,12 @@
#size-cells = <1>;
ranges;
- osc24M: osc24M at 01c20050 {
+ osc24M: clk at 01c20050 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-osc-clk";
reg = <0x01c20050 0x4>;
clock-frequency = <24000000>;
+ clock-output-names = "osc24M";
};
osc32k: clk at 0 {
@@ -60,21 +61,23 @@
clock-output-names = "osc32k";
};
- pll1: pll1 at 01c20000 {
+ pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
};
- pll4: pll4 at 01c20018 {
+ pll4: clk at 01c20018 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20018 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll4";
};
- pll5: pll5 at 01c20020 {
+ pll5: clk at 01c20020 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll5-clk";
reg = <0x01c20020 0x4>;
@@ -82,7 +85,7 @@
clock-output-names = "pll5_ddr", "pll5_other";
};
- pll6: pll6 at 01c20028 {
+ pll6: clk at 01c20028 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll6-clk";
reg = <0x01c20028 0x4>;
@@ -95,6 +98,7 @@
compatible = "allwinner,sun4i-cpu-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll1>, <&pll6 1>;
+ clock-output-names = "cpu";
};
axi: axi at 01c20054 {
@@ -102,6 +106,7 @@
compatible = "allwinner,sun4i-axi-clk";
reg = <0x01c20054 0x4>;
clocks = <&cpu>;
+ clock-output-names = "axi";
};
ahb: ahb at 01c20054 {
@@ -109,9 +114,10 @@
compatible = "allwinner,sun4i-ahb-clk";
reg = <0x01c20054 0x4>;
clocks = <&axi>;
+ clock-output-names = "ahb";
};
- ahb_gates: ahb_gates at 01c20060 {
+ ahb_gates: clk at 01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun7i-a20-ahb-gates-clk";
reg = <0x01c20060 0x8>;
@@ -136,9 +142,10 @@
compatible = "allwinner,sun4i-apb0-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb>;
+ clock-output-names = "apb0";
};
- apb0_gates: apb0_gates at 01c20068 {
+ apb0_gates: clk at 01c20068 {
#clock-cells = <1>;
compatible = "allwinner,sun7i-a20-apb0-gates-clk";
reg = <0x01c20068 0x4>;
@@ -154,6 +161,7 @@
compatible = "allwinner,sun4i-apb1-mux-clk";
reg = <0x01c20058 0x4>;
clocks = <&osc24M>, <&pll6 1>, <&osc32k>;
+ clock-output-names = "apb1_mux";
};
apb1: apb1 at 01c20058 {
@@ -161,9 +169,10 @@
compatible = "allwinner,sun4i-apb1-clk";
reg = <0x01c20058 0x4>;
clocks = <&apb1_mux>;
+ clock-output-names = "apb1";
};
- apb1_gates: apb1_gates at 01c2006c {
+ apb1_gates: clk at 01c2006c {
#clock-cells = <1>;
compatible = "allwinner,sun7i-a20-apb1-gates-clk";
reg = <0x01c2006c 0x4>;
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 7/8] ARM: dts: sun6i: rename clock node names to clk@N
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Device tree naming conventions state that node names should match
node function. Change fully functioning clock nodes to match and
add clock-output-names to all sunxi clock nodes.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 5256ad9..dbc2d29 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -60,17 +60,19 @@
clock-frequency = <24000000>;
};
- osc32k: osc32k {
+ osc32k: clk at 0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32768>;
+ clock-output-names = "osc32k";
};
- pll1: pll1 at 01c20000 {
+ pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun6i-a31-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
};
/*
@@ -97,6 +99,7 @@
* Allwinner.
*/
clocks = <&osc32k>, <&osc24M>, <&pll1>, <&pll1>;
+ clock-output-names = "cpu";
};
axi: axi at 01c20050 {
@@ -104,6 +107,7 @@
compatible = "allwinner,sun4i-axi-clk";
reg = <0x01c20050 0x4>;
clocks = <&cpu>;
+ clock-output-names = "axi";
};
ahb1_mux: ahb1_mux at 01c20054 {
@@ -111,6 +115,7 @@
compatible = "allwinner,sun6i-a31-ahb1-mux-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&axi>, <&pll6>;
+ clock-output-names = "ahb1_mux";
};
ahb1: ahb1 at 01c20054 {
@@ -118,9 +123,10 @@
compatible = "allwinner,sun4i-ahb-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb1_mux>;
+ clock-output-names = "ahb1";
};
- ahb1_gates: ahb1_gates at 01c20060 {
+ ahb1_gates: clk at 01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun6i-a31-ahb1-gates-clk";
reg = <0x01c20060 0x8>;
@@ -146,9 +152,10 @@
compatible = "allwinner,sun4i-apb0-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb1>;
+ clock-output-names = "apb1";
};
- apb1_gates: apb1_gates at 01c20060 {
+ apb1_gates: clk at 01c20068 {
#clock-cells = <1>;
compatible = "allwinner,sun6i-a31-apb1-gates-clk";
reg = <0x01c20068 0x4>;
@@ -163,6 +170,7 @@
compatible = "allwinner,sun4i-apb1-mux-clk";
reg = <0x01c20058 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll6>, <&pll6>;
+ clock-output-names = "apb2_mux";
};
apb2: apb2 at 01c20058 {
@@ -170,9 +178,10 @@
compatible = "allwinner,sun6i-a31-apb2-div-clk";
reg = <0x01c20058 0x4>;
clocks = <&apb2_mux>;
+ clock-output-names = "apb2";
};
- apb2_gates: apb2_gates at 01c2006c {
+ apb2_gates: clk at 01c2006c {
#clock-cells = <1>;
compatible = "allwinner,sun6i-a31-apb2-gates-clk";
reg = <0x01c2006c 0x4>;
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 6/8] ARM: dts: sun5i: rename clock node names to clk@N
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Device tree naming conventions state that node names should match
node function. Change fully functioning clock nodes to match and
add clock-output-names to all sunxi clock nodes.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun5i-a10s.dtsi | 30 ++++++++++++++++++++----------
arch/arm/boot/dts/sun5i-a13.dtsi | 30 ++++++++++++++++++++----------
2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index ea16054..0efad0e 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -47,34 +47,38 @@
clock-frequency = <0>;
};
- osc24M: osc24M at 01c20050 {
+ osc24M: clk at 01c20050 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-osc-clk";
reg = <0x01c20050 0x4>;
clock-frequency = <24000000>;
+ clock-output-names = "osc24M";
};
- osc32k: osc32k {
+ osc32k: clk at 0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32768>;
+ clock-output-names = "osc32k";
};
- pll1: pll1 at 01c20000 {
+ pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
};
- pll4: pll4 at 01c20018 {
+ pll4: clk at 01c20018 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20018 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll4";
};
- pll5: pll5 at 01c20020 {
+ pll5: clk at 01c20020 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll5-clk";
reg = <0x01c20020 0x4>;
@@ -82,7 +86,7 @@
clock-output-names = "pll5_ddr", "pll5_other";
};
- pll6: pll6 at 01c20028 {
+ pll6: clk at 01c20028 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll6-clk";
reg = <0x01c20028 0x4>;
@@ -96,6 +100,7 @@
compatible = "allwinner,sun4i-cpu-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll1>, <&dummy>;
+ clock-output-names = "cpu";
};
axi: axi at 01c20054 {
@@ -103,9 +108,10 @@
compatible = "allwinner,sun4i-axi-clk";
reg = <0x01c20054 0x4>;
clocks = <&cpu>;
+ clock-output-names = "axi";
};
- axi_gates: axi_gates at 01c2005c {
+ axi_gates: clk at 01c2005c {
#clock-cells = <1>;
compatible = "allwinner,sun4i-axi-gates-clk";
reg = <0x01c2005c 0x4>;
@@ -118,9 +124,10 @@
compatible = "allwinner,sun4i-ahb-clk";
reg = <0x01c20054 0x4>;
clocks = <&axi>;
+ clock-output-names = "ahb";
};
- ahb_gates: ahb_gates at 01c20060 {
+ ahb_gates: clk at 01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a10s-ahb-gates-clk";
reg = <0x01c20060 0x8>;
@@ -139,9 +146,10 @@
compatible = "allwinner,sun4i-apb0-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb>;
+ clock-output-names = "apb0";
};
- apb0_gates: apb0_gates at 01c20068 {
+ apb0_gates: clk at 01c20068 {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a10s-apb0-gates-clk";
reg = <0x01c20068 0x4>;
@@ -155,6 +163,7 @@
compatible = "allwinner,sun4i-apb1-mux-clk";
reg = <0x01c20058 0x4>;
clocks = <&osc24M>, <&pll6 1>, <&osc32k>;
+ clock-output-names = "apb1_mux";
};
apb1: apb1 at 01c20058 {
@@ -162,9 +171,10 @@
compatible = "allwinner,sun4i-apb1-clk";
reg = <0x01c20058 0x4>;
clocks = <&apb1_mux>;
+ clock-output-names = "apb1";
};
- apb1_gates: apb1_gates at 01c2006c {
+ apb1_gates: clk at 01c2006c {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a10s-apb1-gates-clk";
reg = <0x01c2006c 0x4>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 320335a..08468b7 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -47,34 +47,38 @@
clock-frequency = <0>;
};
- osc24M: osc24M at 01c20050 {
+ osc24M: clk at 01c20050 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-osc-clk";
reg = <0x01c20050 0x4>;
clock-frequency = <24000000>;
+ clock-output-names = "osc24M";
};
- osc32k: osc32k {
+ osc32k: clk at 0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32768>;
+ clock-output-names = "osc32k";
};
- pll1: pll1 at 01c20000 {
+ pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
};
- pll4: pll4 at 01c20018 {
+ pll4: clk at 01c20018 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20018 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll4";
};
- pll5: pll5 at 01c20020 {
+ pll5: clk at 01c20020 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll5-clk";
reg = <0x01c20020 0x4>;
@@ -82,7 +86,7 @@
clock-output-names = "pll5_ddr", "pll5_other";
};
- pll6: pll6 at 01c20028 {
+ pll6: clk at 01c20028 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll6-clk";
reg = <0x01c20028 0x4>;
@@ -96,6 +100,7 @@
compatible = "allwinner,sun4i-cpu-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll1>, <&dummy>;
+ clock-output-names = "cpu";
};
axi: axi at 01c20054 {
@@ -103,9 +108,10 @@
compatible = "allwinner,sun4i-axi-clk";
reg = <0x01c20054 0x4>;
clocks = <&cpu>;
+ clock-output-names = "axi";
};
- axi_gates: axi_gates at 01c2005c {
+ axi_gates: clk at 01c2005c {
#clock-cells = <1>;
compatible = "allwinner,sun4i-axi-gates-clk";
reg = <0x01c2005c 0x4>;
@@ -118,9 +124,10 @@
compatible = "allwinner,sun4i-ahb-clk";
reg = <0x01c20054 0x4>;
clocks = <&axi>;
+ clock-output-names = "ahb";
};
- ahb_gates: ahb_gates at 01c20060 {
+ ahb_gates: clk at 01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a13-ahb-gates-clk";
reg = <0x01c20060 0x8>;
@@ -138,9 +145,10 @@
compatible = "allwinner,sun4i-apb0-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb>;
+ clock-output-names = "apb0";
};
- apb0_gates: apb0_gates at 01c20068 {
+ apb0_gates: clk at 01c20068 {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a13-apb0-gates-clk";
reg = <0x01c20068 0x4>;
@@ -153,6 +161,7 @@
compatible = "allwinner,sun4i-apb1-mux-clk";
reg = <0x01c20058 0x4>;
clocks = <&osc24M>, <&pll6 1>, <&osc32k>;
+ clock-output-names = "apb1_mux";
};
apb1: apb1 at 01c20058 {
@@ -160,9 +169,10 @@
compatible = "allwinner,sun4i-apb1-clk";
reg = <0x01c20058 0x4>;
clocks = <&apb1_mux>;
+ clock-output-names = "apb1";
};
- apb1_gates: apb1_gates at 01c2006c {
+ apb1_gates: clk at 01c2006c {
#clock-cells = <1>;
compatible = "allwinner,sun5i-a13-apb1-gates-clk";
reg = <0x01c2006c 0x4>;
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 5/8] ARM: dts: sun4i: rename clock node names to clk@N
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Device tree naming conventions state that node names should match
node function. Change fully functioning clock nodes to match and
add clock-output-names to all sunxi clock nodes.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
arch/arm/boot/dts/sun4i-a10.dtsi | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..1a62c5f 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -52,34 +52,38 @@
clock-frequency = <0>;
};
- osc24M: osc24M at 01c20050 {
+ osc24M: clk at 01c20050 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-osc-clk";
reg = <0x01c20050 0x4>;
clock-frequency = <24000000>;
+ clock-output-names = "osc24M";
};
- osc32k: osc32k {
+ osc32k: clk at 0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32768>;
+ clock-output-names = "osc32k";
};
- pll1: pll1 at 01c20000 {
+ pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
};
- pll4: pll4 at 01c20018 {
+ pll4: clk at 01c20018 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20018 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll4";
};
- pll5: pll5 at 01c20020 {
+ pll5: clk at 01c20020 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll5-clk";
reg = <0x01c20020 0x4>;
@@ -87,7 +91,7 @@
clock-output-names = "pll5_ddr", "pll5_other";
};
- pll6: pll6 at 01c20028 {
+ pll6: clk at 01c20028 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-pll6-clk";
reg = <0x01c20028 0x4>;
@@ -101,6 +105,7 @@
compatible = "allwinner,sun4i-cpu-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll1>, <&dummy>;
+ clock-output-names = "cpu";
};
axi: axi at 01c20054 {
@@ -108,9 +113,10 @@
compatible = "allwinner,sun4i-axi-clk";
reg = <0x01c20054 0x4>;
clocks = <&cpu>;
+ clock-output-names = "axi";
};
- axi_gates: axi_gates at 01c2005c {
+ axi_gates: clk at 01c2005c {
#clock-cells = <1>;
compatible = "allwinner,sun4i-axi-gates-clk";
reg = <0x01c2005c 0x4>;
@@ -123,9 +129,10 @@
compatible = "allwinner,sun4i-ahb-clk";
reg = <0x01c20054 0x4>;
clocks = <&axi>;
+ clock-output-names = "ahb";
};
- ahb_gates: ahb_gates at 01c20060 {
+ ahb_gates: clk at 01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-ahb-gates-clk";
reg = <0x01c20060 0x8>;
@@ -148,9 +155,10 @@
compatible = "allwinner,sun4i-apb0-clk";
reg = <0x01c20054 0x4>;
clocks = <&ahb>;
+ clock-output-names = "apb0";
};
- apb0_gates: apb0_gates at 01c20068 {
+ apb0_gates: clk at 01c20068 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-apb0-gates-clk";
reg = <0x01c20068 0x4>;
@@ -165,6 +173,7 @@
compatible = "allwinner,sun4i-apb1-mux-clk";
reg = <0x01c20058 0x4>;
clocks = <&osc24M>, <&pll6 1>, <&osc32k>;
+ clock-output-names = "apb1_mux";
};
apb1: apb1 at 01c20058 {
@@ -172,9 +181,10 @@
compatible = "allwinner,sun4i-apb1-clk";
reg = <0x01c20058 0x4>;
clocks = <&apb1_mux>;
+ clock-output-names = "apb1";
};
- apb1_gates: apb1_gates at 01c2006c {
+ apb1_gates: clk at 01c2006c {
#clock-cells = <1>;
compatible = "allwinner,sun4i-apb1-gates-clk";
reg = <0x01c2006c 0x4>;
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 4/8] clk: sunxi: get divs parent clock name from parent factor clock
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Divs clocks consist of a parent factor clock with multiple outputs,
and seperate clocks for each output. Get the name of the parent
clock from the parent factor clock, instead of the DT node name.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Mike Turquette <mturquette@linaro.org>
---
drivers/clk/sunxi/clk-sunxi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7a2ed98..736fb60 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -869,7 +869,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
struct divs_data *data)
{
struct clk_onecell_data *clk_data;
- const char *parent = node->name;
+ const char *parent;
const char *clk_name;
struct clk **clks, *pclk;
struct clk_hw *gate_hw, *rate_hw;
@@ -883,6 +883,7 @@ static void __init sunxi_divs_clk_setup(struct device_node *node,
/* Set up factor clock that we will be dividing */
pclk = sunxi_factors_clk_setup(node, data->factors);
+ parent = __clk_get_name(pclk);
reg = of_iomap(node, 0);
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 3/8] clk: sunxi: add names for pll5, pll6 parent clocks to factors_data
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
Some factor clocks, such as the parent clock of pll5 and pll6, have
multiple output names. Add the corresponding names to factors_data
tied to compatible string.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Mike Turquette <mturquette@linaro.org>
---
drivers/clk/sunxi/clk-sunxi.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 0ed9794..7a2ed98 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -389,6 +389,7 @@ struct factors_data {
int mux;
struct clk_factors_config *table;
void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
+ const char *name;
};
static struct clk_factors_config sun4i_pll1_config = {
@@ -457,6 +458,14 @@ static const struct factors_data sun4i_pll5_data __initconst = {
.enable = 31,
.table = &sun4i_pll5_config,
.getter = sun4i_get_pll5_factors,
+ .name = "pll5",
+};
+
+static const struct factors_data sun4i_pll6_data __initconst = {
+ .enable = 31,
+ .table = &sun4i_pll5_config,
+ .getter = sun4i_get_pll5_factors,
+ .name = "pll6",
};
static const struct factors_data sun4i_apb1_data __initconst = {
@@ -499,14 +508,14 @@ static struct clk * __init sunxi_factors_clk_setup(struct device_node *node,
(parents[i] = of_clk_get_parent_name(node, i)) != NULL)
i++;
- /* Nodes should be providing the name via clock-output-names
- * but originally our dts didn't, and so we used node->name.
- * The new, better nodes look like clk at deadbeef, so we pull the
- * name just in this case */
- if (!strcmp("clk", clk_name)) {
- of_property_read_string_index(node, "clock-output-names",
- 0, &clk_name);
- }
+ /*
+ * some factor clocks, such as pll5 and pll6, may have multiple
+ * outputs, and have their name designated in factors_data
+ */
+ if (data->name)
+ clk_name = data->name;
+ else
+ of_property_read_string(node, "clock-output-names", &clk_name);
factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
if (!factors)
@@ -838,7 +847,7 @@ static const struct divs_data pll5_divs_data __initconst = {
};
static const struct divs_data pll6_divs_data __initconst = {
- .factors = &sun4i_pll5_data,
+ .factors = &sun4i_pll6_data,
.div = {
{ .shift = 0, .table = pll6_sata_tbl, .gate = 14 }, /* M, SATA */
{ .fixed = 2 }, /* P, other */
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 2/8] clk: sunxi: update clock-output-names dt binding documentation
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
clock-output-names is now required for most of sunxi clock nodes, to
provide the name of the corresponding clock. Add the new requirements,
exceptions, as well as examples.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
Documentation/devicetree/bindings/clock/sunxi.txt | 32 ++++++++++++++++++-----
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index c2cb762..0cf679b 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -44,10 +44,11 @@ Required properties for all clocks:
multiplexed clocks, the list order must match the hardware
programming order.
- #clock-cells : from common clock binding; shall be set to 0 except for
- "allwinner,*-gates-clk" where it shall be set to 1
-
-Additionally, "allwinner,*-gates-clk" clocks require:
-- clock-output-names : the corresponding gate names that the clock controls
+ "allwinner,*-gates-clk", "allwinner,sun4i-pll5-clk" and
+ "allwinner,sun4i-pll6-clk" where it shall be set to 1
+- clock-output-names : shall be the corresponding names of the outputs.
+ If the clock module only has one output, the name shall be the
+ module name.
Clock consumers should specify the desired clocks they use with a
"clocks" phandle cell. Consumers that are using a gated clock should
@@ -56,18 +57,28 @@ offset of the bit controlling this particular gate in the register.
For example:
-osc24M: osc24M at 01c20050 {
+osc24M: clk at 01c20050 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-osc-clk";
reg = <0x01c20050 0x4>;
clocks = <&osc24M_fixed>;
+ clock-output-names = "osc24M";
};
-pll1: pll1 at 01c20000 {
+pll1: clk at 01c20000 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-pll1-clk";
reg = <0x01c20000 0x4>;
clocks = <&osc24M>;
+ clock-output-names = "pll1";
+};
+
+pll5: clk at 01c20020 {
+ #clock-cells = <1>;
+ compatible = "allwinner,sun4i-pll5-clk";
+ reg = <0x01c20020 0x4>;
+ clocks = <&osc24M>;
+ clock-output-names = "pll5_ddr", "pll5_other";
};
cpu: cpu at 01c20054 {
@@ -75,4 +86,13 @@ cpu: cpu at 01c20054 {
compatible = "allwinner,sun4i-cpu-clk";
reg = <0x01c20054 0x4>;
clocks = <&osc32k>, <&osc24M>, <&pll1>;
+ clock-output-names = "cpu";
+};
+
+mmc0_clk: clk at 01c20088 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-mod0-clk";
+ reg = <0x01c20088 0x4>;
+ clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
+ clock-output-names = "mmc0";
};
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 1/8] clk: sunxi: add clock-output-names dt property support
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391392304-4660-1-git-send-email-wens@csie.org>
sunxi clock drivers use dt node name as clock name, but clock
nodes should be named clk at X, so the names would be the same.
Let the drivers read clock names from dt clock-output-names
property.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Mike Turquette <mturquette@linaro.org>
---
drivers/clk/sunxi/clk-sunxi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index abb6c5a..0ed9794 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -51,6 +51,8 @@ static void __init sun4i_osc_clk_setup(struct device_node *node)
if (!gate)
goto err_free_fixed;
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
/* set up gate and fixed rate properties */
gate->reg = of_iomap(node, 0);
gate->bit_idx = SUNXI_OSC24M_GATE;
@@ -601,6 +603,8 @@ static void __init sunxi_mux_clk_setup(struct device_node *node,
(parents[i] = of_clk_get_parent_name(node, i)) != NULL)
i++;
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
clk = clk_register_mux(NULL, clk_name, parents, i,
CLK_SET_RATE_NO_REPARENT, reg,
data->shift, SUNXI_MUX_GATE_WIDTH,
@@ -660,6 +664,8 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
clk_parent = of_clk_get_parent_name(node, 0);
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
clk = clk_register_divider(NULL, clk_name, clk_parent, 0,
reg, data->shift, data->width,
data->pow ? CLK_DIVIDER_POWER_OF_TWO : 0,
--
1.9.rc1
^ permalink raw reply related
* [PATCH v4 0/8] ARM: sunxi: rename DT clock node names to clk@N
From: Chen-Yu Tsai @ 2014-02-03 1:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This is v4 of the clock node renaming patch series, which renames
the clock nodes in sunxi dts to conform to device tree naming
conventions, i.e. clk at N. Dummy clocks that will be renamed/removed
later, or clocks sharing registers are not renamed.
Renamed clock nodes have clock-output-names properties added to
desginate their names. Support for this is added in the first
patch.
Changes since v3:
* Simplify device tree binding documentation: require all sunxi clocks
to have clock-output-names.
* Add clock-output-names to all sunxi clock nodes, including ones not
renamed.
Changes since v2:
* Dropped Cubietruck dts i2c controller patch. Maxime has taken it.
* Changed ARM in commit messages to uppercase.
* Add pll5, pll6 clock names to factors_data tied to compatible strings.
* Dropped pll5 output name in dts due to the previous change.
* Added dts binding documentation for "clock-output-names", as well as
examples.
Changes since v1:
* Fixed pll5, pll6 divs clock name handling
Cheers
ChenYu
Chen-Yu Tsai (8):
clk: sunxi: add clock-output-names dt property support
clk: sunxi: update clock-output-names dt binding documentation
clk: sunxi: add names for pll5, pll6 parent clocks to factors_data
clk: sunxi: get divs parent clock name from parent factor clock
ARM: dts: sun4i: rename clock node names to clk at N
ARM: dts: sun5i: rename clock node names to clk at N
ARM: dts: sun6i: rename clock node names to clk at N
ARM: dts: sun7i: rename clock node names to clk at N
Documentation/devicetree/bindings/clock/sunxi.txt | 32 ++++++++++++++++----
arch/arm/boot/dts/sun4i-a10.dtsi | 30 ++++++++++++-------
arch/arm/boot/dts/sun5i-a10s.dtsi | 30 ++++++++++++-------
arch/arm/boot/dts/sun5i-a13.dtsi | 30 ++++++++++++-------
arch/arm/boot/dts/sun6i-a31.dtsi | 19 ++++++++----
arch/arm/boot/dts/sun7i-a20.dtsi | 25 +++++++++++-----
drivers/clk/sunxi/clk-sunxi.c | 36 ++++++++++++++++-------
7 files changed, 143 insertions(+), 59 deletions(-)
--
1.9.rc1
^ permalink raw reply
* NFS client broken in Linus' tip
From: Trond Myklebust @ 2014-02-02 22:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202122757.GC26684@n2100.arm.linux.org.uk>
On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> > On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> > > On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > > > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > > > Yes and no. I still end up with an empty /etc/mtab, but the file now
> > > > > > exists. However, I can create and echo data into /etc/mtab, but it seems
> > > > > > that can't happen at boot time.
> > > > >
> > > > > Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> > > >
> > > > Unfortunately, that results in some problem at boot time, which
> > > > ultimately ends up with the other three CPUs being stopped, and
> > > > hence the original reason scrolls off the screen before it can be
> > > > read... even at 1920p.
> > > >
> > > Hi Russell,
> > >
> > > The following patch fixes the issue for me.
> >
> > It doesn't entirely fix the issue for me, instead we've got even weirder
> > behaviour:
> >
> > root at cubox-i4:~# ls -al test
> > ls: cannot access test: No such file or directory
> > root at cubox-i4:~# touch test
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
> > root at cubox-i4:~# echo foo > test
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 4 Feb 1 01:01 test
> > root at cubox-i4:~# cat test
> > foo
> > root at cubox-i4:~# rm test
> > root at cubox-i4:~# echo foo > test
> > -bash: test: Operation not supported
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb 1 01:01 test
>
> FYI, I just tested Linus' tip, and NFS is still broken.
>
Hi Russell,
The following patch should fix the above problem. It needs to be applied
on top of the one I sent you previously. In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com
Cheers,
Trond
8<------------------------------------------------------------------------------
>From f9ad7a7b43554bc36eccf03d33617c05b8a2c77d Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Sun, 2 Feb 2014 14:36:42 -0500
Subject: [PATCH] NFSv3: Fix return value of nfs3_proc_setacls
nfs3_proc_setacls is used internally by the NFSv3 create operations
to set the acl after the file has been created. If the operation
fails because the server doesn't support acls, then it must return '0',
not -EOPNOTSUPP.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
fs/nfs/nfs3acl.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 9271a6bb9a41..871d6eda8dba 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -113,7 +113,7 @@ getout:
return ERR_PTR(status);
}
-int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+static int __nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
struct posix_acl *dfacl)
{
struct nfs_server *server = NFS_SERVER(inode);
@@ -198,6 +198,15 @@ out:
return status;
}
+int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+ struct posix_acl *dfacl)
+{
+ int ret;
+ ret = __nfs3_proc_setacls(inode, acl, dfacl);
+ return (ret == -EOPNOTSUPP) ? 0 : ret;
+
+}
+
int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
{
struct posix_acl *alloc = NULL, *dfacl = NULL;
@@ -225,7 +234,7 @@ int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
if (IS_ERR(alloc))
goto fail;
}
- status = nfs3_proc_setacls(inode, acl, dfacl);
+ status = __nfs3_proc_setacls(inode, acl, dfacl);
posix_acl_release(alloc);
return status;
--
1.8.5.3
--
Trond Myklebust
Linux NFS client maintainer
^ permalink raw reply related
* [PATCH v2 RESEND 2/3] ARM: dts: clps711x: Add bindings documentation for CLPS711X irqchip driver
From: Rob Herring @ 2014-02-02 21:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391328520-30923-1-git-send-email-shc_work@mail.ru>
On Sun, Feb 2, 2014 at 2:08 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Add OF document for Cirrus Logic CLPS711X irqchip driver.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> .../interrupt-controller/cirrus,clps711x-intc.txt | 41 ++++++++++++++++++++++
Acked-by: Rob Herring <robh@kernel.org>
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
> new file mode 100644
> index 0000000..759339c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
> @@ -0,0 +1,41 @@
> +Cirrus Logic CLPS711X Interrupt Controller
> +
> +Required properties:
> +
> +- compatible: Should be "cirrus,clps711x-intc".
> +- reg: Specifies base physical address of the registers set.
> +- interrupt-controller: Identifies the node as an interrupt controller.
> +- #interrupt-cells: Specifies the number of cells needed to encode an
> + interrupt source. The value shall be 1.
> +
> +The interrupt sources are as follows:
> +ID Name Description
> +---------------------------
> +1: BLINT Battery low (FIQ)
> +3: MCINT Media changed (FIQ)
> +4: CSINT CODEC sound
> +5: EINT1 External 1
> +6: EINT2 External 2
> +7: EINT3 External 3
> +8: TC1OI TC1 under flow
> +9: TC2OI TC2 under flow
> +10: RTCMI RTC compare match
> +11: TINT 64Hz tick
> +12: UTXINT1 UART1 transmit FIFO half empty
> +13: URXINT1 UART1 receive FIFO half full
> +14: UMSINT UART1 modem status changed
> +15: SSEOTI SSI1 end of transfer
> +16: KBDINT Keyboard
> +17: SS2RX SSI2 receive FIFO half or greater full
> +18: SS2TX SSI2 transmit FIFO less than half empty
> +28: UTXINT2 UART2 transmit FIFO half empty
> +29: URXINT2 UART2 receive FIFO half full
> +32: DAIINT DAI interface (FIQ)
> +
> +Example:
> + intc: interrupt-controller {
> + compatible = "cirrus,clps711x-intc";
> + reg = <0x80000000 0x4000>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] arm64: KVM: Add VGIC device control for arm64
From: Christoffer Dall @ 2014-02-02 21:41 UTC (permalink / raw)
To: linux-arm-kernel
This fixes the build breakage introduced by
c07a0191ef2de1f9510f12d1f88e3b0b5cd8d66f and adds support for the device
control API and save/restore of the VGIC state for ARMv8.
The defines were simply missing from the arm64 header files and
uaccess.h must be implicitly imported from somewhere else on arm.
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++
virt/kvm/arm/vgic.c | 1 +
2 files changed, 10 insertions(+)
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 495ab6f..eaf54a3 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -148,6 +148,15 @@ struct kvm_arch_memory_slot {
#define KVM_REG_ARM_TIMER_CNT ARM64_SYS_REG(3, 3, 14, 3, 2)
#define KVM_REG_ARM_TIMER_CVAL ARM64_SYS_REG(3, 3, 14, 0, 2)
+/* Device Control API: ARM VGIC */
+#define KVM_DEV_ARM_VGIC_GRP_ADDR 0
+#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
+#define KVM_DEV_ARM_VGIC_GRP_CPU_REGS 2
+#define KVM_DEV_ARM_VGIC_CPUID_SHIFT 32
+#define KVM_DEV_ARM_VGIC_CPUID_MASK (0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
+#define KVM_DEV_ARM_VGIC_OFFSET_SHIFT 0
+#define KVM_DEV_ARM_VGIC_OFFSET_MASK (0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+
/* KVM_IRQ_LINE irq field index values */
#define KVM_ARM_IRQ_TYPE_SHIFT 24
#define KVM_ARM_IRQ_TYPE_MASK 0xff
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index be456ce..8ca405c 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -24,6 +24,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/uaccess.h>
#include <linux/irqchip/arm-gic.h>
--
1.8.5.2
^ permalink raw reply related
* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 20:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202191505.GK26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 19:15:05 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> In which case, it may be better to reorder the remaining patches such
> that the DT changes are at the very end - which means we can still
> benefit from the rest of the patches if the DT solution remains an
> open question.
>
> We do have another option now that my generic component support is in
> mainline (merged during this window), which should result in a much
> cleaner solution. If we convert TDA998x to a component, armada DRM
> to a component master (and same for other tda998x users) then we don't
> need the drm_encoder_slave stuff - all that goes away since it's no
> longer necessary.
>
> We also solve this problem as well - because we're then not messing
> around with working out if there's a DT node present: the TDA998x
> device must pre-exist. For non-DT setups, this can be done when
> the I2C bus is created - devices on it would be created using the
> standard mechanisms already present via the i2c_board_data array.
> For DT setups, the devices are created by parsing the I2C bus node
> in DT.
>
> Both cases result in a component being registered upon invocation of
> tda998x_probe(), and removal of the component when tda998x_remove()
> is called. The tda998x driver becomes a standard I2C driver.
>
> This is something I've been intending to look at now that the component
> stuff is in place - as I said previously when the questions around DT
> and Armada DRM were first posed, we need to solve these issues in a
> generic way first, rather than hacking around them.
Agree. I was looking in the same direction...
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202195400.073f4eb4@armhf>
On Sun, Feb 02, 2014 at 07:54:00PM +0100, Jean-Francois Moine wrote:
> I explained how to use the tda998x in a DT context in a message to Jyri
> Sarha:
>
> http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html
Okay, so there's a bunch of changes required to the DRM slave support
which aren't in place yet.
In which case, it may be better to reorder the remaining patches such
that the DT changes are at the very end - which means we can still
benefit from the rest of the patches if the DT solution remains an
open question.
We do have another option now that my generic component support is in
mainline (merged during this window), which should result in a much
cleaner solution. If we convert TDA998x to a component, armada DRM
to a component master (and same for other tda998x users) then we don't
need the drm_encoder_slave stuff - all that goes away since it's no
longer necessary.
We also solve this problem as well - because we're then not messing
around with working out if there's a DT node present: the TDA998x
device must pre-exist. For non-DT setups, this can be done when
the I2C bus is created - devices on it would be created using the
standard mechanisms already present via the i2c_board_data array.
For DT setups, the devices are created by parsing the I2C bus node
in DT.
Both cases result in a component being registered upon invocation of
tda998x_probe(), and removal of the component when tda998x_remove()
is called. The tda998x driver becomes a standard I2C driver.
This is something I've been intending to look at now that the component
stuff is in place - as I said previously when the questions around DT
and Armada DRM were first posed, we need to solve these issues in a
generic way first, rather than hacking around them.
--
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".
^ permalink raw reply
* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 19:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202180433.GI26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 18:04:34 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> So, in summary, I'm pretty happy with this again - and it's all been
> tested here with no apparant detrimental effects. All committed and
> queued up here:
>
> http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel
>
> If you want to pull it back to check:
>
> git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel
>
> I'm proposing to send everything up to the tda998x-fixes tag next week
> for 3.13-rc kernels.
Everything is OK. Thank you.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 18:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202182349.GJ26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 18:23:49 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
> > On Sun, 2 Feb 2014 12:43:58 +0000
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> >
> > > On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > > > This patch set contains various extensions to the tda998x driver:
> > > >
> > > > - simplify the i2c read/write
> > > > - code cleanup and fix some small errors
> > > > - use global constants
> > > > - don't read write-only registers
> > > > - add DT support
> > > > - use IRQ for connection status and EDID read
> > >
> > > I discussed these patches with Rob Clark recently, and the conclusion
> > > we came to is that I'll merge them into a git tree, test them, and
> > > once I'm happy I'll send a pull request as appropriate.
> > >
> > > I'll go through them later today. Those patches which have been re-
> > > posted without any change for the last few times (the first few) I'll
> > > take into my git tree today so you don't have to keep re-posting them
> > > (more importantly, I won't have to keep on looking at them either.)
> >
> > Thanks.
> >
> > BTW, I found some problems with the patch 12 'add DT support' you
> > already acked:
> >
> > - the .of_match_table is not needed because the i2c client is created by
> > the i2c subsystem from the 'reg' in the DT,
>
> Okay - may it be a good idea to have someone knowledgable of I2C give it
> a review?
Sure! There is still a lot of magic in the DT.
I used the tda998x in the DT for a long time without .of_match_table
and it loaded correctly. I added it in the patch just because my other
modules had it.
A few days ago, when I moved the tda998x audio codec description in the
DT as a subnode of the tda998x i2c, the codec module was not loaded. An
of_platform_populate() of the subnodes was needed in the tda998x i2c
driver. I could not find why...
> > - on encoder_destroy(), the function drm_i2c_encoder_destroy()
> > unregisters the i2c client, so, with a DT, a second encoder_init()
> > would crash.
>
> I think this is one of the down-sides of trying to bolt DT into this:
> the drm encoder slave support is not designed to cope with an i2c client
> device pre-created.
>
> In fact, I can't see how this stuff comes anywhere close to working in
> a DT setup: in such a scenario, you declare that there's a tda998x
> device in DT. I2C parses this, and creates an i2c_client itself for
> the tda998x.
>
> When the TDA998x driver initialises, it finds this i2c client and
> binds to it, calling tda998x_probe(), which does nothing.
>
> However, the only way to attach a slave encoder to a DRM device is via
> a call to drm_i2c_encoder_init(), which unconditionally calls
> i2c_new_device(). This creates a _new_ i2c_client structure, again
> unconditionally, for the tda998x. This must be bound by the I2C
> subsystem to a driver - hopefully the tda998x driver, which then
> calls it's encoder_init function.
>
> None of this will happen if DT has already created an i2c_client at
> the appropriate address, because DRMs i2c_new_device() will fail.
>
> My hypothesis is that you have other patches to I2C and/or DRM to
> sort this out which you haven't been posting with this series. So,
> could you please provide some hints as to how this works?
I explained how to use the tda998x in a DT context in a message to Jyri
Sarha:
http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 00/23]
From: Sebastian Hesselbarth @ 2014-02-02 18:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202182349.GJ26684@n2100.arm.linux.org.uk>
On 02/02/2014 07:23 PM, Russell King - ARM Linux wrote:
> On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
>> - on encoder_destroy(), the function drm_i2c_encoder_destroy()
>> unregisters the i2c client, so, with a DT, a second encoder_init()
>> would crash.
>
> I think this is one of the down-sides of trying to bolt DT into this:
> the drm encoder slave support is not designed to cope with an i2c client
> device pre-created.
>
> In fact, I can't see how this stuff comes anywhere close to working in
> a DT setup: in such a scenario, you declare that there's a tda998x
> device in DT. I2C parses this, and creates an i2c_client itself for
> the tda998x.
>
> When the TDA998x driver initialises, it finds this i2c client and
> binds to it, calling tda998x_probe(), which does nothing.
>
> However, the only way to attach a slave encoder to a DRM device is via
> a call to drm_i2c_encoder_init(), which unconditionally calls
> i2c_new_device(). This creates a _new_ i2c_client structure, again
> unconditionally, for the tda998x. This must be bound by the I2C
> subsystem to a driver - hopefully the tda998x driver, which then
> calls it's encoder_init function.
>
> None of this will happen if DT has already created an i2c_client at
> the appropriate address, because DRMs i2c_new_device() will fail.
drm_i2c_encoder_init() could look at .of_node of the i2c_board_info.
If it is there, do not try to i2c_new_device as it has already been
registered by DT i2c auto-probing.
Sebastian
^ permalink raw reply
* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202190606.6fa193ce@armhf>
On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 12:43:58 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
> > On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > > This patch set contains various extensions to the tda998x driver:
> > >
> > > - simplify the i2c read/write
> > > - code cleanup and fix some small errors
> > > - use global constants
> > > - don't read write-only registers
> > > - add DT support
> > > - use IRQ for connection status and EDID read
> >
> > I discussed these patches with Rob Clark recently, and the conclusion
> > we came to is that I'll merge them into a git tree, test them, and
> > once I'm happy I'll send a pull request as appropriate.
> >
> > I'll go through them later today. Those patches which have been re-
> > posted without any change for the last few times (the first few) I'll
> > take into my git tree today so you don't have to keep re-posting them
> > (more importantly, I won't have to keep on looking at them either.)
>
> Thanks.
>
> BTW, I found some problems with the patch 12 'add DT support' you
> already acked:
>
> - the .of_match_table is not needed because the i2c client is created by
> the i2c subsystem from the 'reg' in the DT,
Okay - may it be a good idea to have someone knowledgable of I2C give it
a review?
> - on encoder_destroy(), the function drm_i2c_encoder_destroy()
> unregisters the i2c client, so, with a DT, a second encoder_init()
> would crash.
I think this is one of the down-sides of trying to bolt DT into this:
the drm encoder slave support is not designed to cope with an i2c client
device pre-created.
In fact, I can't see how this stuff comes anywhere close to working in
a DT setup: in such a scenario, you declare that there's a tda998x
device in DT. I2C parses this, and creates an i2c_client itself for
the tda998x.
When the TDA998x driver initialises, it finds this i2c client and
binds to it, calling tda998x_probe(), which does nothing.
However, the only way to attach a slave encoder to a DRM device is via
a call to drm_i2c_encoder_init(), which unconditionally calls
i2c_new_device(). This creates a _new_ i2c_client structure, again
unconditionally, for the tda998x. This must be bound by the I2C
subsystem to a driver - hopefully the tda998x driver, which then
calls it's encoder_init function.
None of this will happen if DT has already created an i2c_client at
the appropriate address, because DRMs i2c_new_device() will fail.
My hypothesis is that you have other patches to I2C and/or DRM to
sort this out which you haven't been posting with this series. So,
could you please provide some hints as to how this works?
--
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".
^ permalink raw reply
* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 18:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202124358.GD26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 12:43:58 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > This patch set contains various extensions to the tda998x driver:
> >
> > - simplify the i2c read/write
> > - code cleanup and fix some small errors
> > - use global constants
> > - don't read write-only registers
> > - add DT support
> > - use IRQ for connection status and EDID read
>
> I discussed these patches with Rob Clark recently, and the conclusion
> we came to is that I'll merge them into a git tree, test them, and
> once I'm happy I'll send a pull request as appropriate.
>
> I'll go through them later today. Those patches which have been re-
> posted without any change for the last few times (the first few) I'll
> take into my git tree today so you don't have to keep re-posting them
> (more importantly, I won't have to keep on looking at them either.)
Thanks.
BTW, I found some problems with the patch 12 'add DT support' you
already acked:
- the .of_match_table is not needed because the i2c client is created by
the i2c subsystem from the 'reg' in the DT,
- on encoder_destroy(), the function drm_i2c_encoder_destroy()
unregisters the i2c client, so, with a DT, a second encoder_init()
would crash.
Do you better like a new patch or a fix?
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 18:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202124358.GD26684@n2100.arm.linux.org.uk>
On Sun, Feb 02, 2014 at 12:43:58PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > This patch set contains various extensions to the tda998x driver:
> >
> > - simplify the i2c read/write
> > - code cleanup and fix some small errors
> > - use global constants
> > - don't read write-only registers
> > - add DT support
> > - use IRQ for connection status and EDID read
>
> I discussed these patches with Rob Clark recently, and the conclusion
> we came to is that I'll merge them into a git tree, test them, and
> once I'm happy I'll send a pull request as appropriate.
>
> I'll go through them later today. Those patches which have been re-
> posted without any change for the last few times (the first few) I'll
> take into my git tree today so you don't have to keep re-posting them
> (more importantly, I won't have to keep on looking at them either.)
Okay, out of your patches, I would like to queue up the following
for submission into -rc kernels:
drm/i2c: tda998x: fix bad value in the AIF
drm/i2c: tda998x: check the CEC device creation
drm/i2c: tda998x: free the CEC device on encoder_destroy
drm/i2c: tda998x: force the page register at startup time
drm/i2c: tda998x: set the PLL division factor in range 0..3
drm/i2c: tda998x: fix the ENABLE_SPACE register
since these fix real bugs. Moving them to the front of the queue isn't
that big a deal (I've done it here in my git tree - yes, there were a
few conflicts, but nothing serious.)
I'll also consider these for -rc too:
drm/i2c: tda998x: use HDMI constants
drm/i2c: tda998x: add the active aspect in HDMI AVI frame
drm/i2c: tda998x: change the frequence in the audio channel
if we have reports of display devices affected by the info frame errors.
As far as the last summary line goes, I'll change it to:
"Use ALSA IEC958 definitions and update audio frequency"
Note that in general, bug fixes should always be towards the beginning
of a patch series, so that they can be applied independently of the
remainder of the development, which also makes them easier to backport
to stable kernels if that's deemed necessary.
As for the remainder, I think moving the DT documentation patch
immediately after the addition of DT code (or even just before it) is
good form.
So, in summary, I'm pretty happy with this again - and it's all been
tested here with no apparant detrimental effects. All committed and
queued up here:
http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel
If you want to pull it back to check:
git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel
I'm proposing to send everything up to the tda998x-fixes tag next week
for 3.13-rc kernels.
Rob, do you want to provide any acks for this or are you happy?
--
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".
^ permalink raw reply
* [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
From: Russell King - ARM Linux @ 2014-02-02 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202184512.7fa2e3cf@armhf>
On Sun, Feb 02, 2014 at 06:45:12PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 16:23:09 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
> > On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote:
> > > This patch takes care of the write-only registers of the tda998x.
> > >
> > > The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits
> > > cleared after reset, so, they may be fully re-written.
> > >
> > > The register MAT_CONTRL is set to
> > > MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)
> > > after reset, so, it may be fully set again to this value.
> >
> > I said in v3 of this patch, which seems to remain unaddressed:
> >
> > > /* must be last register set: */
> > > - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> > > + reg_write(priv, REG_TBG_CNTRL_0, 0);
> >
> > Register changes which have a potential effect shouldn't be part of a
> > patch which is really only trying to avoid reading from write only
> > registers.
> >
> > This could be a potential functional change - and it's probably one
> > which Rob Clark should at least be made aware of. As I commented last
> > time, when you're changing register values in an otherwise innocuous
> > patch, you should comment about them in the patch description.
>
> According to the tda9983b documentation, the register TBG_CNTRL_0 is
> set to 0 at reset time. I think that it is the same for all the tda998x
> family. In the other hand, this register is supposed to be write only,
> so reading it may return any value, and the reg_clear() function may
> set any other bits. Then, clearing one bit is less secure than clearing
> the full register.
Okay, in that case I'm happy with this patch.
--
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".
^ permalink raw reply
* [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
From: Russell King - ARM Linux @ 2014-02-02 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202183000.1cf9ca9c@armhf>
On Sun, Feb 02, 2014 at 06:30:00PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 16:20:58 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
> > On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
> > > This patch adds more error checking inn I2C I/O functions.
> > > In case of I/O error, this permits to avoid writing in bad controller
> > > pages, a bad chipset detection or looping when getting the EDID.
> >
> > I've just looked at this again, and spotted something:
> >
> > > -static uint8_t
> > > +static int
> > > reg_read(struct tda998x_priv *priv, uint16_t reg)
> > > {
> > > uint8_t val = 0;
> > > - reg_read_range(priv, reg, &val, sizeof(val));
> > > + int ret;
> > > +
> > > + ret = reg_read_range(priv, reg, &val, sizeof(val));
> > > + if (ret < 0)
> > > + return ret;
> >
> > So yes, this can return negative numbers.
> >
> > > @@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
> > > tda998x_reset(priv);
> > >
> > > /* read version: */
> > > - priv->rev = reg_read(priv, REG_VERSION_LSB) |
> > > - reg_read(priv, REG_VERSION_MSB) << 8;
> > > + ret = reg_read(priv, REG_VERSION_LSB) |
> > > + (reg_read(priv, REG_VERSION_MSB) << 8);
> > > + if (ret < 0)
> > > + goto fail;
> > > + priv->rev = ret;
> >
> > Two issues here:
> >
> > 1. The additional parens are /really/ not required.
> > 2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
> >
> > If we're going to the extent of attempting to make the read/write
> > functions return errors, we should at least handle errors generated
> > by them properly, otherwise it's pointless making them return errors.
> >
> > ret = reg_read(priv, REG_VERSION_LSB);
> > if (ret < 0)
> > goto fail;
> >
> > priv->rev = ret;
> >
> > ret = reg_read(priv, REG_VERSION_MSB);
> > if (ret < 0)
> > goto fail;
> >
> > priv->rev |= ret << 8;
> >
> > If you want it to look slightly nicer:
> >
> > int rev_lo, rev_hi;
> >
> > rev_lo = reg_read(priv, REG_VERSION_LSB);
> > rev_hi = reg_read(priv, REG_VERSION_MSB);
> > if (rev_lo < 0 || rev_hi < 0) {
> > ret = rev_lo < 0 ? rev_lo : rev_hi;
> > goto fail;
> > }
> >
> > priv->rev = rev_lo | rev_hi << 8;
> >
> > I'm happy to commit such a change after this patch to clean it up, or if
> > you want to regenerate your patch 2 and post /just/ that incorporating
> > this change.
>
> I think that my code works correctly: when there is an error, the
> result of reg_read() is minus the error code, and this error code is
> always lower than 8388607 (0x7fffff). Then, reg_read() << 8 will always
> be negative.
The issue I'm pointing out is not whether it will be interpreted as an
error or not, it's whether the value is a correct error code. If we
don't care about the reason why an error occurs, we might as well just
return -1.
I've added my own patch which adjusts it as per above to my tree anyway,
so I'm not that worried about this.
--
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".
^ permalink raw reply
* [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
From: Jean-Francois Moine @ 2014-02-02 17:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202162309.GF26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 16:23:09 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote:
> > This patch takes care of the write-only registers of the tda998x.
> >
> > The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits
> > cleared after reset, so, they may be fully re-written.
> >
> > The register MAT_CONTRL is set to
> > MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)
> > after reset, so, it may be fully set again to this value.
>
> I said in v3 of this patch, which seems to remain unaddressed:
>
> > /* must be last register set: */
> > - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> > + reg_write(priv, REG_TBG_CNTRL_0, 0);
>
> Register changes which have a potential effect shouldn't be part of a
> patch which is really only trying to avoid reading from write only
> registers.
>
> This could be a potential functional change - and it's probably one
> which Rob Clark should at least be made aware of. As I commented last
> time, when you're changing register values in an otherwise innocuous
> patch, you should comment about them in the patch description.
According to the tda9983b documentation, the register TBG_CNTRL_0 is
set to 0 at reset time. I think that it is the same for all the tda998x
family. In the other hand, this register is supposed to be write only,
so reading it may return any value, and the reg_clear() function may
set any other bits. Then, clearing one bit is less secure than clearing
the full register.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
From: Jean-Francois Moine @ 2014-02-02 17:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140202162057.GE26684@n2100.arm.linux.org.uk>
On Sun, 2 Feb 2014 16:20:58 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
> > This patch adds more error checking inn I2C I/O functions.
> > In case of I/O error, this permits to avoid writing in bad controller
> > pages, a bad chipset detection or looping when getting the EDID.
>
> I've just looked at this again, and spotted something:
>
> > -static uint8_t
> > +static int
> > reg_read(struct tda998x_priv *priv, uint16_t reg)
> > {
> > uint8_t val = 0;
> > - reg_read_range(priv, reg, &val, sizeof(val));
> > + int ret;
> > +
> > + ret = reg_read_range(priv, reg, &val, sizeof(val));
> > + if (ret < 0)
> > + return ret;
>
> So yes, this can return negative numbers.
>
> > @@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
> > tda998x_reset(priv);
> >
> > /* read version: */
> > - priv->rev = reg_read(priv, REG_VERSION_LSB) |
> > - reg_read(priv, REG_VERSION_MSB) << 8;
> > + ret = reg_read(priv, REG_VERSION_LSB) |
> > + (reg_read(priv, REG_VERSION_MSB) << 8);
> > + if (ret < 0)
> > + goto fail;
> > + priv->rev = ret;
>
> Two issues here:
>
> 1. The additional parens are /really/ not required.
> 2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
>
> If we're going to the extent of attempting to make the read/write
> functions return errors, we should at least handle errors generated
> by them properly, otherwise it's pointless making them return errors.
>
> ret = reg_read(priv, REG_VERSION_LSB);
> if (ret < 0)
> goto fail;
>
> priv->rev = ret;
>
> ret = reg_read(priv, REG_VERSION_MSB);
> if (ret < 0)
> goto fail;
>
> priv->rev |= ret << 8;
>
> If you want it to look slightly nicer:
>
> int rev_lo, rev_hi;
>
> rev_lo = reg_read(priv, REG_VERSION_LSB);
> rev_hi = reg_read(priv, REG_VERSION_MSB);
> if (rev_lo < 0 || rev_hi < 0) {
> ret = rev_lo < 0 ? rev_lo : rev_hi;
> goto fail;
> }
>
> priv->rev = rev_lo | rev_hi << 8;
>
> I'm happy to commit such a change after this patch to clean it up, or if
> you want to regenerate your patch 2 and post /just/ that incorporating
> this change.
I think that my code works correctly: when there is an error, the
result of reg_read() is minus the error code, and this error code is
always lower than 8388607 (0x7fffff). Then, reg_read() << 8 will always
be negative.
Otherwise, I may redo a patch about the useless parenthesis.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox