linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] ARM: berlin: add SDHCI support
@ 2014-04-16 12:40 Antoine Ténart
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 12:40 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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


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

* [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:40 [PATCH 0/4] ARM: berlin: add SDHCI support Antoine Ténart
@ 2014-04-16 12:40 ` Antoine Ténart
  2014-04-16 12:56   ` Joe Perches
                     ` (3 more replies)
  2014-04-16 12:40 ` [PATCH 2/4] Documentation: bindings: add the sdhci-berlin Antoine Ténart
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 12:40 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 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 <antoine.tenart@free-electrons.com>
+ *
+ * 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 <antoine.tenart@free-electrons.com>");
+MODULE_LICENSE("GPL");
-- 
1.8.3.2


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

* [PATCH 2/4] Documentation: bindings: add the sdhci-berlin
  2014-04-16 12:40 [PATCH 0/4] ARM: berlin: add SDHCI support Antoine Ténart
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
@ 2014-04-16 12:40 ` Antoine Ténart
  2014-04-16 12:40 ` [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q Antoine Ténart
  2014-04-16 12:40 ` [PATCH 4/4] ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP Antoine Ténart
  3 siblings, 0 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 12:40 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

Add the binding documentation for the Marvell Berlin SDHCI driver.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 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


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

* [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q
  2014-04-16 12:40 [PATCH 0/4] ARM: berlin: add SDHCI support Antoine Ténart
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
  2014-04-16 12:40 ` [PATCH 2/4] Documentation: bindings: add the sdhci-berlin Antoine Ténart
@ 2014-04-16 12:40 ` Antoine Ténart
  2014-04-16 13:09   ` Andrew Lunn
  2014-04-17  3:33   ` Jisheng Zhang
  2014-04-16 12:40 ` [PATCH 4/4] ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP Antoine Ténart
  3 siblings, 2 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 12:40 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 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


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

* [PATCH 4/4] ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP
  2014-04-16 12:40 [PATCH 0/4] ARM: berlin: add SDHCI support Antoine Ténart
                   ` (2 preceding siblings ...)
  2014-04-16 12:40 ` [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q Antoine Ténart
@ 2014-04-16 12:40 ` Antoine Ténart
  3 siblings, 0 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 12:40 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: Antoine Ténart, alexandre.belloni, zmxu, jszhang,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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 <antoine.tenart@free-electrons.com>
---
 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


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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
@ 2014-04-16 12:56   ` Joe Perches
  2014-04-16 13:09     ` Antoine Ténart
  2014-04-16 14:26   ` Sebastian Hesselbarth
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2014-04-16 12:56 UTC (permalink / raw)
  To: Antoine Ténart
  Cc: sebastian.hesselbarth, chris, anton, alexandre.belloni, zmxu,
	jszhang, linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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



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

* Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q
  2014-04-16 12:40 ` [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q Antoine Ténart
@ 2014-04-16 13:09   ` Andrew Lunn
  2014-04-16 13:23     ` Antoine Ténart
  2014-04-17  3:33   ` Jisheng Zhang
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2014-04-16 13:09 UTC (permalink / raw)
  To: Antoine Ténart
  Cc: sebastian.hesselbarth, chris, anton, zmxu, jszhang, devicetree,
	linux-mmc, linux-kernel, alexandre.belloni, linux-arm-kernel

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 <antoine.tenart@free-electrons.com>
> ---
>  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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:56   ` Joe Perches
@ 2014-04-16 13:09     ` Antoine Ténart
  0 siblings, 0 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 13:09 UTC (permalink / raw)
  To: Joe Perches
  Cc: sebastian.hesselbarth, chris, anton, alexandre.belloni, zmxu,
	jszhang, linux-arm-kernel, devicetree, linux-mmc, linux-kernel

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

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

* Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q
  2014-04-16 13:09   ` Andrew Lunn
@ 2014-04-16 13:23     ` Antoine Ténart
  0 siblings, 0 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-16 13:23 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: sebastian.hesselbarth, chris, anton, zmxu, jszhang, devicetree,
	linux-mmc, linux-kernel, alexandre.belloni, linux-arm-kernel

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 <antoine.tenart@free-electrons.com>
> > ---
> >  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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
  2014-04-16 12:56   ` Joe Perches
@ 2014-04-16 14:26   ` Sebastian Hesselbarth
  2014-04-17 13:33     ` Antoine Ténart
  2014-04-18  7:20   ` Antoine Ténart
  2014-05-09 15:55   ` Ben Dooks
  3 siblings, 1 reply; 16+ messages in thread
From: Sebastian Hesselbarth @ 2014-04-16 14:26 UTC (permalink / raw)
  To: Antoine Ténart, chris, anton
  Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree,
	linux-mmc, linux-kernel

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 <antoine.tenart@free-electrons.com>
> ---
>   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 <antoine.tenart@free-electrons.com>
> + *
> + * 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 <sebastian.hesselbarth@gmail.com>

> +
> +	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 <antoine.tenart@free-electrons.com>");
> +MODULE_LICENSE("GPL");
>


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

* Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q
  2014-04-16 12:40 ` [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q Antoine Ténart
  2014-04-16 13:09   ` Andrew Lunn
@ 2014-04-17  3:33   ` Jisheng Zhang
  2014-04-17  6:54     ` Sebastian Hesselbarth
  1 sibling, 1 reply; 16+ messages in thread
From: Jisheng Zhang @ 2014-04-17  3:33 UTC (permalink / raw)
  To: Antoine Ténart
  Cc: sebastian.hesselbarth, chris, anton, alexandre.belloni, Jimmy Xu,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

Hi Antoine,

On Wed, 16 Apr 2014 05:40:10 -0700
Antoine Ténart <antoine.tenart@free-electrons.com> wrote:

> Add the SDHCI nodes for the Marvell Berlin BG2Q, using the berlin-sdhci
> driver.
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
>  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

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

* Re: [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q
  2014-04-17  3:33   ` Jisheng Zhang
@ 2014-04-17  6:54     ` Sebastian Hesselbarth
  0 siblings, 0 replies; 16+ messages in thread
From: Sebastian Hesselbarth @ 2014-04-17  6:54 UTC (permalink / raw)
  To: Jisheng Zhang, Antoine Ténart
  Cc: chris, anton, alexandre.belloni, Jimmy Xu, linux-arm-kernel,
	devicetree, linux-mmc, linux-kernel

On 04/17/2014 05:33 AM, Jisheng Zhang wrote:
> On Wed, 16 Apr 2014 05:40:10 -0700
> Antoine Ténart <antoine.tenart@free-electrons.com> 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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 14:26   ` Sebastian Hesselbarth
@ 2014-04-17 13:33     ` Antoine Ténart
  2014-04-18  6:06       ` Jisheng Zhang
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Ténart @ 2014-04-17 13:33 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: chris, anton, alexandre.belloni, zmxu, jszhang, linux-arm-kernel,
	devicetree, linux-mmc, linux-kernel

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 <antoine.tenart@free-electrons.com>
> >---
> >  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 <antoine.tenart@free-electrons.com>
> >+ *
> >+ * 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 <sebastian.hesselbarth@gmail.com>
> 
> >+
> >+	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 <antoine.tenart@free-electrons.com>");
> >+MODULE_LICENSE("GPL");
> >
> 

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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-17 13:33     ` Antoine Ténart
@ 2014-04-18  6:06       ` Jisheng Zhang
  0 siblings, 0 replies; 16+ messages in thread
From: Jisheng Zhang @ 2014-04-18  6:06 UTC (permalink / raw)
  To: Antoine Ténart
  Cc: Sebastian Hesselbarth, chris, anton, alexandre.belloni, Jimmy Xu,
	linux-arm-kernel, devicetree, linux-mmc, linux-kernel

Hi,

On Thu, 17 Apr 2014 06:33:59 -0700
Antoine Ténart <antoine.tenart@free-electrons.com> 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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
  2014-04-16 12:56   ` Joe Perches
  2014-04-16 14:26   ` Sebastian Hesselbarth
@ 2014-04-18  7:20   ` Antoine Ténart
  2014-05-09 15:55   ` Ben Dooks
  3 siblings, 0 replies; 16+ messages in thread
From: Antoine Ténart @ 2014-04-18  7:20 UTC (permalink / raw)
  To: sebastian.hesselbarth, chris, anton
  Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree,
	linux-mmc, linux-kernel

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

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

* Re: [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs
  2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
                     ` (2 preceding siblings ...)
  2014-04-18  7:20   ` Antoine Ténart
@ 2014-05-09 15:55   ` Ben Dooks
  3 siblings, 0 replies; 16+ messages in thread
From: Ben Dooks @ 2014-05-09 15:55 UTC (permalink / raw)
  To: Antoine Ténart, sebastian.hesselbarth, chris, anton
  Cc: alexandre.belloni, zmxu, jszhang, linux-arm-kernel, devicetree,
	linux-mmc, linux-kernel

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

[snip]

> +
> +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,
> +	},
> +	{}

I think the hardware names should be used instead of the
quirks the hardware has (broken wp / broken adma)



-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

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

end of thread, other threads:[~2014-05-09 15:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-16 12:40 [PATCH 0/4] ARM: berlin: add SDHCI support Antoine Ténart
2014-04-16 12:40 ` [PATCH 1/4] mmc: sdhci: add a driver for Berlin SoCs Antoine Ténart
2014-04-16 12:56   ` Joe Perches
2014-04-16 13:09     ` Antoine Ténart
2014-04-16 14:26   ` Sebastian Hesselbarth
2014-04-17 13:33     ` Antoine Ténart
2014-04-18  6:06       ` Jisheng Zhang
2014-04-18  7:20   ` Antoine Ténart
2014-05-09 15:55   ` Ben Dooks
2014-04-16 12:40 ` [PATCH 2/4] Documentation: bindings: add the sdhci-berlin Antoine Ténart
2014-04-16 12:40 ` [PATCH 3/4] ARM: dts: berlin: add the SDHCI nodes for the BG2Q Antoine Ténart
2014-04-16 13:09   ` Andrew Lunn
2014-04-16 13:23     ` Antoine Ténart
2014-04-17  3:33   ` Jisheng Zhang
2014-04-17  6:54     ` Sebastian Hesselbarth
2014-04-16 12:40 ` [PATCH 4/4] ARM: dts: berlin: enable SD card reader and eMMC for the BG2Q DMP Antoine Ténart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).