All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes
@ 2015-01-16 16:11 Thomas Petazzoni
  2015-01-16 16:11 ` [PATCHv2 1/3] ARM: mvebu: completely disable hardware I/O coherency Thomas Petazzoni
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-16 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

Arnd, Andrew,

As we discussed, here is a set of 3 patches related to HW I/O
coherency on mvebu:

 * The first patch disables HW I/O coherency entirely, and is meant to
   be backported to stable, until we do enough testing of the new HW
   I/O coherency stuff and backport it to stabel.

 * The last two patches add the new HW I/O coherency strategy based on
   automatic I/O synchronization barriers, and obviously re-enable the
   HW I/O coherency.

I'd like all three patches to be merged for 3.20. The first patch will
be backported to stable right now, and we will continue to do more
testing around HW I/O coherency in 3.20 to validate that we have
reached a proper solution.

Special thanks to Simon Guinot who did quite a bit of testing of the
automatic I/O synchronization barriers, and to Arnd for all the
discussions related to this topic.

Thanks,

Thomas

Thomas Petazzoni (3):
  ARM: mvebu: completely disable hardware I/O coherency
  bus: mvebu-mbus: use automatic I/O synchronization barriers
  ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O
    coherency

 arch/arm/mach-mvebu/coherency.c | 56 ++++++-----------------------------------
 drivers/bus/mvebu-mbus.c        | 17 ++++++++++---
 2 files changed, 21 insertions(+), 52 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 1/3] ARM: mvebu: completely disable hardware I/O coherency
  2015-01-16 16:11 [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Thomas Petazzoni
@ 2015-01-16 16:11 ` Thomas Petazzoni
  2015-01-16 16:11 ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Thomas Petazzoni
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-16 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

The current hardware I/O coherency is known to cause problems with DMA
coherent buffers, as it still requires explicit I/O synchronization
barriers, which is not compatible with the semantics expected by the
Linux DMA coherent buffers API.

So, in order to have enough time to validate a new solution based on
automatic I/O synchronization barriers, this commit disables hardware
I/O coherency entirely. Future patches will re-enable it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.8+
---
Due to the coherent mapping problem, the I/O coherency has in fact
been broken since its introduction, which is why we request the
backport of this patch all the way up to 3.8.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index 3585cb3..caa21e9 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -246,9 +246,14 @@ static int coherency_type(void)
 	return type;
 }
 
+/*
+ * As a precaution, we currently completely disable hardware I/O
+ * coherency, until enough testing is done with automatic I/O
+ * synchronization barriers to validate that it is a proper solution.
+ */
 int coherency_available(void)
 {
-	return coherency_type() != COHERENCY_FABRIC_TYPE_NONE;
+	return false;
 }
 
 int __init coherency_init(void)
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers
  2015-01-16 16:11 [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Thomas Petazzoni
  2015-01-16 16:11 ` [PATCHv2 1/3] ARM: mvebu: completely disable hardware I/O coherency Thomas Petazzoni
@ 2015-01-16 16:11 ` Thomas Petazzoni
  2015-04-17 10:45   ` Nicolas Schichan
  2015-01-16 16:11 ` [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency Thomas Petazzoni
  2015-01-19 22:36 ` [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Andrew Lunn
  3 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-16 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of using explicit I/O synchronization barriers shoehorned
inside the streaming DMA mappings API (in
arch/arm/mach-mvebu/coherency.c), we are switching to use automatic
I/O synchronization barrier.

The primary motivation for this change is that explicit I/O
synchronization barriers are not only needed for streaming DMA
mappings (which can easily be done by overriding the dma_map_ops), but
also for coherent DMA mappings (which is a lot less easy to do, since
the kernel assumes such mappings are coherent and don't require any
sort of cache maintenance operation to ensure the consistency of the
buffers).

Switching to automatic I/O synchronization barriers will also allow us
to use the existing arm_coherent_dma_ops instead of our custom
arm_dma_ops.

In order to use automatic I/O synchronization barriers, this commit
changes mvebu-mbus in two ways:

 - It enables automatic I/O synchronization barriers in the 0x84
   register of the MBus bridge, by enabling such barriers for all MBus
   units. This enables automatic barriers for the on-SoC peripherals
   that are doing DMA.

 - It enables the SyncEnable bit in the MBus windows, so that PCIe
   devices also use automatic I/O synchronization barrier.

This automatic synchronization barrier relies on the assumption that
at least one register of a given hardware unit is read before the
driver accesses the DMA mappings modified by this unit. This
assumption is guaranteed for PCI devices by vertue of the PCI
standard, and we can reasonably verify that this assumption is also
true for the limited number of platform drivers doing DMA used on
Marvell EBU platforms.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/bus/mvebu-mbus.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index eb7682d..398f0ee 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -69,6 +69,7 @@
  */
 #define WIN_CTRL_OFF		0x0000
 #define   WIN_CTRL_ENABLE       BIT(0)
+#define   WIN_CTRL_SYNCBARRIER  BIT(1)
 #define   WIN_CTRL_TGT_MASK     0xf0
 #define   WIN_CTRL_TGT_SHIFT    4
 #define   WIN_CTRL_ATTR_MASK    0xff00
@@ -82,6 +83,9 @@
 #define   WIN_REMAP_LOW         0xffff0000
 #define WIN_REMAP_HI_OFF	0x000c
 
+#define UNIT_SYNC_BARRIER_OFF   0x84
+#define   UNIT_SYNC_BARRIER_ALL 0xFFFF
+
 #define ATTR_HW_COHERENCY	(0x1 << 4)
 
 #define DDR_BASE_CS_OFF(n)	(0x0000 + ((n) << 3))
@@ -303,6 +307,7 @@ static int mvebu_mbus_setup_window(struct mvebu_mbus_state *mbus,
 	ctrl = ((size - 1) & WIN_CTRL_SIZE_MASK) |
 		(attr << WIN_CTRL_ATTR_SHIFT)    |
 		(target << WIN_CTRL_TGT_SHIFT)   |
+		WIN_CTRL_SYNCBARRIER             |
 		WIN_CTRL_ENABLE;
 
 	writel(base & WIN_BASE_LOW, addr + WIN_BASE_OFF);
@@ -844,7 +849,8 @@ static int __init mvebu_mbus_common_init(struct mvebu_mbus_state *mbus,
 					 phys_addr_t sdramwins_phys_base,
 					 size_t sdramwins_size,
 					 phys_addr_t mbusbridge_phys_base,
-					 size_t mbusbridge_size)
+					 size_t mbusbridge_size,
+					 bool is_coherent)
 {
 	int win;
 
@@ -876,6 +882,10 @@ static int __init mvebu_mbus_common_init(struct mvebu_mbus_state *mbus,
 
 	mbus->soc->setup_cpu_target(mbus);
 
+	if (is_coherent)
+		writel(UNIT_SYNC_BARRIER_ALL,
+		       mbus->mbuswins_base + UNIT_SYNC_BARRIER_OFF);
+
 	register_syscore_ops(&mvebu_mbus_syscore_ops);
 
 	return 0;
@@ -903,7 +913,7 @@ int __init mvebu_mbus_init(const char *soc, phys_addr_t mbuswins_phys_base,
 			mbuswins_phys_base,
 			mbuswins_size,
 			sdramwins_phys_base,
-			sdramwins_size, 0, 0);
+			sdramwins_size, 0, 0, false);
 }
 
 #ifdef CONFIG_OF
@@ -1105,7 +1115,8 @@ int __init mvebu_mbus_dt_init(bool is_coherent)
 				     sdramwins_res.start,
 				     resource_size(&sdramwins_res),
 				     mbusbridge_res.start,
-				     resource_size(&mbusbridge_res));
+				     resource_size(&mbusbridge_res),
+				     is_coherent);
 	if (ret)
 		return ret;
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency
  2015-01-16 16:11 [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Thomas Petazzoni
  2015-01-16 16:11 ` [PATCHv2 1/3] ARM: mvebu: completely disable hardware I/O coherency Thomas Petazzoni
  2015-01-16 16:11 ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Thomas Petazzoni
@ 2015-01-16 16:11 ` Thomas Petazzoni
  2015-01-16 16:15   ` Andrew Lunn
  2015-01-19 22:36 ` [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Andrew Lunn
  3 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-16 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we have enabled automatic I/O synchronization barriers, we no
longer need any explicit barriers. We can therefore simplify
arch/arm/mach-mvebu/coherency.c by using the existing
arm_coherent_dma_ops instead of our custom mvebu_hwcc_dma_ops, and
re-enable hardware I/O coherency support.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c | 53 +++--------------------------------------
 1 file changed, 3 insertions(+), 50 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index caa21e9..5e7df71 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -33,6 +33,7 @@
 #include <asm/smp_plat.h>
 #include <asm/cacheflush.h>
 #include <asm/mach/map.h>
+#include <asm/dma-mapping.h>
 #include "coherency.h"
 #include "mvebu-soc-id.h"
 
@@ -76,54 +77,6 @@ int set_cpu_coherent(void)
 	return ll_enable_coherency();
 }
 
-static inline void mvebu_hwcc_sync_io_barrier(void)
-{
-	writel(0x1, coherency_cpu_base + IO_SYNC_BARRIER_CTL_OFFSET);
-	while (readl(coherency_cpu_base + IO_SYNC_BARRIER_CTL_OFFSET) & 0x1);
-}
-
-static dma_addr_t mvebu_hwcc_dma_map_page(struct device *dev, struct page *page,
-				  unsigned long offset, size_t size,
-				  enum dma_data_direction dir,
-				  struct dma_attrs *attrs)
-{
-	if (dir != DMA_TO_DEVICE)
-		mvebu_hwcc_sync_io_barrier();
-	return pfn_to_dma(dev, page_to_pfn(page)) + offset;
-}
-
-
-static void mvebu_hwcc_dma_unmap_page(struct device *dev, dma_addr_t dma_handle,
-			      size_t size, enum dma_data_direction dir,
-			      struct dma_attrs *attrs)
-{
-	if (dir != DMA_TO_DEVICE)
-		mvebu_hwcc_sync_io_barrier();
-}
-
-static void mvebu_hwcc_dma_sync(struct device *dev, dma_addr_t dma_handle,
-			size_t size, enum dma_data_direction dir)
-{
-	if (dir != DMA_TO_DEVICE)
-		mvebu_hwcc_sync_io_barrier();
-}
-
-static struct dma_map_ops mvebu_hwcc_dma_ops = {
-	.alloc			= arm_dma_alloc,
-	.free			= arm_dma_free,
-	.mmap			= arm_dma_mmap,
-	.map_page		= mvebu_hwcc_dma_map_page,
-	.unmap_page		= mvebu_hwcc_dma_unmap_page,
-	.get_sgtable		= arm_dma_get_sgtable,
-	.map_sg			= arm_dma_map_sg,
-	.unmap_sg		= arm_dma_unmap_sg,
-	.sync_single_for_cpu	= mvebu_hwcc_dma_sync,
-	.sync_single_for_device	= mvebu_hwcc_dma_sync,
-	.sync_sg_for_cpu	= arm_dma_sync_sg_for_cpu,
-	.sync_sg_for_device	= arm_dma_sync_sg_for_device,
-	.set_dma_mask		= arm_dma_set_mask,
-};
-
 static int mvebu_hwcc_notifier(struct notifier_block *nb,
 			       unsigned long event, void *__dev)
 {
@@ -131,7 +84,7 @@ static int mvebu_hwcc_notifier(struct notifier_block *nb,
 
 	if (event != BUS_NOTIFY_ADD_DEVICE)
 		return NOTIFY_DONE;
-	set_dma_ops(dev, &mvebu_hwcc_dma_ops);
+	set_dma_ops(dev, &arm_coherent_dma_ops);
 
 	return NOTIFY_OK;
 }
@@ -253,7 +206,7 @@ static int coherency_type(void)
  */
 int coherency_available(void)
 {
-	return false;
+	return coherency_type() != COHERENCY_FABRIC_TYPE_NONE;
 }
 
 int __init coherency_init(void)
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency
  2015-01-16 16:11 ` [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency Thomas Petazzoni
@ 2015-01-16 16:15   ` Andrew Lunn
  2015-01-16 16:20     ` Thomas Petazzoni
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2015-01-16 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas

In patch 1/3 you add a comment here saying why you have turned it off.
This hunk turns it back on again. But i don't see the comment being
removed?

Thanks
	Andrew

> @@ -253,7 +206,7 @@ static int coherency_type(void)
>   */
>  int coherency_available(void)
>  {
> -	return false;
> +	return coherency_type() != COHERENCY_FABRIC_TYPE_NONE;
>  }
>  
>  int __init coherency_init(void)
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency
  2015-01-16 16:15   ` Andrew Lunn
@ 2015-01-16 16:20     ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-16 16:20 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Andrew Lunn,

On Fri, 16 Jan 2015 17:15:48 +0100, Andrew Lunn wrote:

> In patch 1/3 you add a comment here saying why you have turned it off.
> This hunk turns it back on again. But i don't see the comment being
> removed?

Stupid mistake, indeed :-/

Do you want a v3, or can you kill the comment when applying the patch?

Thanks, and sorry about this,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes
  2015-01-16 16:11 [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Thomas Petazzoni
                   ` (2 preceding siblings ...)
  2015-01-16 16:11 ` [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency Thomas Petazzoni
@ 2015-01-19 22:36 ` Andrew Lunn
  2015-01-20 15:22   ` Thomas Petazzoni
  3 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2015-01-19 22:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 16, 2015 at 05:11:26PM +0100, Thomas Petazzoni wrote:
> Arnd, Andrew,
> 
> As we discussed, here is a set of 3 patches related to HW I/O
> coherency on mvebu:
> 
>  * The first patch disables HW I/O coherency entirely, and is meant to
>    be backported to stable, until we do enough testing of the new HW
>    I/O coherency stuff and backport it to stabel.

Added to mvebu/fixes-3

> 
>  * The last two patches add the new HW I/O coherency strategy based on
>    automatic I/O synchronization barriers, and obviously re-enable the
>    HW I/O coherency.

Added to mvebu/sox. I also removed the comment you forgot to removed
in 3/3.

   Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes
  2015-01-19 22:36 ` [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Andrew Lunn
@ 2015-01-20 15:22   ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-01-20 15:22 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Andrew Lunn,

On Mon, 19 Jan 2015 23:36:57 +0100, Andrew Lunn wrote:
> On Fri, Jan 16, 2015 at 05:11:26PM +0100, Thomas Petazzoni wrote:
> > Arnd, Andrew,
> > 
> > As we discussed, here is a set of 3 patches related to HW I/O
> > coherency on mvebu:
> > 
> >  * The first patch disables HW I/O coherency entirely, and is meant to
> >    be backported to stable, until we do enough testing of the new HW
> >    I/O coherency stuff and backport it to stabel.
> 
> Added to mvebu/fixes-3
> 
> > 
> >  * The last two patches add the new HW I/O coherency strategy based on
> >    automatic I/O synchronization barriers, and obviously re-enable the
> >    HW I/O coherency.
> 
> Added to mvebu/sox. I also removed the comment you forgot to removed
> in 3/3.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers
  2015-01-16 16:11 ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Thomas Petazzoni
@ 2015-04-17 10:45   ` Nicolas Schichan
  2015-04-17 11:23     ` Thomas Petazzoni
  2015-05-28 19:43     ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Aaro Koskinen
  0 siblings, 2 replies; 15+ messages in thread
From: Nicolas Schichan @ 2015-04-17 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/16/2015 05:11 PM, Thomas Petazzoni wrote:
> Instead of using explicit I/O synchronization barriers shoehorned
> inside the streaming DMA mappings API (in
> arch/arm/mach-mvebu/coherency.c), we are switching to use automatic
> I/O synchronization barrier.

Hello Thomas,

I'm affraid this patche causes issue on mv88f6282 (and most probably on
mv88f6281 as well): see below.

[...]

> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index eb7682d..398f0ee 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -69,6 +69,7 @@
>   */
>  #define WIN_CTRL_OFF		0x0000
>  #define   WIN_CTRL_ENABLE       BIT(0)
> +#define   WIN_CTRL_SYNCBARRIER  BIT(1)

In the 88f6282 datasheet this bit in the "WindowX Control Register" is
documented as reserved on all windows but window 6 and 7.

For windows 6 and 7 this bit is documented to write protect the window when set.

In our configuration, this window gets chosen for the PCI memory accesses
(target 0x4, attribute 0xe8) and we effectively end up only being able to read
from the PCI devices (mwl8k fails to load the firmware and sky2 timeouts on
MDIO accesses). Write accesses are silently discarded (no external aborts of
any kind), but that's probably expected as the AHB error propagation is disabled.

Reverting this patch made all the PCI device work correctly.

Regards,

-- 
Nicolas Schichan
Freebox SAS

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers
  2015-04-17 10:45   ` Nicolas Schichan
@ 2015-04-17 11:23     ` Thomas Petazzoni
  2015-04-24 14:44       ` [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms Nicolas Schichan
  2015-04-24 15:12       ` [PATCH] " Nicolas Schichan
  2015-05-28 19:43     ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Aaro Koskinen
  1 sibling, 2 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-04-17 11:23 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Nicolas Schichan,

On Fri, 17 Apr 2015 12:45:34 +0200, Nicolas Schichan wrote:

> I'm affraid this patche causes issue on mv88f6282 (and most probably on
> mv88f6281 as well): see below.
> 
> [...]
> 
> > diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> > index eb7682d..398f0ee 100644
> > --- a/drivers/bus/mvebu-mbus.c
> > +++ b/drivers/bus/mvebu-mbus.c
> > @@ -69,6 +69,7 @@
> >   */
> >  #define WIN_CTRL_OFF		0x0000
> >  #define   WIN_CTRL_ENABLE       BIT(0)
> > +#define   WIN_CTRL_SYNCBARRIER  BIT(1)
> 
> In the 88f6282 datasheet this bit in the "WindowX Control Register" is
> documented as reserved on all windows but window 6 and 7.
> 
> For windows 6 and 7 this bit is documented to write protect the window when set.
> 
> In our configuration, this window gets chosen for the PCI memory accesses
> (target 0x4, attribute 0xe8) and we effectively end up only being able to read
> from the PCI devices (mwl8k fails to load the firmware and sky2 timeouts on
> MDIO accesses). Write accesses are silently discarded (no external aborts of
> any kind), but that's probably expected as the AHB error propagation is disabled.
> 
> Reverting this patch made all the PCI device work correctly.

Ah, thanks a lot for the bug report. I'll cook a fix for this problem,
and Cc: you when submitting.

Thanks again!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms.
  2015-04-17 11:23     ` Thomas Petazzoni
@ 2015-04-24 14:44       ` Nicolas Schichan
  2015-04-24 14:54         ` Thomas Petazzoni
  2015-04-24 15:12       ` [PATCH] " Nicolas Schichan
  1 sibling, 1 reply; 15+ messages in thread
From: Nicolas Schichan @ 2015-04-24 14:44 UTC (permalink / raw)
  To: linux-arm-kernel

On those platforms (orion5x, kirkwood and dove AFAICS) the
WIN_CTRL_SYNCBARRIER bit in the window control register is either
reserved (all windows except 6 and 7) or enables read-only protection
(windows 6 and 7).

Signed-off-by: Nicolas Schichan <nschichan@freebox.fr>
---

Hello Thomas,

Please find here my attempt at fixing this. I have been relying on the
hw_io_coherency field of the struct mvebu_mbus_state which is always
false for the platforms without the WIN_CTRL_SYNCBARRIER bit available
(orion5x, kirkwood, and dove).

I was going to add a field to mvebu_soc_data documenting the
availability of the WIN_CTRL_SYNCBARRIER bit for each platform but for
that purpose, the mvebu_mbus_state struct hw_io_coherency field looks
to be enough.

This has been tested on mv88f6282 (kirkwood).

Regards,

 drivers/bus/mvebu-mbus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index fb9ec62..019a033 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -323,8 +323,9 @@ static int mvebu_mbus_setup_window(struct mvebu_mbus_state *mbus,
 	ctrl = ((size - 1) & WIN_CTRL_SIZE_MASK) |
 		(attr << WIN_CTRL_ATTR_SHIFT)    |
 		(target << WIN_CTRL_TGT_SHIFT)   |
-		WIN_CTRL_SYNCBARRIER             |
 		WIN_CTRL_ENABLE;
+	if (mbus->hw_io_coherency)
+		ctrl |= WIN_CTRL_SYNCBARRIER;
 
 	writel(base & WIN_BASE_LOW, addr + WIN_BASE_OFF);
 	writel(ctrl, addr + WIN_CTRL_OFF);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms.
  2015-04-24 14:44       ` [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms Nicolas Schichan
@ 2015-04-24 14:54         ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-04-24 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Nicolas Schichan,

On Fri, 24 Apr 2015 16:44:31 +0200, Nicolas Schichan wrote:
> On those platforms (orion5x, kirkwood and dove AFAICS) the
> WIN_CTRL_SYNCBARRIER bit in the window control register is either
> reserved (all windows except 6 and 7) or enables read-only protection
> (windows 6 and 7).
> 
> Signed-off-by: Nicolas Schichan <nschichan@freebox.fr>
> ---
> 
> Hello Thomas,
> 
> Please find here my attempt at fixing this. I have been relying on the
> hw_io_coherency field of the struct mvebu_mbus_state which is always
> false for the platforms without the WIN_CTRL_SYNCBARRIER bit available
> (orion5x, kirkwood, and dove).
> 
> I was going to add a field to mvebu_soc_data documenting the
> availability of the WIN_CTRL_SYNCBARRIER bit for each platform but for
> that purpose, the mvebu_mbus_state struct hw_io_coherency field looks
> to be enough.
> 
> This has been tested on mv88f6282 (kirkwood).

Thanks, it looks good to me. Can you resend with:

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: <stable@vger.kernel.org> # v4.0+
Fixes: a0b5cd4ac2d6 ("bus: mvebu-mbus: use automatic I/O synchronization barriers")

An addition to the commit log like:

Commit a0b5cd4ac2d6 ("bus: mvebu-mbus: use automatic I/O
synchronization barriers") enabled the usage of automatic I/O
synchronization barriers by enabling bit WIN_CTRL_SYNCBARRIER in the
control registers of MBus windows, but....

And in the code, maybe add a comment above the WIN_CTRL_SYNCBARRIER
definition like:

/* Only on HW I/O coherency capable platforms */
#define WIN_CTRL_SYNCBARRIER ...

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms.
  2015-04-17 11:23     ` Thomas Petazzoni
  2015-04-24 14:44       ` [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms Nicolas Schichan
@ 2015-04-24 15:12       ` Nicolas Schichan
  1 sibling, 0 replies; 15+ messages in thread
From: Nicolas Schichan @ 2015-04-24 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

Commit a0b5cd4ac2d6 ("bus: mvebu-mbus: use automatic I/O
synchronization barriers") enabled the usage of automatic I/O
synchronization barriers by enabling bit WIN_CTRL_SYNCBARRIER in the
control registers of MBus windows, but on non io-coherent platforms
(orion5x, kirkwood and dove) the WIN_CTRL_SYNCBARRIER bit in
the window control register is either reserved (all windows except 6
and 7) or enables read-only protection (windows 6 and 7).

Signed-off-by: Nicolas Schichan <nschichan@freebox.fr>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: <stable@vger.kernel.org> # v4.0+
Fixes: a0b5cd4ac2d6 ("bus: mvebu-mbus: use automatic I/O synchronization barriers")

---
 drivers/bus/mvebu-mbus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index fb9ec62..7fa4510 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -70,6 +70,7 @@
  */
 #define WIN_CTRL_OFF		0x0000
 #define   WIN_CTRL_ENABLE       BIT(0)
+/* Only on HW I/O coherency capable platforms */
 #define   WIN_CTRL_SYNCBARRIER  BIT(1)
 #define   WIN_CTRL_TGT_MASK     0xf0
 #define   WIN_CTRL_TGT_SHIFT    4
@@ -323,8 +324,9 @@ static int mvebu_mbus_setup_window(struct mvebu_mbus_state *mbus,
 	ctrl = ((size - 1) & WIN_CTRL_SIZE_MASK) |
 		(attr << WIN_CTRL_ATTR_SHIFT)    |
 		(target << WIN_CTRL_TGT_SHIFT)   |
-		WIN_CTRL_SYNCBARRIER             |
 		WIN_CTRL_ENABLE;
+	if (mbus->hw_io_coherency)
+		ctrl |= WIN_CTRL_SYNCBARRIER;
 
 	writel(base & WIN_BASE_LOW, addr + WIN_BASE_OFF);
 	writel(ctrl, addr + WIN_CTRL_OFF);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers
  2015-04-17 10:45   ` Nicolas Schichan
  2015-04-17 11:23     ` Thomas Petazzoni
@ 2015-05-28 19:43     ` Aaro Koskinen
  2015-05-28 19:57       ` Thomas Petazzoni
  1 sibling, 1 reply; 15+ messages in thread
From: Aaro Koskinen @ 2015-05-28 19:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Fri, Apr 17, 2015 at 12:45:34PM +0200, Nicolas Schichan wrote:
> On 01/16/2015 05:11 PM, Thomas Petazzoni wrote:
> > Instead of using explicit I/O synchronization barriers shoehorned
> > inside the streaming DMA mappings API (in
> > arch/arm/mach-mvebu/coherency.c), we are switching to use automatic
> > I/O synchronization barrier.
> 
> Hello Thomas,
> 
> I'm affraid this patche causes issue on mv88f6282 (and most probably on
> mv88f6281 as well): see below.

Is this issue already fixed in 4.1-rc5?

I just noticed that on mvebu/openrd-client the built-in PCI XGI framebuffer
stopped working somewhere between 3.19 and 4.0, and I bisected it to this
commit. The issue is also present in 4.1-rc5 and reverting this commit
makes the display to work again.

A.

> [...]
> 
> > diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> > index eb7682d..398f0ee 100644
> > --- a/drivers/bus/mvebu-mbus.c
> > +++ b/drivers/bus/mvebu-mbus.c
> > @@ -69,6 +69,7 @@
> >   */
> >  #define WIN_CTRL_OFF		0x0000
> >  #define   WIN_CTRL_ENABLE       BIT(0)
> > +#define   WIN_CTRL_SYNCBARRIER  BIT(1)
> 
> In the 88f6282 datasheet this bit in the "WindowX Control Register" is
> documented as reserved on all windows but window 6 and 7.
> 
> For windows 6 and 7 this bit is documented to write protect the window when set.
> 
> In our configuration, this window gets chosen for the PCI memory accesses
> (target 0x4, attribute 0xe8) and we effectively end up only being able to read
> from the PCI devices (mwl8k fails to load the firmware and sky2 timeouts on
> MDIO accesses). Write accesses are silently discarded (no external aborts of
> any kind), but that's probably expected as the AHB error propagation is disabled.
> 
> Reverting this patch made all the PCI device work correctly.
> 
> Regards,
> 
> -- 
> Nicolas Schichan
> Freebox SAS
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers
  2015-05-28 19:43     ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Aaro Koskinen
@ 2015-05-28 19:57       ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2015-05-28 19:57 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Aaro Koskinen,

On Thu, 28 May 2015 22:43:33 +0300, Aaro Koskinen wrote:

> On Fri, Apr 17, 2015 at 12:45:34PM +0200, Nicolas Schichan wrote:
> > On 01/16/2015 05:11 PM, Thomas Petazzoni wrote:
> > > Instead of using explicit I/O synchronization barriers shoehorned
> > > inside the streaming DMA mappings API (in
> > > arch/arm/mach-mvebu/coherency.c), we are switching to use automatic
> > > I/O synchronization barrier.
> > 
> > Hello Thomas,
> > 
> > I'm affraid this patche causes issue on mv88f6282 (and most probably on
> > mv88f6281 as well): see below.
> 
> Is this issue already fixed in 4.1-rc5?

No. I just resent the patch from Nicolas earlier today, he has been
merged by one of the mvebu maintainer, and he will send a pull request
for it. So it should be part of the final 4.1, and the patch is also
marked for stable, so it will be backported to 4.0.x as well.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-05-28 19:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-16 16:11 [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Thomas Petazzoni
2015-01-16 16:11 ` [PATCHv2 1/3] ARM: mvebu: completely disable hardware I/O coherency Thomas Petazzoni
2015-01-16 16:11 ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Thomas Petazzoni
2015-04-17 10:45   ` Nicolas Schichan
2015-04-17 11:23     ` Thomas Petazzoni
2015-04-24 14:44       ` [RFC PATCH] bus: mvebu-mbus: do not set WIN_CTRL_SYNCBARRIER on non io-coherent platforms Nicolas Schichan
2015-04-24 14:54         ` Thomas Petazzoni
2015-04-24 15:12       ` [PATCH] " Nicolas Schichan
2015-05-28 19:43     ` [PATCHv2 2/3] bus: mvebu-mbus: use automatic I/O synchronization barriers Aaro Koskinen
2015-05-28 19:57       ` Thomas Petazzoni
2015-01-16 16:11 ` [PATCHv2 3/3] ARM: mvebu: use arm_coherent_dma_ops and re-enable hardware I/O coherency Thomas Petazzoni
2015-01-16 16:15   ` Andrew Lunn
2015-01-16 16:20     ` Thomas Petazzoni
2015-01-19 22:36 ` [PATCHv2 0/3] ARM: mvebu: I/O coherency related fixes Andrew Lunn
2015-01-20 15:22   ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.