2014-04-16 12:40:18

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 0/4] ARM: berlin: add SDHCI support

This series adds the support for the SDHCI controller for Marvell Berlin
SoCs and enable the SD card reader and eMMC for the BG2Q DMP. The
controller supports 3 sockets.

Tested on the BG2Q DMP.

This series applies on top of Alexandre's patches for the Berlin's pll:
https://patchwork.kernel.org/patch/3890341/
https://patchwork.kernel.org/patch/3876271/

Antoine Ténart (4):
mmc: sdhci: add a driver for Berlin SoCs
Documentation: bindings: add the sdhci-berlin
ARM: dts: berlin: add the SDHCI nodes for the BG2Q
ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP

.../devicetree/bindings/mmc/sdhci-berlin.txt | 18 +++
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 ++
arch/arm/boot/dts/berlin2q.dtsi | 40 +++++++
drivers/mmc/host/Kconfig | 11 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-berlin.c | 129 +++++++++++++++++++++
6 files changed, 207 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-berlin.txt
create mode 100644 drivers/mmc/host/sdhci-berlin.c

--
1.8.3.2


2014-04-16 12:40:26

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q

Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
driver.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 5925e6a16749..8f897d461460 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -67,6 +67,14 @@
clock-div = <3>;
};

+ sdio1clk: sdio1clk {
+ compatible = "fixed-factor-clock";
+ #clock-cells = <0>;
+ clocks = <&syspll>;
+ clock-mult = <1>;
+ clock-div = <4>;
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <1>;
@@ -75,6 +83,38 @@
ranges = <0 0xf7000000 0x1000000>;
interrupt-parent = <&gic>;

+ sdhci0: sdhci@ab0000 {
+ compatible = "marvell,berlin2q-sdhci";
+ reg = <0xab0000 0x200>;
+ clocks = <&sdio1clk>;
+ interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+ keep-power-in-suspend;
+ enable-sdio-wakeup;
+ broken-cd;
+ status = "disabled";
+ };
+
+ sdhci1: sdhci@ab0800 {
+ compatible = "marvell,berlin2q-sdhci";
+ reg = <0xab0800 0x200>;
+ clocks = <&sdio1clk>;
+ interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+ keep-power-in-suspend;
+ enable-sdio-wakeup;
+ status = "disabled";
+ };
+
+ sdhci2: sdhci@ab1000 {
+ compatible = "marvell,berlin2q-sdhci";
+ reg = <0xab1000 0x200>;
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&sdio1clk>;
+ keep-power-in-suspend;
+ enable-sdio-wakeup;
+ broken-cd;
+ status = "disabled";
+ };
+
l2: l2-cache-controller@ac0000 {
compatible = "arm,pl310-cache";
reg = <0xac0000 0x1000>;
--
1.8.3.2

2014-04-16 12:40:24

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 2/4] Documentation: bindings: add the sdhci-berlin

Add the binding documentation for the Marvell Berlin SDHCI driver.

Signed-off-by: Antoine Ténart <[email protected]>
---
Documentation/devicetree/bindings/mmc/sdhci-berlin.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-berlin.txt

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-berlin.txt b/Documentation/devicetree/bindings/mmc/sdhci-berlin.txt
new file mode 100644
index 000000000000..1a5591f85a64
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-berlin.txt
@@ -0,0 +1,18 @@
+* Marvell Berlin SDHCI controller
+
+All common properties are documented in mmc.txt
+
+Required properties:
+- compatible: "marvell,berlin2-sdhci",
+ "marvell,berlin2cd-sdhci",
+ "marvell,berlin2q-sdhci"
+- clocks: reference to the SDHCI controller clock
+
+Example:
+
+sdhci0: sdhci@ab0000 {
+ compatible = "marvell,berlin2q-sdhci";
+ reg = <0xab0000 0x200>;
+ clocks = <&sdio1clk>;
+ interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+};
--
1.8.3.2

2014-04-16 12:40:20

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
This controller supports 3 sockets.

Signed-off-by: Antoine Ténart <[email protected]>
---
drivers/mmc/host/Kconfig | 11 ++++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-berlin.c | 129 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 141 insertions(+)
create mode 100644 drivers/mmc/host/sdhci-berlin.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1384f67abe21..42db70031eb2 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -283,6 +283,17 @@ config MMC_SDHCI_BCM2835

If unsure, say N.

+config MMC_SDHCI_BERLIN
+ tristate "Marvell Berlin SD/MMC Host Controller support"
+ depends on ARCH_BERLIN
+ depends on MMC_SDHCI_PLTFM
+ select MMC_SDHCI_IO_ACCESSORS
+ help
+ This selects the Berlin SD/MMC controller. If you have a Berlin
+ platform with SD or MMC devices, say Y or M here.
+
+ If unsure, say N.
+
config MMC_OMAP
tristate "TI OMAP Multimedia Card Interface support"
depends on ARCH_OMAP
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 3483b6b6b880..b0256290530c 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o
+obj-$(CONFIG_MMC_SDHCI_BERLIN) += sdhci-berlin.o

ifeq ($(CONFIG_CB710_DEBUG),y)
CFLAGS-cb710-mmc += -DDEBUG
diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c
new file mode 100644
index 000000000000..ece8f7863937
--- /dev/null
+++ b/drivers/mmc/host/sdhci-berlin.c
@@ -0,0 +1,129 @@
+/*
+ * Marvell Berlin SDHCI driver
+ *
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mmc/host.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include "sdhci-pltfm.h"
+
+static struct sdhci_ops sdhci_berlin_ops = {
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
+};
+
+static struct sdhci_pltfm_data sdhci_berlin2_pdata = {
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_BROKEN_ADMA |
+ SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
+ .ops = &sdhci_berlin_ops,
+};
+
+static struct sdhci_pltfm_data sdhci_berlin2q_pdata = {
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+ SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
+ .ops = &sdhci_berlin_ops,
+};
+
+static const struct of_device_id sdhci_berlin_of_match_table[] = {
+ {
+ .compatible = "marvell,berlin2-sdhci",
+ .data = &sdhci_berlin2_pdata,
+ },
+ {
+ .compatible = "marvell,berlin2cd-sdhci",
+ .data = &sdhci_berlin2_pdata,
+ },
+ {
+ .compatible = "marvell,berlin2q-sdhci",
+ .data = &sdhci_berlin2q_pdata,
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, sdhci_berlin_of_match_table);
+
+static int sdhci_berlin_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sdhci_host *host;
+ struct sdhci_pltfm_host *pltfm_host;
+ struct clk *clk;
+ const struct of_device_id *device =
+ of_match_device(sdhci_berlin_of_match_table, dev);
+ int ret;
+
+ host = sdhci_pltfm_init(pdev,
+ (struct sdhci_pltfm_data *)device->data, 0);
+ if (IS_ERR(host))
+ return PTR_ERR(host);
+
+ pltfm_host = sdhci_priv(host);
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk)) {
+ dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk));
+ ret = PTR_ERR(clk);
+ goto err_clk_get;
+ }
+
+ clk_prepare_enable(clk);
+ pltfm_host->clk = clk;
+
+ sdhci_get_of_property(pdev);
+
+ if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+ host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+
+ ret = sdhci_add_host(host);
+ if (ret)
+ goto err_add_host;
+
+ return 0;
+
+err_add_host:
+ clk_disable_unprepare(pltfm_host->clk);
+err_clk_get:
+ sdhci_pltfm_free(pdev);
+
+ return ret;
+}
+
+static int sdhci_berlin_remove(struct platform_device *pdev)
+{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+ sdhci_pltfm_unregister(pdev);
+ clk_disable_unprepare(pltfm_host->clk);
+
+ return 0;
+}
+
+static struct platform_driver sdhci_berlin_driver = {
+ .driver = {
+ .name = "berlin-sdhci",
+ .owner = THIS_MODULE,
+ .pm = SDHCI_PLTFM_PMOPS,
+ .of_match_table = sdhci_berlin_of_match_table,
+ },
+ .probe = sdhci_berlin_probe,
+ .remove = sdhci_berlin_remove,
+};
+module_platform_driver(sdhci_berlin_driver);
+
+MODULE_DESCRIPTION("SDHCI driver for Marvell Berlin SoC");
+MODULE_AUTHOR("Antoine Ténart <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.8.3.2

2014-04-16 12:42:21

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 4/4] ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP

Enable the SD Card reader and the internal eMMC on the Berlin BG2Q DMP
using two of the SDHCI nodes of the Berlin BG2Q.

Signed-off-by: Antoine Ténart <[email protected]>
---
arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 2da9c41e29d8..671ed4d055ed 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -26,3 +26,11 @@
&uart0 {
status = "okay";
};
+
+&sdhci1 {
+ status = "okay";
+};
+
+&sdhci2 {
+ status = "okay";
+};
--
1.8.3.2

2014-04-16 12:56:40

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

On Wed, 2014-04-16 at 14:40 +0200, Antoine T?nart wrote:
> Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> This controller supports 3 sockets.

trivial notes:

> diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c
[]
> +static struct sdhci_ops sdhci_berlin_ops = {
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_berlin2_pdata = {
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> + SDHCI_QUIRK_BROKEN_ADMA |
> + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> + .ops = &sdhci_berlin_ops,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_berlin2q_pdata = {
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> + SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> + .ops = &sdhci_berlin_ops,
> +};

Perhaps const?

> +static int sdhci_berlin_probe(struct platform_device *pdev)
> +{
[]
> + host = sdhci_pltfm_init(pdev,
> + (struct sdhci_pltfm_data *)device->data, 0);

Unnecessary cast? Maybe:

host = sdhci_pltfm_init(pdev, device->data, 0);

> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk));
> + ret = PTR_ERR(clk);

PTR_ERR is an int. Does this produce a compile warning?
Maybe reverse these lines and use ret?

ret = PTR_ERR(clk);
dev_err(dev, count not get clock: %d\n", ret);

2014-04-16 13:09:49

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

Joe,

On Wed, Apr 16, 2014 at 05:56:34AM -0700, Joe Perches wrote:
> On Wed, 2014-04-16 at 14:40 +0200, Antoine T?nart wrote:
> > Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> > This controller supports 3 sockets.
>
> trivial notes:
>
> > diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c
> []
> > +static struct sdhci_ops sdhci_berlin_ops = {
> > + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> > +};
> > +
> > +static struct sdhci_pltfm_data sdhci_berlin2_pdata = {
> > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> > + SDHCI_QUIRK_BROKEN_ADMA |
> > + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> > + .ops = &sdhci_berlin_ops,
> > +};
> > +
> > +static struct sdhci_pltfm_data sdhci_berlin2q_pdata = {
> > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> > + SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> > + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> > + .ops = &sdhci_berlin_ops,
> > +};
>
> Perhaps const?

Yes, sure. I'll fix that.

>
> > +static int sdhci_berlin_probe(struct platform_device *pdev)
> > +{
> []
> > + host = sdhci_pltfm_init(pdev,
> > + (struct sdhci_pltfm_data *)device->data, 0);
>
> Unnecessary cast? Maybe:
>
> host = sdhci_pltfm_init(pdev, device->data, 0);

Right.

>
> > + if (IS_ERR(host))
> > + return PTR_ERR(host);
> > +
> > + pltfm_host = sdhci_priv(host);
> > +
> > + clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(clk)) {
> > + dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk));
> > + ret = PTR_ERR(clk);
>
> PTR_ERR is an int. Does this produce a compile warning?

It does, I had a warning.

> Maybe reverse these lines and use ret?
>
> ret = PTR_ERR(clk);
> dev_err(dev, count not get clock: %d\n", ret);

That's better. Will do.

Thanks !

Antoine


--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-16 13:11:38

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q

On Wed, Apr 16, 2014 at 02:40:10PM +0200, Antoine T?nart wrote:
> Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
> driver.
>
> Signed-off-by: Antoine T?nart <[email protected]>
> ---
> arch/arm/boot/dts/berlin2q.dtsi | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> index 5925e6a16749..8f897d461460 100644
> --- a/arch/arm/boot/dts/berlin2q.dtsi
> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> @@ -67,6 +67,14 @@
> clock-div = <3>;
> };
>
> + sdio1clk: sdio1clk {
> + compatible = "fixed-factor-clock";
> + #clock-cells = <0>;
> + clocks = <&syspll>;
> + clock-mult = <1>;
> + clock-div = <4>;
> + };
> +
> soc {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -75,6 +83,38 @@
> ranges = <0 0xf7000000 0x1000000>;
> interrupt-parent = <&gic>;
>
> + sdhci0: sdhci@ab0000 {
> + compatible = "marvell,berlin2q-sdhci";
> + reg = <0xab0000 0x200>;
> + clocks = <&sdio1clk>;
> + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> + keep-power-in-suspend;
> + enable-sdio-wakeup;
> + broken-cd;

Hi Antoine

I would expect these three last properties to be a property of the
board, not the SoC. Or am i missing something?

Thanks
Andrew

2014-04-16 13:23:18

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q

Hi Andrew,

On Wed, Apr 16, 2014 at 03:09:15PM +0200, Andrew Lunn wrote:
> On Wed, Apr 16, 2014 at 02:40:10PM +0200, Antoine T?nart wrote:
> > Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
> > driver.
> >
> > Signed-off-by: Antoine T?nart <[email protected]>
> > ---
> > arch/arm/boot/dts/berlin2q.dtsi | 40 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
> > index 5925e6a16749..8f897d461460 100644
> > --- a/arch/arm/boot/dts/berlin2q.dtsi
> > +++ b/arch/arm/boot/dts/berlin2q.dtsi
> > @@ -67,6 +67,14 @@
> > clock-div = <3>;
> > };
> >
> > + sdio1clk: sdio1clk {
> > + compatible = "fixed-factor-clock";
> > + #clock-cells = <0>;
> > + clocks = <&syspll>;
> > + clock-mult = <1>;
> > + clock-div = <4>;
> > + };
> > +
> > soc {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > @@ -75,6 +83,38 @@
> > ranges = <0 0xf7000000 0x1000000>;
> > interrupt-parent = <&gic>;
> >
> > + sdhci0: sdhci@ab0000 {
> > + compatible = "marvell,berlin2q-sdhci";
> > + reg = <0xab0000 0x200>;
> > + clocks = <&sdio1clk>;
> > + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> > + keep-power-in-suspend;
> > + enable-sdio-wakeup;
> > + broken-cd;
>
> Hi Antoine
>
> I would expect these three last properties to be a property of the
> board, not the SoC. Or am i missing something?

No reason, I'll move them.

Thanks !

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-16 14:26:18

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

On 04/16/2014 02:40 PM, Antoine Ténart wrote:
> Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> This controller supports 3 sockets.

nit: Isn't it three controller with one socket each?

> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 11 ++++
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-berlin.c | 129 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 141 insertions(+)
> create mode 100644 drivers/mmc/host/sdhci-berlin.c
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 1384f67abe21..42db70031eb2 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -283,6 +283,17 @@ config MMC_SDHCI_BCM2835
>
> If unsure, say N.
>
> +config MMC_SDHCI_BERLIN
> + tristate "Marvell Berlin SD/MMC Host Controller support"
> + depends on ARCH_BERLIN
> + depends on MMC_SDHCI_PLTFM
> + select MMC_SDHCI_IO_ACCESSORS
> + help
> + This selects the Berlin SD/MMC controller. If you have a Berlin
> + platform with SD or MMC devices, say Y or M here.
> +
> + If unsure, say N.
> +
> config MMC_OMAP
> tristate "TI OMAP Multimedia Card Interface support"
> depends on ARCH_OMAP
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 3483b6b6b880..b0256290530c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
> obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
> obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o
> +obj-$(CONFIG_MMC_SDHCI_BERLIN) += sdhci-berlin.o
>
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc += -DDEBUG
> diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c
> new file mode 100644
> index 000000000000..ece8f7863937
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-berlin.c
> @@ -0,0 +1,129 @@
> +/*
> + * Marvell Berlin SDHCI driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include "sdhci-pltfm.h"
> +
> +static struct sdhci_ops sdhci_berlin_ops = {
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_berlin2_pdata = {
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> + SDHCI_QUIRK_BROKEN_ADMA |
> + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> + .ops = &sdhci_berlin_ops,
> +};
> +
> +static struct sdhci_pltfm_data sdhci_berlin2q_pdata = {
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> + SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> + .ops = &sdhci_berlin_ops,
> +};
> +
> +static const struct of_device_id sdhci_berlin_of_match_table[] = {
> + {
> + .compatible = "marvell,berlin2-sdhci",
> + .data = &sdhci_berlin2_pdata,
> + },
> + {
> + .compatible = "marvell,berlin2cd-sdhci",
> + .data = &sdhci_berlin2_pdata,
> + },
> + {
> + .compatible = "marvell,berlin2q-sdhci",
> + .data = &sdhci_berlin2q_pdata,
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_berlin_of_match_table);
> +
> +static int sdhci_berlin_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sdhci_host *host;
> + struct sdhci_pltfm_host *pltfm_host;
> + struct clk *clk;
> + const struct of_device_id *device =
> + of_match_device(sdhci_berlin_of_match_table, dev);
> + int ret;
> +
> + host = sdhci_pltfm_init(pdev,
> + (struct sdhci_pltfm_data *)device->data, 0);
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk));
> + ret = PTR_ERR(clk);
> + goto err_clk_get;
> + }
> +
> + clk_prepare_enable(clk);
> + pltfm_host->clk = clk;
> +
> + sdhci_get_of_property(pdev);
> +
> + if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> + host->mmc->caps |= MMC_CAP_NONREMOVABLE;

Documentation/devicetree/bindings/mmc/mmc.txt names "non-removable"
as a standard property. Any chance we can shove this two lines above
right into sdhci_get_of_property()?

Besides the other comments from Joe, this looks good to me,

Reviewed-by: Sebastian Hesselbarth <[email protected]>

> +
> + ret = sdhci_add_host(host);
> + if (ret)
> + goto err_add_host;
> +
> + return 0;
> +
> +err_add_host:
> + clk_disable_unprepare(pltfm_host->clk);
> +err_clk_get:
> + sdhci_pltfm_free(pdev);
> +
> + return ret;
> +}
> +
> +static int sdhci_berlin_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> + sdhci_pltfm_unregister(pdev);
> + clk_disable_unprepare(pltfm_host->clk);
> +
> + return 0;
> +}
> +
> +static struct platform_driver sdhci_berlin_driver = {
> + .driver = {
> + .name = "berlin-sdhci",
> + .owner = THIS_MODULE,
> + .pm = SDHCI_PLTFM_PMOPS,
> + .of_match_table = sdhci_berlin_of_match_table,
> + },
> + .probe = sdhci_berlin_probe,
> + .remove = sdhci_berlin_remove,
> +};
> +module_platform_driver(sdhci_berlin_driver);
> +
> +MODULE_DESCRIPTION("SDHCI driver for Marvell Berlin SoC");
> +MODULE_AUTHOR("Antoine Ténart <[email protected]>");
> +MODULE_LICENSE("GPL");
>

2014-04-17 03:35:23

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q

Hi Antoine,

On Wed, 16 Apr 2014 05:40:10 -0700
Antoine Ténart <[email protected]> wrote:

> Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
> driver.
>
> Signed-off-by: Antoine Ténart <[email protected]>
> ---
> arch/arm/boot/dts/berlin2q.dtsi | 40
> ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm/boot/dts/berlin2q.dtsi
> b/arch/arm/boot/dts/berlin2q.dtsi index 5925e6a16749..8f897d461460 100644
> --- a/arch/arm/boot/dts/berlin2q.dtsi
> +++ b/arch/arm/boot/dts/berlin2q.dtsi
> @@ -67,6 +67,14 @@
> clock-div = <3>;
> };
>
> + sdio1clk: sdio1clk {
> + compatible = "fixed-factor-clock";
> + #clock-cells = <0>;
> + clocks = <&syspll>;
> + clock-mult = <1>;
> + clock-div = <4>;
> + };
> +
> soc {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -75,6 +83,38 @@
> ranges = <0 0xf7000000 0x1000000>;
> interrupt-parent = <&gic>;
>
> + sdhci0: sdhci@ab0000 {
> + compatible = "marvell,berlin2q-sdhci";
> + reg = <0xab0000 0x200>;
> + clocks = <&sdio1clk>;
> + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> + keep-power-in-suspend;
> + enable-sdio-wakeup;
> + broken-cd;
> + status = "disabled";
> + };
> +
> + sdhci1: sdhci@ab0800 {
> + compatible = "marvell,berlin2q-sdhci";
> + reg = <0xab0800 0x200>;
> + clocks = <&sdio1clk>;
> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> + keep-power-in-suspend;
> + enable-sdio-wakeup;
> + status = "disabled";
> + };
> +
> + sdhci2: sdhci@ab1000 {
> + compatible = "marvell,berlin2q-sdhci";
> + reg = <0xab1000 0x200>;
> + interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&sdio1clk>;
> + keep-power-in-suspend;
> + enable-sdio-wakeup;
> + broken-cd;
> + status = "disabled";
> + };

could we put sdhci@ab1000 at the first of sdhci lists? For two reasons:

1. sdhci@ab0000 and sdhci@ab0800 is called as sdhci1 and sdhci2 in mrvl
internal discussion, so this would make the name consistent when we
upgrade linux kernel to one mainline version.

2. sdhci@ab1000 is always used for emmc. if sdhci@ab0800 is put at the
head of sdhci@ab1000, and there's one sdcard in it, mmcblock0 would be
the sdcard rather than emmc.

I dunno whether there's elegant solutions for these two issues. alias? Could
anyone kindly help?

Thanks in advance,
Jisheng

2014-04-17 06:54:29

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q

On 04/17/2014 05:33 AM, Jisheng Zhang wrote:
> On Wed, 16 Apr 2014 05:40:10 -0700
> Antoine Ténart <[email protected]> wrote:
>> Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
>> driver.
[...]
>> + sdhci0: sdhci@ab0000 {
>> + compatible = "marvell,berlin2q-sdhci";
>> + reg = <0xab0000 0x200>;
>> + clocks = <&sdio1clk>;
>> + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
>> + keep-power-in-suspend;
>> + enable-sdio-wakeup;
>> + broken-cd;
>> + status = "disabled";
>> + };
>> +
>> + sdhci1: sdhci@ab0800 {
>> + compatible = "marvell,berlin2q-sdhci";
>> + reg = <0xab0800 0x200>;
>> + clocks = <&sdio1clk>;
>> + interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
>> + keep-power-in-suspend;
>> + enable-sdio-wakeup;
>> + status = "disabled";
>> + };
>> +
>> + sdhci2: sdhci@ab1000 {
>> + compatible = "marvell,berlin2q-sdhci";
>> + reg = <0xab1000 0x200>;
>> + interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&sdio1clk>;
>> + keep-power-in-suspend;
>> + enable-sdio-wakeup;
>> + broken-cd;
>> + status = "disabled";
>> + };
>
> could we put sdhci@ab1000 at the first of sdhci lists? For two reasons:

Don't reorder the nodes, but use aliases.

> 1. sdhci@ab0000 and sdhci@ab0800 is called as sdhci1 and sdhci2 in mrvl
> internal discussion, so this would make the name consistent when we
> upgrade linux kernel to one mainline version.

How about we only move the node labels?

> 2. sdhci@ab1000 is always used for emmc. if sdhci@ab0800 is put at the
> head of sdhci@ab1000, and there's one sdcard in it, mmcblock0 would be
> the sdcard rather than emmc.

And label this one sdhci0?

> I dunno whether there's elegant solutions for these two issues. alias? Could
> anyone kindly help?

Have a look at drivers/mmc/host/dw_mmc.c:

ctrl_id = of_alias_get_id(host->dev->of_node, "mshc");

this also requires an aliases node in berlin2foo.dtsi:

aliases {
mshc0 = &sdhci0;
mshc1 = &sdhci1;
mshc2 = &sdhci2;
};

Rather than using "mshc", I'd prefer something like "sdio" or "mmc".

Also, if that alias would be part of generic mmc OF code would be
good too, but we'll have to wait for Chris' call here.

Sebastian

2014-04-17 13:34:17

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

Sebastian,

On Wed, Apr 16, 2014 at 04:26:06PM +0200, Sebastian Hesselbarth wrote:
> On 04/16/2014 02:40 PM, Antoine T?nart wrote:
> >Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> >This controller supports 3 sockets.
>
> nit: Isn't it three controller with one socket each?

It's a little bit confusing, there are 3 different register regions and
I can see 3 separate data lines but, I quote: "The SD host controller
supports three sockets".

Maybe Jisheng can advise on that matter ?

>
> >Signed-off-by: Antoine T?nart <[email protected]>
> >---
> > drivers/mmc/host/Kconfig | 11 ++++
> > drivers/mmc/host/Makefile | 1 +
> > drivers/mmc/host/sdhci-berlin.c | 129 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 141 insertions(+)
> > create mode 100644 drivers/mmc/host/sdhci-berlin.c
> >
> >diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> >index 1384f67abe21..42db70031eb2 100644
> >--- a/drivers/mmc/host/Kconfig
> >+++ b/drivers/mmc/host/Kconfig
> >@@ -283,6 +283,17 @@ config MMC_SDHCI_BCM2835
> >
> > If unsure, say N.
> >
> >+config MMC_SDHCI_BERLIN
> >+ tristate "Marvell Berlin SD/MMC Host Controller support"
> >+ depends on ARCH_BERLIN
> >+ depends on MMC_SDHCI_PLTFM
> >+ select MMC_SDHCI_IO_ACCESSORS
> >+ help
> >+ This selects the Berlin SD/MMC controller. If you have a Berlin
> >+ platform with SD or MMC devices, say Y or M here.
> >+
> >+ If unsure, say N.
> >+
> > config MMC_OMAP
> > tristate "TI OMAP Multimedia Card Interface support"
> > depends on ARCH_OMAP
> >diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> >index 3483b6b6b880..b0256290530c 100644
> >--- a/drivers/mmc/host/Makefile
> >+++ b/drivers/mmc/host/Makefile
> >@@ -64,6 +64,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> > obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
> > obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
> > obj-$(CONFIG_MMC_SDHCI_BCM2835) += sdhci-bcm2835.o
> >+obj-$(CONFIG_MMC_SDHCI_BERLIN) += sdhci-berlin.o
> >
> > ifeq ($(CONFIG_CB710_DEBUG),y)
> > CFLAGS-cb710-mmc += -DDEBUG
> >diff --git a/drivers/mmc/host/sdhci-berlin.c b/drivers/mmc/host/sdhci-berlin.c
> >new file mode 100644
> >index 000000000000..ece8f7863937
> >--- /dev/null
> >+++ b/drivers/mmc/host/sdhci-berlin.c
> >@@ -0,0 +1,129 @@
> >+/*
> >+ * Marvell Berlin SDHCI driver
> >+ *
> >+ * Copyright (C) 2014 Marvell Technology Group Ltd.
> >+ *
> >+ * Antoine T?nart <[email protected]>
> >+ *
> >+ * This file is licensed under the terms of the GNU General Public
> >+ * License version 2. This program is licensed "as is" without any
> >+ * warranty of any kind, whether express or implied
> >+ */
> >+
> >+#include <linux/clk.h>
> >+#include <linux/err.h>
> >+#include <linux/io.h>
> >+#include <linux/mmc/host.h>
> >+#include <linux/module.h>
> >+#include <linux/of.h>
> >+#include <linux/of_device.h>
> >+
> >+#include "sdhci-pltfm.h"
> >+
> >+static struct sdhci_ops sdhci_berlin_ops = {
> >+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> >+};
> >+
> >+static struct sdhci_pltfm_data sdhci_berlin2_pdata = {
> >+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> >+ SDHCI_QUIRK_BROKEN_ADMA |
> >+ SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> >+ .ops = &sdhci_berlin_ops,
> >+};
> >+
> >+static struct sdhci_pltfm_data sdhci_berlin2q_pdata = {
> >+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> >+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
> >+ SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> >+ .ops = &sdhci_berlin_ops,
> >+};
> >+
> >+static const struct of_device_id sdhci_berlin_of_match_table[] = {
> >+ {
> >+ .compatible = "marvell,berlin2-sdhci",
> >+ .data = &sdhci_berlin2_pdata,
> >+ },
> >+ {
> >+ .compatible = "marvell,berlin2cd-sdhci",
> >+ .data = &sdhci_berlin2_pdata,
> >+ },
> >+ {
> >+ .compatible = "marvell,berlin2q-sdhci",
> >+ .data = &sdhci_berlin2q_pdata,
> >+ },
> >+ {}
> >+};
> >+MODULE_DEVICE_TABLE(of, sdhci_berlin_of_match_table);
> >+
> >+static int sdhci_berlin_probe(struct platform_device *pdev)
> >+{
> >+ struct device *dev = &pdev->dev;
> >+ struct sdhci_host *host;
> >+ struct sdhci_pltfm_host *pltfm_host;
> >+ struct clk *clk;
> >+ const struct of_device_id *device =
> >+ of_match_device(sdhci_berlin_of_match_table, dev);
> >+ int ret;
> >+
> >+ host = sdhci_pltfm_init(pdev,
> >+ (struct sdhci_pltfm_data *)device->data, 0);
> >+ if (IS_ERR(host))
> >+ return PTR_ERR(host);
> >+
> >+ pltfm_host = sdhci_priv(host);
> >+
> >+ clk = devm_clk_get(dev, NULL);
> >+ if (IS_ERR(clk)) {
> >+ dev_err(dev, "could not get clock: %ld\n", PTR_ERR(clk));
> >+ ret = PTR_ERR(clk);
> >+ goto err_clk_get;
> >+ }
> >+
> >+ clk_prepare_enable(clk);
> >+ pltfm_host->clk = clk;
> >+
> >+ sdhci_get_of_property(pdev);
> >+
> >+ if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> >+ host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>
> Documentation/devicetree/bindings/mmc/mmc.txt names "non-removable"
> as a standard property. Any chance we can shove this two lines above
> right into sdhci_get_of_property()?

Well, I gave it a try without this quirk and it seemed to work fine.
If we find out we do need it, I agree to add this quirk in
sdhci_get_of_property().

Antoine

>
> Besides the other comments from Joe, this looks good to me,
>
> Reviewed-by: Sebastian Hesselbarth <[email protected]>
>
> >+
> >+ ret = sdhci_add_host(host);
> >+ if (ret)
> >+ goto err_add_host;
> >+
> >+ return 0;
> >+
> >+err_add_host:
> >+ clk_disable_unprepare(pltfm_host->clk);
> >+err_clk_get:
> >+ sdhci_pltfm_free(pdev);
> >+
> >+ return ret;
> >+}
> >+
> >+static int sdhci_berlin_remove(struct platform_device *pdev)
> >+{
> >+ struct sdhci_host *host = platform_get_drvdata(pdev);
> >+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >+
> >+ sdhci_pltfm_unregister(pdev);
> >+ clk_disable_unprepare(pltfm_host->clk);
> >+
> >+ return 0;
> >+}
> >+
> >+static struct platform_driver sdhci_berlin_driver = {
> >+ .driver = {
> >+ .name = "berlin-sdhci",
> >+ .owner = THIS_MODULE,
> >+ .pm = SDHCI_PLTFM_PMOPS,
> >+ .of_match_table = sdhci_berlin_of_match_table,
> >+ },
> >+ .probe = sdhci_berlin_probe,
> >+ .remove = sdhci_berlin_remove,
> >+};
> >+module_platform_driver(sdhci_berlin_driver);
> >+
> >+MODULE_DESCRIPTION("SDHCI driver for Marvell Berlin SoC");
> >+MODULE_AUTHOR("Antoine T?nart <[email protected]>");
> >+MODULE_LICENSE("GPL");
> >
>

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2014-04-18 06:08:58

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

Hi,

On Thu, 17 Apr 2014 06:33:59 -0700
Antoine Ténart <[email protected]> wrote:

> Sebastian,
>
> On Wed, Apr 16, 2014 at 04:26:06PM +0200, Sebastian Hesselbarth wrote:
> > On 04/16/2014 02:40 PM, Antoine Ténart wrote:
> > >Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> > >This controller supports 3 sockets.
> >
> > nit: Isn't it three controller with one socket each?
>
> It's a little bit confusing, there are 3 different register regions and
> I can see 3 separate data lines but, I quote: "The SD host controller
> supports three sockets".
>
> Maybe Jisheng can advise on that matter ?


It's three controller with one socket each ;)

Thanks

2014-04-18 07:21:03

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs

Hi all,

On Wed, Apr 16, 2014 at 02:40:08PM +0200, Antoine T?nart wrote:
> Add a Driver to support the SDHCI controller of the Marvell Berlin SoCs.
> This controller supports 3 sockets.

After talking a bit with Sebastian and Jisheng I decided to have a look on the
pxav3 driver. I gave it a try, it seemed to be compatible with the Berlin SDHCI
controller so I'll use this one instead of adding a Berlin specific one.

You all can forget this one, I'll send a v2 using the pxav3 driver.

Thanks!

Antoine

--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com