Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 10/24] drm/i2c: tda998x: don't read write-only registers
From: Russell King - ARM Linux @ 2014-01-22 21:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140119195842.24c40e22@armhf>

On Sun, Jan 19, 2014 at 07:58:42PM +0100, Jean-Francois Moine wrote:
> This patch takes care of the write-only registers of the tda998x.
> 
> The value 'MAT_CONTRL_MAT_SC(1)' in the register MAT_CONTRL has been
> set as it is at reset time.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
> v3
>     - remarks from Russell King
> 	- don't move the sync polarity setting after the setting of the
> 	  register TBG_CNTRL_0 which must be the last setting of the
> 	  init sequence

This is better, except I find that there's an additional change in this
version which wasn't in the original patch 9:

>  	/* 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.

-- 
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 v2 0/5] clk: sunxi: Add support for USB clocks and reset bits
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Emilio, Maxime, et al,

Emilio, here is v2 of my patch-set adding support for sunxi-clk USB clocks and
reset bits. This addresses all your review comments from v1.

Can you add the first 2 patches to your queue of patches for Mike for 3.15 ?

Maxime, can you add patch 3-5 which add the dt bindings for this to your
tree please ?

Thanks & Regards,

Hans

^ permalink raw reply

* [PATCH v2 1/5] clk: sunxi: Add support for USB clock-register reset bits
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390426587-16287-1-git-send-email-hdegoede@redhat.com>

The usb-clk register is special in that it not only contains clk gate bits,
but also has a few reset bits. This commit adds support for this by allowing
gates type sunxi clks to also register a reset controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/clk/sunxi/clk-sunxi.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 03bb8b8..f1a147c 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -18,6 +18,7 @@
 #include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/reset-controller.h>
 
 #include "clk-factors.h"
 
@@ -741,6 +742,59 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 
 
 /**
+ * sunxi_gates_reset... - reset bits in leaf gate clk registers handling
+ */
+
+struct gates_reset_data {
+	void __iomem			*reg;
+	spinlock_t			*lock;
+	struct reset_controller_dev	rcdev;
+};
+
+static int sunxi_gates_reset_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct gates_reset_data *data = container_of(rcdev,
+						     struct gates_reset_data,
+						     rcdev);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(data->lock, flags);
+
+	reg = readl(data->reg);
+	writel(reg & ~BIT(id), data->reg);
+
+	spin_unlock_irqrestore(data->lock, flags);
+
+	return 0;
+}
+
+static int sunxi_gates_reset_deassert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct gates_reset_data *data = container_of(rcdev,
+						     struct gates_reset_data,
+						     rcdev);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(data->lock, flags);
+
+	reg = readl(data->reg);
+	writel(reg | BIT(id), data->reg);
+
+	spin_unlock_irqrestore(data->lock, flags);
+
+	return 0;
+}
+
+static struct reset_control_ops sunxi_gates_reset_ops = {
+	.assert		= sunxi_gates_reset_assert,
+	.deassert	= sunxi_gates_reset_deassert,
+};
+
+/**
  * sunxi_gates_clk_setup() - Setup function for leaf gates on clocks
  */
 
@@ -748,6 +802,7 @@ static void __init sunxi_divider_clk_setup(struct device_node *node,
 
 struct gates_data {
 	DECLARE_BITMAP(mask, SUNXI_GATES_MAX_SIZE);
+	u32 reset_mask;
 };
 
 static const struct gates_data sun4i_axi_gates_data __initconst = {
@@ -818,6 +873,7 @@ static void __init sunxi_gates_clk_setup(struct device_node *node,
 					 struct gates_data *data)
 {
 	struct clk_onecell_data *clk_data;
+	struct gates_reset_data *reset_data;
 	const char *clk_parent;
 	const char *clk_name;
 	void *reg;
@@ -861,6 +917,21 @@ static void __init sunxi_gates_clk_setup(struct device_node *node,
 	clk_data->clk_num = i;
 
 	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+	/* Register a reset controler for gates with reset bits */
+	if (data->reset_mask == 0)
+		return;
+
+	reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
+	if (!reset_data)
+		return;
+
+	reset_data->reg = reg;
+	reset_data->lock = &clk_lock;
+	reset_data->rcdev.nr_resets = hweight32(data->reset_mask);
+	reset_data->rcdev.ops = &sunxi_gates_reset_ops;
+	reset_data->rcdev.of_node = node;
+	reset_controller_register(&reset_data->rcdev);
 }
 
 
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390426587-16287-1-git-send-email-hdegoede@redhat.com>

From: Roman Byshko <rbyshko@gmail.com>

Add register definitions for the usb-clk register found on sun4i, sun5i and
sun7i SoCs.

Signed-off-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/clock/sunxi.txt |  5 +++++
 drivers/clk/sunxi/clk-sunxi.c                     | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 79c7197..8bccb6a 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -37,6 +37,8 @@ Required properties:
 	"allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
 	"allwinner,sun4i-mod0-clk" - for the module 0 family of clocks
 	"allwinner,sun7i-a20-out-clk" - for the external output clocks
+	"allwinner,sun4i-usb-gates-clk" - for usb gates + resets on A10 / A20
+	"allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
 
 Required properties for all clocks:
 - reg : shall be the control register address for the clock.
@@ -49,6 +51,9 @@ Required properties for all clocks:
 Additionally, "allwinner,*-gates-clk" clocks require:
 - clock-output-names : the corresponding gate names that the clock controls
 
+And "allwinner,*-usb-gates-clk" clocks also require:
+- reset-cells : shall be set to 1
+
 Clock consumers should specify the desired clocks they use with a
 "clocks" phandle cell. Consumers that are using a gated clock should
 provide an additional ID in their clock property. This ID is the
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index f1a147c..18cbc3c 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -813,6 +813,16 @@ static const struct gates_data sun4i_ahb_gates_data __initconst = {
 	.mask = {0x7F77FFF, 0x14FB3F},
 };
 
+static const struct gates_data sun4i_usb_gates_data __initconst = {
+	.mask = {0x1C0},
+	.reset_mask = 0x07,
+};
+
+static const struct gates_data sun5i_a13_usb_gates_data __initconst = {
+	.mask = {0x140},
+	.reset_mask = 0x03,
+};
+
 static const struct gates_data sun5i_a10s_ahb_gates_data __initconst = {
 	.mask = {0x147667e7, 0x185915},
 };
@@ -1159,6 +1169,8 @@ static const struct of_device_id clk_gates_match[] __initconst = {
 	{.compatible = "allwinner,sun6i-a31-apb1-gates-clk", .data = &sun6i_a31_apb1_gates_data,},
 	{.compatible = "allwinner,sun7i-a20-apb1-gates-clk", .data = &sun7i_a20_apb1_gates_data,},
 	{.compatible = "allwinner,sun6i-a31-apb2-gates-clk", .data = &sun6i_a31_apb2_gates_data,},
+	{.compatible = "allwinner,sun4i-usb-gates-clk", .data = &sun4i_usb_gates_data,},
+	{.compatible = "allwinner,sun5i-a13-usb-gates-clk", .data = &sun5i_a13_usb_gates_data,},
 	{}
 };
 
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v2 3/5] ARM: sun4i: dt: Add bindings for USB clocks
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390426587-16287-1-git-send-email-hdegoede@redhat.com>

From: Roman Byshko <rbyshko@gmail.com>

Signed-off-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 7264ef9..913674b 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -290,6 +290,15 @@
 			clock-output-names = "ir1";
 		};
 
+		usb_clk: clk at 01c200cc {
+			#clock-cells = <1>;
+		        #reset-cells = <1>;
+			compatible = "allwinner,sun4i-usb-gates-clk";
+			reg = <0x01c200cc 0x4>;
+			clocks = <&pll6 1>;
+			clock-output-names = "usb_ohci0", "usb_ohci1", "usb_phy";
+		};
+
 		spi3_clk: clk at 01c200d4 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-mod0-clk";
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v2 4/5] ARM: sun5i: dt: Add bindings for USB clocks
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390426587-16287-1-git-send-email-hdegoede@redhat.com>

From: Roman Byshko <rbyshko@gmail.com>

Signed-off-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun5i-a10s.dtsi | 9 +++++++++
 arch/arm/boot/dts/sun5i-a13.dtsi  | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index 9591959..44235e7 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -255,6 +255,15 @@
 			clock-output-names = "ir0";
 		};
 
+		usb_clk: clk at 01c200cc {
+			#clock-cells = <1>;
+		        #reset-cells = <1>;
+			compatible = "allwinner,sun5i-a13-usb-gates-clk";
+			reg = <0x01c200cc 0x4>;
+			clocks = <&pll6 1>;
+			clock-output-names = "usb_ohci0", "usb_phy";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-mod0-clk";
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index f6f2c81..4193fdf 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -252,6 +252,15 @@
 			clock-output-names = "ir0";
 		};
 
+		usb_clk: clk at 01c200cc {
+			#clock-cells = <1>;
+		        #reset-cells = <1>;
+			compatible = "allwinner,sun5i-a13-usb-gates-clk";
+			reg = <0x01c200cc 0x4>;
+			clocks = <&pll6 1>;
+			clock-output-names = "usb_ohci0", "usb_phy";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-mod0-clk";
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v2 5/5] ARM: sun7i: dt: Add bindings for USB clocks
From: Hans de Goede @ 2014-01-22 21:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390426587-16287-1-git-send-email-hdegoede@redhat.com>

From: Roman Byshko <rbyshko@gmail.com>

Signed-off-by: Roman Byshko <rbyshko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 57f1fc9..a44b3b3 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -280,6 +280,15 @@
 			clock-output-names = "ir1";
 		};
 
+		usb_clk: clk at 01c200cc {
+			#clock-cells = <1>;
+		        #reset-cells = <1>;
+			compatible = "allwinner,sun4i-usb-gates-clk";
+			reg = <0x01c200cc 0x4>;
+			clocks = <&pll6 1>;
+			clock-output-names = "usb_ohci0", "usb_ohci1", "usb_phy";
+		};
+
 		spi3_clk: clk at 01c200d4 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-mod0-clk";
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH v3 14/24] drm/i2c: tda998x: add DT support
From: Russell King - ARM Linux @ 2014-01-22 21:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140119195843.1491091f@armhf>

On Sun, Jan 19, 2014 at 07:58:43PM +0100, Jean-Francois Moine wrote:
> This patch adds DT support to the tda998x.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>

-- 
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 v3 13/24] drm/i2c: tda998x: fix a NULL pointer dereference
From: Russell King - ARM Linux @ 2014-01-22 21:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140119195843.59810a46@armhf>

On Sun, Jan 19, 2014 at 07:58:43PM +0100, Jean-Francois Moine wrote:
> This patch fixes a NULL pointer dereference when the set_config
> function has not been called (priv->params == NULL).

No, that's not what this patch is doing.  Maybe you could enlighten me
how priv->params could ever be NULL when that is _not_ a pointer?  That's
completely impossible as it isn't a pointer.

If you tried "priv->params = NULL" the C compiler would barf on it.

I suspect you've misunderstood the code, and this change isn't actually
necessary.

-- 
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

* Freescale FEC packet loss
From: Marek Vasut @ 2014-01-22 21:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi guys,

I am running stock Linux 3.13 on i.MX6Q SabreLite board. The CPU is i.MX6Q TO 
1.0 .

I am hitting a WARNING when I use the FEC ethernet to transfer data, thus I 
started investigating this problem. TL;DR I am not able to figure this problem 
out, so I am not attaching a patch :-(

Steps to reproduce:
-------------------
1) Boot stock Linux 3.13 on i.MX6Q SabreLite board
2) Plug in an SD card into one of the SD slots (I use the full-size one)
3) Plug in an USB stick into one of the USB ports (I use the upper one)
4) Plug in an ethernet cable into the board
   -> Connect the other side into a gigabit-capable PC
   -> Let us assume the PC has IP address 192.168.1.1/24 and
      the board 192.168.1.2/24
5) Install iperf on both boards
6) Bring up the ethernet link on both ends:
   $ ifconfig ...
7) On the PC, run:
   $ iperf -s -i 1
8) Start producing load on the in-CPU busses. Run this on the board:
   $ sha1sum /dev/mmcblk0 &
   $ sha1sum /dev/sda &
9) Now let the board generate ethernet traffic:
   $ iperf -c 192.168.1.1 -t 1000 -i 1

Now wait about 10 minutes and the system will produce a warning and you will 
observe dips in the transmission speed. You can see the output at the end of the 
email.

I observe that this happens more often when there is a severe load on the 
busses, which in this case I simulate by running the sha1sum on /dev/mmcblk0 and 
sha1sum /dev/sda in the background in step 8) . I can also well simulate this by 
running 'sha1sum /dev/mmcblk0 & sha1sum /dev/mmcblk1 &' when I have both SD 
cards populated on the MX6Q sabrelite with the same result (WARNING and dips in 
speed).

There was apparently a thread about this problem already [1] where the person 
used SATA to produce high bus load and had exactly the same result.

One more observation is that I managed to replicate this problem all the way 
back to Linux 3.3.8 , which is one of the first kernel versions where sabrelite 
was supported. I also see this one the Freescale's 3.0.35-4.1.0 .

I have trouble figuring out what this problem is all about. Can you please help 
me? Thank you!

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
October/202519.html

root@(none):~# iperf -c 192.168.1.1 -t 1000 -i 1
------------------------------------------------------------
Client connecting to 192.168.1.1, TCP port 5001
TCP window size: 43.8 KByte (default)
------------------------------------------------------------
[  3] local 192.168.1.128 port 36760 connected with 192.168.1.1 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 1.0 sec  25.5 MBytes   214 Mbits/sec
[  3]  1.0- 2.0 sec  28.9 MBytes   242 Mbits/sec
[  3]  2.0- 3.0 sec  24.1 MBytes   202 Mbits/sec
[  3]  3.0- 4.0 sec  21.1 MBytes   177 Mbits/sec
[  3]  4.0- 5.0 sec  20.2 MBytes   170 Mbits/sec
[  3]  5.0- 6.0 sec  20.0 MBytes   168 Mbits/sec
[  3]  6.0- 7.0 sec  20.0 MBytes   168 Mbits/sec
[  3]  7.0- 8.0 sec  20.1 MBytes   169 Mbits/sec
[  3]  8.0- 9.0 sec  20.5 MBytes   172 Mbits/sec
[  3]  9.0-10.0 sec  21.5 MBytes   180 Mbits/sec
[  3] 10.0-11.0 sec  20.0 MBytes   168 Mbits/sec
[  3] 11.0-12.0 sec  19.4 MBytes   163 Mbits/sec
[  3] 12.0-13.0 sec  19.6 MBytes   165 Mbits/sec
[  3] 13.0-14.0 sec  19.8 MBytes   166 Mbits/sec
[  3] 14.0-15.0 sec  19.4 MBytes   163 Mbits/sec
[  275.945247] ------------[ cut here ]------------
[  275.949920] WARNING: CPU: 3 PID: 1155 at net/sched/sch_generic.c:264 
dev_watchdog+0x280/0x2a4()
[  275.958679] NETDEV WATCHDOG: eth3 (fec): transmit queue 0 timed out
[  275.964985] Modules linked in:
[  275.968096] CPU: 3 PID: 1155 Comm: sha1sum Not tainted 3.13.0 #18
[  275.974217] Backtrace:                                                                                                                                                      
[  275.976787] [<80012410>] (dump_backtrace+0x0/0x10c) from [<800125ac>] 
(show_stack+0x18/0x1c)                                                                                
[  275.985271]  r6:00000108 r5:00000009 r4:00000000 r3:00000000                                                                                                                
[  275.991050] [<80012594>] (show_stack+0x0/0x1c) from [<8060f9c8>] 
(dump_stack+0x80/0x9c)                                                                                     
[  275.999103] [<8060f948>] (dump_stack+0x0/0x9c) from [<8002703c>] 
(warn_slowpath_common+0x6c/0x90)                                                                           
[  276.008017]  r4:bef43e10 r3:bef96c00                                                                                                                                        
[  276.011663] [<80026fd0>] (warn_slowpath_common+0x0/0x90) from [<80027104>] 
(warn_slowpath_fmt+0x3)                                                                          
[  276.021170]  r8:bf098240 r7:bef42000 r6:bf098200 r5:bf068000 r4:00000000                                                                                                    
[  276.028083] [<800270cc>] (warn_slowpath_fmt+0x0/0x40) from [<804e2754>] 
(dev_watchdog+0x280/0x2a4)                                                                          
[  276.037095]  r3:bf068000 r2:807d2fb4                                                                                                                                        
[  276.040734] [<804e24d4>] (dev_watchdog+0x0/0x2a4) from [<80030e1c>] 
(call_timer_fn+0x70/0xec)                                                                               
[  276.049313] [<80030dac>] (call_timer_fn+0x0/0xec) from [<80031a90>] 
(run_timer_softirq+0x198/0x23)                                                                          
[  276.058407]  r8:00200200 r7:00000000 r6:bef43ec8 r5:bf836000 r4:bf068284                                                                                                    
[  276.065257] [<800318f8>] (run_timer_softirq+0x0/0x230) from [<8002b480>] 
(__do_softirq+0x100/0x25)                                                                          
[  276.074338] [<8002b380>] (__do_softirq+0x0/0x254) from [<8002b9a8>] 
(irq_exit+0xb0/0x108)                                                                                   
[  276.082570] [<8002b8f8>] (irq_exit+0x0/0x108) from [<8000f3dc>] 
(handle_IRQ+0x58/0xb8)                                                                                      
[  276.090528]  r4:8086ccc8 r3:00000184
[  276.094162] [<8000f384>] (handle_IRQ+0x0/0xb8) from [<80008640>] 
(gic_handle_irq+0x30/0x64)
[  276.102552]  r8:5590d9fa r7:f4000100 r6:bef43fb0 r5:8086ce14 r4:f400010c
r3:000000a0
[  276.110575] [<80008610>] (gic_handle_irq+0x0/0x64) from [<800133a0>] 
(__irq_usr+0x40/0x60)
[  276.118871] Exception stack(0xbef43fb0 to 0xbef43ff8)
[  276.123942] 3fa0:                                     fbe9d585 13e98d33 
6ed9eba1 21e67a57
[  276.132173] 3fc0: 170dd9fc bc58d89e 5d1b7878 c19f2bf4 5590d9fa 2577bcc4 
7b2ac1ea 6cee44dd
[  276.140398] 3fe0: cf486f09 7ea92a58 0000ba1f 0000ab72 a0000030 ffffffff
[  276.147041]  r7:c19f2bf4 r6:ffffffff r5:a0000030 r4:0000ab72
[  276.152846] ---[ end trace 054500acb8edb763 ]---
[  3] 15.0-16.0 sec  18.8 MBytes   157 Mbits/sec
[  3] 16.0-17.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 17.0-18.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 18.0-19.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 19.0-20.0 sec  4.25 MBytes  35.7 Mbits/sec
[  3] 20.0-21.0 sec  19.8 MBytes   166 Mbits/sec
[  3] 21.0-22.0 sec  19.8 MBytes   166 Mbits/sec
[  3] 22.0-23.0 sec  19.5 MBytes   164 Mbits/sec
[  3] 23.0-24.0 sec  8.38 MBytes  70.3 Mbits/sec
[  3] 24.0-25.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 25.0-26.0 sec  7.88 MBytes  66.1 Mbits/sec
[  3] 26.0-27.0 sec  20.1 MBytes   169 Mbits/sec
[...]
[  3] 71.0-72.0 sec  23.4 MBytes   196 Mbits/sec
[  3] 72.0-73.0 sec  12.2 MBytes   103 Mbits/sec
[  3] 73.0-74.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 74.0-75.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 75.0-76.0 sec  10.9 MBytes  91.2 Mbits/sec
[  3] 76.0-77.0 sec  22.4 MBytes   188 Mbits/sec
[  3] 77.0-78.0 sec  23.0 MBytes   193 Mbits/sec

^ permalink raw reply

* [PATCH v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
From: Ezequiel Garcia @ 2014-01-22 22:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140122205213.GW18269@obsidianresearch.com>

On Wed, Jan 22, 2014 at 01:52:13PM -0700, Jason Gunthorpe wrote:
> > Clearing BRIDGE_CAUSE will only clear all currently pending upstream
> > IRQs, of course. If WDT IRQ will be re-raised right after that in
> > BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
> > clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
> 
> Which is why it makes no sense to clear it one time at kernel start.
> 

So, it seems we need to handle irq_startup(), as you suggested.
I've just tested the attached patch, and it's working fine: the driver's
probe() fully stops the watchdog, and then request_irq() acks and
pending interrupts, through the added irq_startup().

How does it look?

diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
index e51d400..91a3955 100644
--- a/drivers/irqchip/irq-orion.c
+++ b/drivers/irqchip/irq-orion.c
@@ -108,6 +108,16 @@ IRQCHIP_DECLARE(orion_intc, "marvell,orion-intc", orion_irq_init);
 #define ORION_BRIDGE_IRQ_CAUSE	0x00
 #define ORION_BRIDGE_IRQ_MASK	0x04
 
+static unsigned int orion_bridge_irq_startup(struct irq_data *data)
+{
+	/* Ack pending interrupts */
+	data->chip->irq_ack(data);
+
+	/* Unmask the interrupt */
+	data->chip->irq_unmask(data);
+	return 0;
+}
+
 static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
 	struct irq_domain *d = irq_get_handler_data(irq);
@@ -176,6 +186,7 @@ static int __init orion_bridge_irq_init(struct device_node *np,
 
 	gc->chip_types[0].regs.ack = ORION_BRIDGE_IRQ_CAUSE;
 	gc->chip_types[0].regs.mask = ORION_BRIDGE_IRQ_MASK;
+	gc->chip_types[0].chip.irq_startup = orion_bridge_irq_startup;
 	gc->chip_types[0].chip.irq_ack = irq_gc_ack_clr_bit;
 	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
 	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;


-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply related

* [BUG] FL1009: xHCI host not responding to stop endpoint command.
From: Arnaud Ebalard @ 2014-01-22 22:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140121211741.GA12657@xanatos>

Hi Sarah,

Sarah Sharp <sarah.a.sharp@linux.intel.com> writes:

> On Sat, Jan 18, 2014 at 10:49:17PM +0100, Arnaud Ebalard wrote:
>> Hi,
>> 
>> I have added Thomas in the recipients, because I guess he may be of some
>> help debugging the issue further. Thomas, the beginning of the thread is
>> here: http://thread.gmane.org/gmane.linux.usb.general/101531
>
> ...
>
>> I started suspecting the introduction of MSI support in Marvell PCIe
>> host controller driver (FL1009 is on the PCIe bus) and compiled a
>> a 3.13.0-rc8 w/ CONFIG_PCI_MSI disabled (it was enabled in all my
>> previous tests): I did not manage to reproduce the issue with this
>> kernel. As a side note, commits 5b4deb6526bd, 31f614edb726 and
>> 627dfcc249e2 are
>> 
>> ATM, I do not know if the problem is related to a bug in introduced MSI
>> support or some weird incompatibility of that functionality with the
>> FL1009 which would require some quirk in XHCI stack.
>
> We've actually had issues in the past with Fresco Logic hosts not
> supporting MSI properly, even though the PCI devices claim to have MSI
> support.  So turning off CONFIG_PCI_MSI may actually mean the Fresco
> Logic host is to blame, rather than the Marvell patches.  I assume MSI
> wouldn't have been turned on for the Fresco Logic host unless the parent
> PCI host controller supported it.
>
> Let's see if the Fresco Logic host is really the root cause.  Please
> apply the this patch to 3.13.0-rc8 and recompile with CONFIG_PCI_MSI
> enabled:
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 6c03584ac15f..74748444c040 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -30,6 +30,7 @@
>  /* Device for a quirk */
>  #define PCI_VENDOR_ID_FRESCO_LOGIC	0x1b73
>  #define PCI_DEVICE_ID_FRESCO_LOGIC_PDK	0x1000
> +#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1009	0x1009
>  #define PCI_DEVICE_ID_FRESCO_LOGIC_FL1400	0x1400
>  
>  #define PCI_VENDOR_ID_ETRON		0x1b6f
> @@ -63,6 +64,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>  
>  	/* Look for vendor-specific quirks */
>  	if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
> +			pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1009)
> +		xhci->quirks |= XHCI_BROKEN_MSI;
> +	if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
>  			(pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK ||
>  			 pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1400)) {
>  		if (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK &&

With the patch applied on top of 3.13.0 kernel recompiled w/
CONFIG_PCI_MSI enabled, I cannot reproduce the bug. I guess
you can add my:

 Reported-and-tested-By: Arnaud Ebalard <arno@natisbad.org>

Since you'll have to push the patch to -stable team at least for 3.13,
I wonder if it would not make sense to extend that at least to 3.12.
and possibly 3.10 (3.2 is still widely used but I wonder if it makes
sense to go that far).

Cheers,

a+

^ permalink raw reply

* [BUG] FL1009: xHCI host not responding to stop endpoint command.
From: Jason Cooper @ 2014-01-22 22:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87bnz364h0.fsf@natisbad.org>

On Wed, Jan 22, 2014 at 11:23:23PM +0100, Arnaud Ebalard wrote:
> With the patch applied on top of 3.13.0 kernel recompiled w/
> CONFIG_PCI_MSI enabled, I cannot reproduce the bug. I guess
> you can add my:
> 
>  Reported-and-tested-By: Arnaud Ebalard <arno@natisbad.org>
> 
> Since you'll have to push the patch to -stable team at least for 3.13,
> I wonder if it would not make sense to extend that at least to 3.12.
> and possibly 3.10 (3.2 is still widely used but I wonder if it makes
> sense to go that far).

Can you pinpoint the commit that introduced the regression?

thx,

Jason.

^ permalink raw reply

* [PATCH v3 15/24] drm/i2c: tda998x: use irq for connection status and EDID read
From: Russell King - ARM Linux @ 2014-01-22 22:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140119195843.7ae9753d@armhf>

On Sun, Jan 19, 2014 at 07:58:43PM +0100, Jean-Francois Moine wrote:
> This patch adds the optional treatment of the tda998x IRQ.
> 
> The interrupt function is used to know the display connection status
> without polling and to speedup reading the EDID.
> 
> The interrupt number may be defined either in the DT or at encoder set
> config time for non-DT boards.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
> v3
>     - remarks from Russell King
> 	- move the setting of the wq_edid_wait flag before asking the
> 	  device to read the EDID
> 	  Thanks about this bug fix, but I don't see the other problem:
> 	  there is no reason for the read_edid_block function to be
> 	  called more than once...

It will happen whenever DRM asks the connector for the modes.  However,
since the read process is triggered each time, I think this is fine.

> 	- use '0' as 'no irq'
> 	- remove the useless delay after irq init

Some further comments...

> @@ -720,6 +787,10 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
>  		priv->audio_port = p->audio_cfg;
>  		priv->audio_format = p->audio_format;
>  	}
> +
> +	priv->irq = p->irq;
> +	if (p->irq)
> +		tda_irq_init(priv);

If we're going to do it this way, this should probably release the IRQ if
there was one before re-claiming it, just in case this function gets called
more than once by some driver using it.

The alternative is, as I said before, to use the infrastructure which is
already there, namely setting the interrupt via struct i2c_client's
irq member.  Yes, that doesn't satisfy Sebastian's comment about using
a GPIO, but there's no sign of GPIO usage in here at the moment anyway.
So we might as well use what's already provided.

That would then avoid the need to deal with IRQ changes etc in
tda998x_encoder_set_config().

In any case, I've tested this patch in both non-IRQ and IRQ modes, and it
appears to work - but I'd like the suggestion above before I provide any
attributations.

Thanks.

-- 
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 v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
From: Jason Gunthorpe @ 2014-01-22 22:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140122221237.GA30763@localhost>

On Wed, Jan 22, 2014 at 07:12:38PM -0300, Ezequiel Garcia wrote:
> On Wed, Jan 22, 2014 at 01:52:13PM -0700, Jason Gunthorpe wrote:
> > > Clearing BRIDGE_CAUSE will only clear all currently pending upstream
> > > IRQs, of course. If WDT IRQ will be re-raised right after that in
> > > BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
> > > clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
> > 
> > Which is why it makes no sense to clear it one time at kernel start.
> > 
> 
> So, it seems we need to handle irq_startup(), as you suggested.
> I've just tested the attached patch, and it's working fine: the driver's
> probe() fully stops the watchdog, and then request_irq() acks and
> pending interrupts, through the added irq_startup().
> 
> How does it look?

Looks sane to me.

I looked some more and there are other drivers (eg irq-metag-ext) that
take this same approach.

Sebastian:
I looked at the irq-orion driver a bit more and noticed this:

        ret = irq_alloc_domain_generic_chips(domain, nrirqs, 1, np->name,
                             handle_level_irq, clr, 0, IRQ_GC_INIT_MASK_CACHE);
                           ^^^^^^^^^^^^^^^^^^^^^
Shouldn't it be handle_edge_irq? Otherwise who is calling irq_ack? How
does this work at all? :)

Jason

^ permalink raw reply

* [PATCH v5 7/8] ARM: brcmstb: gic: add compatible string for Broadcom Brahma15
From: Florian Fainelli @ 2014-01-22 22:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390361452-3124-8-git-send-email-marc.ceeeee@gmail.com>

Hi Marc,

2014/1/21 Marc Carino <marc.ceeeee@gmail.com>:
> Document the Broadcom Brahma B15 GIC implementation as compatible
> with the ARM GIC standard.
>
> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Do not we also need to update drivers/irqchip/irq-gic.c to look for
this compatible property? Alternatively should the example DTS contain
the following:

compatible = "brcm,brahma-b15-gic", "arm,cortex-a15-gic"?

> ---
>  Documentation/devicetree/bindings/arm/gic.txt |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 3dfb0c0..d7409fd 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -15,6 +15,7 @@ Main node required properties:
>         "arm,cortex-a9-gic"
>         "arm,cortex-a7-gic"
>         "arm,arm11mp-gic"
> +       "brcm,brahma-b15-gic"
>  - interrupt-controller : Identifies the node as an interrupt controller
>  - #interrupt-cells : Specifies the number of cells needed to encode an
>    interrupt source.  The type shall be a <u32> and the value shall be 3.
> --
> 1.7.1
>



-- 
Florian

^ permalink raw reply

* [PATCH v3 16/24] drm/i2c: tda998x: add DT documentation
From: Russell King - ARM Linux @ 2014-01-22 22:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOesGMhngiw+f8bkfB9C4COTPksXg_ZsYA-p1r5WJ6FZuBE3KQ@mail.gmail.com>

On Tue, Jan 21, 2014 at 10:17:56AM -0800, Olof Johansson wrote:
> Ok, this is a classic case where the binding should describe how
> things are configured/wired up instead of hardcoding a register value
> like this. From the data sheet there seems to be a _lot_ of settings,
> so selecting the needed subset for now might be acceptable (and go
> with defaults on the rest).

The issue is not that simple, because the register is kind of backwards.

Rather than describing what is on each nibble of the input bus, you
specify how each nibble of the internal bus maps to the external world.
The internal bus is mostly fixed as to which nibbles are red, green and
blue (there is internal support for swapping red/blue on the internal
bus, but we don't use that - it just adds extra unnecessary headaches
to the mapping.)

The way I ended up describing it in platform data was via the individual
nibble settings for the register:

        .swap_a = 2,
        .swap_b = 3,
        .swap_c = 4,
        .swap_d = 5,
        .swap_e = 0,
        .swap_f = 1,

which gives you a way to refer back to the data sheet.  There's also the
control whether the nibble is bit-big-endian or bit-little-endian.

So basically, swap_a controls which nibble of the input bus is mapped to
the internal bus bits 23:20, and mirror_a whether it's bit 0123 -> 0123
or 0123 -> 3210.

This means it's possible to tell the chip that each nibble of the interal
bus comes from the same four pins!

If we want to describe what each nibble set on the input pins does, it's
going to make for some complex code to convert that from "forward" notation
(which we'll have to invent) to the "reverse" notation that the chip needs.

-- 
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

* [BUG] FL1009: xHCI host not responding to stop endpoint command.
From: Arnaud Ebalard @ 2014-01-22 22:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140122222641.GD29184@titan.lakedaemon.net>

Hi Jason,

Jason Cooper <jason@lakedaemon.net> writes:

> On Wed, Jan 22, 2014 at 11:23:23PM +0100, Arnaud Ebalard wrote:
>> With the patch applied on top of 3.13.0 kernel recompiled w/
>> CONFIG_PCI_MSI enabled, I cannot reproduce the bug. I guess
>> you can add my:
>> 
>>  Reported-and-tested-By: Arnaud Ebalard <arno@natisbad.org>
>> 
>> Since you'll have to push the patch to -stable team at least for 3.13,
>> I wonder if it would not make sense to extend that at least to 3.12.
>> and possibly 3.10 (3.2 is still widely used but I wonder if it makes
>> sense to go that far).
>
> Can you pinpoint the commit that introduced the regression?

f5182b4155b9d686c5540a6822486400e34ddd98 "xhci: Disable MSI for some Fresco Logic hosts."

Technically, this is not per se the commit which introduced the
regression but the one that *partially* fixed it by introducing the XHCI
quirk to skip MSI enabling for Fresco Logic chips. The thing is it
should have included the FL1009 in the targets. Sarah, can you confirm
this?

Jason, the logic is summarized here, AFAICT:

commit 455f58925247e8a1a1941e159f3636ad6ee4c90b
Author: Oliver Neukum <oneukum@suse.de>
Date:   Mon Sep 30 15:50:54 2013 +0200

    xhci: quirk for extra long delay for S4
    
    It has been reported that this chipset really cannot
    sleep without this extraordinary delay.
    
    This patch should be backported, in order to ensure this host functions
    under stable kernels.  The last quirk for Fresco Logic hosts (commit
    bba18e33f25072ebf70fd8f7f0cdbf8cdb59a746 "xhci: Extend Fresco Logic MSI
    quirk.") was backported to stable kernels as old as 2.6.36.
    
    Signed-off-by: Oliver Neukum <oneukum@suse.de>
    Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
    Cc: stable at vger.kernel.org


commit bba18e33f25072ebf70fd8f7f0cdbf8cdb59a746
Author: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Date:   Wed Oct 17 13:44:06 2012 -0700

    xhci: Extend Fresco Logic MSI quirk.
    
    Ali reports that plugging a device into the Fresco Logic xHCI host with
    PCI device ID 1400 produces an IRQ error:
    
     do_IRQ: 3.176 No irq handler for vector (irq -1)
    
    Other early Fresco Logic host revisions don't support MSI, even though
    their PCI config space claims they do.  Extend the quirk to disabling
    MSI to this chipset revision.  Also enable the short transfer quirk,
    since it's likely this revision also has that quirk, and it should be
    harmless to enable.
    
    <SNIP>

    This patch should be backported to stable kernels as old as 2.6.36, that
    contain the commit f5182b4155b9d686c5540a6822486400e34ddd98 "xhci:
    Disable MSI for some Fresco Logic hosts."
    
    Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
    Reported-by: A Sh <smr.ash1991@gmail.com>
    Tested-by: A Sh <smr.ash1991@gmail.com>
    Cc: stable at vger.kernel.org


commit f5182b4155b9d686c5540a6822486400e34ddd98
Author: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Date:   Thu Jun 2 11:33:02 2011 -0700

    xhci: Disable MSI for some Fresco Logic hosts.
    
    Some Fresco Logic hosts, including those found in the AUAU N533V laptop,
    advertise MSI, but fail to actually generate MSI interrupts.  Add a new
    xHCI quirk to skip MSI enabling for the Fresco Logic host controllers.
    Fresco Logic confirms that all chips with PCI vendor ID 0x1b73 and device
    ID 0x1000, regardless of PCI revision ID, do not support MSI.
    
    This should be backported to stable kernels as far back as 2.6.36, which
    was the first kernel to support MSI on xHCI hosts.
    
    Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
    Reported-by: Sergey Galanov <sergey.e.galanov@gmail.com>
    Cc: stable at kernel.org

^ permalink raw reply

* [PATCH v3 17/24] drm/i2c: tda998x: get a better status of the connection
From: Russell King - ARM Linux @ 2014-01-22 22:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140119195844.202f021b@armhf>

On Sun, Jan 19, 2014 at 07:58:44PM +0100, Jean-Francois Moine wrote:
> This patch refines the connection status testing both bits RXSENS and
> HPD of the CEC register giving the connection level.

Can you explain why this is necessary?

I believe this is not necessary for the following reason:

HDMI cables are required to mate in a certain order:
- First to mate is the shell.
- Second are all pins with the exception of the +5V line.
- Third is the +5V line.

Sinks are not permitted to assert the HPD line until the EDID memory is
ready to be read, and +5V is indicated from the source.

Hence, HPD will not be asserted until the connector is fully mated, and
there are receivers available on the line.

This doesn't indicate whether the sink is powered and able to display
a picture - it merely means that the sink is present and valid EDID is
available to be read.  That's all that we really need here.

-- 
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 v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
From: Ezequiel Garcia @ 2014-01-22 22:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140122223117.GX18269@obsidianresearch.com>

On Wed, Jan 22, 2014 at 03:31:17PM -0700, Jason Gunthorpe wrote:
> On Wed, Jan 22, 2014 at 07:12:38PM -0300, Ezequiel Garcia wrote:
> > On Wed, Jan 22, 2014 at 01:52:13PM -0700, Jason Gunthorpe wrote:
> > > > Clearing BRIDGE_CAUSE will only clear all currently pending upstream
> > > > IRQs, of course. If WDT IRQ will be re-raised right after that in
> > > > BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
> > > > clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
> > > 
> > > Which is why it makes no sense to clear it one time at kernel start.
> > > 
> > 
> > So, it seems we need to handle irq_startup(), as you suggested.
> > I've just tested the attached patch, and it's working fine: the driver's
> > probe() fully stops the watchdog, and then request_irq() acks and
> > pending interrupts, through the added irq_startup().
> > 
> > How does it look?
> 
> Looks sane to me.
> 
> I looked some more and there are other drivers (eg irq-metag-ext) that
> take this same approach.
> 

Yup, I took that one as a starting point.

[..]
> I looked at the irq-orion driver a bit more and noticed this:
> 
>         ret = irq_alloc_domain_generic_chips(domain, nrirqs, 1, np->name,
>                              handle_level_irq, clr, 0, IRQ_GC_INIT_MASK_CACHE);
>                            ^^^^^^^^^^^^^^^^^^^^^
> Shouldn't it be handle_edge_irq? Otherwise who is calling irq_ack? How
> does this work at all? :)
> 

I'm not familiar with the differences between handle_level_irq and
handle_edge_irq, but -AFAICS- both seem to ack the IRQ.

In fact handle_level_irq(), masks and acks the IRQ as the first thing.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Jonas Gorski @ 2014-01-22 23:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1401221614550.1310-100000@iolanthe.rowland.org>

On Wed, 22 Jan 2014 16:17:42 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Wed, 22 Jan 2014, Jonas Gorski wrote:
> 
> > If it's safe to set ehci->big_endian_{desc,mmio} from the _probe()
> > routine, then maybe the pdata sets in _reset() should be moved into here
> > instead of adding extra cludges/checks into _reset().
> 
> Why?  What difference would it make?

Effectivewise none, but to me it seems to be cleaner to set them once in
probe() instead of everytime reset() is called.

I admit I don't know the code flow good enough if reset() is called
more than once in the lifetime of a hcd device.

And as I said, it would allow doing the checks the patch adds for both
DT and !DT, not just DT only.


Regards
Jonas

^ permalink raw reply

* [PATCH v4 00/18] Armada 370/XP watchdog support
From: Ezequiel Garcia @ 2014-01-22 23:04 UTC (permalink / raw)
  To: linux-arm-kernel

After some lengthy discussion on the [v2] and [v3] patchsets, here's a new
round. I hope I haven't forgotten anything.

[v2] http://www.spinics.net/lists/arm-kernel/msg302104.html

[v3] http://www.spinics.net/lists/arm-kernel/msg302245.html

The main changes from v3 are:

  * It wasn't nice to break DT compatibility by adding a second resource
    requirement, so we provided a fallback to use the RSTOUT address.
    All in all, the solution doesn't look too bad.

  * Added a full watchdog stop at driver probe time, *before* the call
    to request_irq().

    Notice that currently the request_irq() doesn't seem to clear the
    pending interruption. This means the BRIDGE_CAUSE clear removal is
    still not safe.

    This should be fixed sooner than later and, of course, before this
    gets merged.

  * Rework the interruption request, to use devm_request_irq() and
    avoid dealing with IRQ releasing.

  * Added proper clock error handling and fixed the probe() error path.

  * Typos and minor issues got fixed

Thanks to everyone who has helped reviewing the previous sets! It's certainly
easier to move forward when you get so much nice feedback...

Ezequiel Garcia (18):
  ARM: Introduce atomic MMIO modify
  clocksource: orion: Use atomic access for shared registers
  watchdog: orion: Add clock error handling
  watchdog: orion: Use atomic access for shared registers
  watchdog: orion: Remove unused macros
  watchdog: orion: Make sure the watchdog is initially stopped
  watchdog: orion: Handle IRQ
  watchdog: orion: Make RSTOUT register a separate resource
  watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
  watchdog: orion: Introduce an orion_watchdog device structure
  watchdog: orion: Introduce per-compatible of_device_id data
  watchdog: orion: Add per-compatible clock initialization
  watchdog: orion: Add per-compatible watchdog start implementation
  watchdog: orion: Add support for Armada 370 and Armada XP SoC
  ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree
  ARM: kirkwood: Add RSTOUT 'reg' entry to devicetree
  watchdog: orion: Enable the build on ARCH_MVEBU
  ARM: mvebu: Enable watchdog support in defconfig

 .../devicetree/bindings/watchdog/marvel.txt        |   8 +-
 arch/arm/boot/dts/armada-370-xp.dtsi               |   4 +
 arch/arm/boot/dts/armada-370.dtsi                  |   5 +
 arch/arm/boot/dts/armada-xp.dtsi                   |   6 +
 arch/arm/boot/dts/kirkwood.dtsi                    |   2 +-
 arch/arm/configs/mvebu_defconfig                   |   2 +
 arch/arm/include/asm/io.h                          |   6 +
 arch/arm/kernel/io.c                               |  35 ++
 arch/arm/mach-dove/include/mach/bridge-regs.h      |   1 +
 arch/arm/mach-kirkwood/include/mach/bridge-regs.h  |   1 +
 arch/arm/mach-mv78xx0/include/mach/bridge-regs.h   |   1 +
 arch/arm/mach-orion5x/include/mach/bridge-regs.h   |   1 +
 arch/arm/plat-orion/common.c                       |  10 +-
 drivers/clocksource/time-orion.c                   |  28 +-
 drivers/watchdog/Kconfig                           |   2 +-
 drivers/watchdog/orion_wdt.c                       | 370 ++++++++++++++++-----
 16 files changed, 373 insertions(+), 109 deletions(-)

-- 
1.8.1.5

^ permalink raw reply

* [PATCH v4 01/18] ARM: Introduce atomic MMIO modify
From: Ezequiel Garcia @ 2014-01-22 23:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390431915-5115-1-git-send-email-ezequiel.garcia@free-electrons.com>

Some SoC have MMIO regions that are shared across orthogonal
subsystems. This commit implements a possible solution for the
thread-safe access of such regions through a spinlock-protected API.

Concurrent access is protected with a single spinlock for the
entire MMIO address space. While this protects shared-registers,
it also serializes access to unrelated/unshared registers.

We add relaxed and non-relaxed variants, by using writel_relaxed and writel,
respectively. The rationale for this is that some users may not require
register write completion but only thread-safe access to a register.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/include/asm/io.h |  6 ++++++
 arch/arm/kernel/io.c      | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index fbeb39c..8aa4cca 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -38,6 +38,12 @@
 #define isa_bus_to_virt phys_to_virt
 
 /*
+ * Atomic MMIO-wide IO modify
+ */
+extern void atomic_io_modify(void __iomem *reg, u32 mask, u32 set);
+extern void atomic_io_modify_relaxed(void __iomem *reg, u32 mask, u32 set);
+
+/*
  * Generic IO read/write.  These perform native-endian accesses.  Note
  * that some architectures will want to re-define __raw_{read,write}w.
  */
diff --git a/arch/arm/kernel/io.c b/arch/arm/kernel/io.c
index dcd5b4d..9203cf8 100644
--- a/arch/arm/kernel/io.c
+++ b/arch/arm/kernel/io.c
@@ -1,6 +1,41 @@
 #include <linux/export.h>
 #include <linux/types.h>
 #include <linux/io.h>
+#include <linux/spinlock.h>
+
+static DEFINE_RAW_SPINLOCK(__io_lock);
+
+/*
+ * Generic atomic MMIO modify.
+ *
+ * Allows thread-safe access to registers shared by unrelated subsystems.
+ * The access is protected by a single MMIO-wide lock.
+ */
+void atomic_io_modify_relaxed(void __iomem *reg, u32 mask, u32 set)
+{
+	unsigned long flags;
+	u32 value;
+
+	raw_spin_lock_irqsave(&__io_lock, flags);
+	value = readl_relaxed(reg) & ~mask;
+	value |= (set & mask);
+	writel_relaxed(value, reg);
+	raw_spin_unlock_irqrestore(&__io_lock, flags);
+}
+EXPORT_SYMBOL(atomic_io_modify_relaxed);
+
+void atomic_io_modify(void __iomem *reg, u32 mask, u32 set)
+{
+	unsigned long flags;
+	u32 value;
+
+	raw_spin_lock_irqsave(&__io_lock, flags);
+	value = readl_relaxed(reg) & ~mask;
+	value |= (set & mask);
+	writel(value, reg);
+	raw_spin_unlock_irqrestore(&__io_lock, flags);
+}
+EXPORT_SYMBOL(atomic_io_modify);
 
 /*
  * Copy data from IO memory space to "real" memory space.
-- 
1.8.1.5

^ permalink raw reply related

* [PATCH v4 02/18] clocksource: orion: Use atomic access for shared registers
From: Ezequiel Garcia @ 2014-01-22 23:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390431915-5115-1-git-send-email-ezequiel.garcia@free-electrons.com>

Replace the driver-specific thread-safe shared register API
by the recently introduced atomic_io_clear_set().

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/clocksource/time-orion.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/clocksource/time-orion.c b/drivers/clocksource/time-orion.c
index 9c7f018..3f14e56 100644
--- a/drivers/clocksource/time-orion.c
+++ b/drivers/clocksource/time-orion.c
@@ -35,20 +35,6 @@
 #define ORION_ONESHOT_MAX	0xfffffffe
 
 static void __iomem *timer_base;
-static DEFINE_SPINLOCK(timer_ctrl_lock);
-
-/*
- * Thread-safe access to TIMER_CTRL register
- * (shared with watchdog timer)
- */
-void orion_timer_ctrl_clrset(u32 clr, u32 set)
-{
-	spin_lock(&timer_ctrl_lock);
-	writel((readl(timer_base + TIMER_CTRL) & ~clr) | set,
-		timer_base + TIMER_CTRL);
-	spin_unlock(&timer_ctrl_lock);
-}
-EXPORT_SYMBOL(orion_timer_ctrl_clrset);
 
 /*
  * Free-running clocksource handling.
@@ -68,7 +54,8 @@ static int orion_clkevt_next_event(unsigned long delta,
 {
 	/* setup and enable one-shot timer */
 	writel(delta, timer_base + TIMER1_VAL);
-	orion_timer_ctrl_clrset(TIMER1_RELOAD_EN, TIMER1_EN);
+	atomic_io_modify(timer_base + TIMER_CTRL,
+		TIMER1_RELOAD_EN | TIMER1_EN, TIMER1_EN);
 
 	return 0;
 }
@@ -80,10 +67,13 @@ static void orion_clkevt_mode(enum clock_event_mode mode,
 		/* setup and enable periodic timer at 1/HZ intervals */
 		writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD);
 		writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL);
-		orion_timer_ctrl_clrset(0, TIMER1_RELOAD_EN | TIMER1_EN);
+		atomic_io_modify(timer_base + TIMER_CTRL,
+			TIMER1_RELOAD_EN | TIMER1_EN,
+			TIMER1_RELOAD_EN | TIMER1_EN);
 	} else {
 		/* disable timer */
-		orion_timer_ctrl_clrset(TIMER1_RELOAD_EN | TIMER1_EN, 0);
+		atomic_io_modify(timer_base + TIMER_CTRL,
+			TIMER1_RELOAD_EN | TIMER1_EN, 0);
 	}
 }
 
@@ -131,7 +121,9 @@ static void __init orion_timer_init(struct device_node *np)
 	/* setup timer0 as free-running clocksource */
 	writel(~0, timer_base + TIMER0_VAL);
 	writel(~0, timer_base + TIMER0_RELOAD);
-	orion_timer_ctrl_clrset(0, TIMER0_RELOAD_EN | TIMER0_EN);
+	atomic_io_modify(timer_base + TIMER_CTRL,
+		TIMER0_RELOAD_EN | TIMER0_EN,
+		TIMER0_RELOAD_EN | TIMER0_EN);
 	clocksource_mmio_init(timer_base + TIMER0_VAL, "orion_clocksource",
 			      clk_get_rate(clk), 300, 32,
 			      clocksource_mmio_readl_down);
-- 
1.8.1.5

^ permalink raw reply related

* [PATCH v4 03/18] watchdog: orion: Add clock error handling
From: Ezequiel Garcia @ 2014-01-22 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1390431915-5115-1-git-send-email-ezequiel.garcia@free-electrons.com>

This commit adds a check for clk_prepare_enable success and introduces
an error path to disable the clock properly.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/watchdog/orion_wdt.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index f7722a4..7f19fa3 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -151,17 +151,24 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "Orion Watchdog missing clock\n");
-		return -ENODEV;
+		return PTR_ERR(clk);
 	}
-	clk_prepare_enable(clk);
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		return ret;
 	wdt_tclk = clk_get_rate(clk);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
+	if (!res) {
+		ret = -ENODEV;
+		goto disable_clk;
+	}
+
 	wdt_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
-	if (!wdt_reg)
-		return -ENOMEM;
+	if (!wdt_reg) {
+		ret = -ENOMEM;
+		goto disable_clk;
+	}
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
@@ -171,14 +178,16 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
-	if (ret) {
-		clk_disable_unprepare(clk);
-		return ret;
-	}
+	if (ret)
+		goto disable_clk;
 
 	pr_info("Initial timeout %d sec%s\n",
 		orion_wdt.timeout, nowayout ? ", nowayout" : "");
 	return 0;
+
+disable_clk:
+	clk_disable_unprepare(clk);
+	return ret;
 }
 
 static int orion_wdt_remove(struct platform_device *pdev)
-- 
1.8.1.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox