[v3,for,3.10] Introduce a Marvell EBU MBus driver

Commit Message

Neil Greatorex March 28, 2013, 10:27 p.m. UTC

Thomas,

On Thu, 28 Mar 2013, Thomas Petazzoni wrote:

> Dear Neil Greatorex,
>
> On Thu, 28 Mar 2013 01:32:11 +0000, Neil Greatorex wrote:
>
>> This set of patches seems to break the mvsdio driver (and hence
>> mwifiex_sdio) on the Globalscale Mirabox. I believe this is because
>> mvsdio.c contains a function mv_conf_mbus_windows that seems to
>> conflict with the new mbus driver. I don't understand enough about the
>> hardware (without the datasheets anyway) to be able to understand the
>> exact problem, but I hope that this is enough to point you (or someone
>> else) to it!
>
> Thanks for reporting this problem. As Ryan Press said in reply to your
> e-mail, it is well known that the mvsdio driver doesn't handle DMA
> properly with SDIO devices that are not SD cards, and this, regardless
> of whether the mvebu-mbus driver patches are applied or not.
>
> Therefore, are you sure that this problem is really introduced by the
> mvebu-mbus driver, and doesn't exist without applying the patches?
>
> The mv_conf_mbus_windows() function of the mvsdio driver doesn't
> conflict with the mvebu-mbus driver. What the mvsdio driver does is
> request the list of DRAM chip-selects using mv_mbus_dram_info(), and
> configure the SDIO unit with those informations in
> mv_conf_mbus_windows(). But yes, the list of DRAM chip-selects is
> provided by mv_mbus_dram_info() and this function has changed by the
> introduction of the mvebu-mbus driver.
>
> I'm on the move right now, so I don't have hardware around to test
> this. Would it be possible for you to test the Mirabox with the
> following patch applied on mvsdio, and do a run with and without the
> patch series that introduces mvebu-mbus? The idea is that the
> informations listed should be identical. If not, then indeed there is
> potentially a problem with the mvebu-mbus driver.
>

Thanks for your information. I have managed to track down the bug. It 
seems that two crucial lines were missed in the conversion to the 
mvebu-mbus driver - the lines that set the hw_io_coherency flag if a 
marvell,coherency-fabric compatible node is in the device tree. This leads 
to the differences below that I found using your printk patch:

Before mvebu-mbus patches:

[SD] root@mirabox:~# modprobe mvsdio
[  134.595899] mvsdio: CS[0], base = 0x0, size = 0x20000000, mbus_attr = 
0x1e
[  134.602821] mvsdio: CS[1], base = 0x20000000, size = 0x20000000, 
mbus_attr = 0x1d
[  134.643933] mmc0: mvsdio driver initialized, lacking card detect (fall 
back to polling)
[  134.660814] mmc0: new high speed SDIO card at address 0001

After mvebu-mbus patches:

[SD] root@mirabox:~# modprobe mvsdio
[  107.462881] mvsdio: CS[0], base = 0x0, size = 0x20000000, mbus_attr = 
0xe
[  107.469766] mvsdio: CS[1], base = 0x20000000, size = 0x20000000, 
mbus_attr = 0xd
[  107.516219] mmc0: mvsdio driver initialized, lacking card detect (fall 
back to polling)
[  107.533029] mmc0: new high speed SDIO card at address 0001

Here is a patch that restores the behaviour to how it was before the mbus 
patches, but it obviously needs testing on other hardware than the 
Mirabox. It has been based on your marvell-mvebu-mbus-v3 branch from 
Github:

-- >8 --
Subject: [PATCH] bus: mvebu-mbus: Restore checking for coherency fabric
  hardware

The new mvebu-mbus driver was not checking the device tree for
coherency fabric hardware and hence was not setting the hw_io_coherency
flag in mbus_state. This prevented the mvsdio driver from operating
correctly. This patch restores the check.
---
  drivers/bus/mvebu-mbus.c |    3 +++
  1 file changed, 3 insertions(+)

  		mvebu_mbus_disable_window(mbus, win);

Comments

Jason Cooper March 29, 2013, 1 a.m. UTC | #1

Thomas,

On Thu, Mar 28, 2013 at 10:27:28PM +0000, Neil Greatorex wrote:
> Thomas,
> 
> On Thu, 28 Mar 2013, Thomas Petazzoni wrote:
> 
> >Dear Neil Greatorex,
> >
> >On Thu, 28 Mar 2013 01:32:11 +0000, Neil Greatorex wrote:
> >
...
> Thanks for your information. I have managed to track down the bug.
> It seems that two crucial lines were missed in the conversion to the
> mvebu-mbus driver - the lines that set the hw_io_coherency flag if a
> marvell,coherency-fabric compatible node is in the device tree. This
> leads to the differences below that I found using your printk patch:
> 
> Before mvebu-mbus patches:
> 
> [SD] root@mirabox:~# modprobe mvsdio
> [  134.595899] mvsdio: CS[0], base = 0x0, size = 0x20000000,
> mbus_attr = 0x1e
> [  134.602821] mvsdio: CS[1], base = 0x20000000, size = 0x20000000,
> mbus_attr = 0x1d
> [  134.643933] mmc0: mvsdio driver initialized, lacking card detect
> (fall back to polling)
> [  134.660814] mmc0: new high speed SDIO card at address 0001
> 
> After mvebu-mbus patches:
> 
> [SD] root@mirabox:~# modprobe mvsdio
> [  107.462881] mvsdio: CS[0], base = 0x0, size = 0x20000000,
> mbus_attr = 0xe
> [  107.469766] mvsdio: CS[1], base = 0x20000000, size = 0x20000000,
> mbus_attr = 0xd
> [  107.516219] mmc0: mvsdio driver initialized, lacking card detect
> (fall back to polling)
> [  107.533029] mmc0: new high speed SDIO card at address 0001
> 
> Here is a patch that restores the behaviour to how it was before the
> mbus patches, but it obviously needs testing on other hardware than
> the Mirabox. It has been based on your marvell-mvebu-mbus-v3 branch
> from Github:
> 
> -- >8 --
> Subject: [PATCH] bus: mvebu-mbus: Restore checking for coherency fabric
>  hardware
> 
> The new mvebu-mbus driver was not checking the device tree for
> coherency fabric hardware and hence was not setting the hw_io_coherency
> flag in mbus_state. This prevented the mvsdio driver from operating
> correctly. This patch restores the check.
> ---
>  drivers/bus/mvebu-mbus.c |    3 +++
>  1 file changed, 3 insertions(+)

Thomas,

If you can give an Ack/Tested-by, I'll apply this on top of what I just
pulled in.

thx,

Jason.

> 
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index 586d03e..a0250c6 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -858,6 +858,9 @@ int __init mvebu_mbus_init(const char *soc,
> phys_addr_t mbuswins_phys_base,
>  		return -ENOMEM;
>  	}
> 
> +	if (of_find_compatible_node(NULL, NULL,
> "marvell,coherency-fabric"))
> +		mbus->hw_io_coherency = 1;
> +
>  	for (win = 0; win < mbus->soc->num_wins; win++)
>  		mvebu_mbus_disable_window(mbus, win);
> 
> -- 
> 1.7.10.4
> 
> I hope that works for you on the Mirabox and the other platforms...
> 
> <Thomas' patch snipped>
> 
> Cheers,
> Neil

Jason Cooper March 30, 2013, 8:21 p.m. UTC | #2

On Thu, Mar 28, 2013 at 10:27:28PM +0000, Neil Greatorex wrote:
...
> -- >8 --
> Subject: [PATCH] bus: mvebu-mbus: Restore checking for coherency fabric
>  hardware
> 
> The new mvebu-mbus driver was not checking the device tree for
> coherency fabric hardware and hence was not setting the hw_io_coherency
> flag in mbus_state. This prevented the mvsdio driver from operating
> correctly. This patch restores the check.

Neil,

If I could get you to put a Signed-off-by: on this, I'll queue it up so
folks can test it more easily.

thx,

Jason.

> ---
>  drivers/bus/mvebu-mbus.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index 586d03e..a0250c6 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -858,6 +858,9 @@ int __init mvebu_mbus_init(const char *soc,
> phys_addr_t mbuswins_phys_base,
>  		return -ENOMEM;
>  	}
> 
> +	if (of_find_compatible_node(NULL, NULL,
> "marvell,coherency-fabric"))
> +		mbus->hw_io_coherency = 1;
> +
>  	for (win = 0; win < mbus->soc->num_wins; win++)
>  		mvebu_mbus_disable_window(mbus, win);
> 
> -- 
> 1.7.10.4

Patch

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index 586d03e..a0250c6 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -858,6 +858,9 @@  int __init mvebu_mbus_init(const char *soc, 
phys_addr_t mbuswins_phys_base,
  		return -ENOMEM;
  	}

+	if (of_find_compatible_node(NULL, NULL, 
"marvell,coherency-fabric"))
+		mbus->hw_io_coherency = 1;
+
  	for (win = 0; win < mbus->soc->num_wins; win++)