Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
[...] >> >> I think a better approach is to use the new sdhci-caps* bindings to >> mask those caps that can't be trusted. And then use the generic mmc >> bindings for speed modes instead. >> >> That should be a safer approach, right!? > > Right. > > But, if I know the caps register bits 63-32 are all zero, > I need not explicitly specify sdhci-caps-mask, right? Maybe not. I don't have a strong opinion here, so l am fine with whatever you choose. Kind regards Uffe
Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
Hi Ulf, 2016-12-13 16:52 GMT+09:00 Ulf Hansson : > [...] > + +Optional properties: +For eMMC configuration, supported speed modes are not indicated by the SDHCI +Capabilities Register. Instead, the following properties should be specified +if supported. See mmc.txt for details. +- mmc-ddr-1_8v +- mmc-ddr-1_2v +- mmc-hs200-1_8v +- mmc-hs200-1_2v +- mmc-hs400-1_8v +- mmc-hs400-1_2v >>> >>> There's now a property to override SDHCI capabilities register. Maybe >>> you should use that instead? I'll defer to Ulf. >>> >> >> I did not know this new property. >> >> So, now we have two ways to specify MMC speed mode capabilities >> by only touching DT. > > Let me clarify a bit. > > The point with the new bindings is to be able to override *broken* > SDHCI caps bits. So, not only related to the speed modes. Right. So my approach ([1] Add MMC mode flags directly) basically sounds OK. >> >> [1] Add MMC mode flags directly, like I did. >> [2] Use "sdhci-caps-mask" and "sdhci-caps" >> >> >> The problem for [2] is that eMMC capabilities >> do not perfectly correspond to the SDHCI capabilities register. >> >> +- mmc-hs400-1_8v +- mmc-hs400-1_2v >> >> If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, >> we can use the bit63 of caps for specifying HS400. >> >> But, this is not defined in the SDHCI standard. >> #define SDHCI_SUPPORT_HS400 0x8000 /* Non-standard */ >> >> >> +- mmc-ddr-1_8v >> >> For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR >> from MMC_CAP_UHS_DDR50 (bit34 of caps) >> >> This is not supported in the current code, but >> if this is a good idea, I can send a patch. >> >> +- mmc-ddr-1_2v >> >> This does not have the corresponding bit, but >> 1.2V is not commonly used, so this is not a fatal problem. >> >> >> >> What I can do at most now, is to delete the >> Optional properties section entirely >> so users can choose [1] or [2] as they like. >> > > I think a better approach is to use the new sdhci-caps* bindings to > mask those caps that can't be trusted. And then use the generic mmc > bindings for speed modes instead. > > That should be a safer approach, right!? Right. But, if I know the caps register bits 63-32 are all zero, I need not explicitly specify sdhci-caps-mask, right? -- Best Regards Masahiro Yamada
Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
[...] >>> + >>> +Optional properties: >>> +For eMMC configuration, supported speed modes are not indicated by the >>> SDHCI >>> +Capabilities Register. Instead, the following properties should be >>> specified >>> +if supported. See mmc.txt for details. >>> +- mmc-ddr-1_8v >>> +- mmc-ddr-1_2v >>> +- mmc-hs200-1_8v >>> +- mmc-hs200-1_2v >>> +- mmc-hs400-1_8v >>> +- mmc-hs400-1_2v >> >> There's now a property to override SDHCI capabilities register. Maybe >> you should use that instead? I'll defer to Ulf. >> > > I did not know this new property. > > So, now we have two ways to specify MMC speed mode capabilities > by only touching DT. Let me clarify a bit. The point with the new bindings is to be able to override *broken* SDHCI caps bits. So, not only related to the speed modes. > > [1] Add MMC mode flags directly, like I did. > [2] Use "sdhci-caps-mask" and "sdhci-caps" > > > The problem for [2] is that eMMC capabilities > do not perfectly correspond to the SDHCI capabilities register. > > >>> +- mmc-hs400-1_8v >>> +- mmc-hs400-1_2v > > If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, > we can use the bit63 of caps for specifying HS400. > > But, this is not defined in the SDHCI standard. > #define SDHCI_SUPPORT_HS400 0x8000 /* Non-standard */ > > > >>> +- mmc-ddr-1_8v > > For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR > from MMC_CAP_UHS_DDR50 (bit34 of caps) > > This is not supported in the current code, but > if this is a good idea, I can send a patch. > > >>> +- mmc-ddr-1_2v > > This does not have the corresponding bit, but > 1.2V is not commonly used, so this is not a fatal problem. > > > > What I can do at most now, is to delete the > Optional properties section entirely > so users can choose [1] or [2] as they like. > I think a better approach is to use the new sdhci-caps* bindings to mask those caps that can't be trusted. And then use the generic mmc bindings for speed modes instead. That should be a safer approach, right!? Kind regards Uffe
Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
Hi Rob. 2016-12-13 2:14 GMT+09:00 Rob Herring : >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> new file mode 100644 >> index 000..750374f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt >> @@ -0,0 +1,30 @@ >> +* Cadence SD/SDIO/eMMC Host Controller >> + >> +Required properties: >> +- compatible: should be "cdns,sd4hc". > > Needs SoC specific compatible strings too. I remember you mentioned the vendor prefix stands for the SoC vendor, not the IP vendor. The compatible prefixed with the IP vendor is prepared for a fallback. I will add "socionext,sd4hc". >> +- reg: offset and length of the register set for the device. >> +- interrupts: a single interrupt specifier. >> +- clocks: phandle to the input clock. >> + >> +Optional properties: >> +For eMMC configuration, supported speed modes are not indicated by the SDHCI >> +Capabilities Register. Instead, the following properties should be >> specified >> +if supported. See mmc.txt for details. >> +- mmc-ddr-1_8v >> +- mmc-ddr-1_2v >> +- mmc-hs200-1_8v >> +- mmc-hs200-1_2v >> +- mmc-hs400-1_8v >> +- mmc-hs400-1_2v > > There's now a property to override SDHCI capabilities register. Maybe > you should use that instead? I'll defer to Ulf. > I did not know this new property. So, now we have two ways to specify MMC speed mode capabilities by only touching DT. [1] Add MMC mode flags directly, like I did. [2] Use "sdhci-caps-mask" and "sdhci-caps" The problem for [2] is that eMMC capabilities do not perfectly correspond to the SDHCI capabilities register. >> +- mmc-hs400-1_8v >> +- mmc-hs400-1_2v If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, we can use the bit63 of caps for specifying HS400. But, this is not defined in the SDHCI standard. #define SDHCI_SUPPORT_HS400 0x8000 /* Non-standard */ >> +- mmc-ddr-1_8v For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR from MMC_CAP_UHS_DDR50 (bit34 of caps) This is not supported in the current code, but if this is a good idea, I can send a patch. >> +- mmc-ddr-1_2v This does not have the corresponding bit, but 1.2V is not commonly used, so this is not a fatal problem. What I can do at most now, is to delete the Optional properties section entirely so users can choose [1] or [2] as they like. -- Best Regards Masahiro Yamada
Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
On Thu, Dec 08, 2016 at 09:50:55PM +0900, Masahiro Yamada wrote:
> Add a driver for the Cadence SD4HC SD/SDIO/eMMC Controller.
>
> For SD, it basically relies on the SDHCI standard code.
> For eMMC, this driver provides some callbacks to support the
> hardware part that is specific to this IP design.
>
> Signed-off-by: Masahiro Yamada
> Acked-by: Adrian Hunter
> ---
>
> Changes in v5:
> - Fix calculation of the center of the tuned window
>
> Changes in v4:
> - Override mmc_host_ops.execute_tuning instead of the
> .platform_execute_tuning implementation
>
> Changes in v3:
> - Remove unneeded explanation about HRS and SRS from DT binding;
> the offsets to HRS/SRS are fixed for this hardware and this is
> quite normal, like each hardware has a fixed register view except
> the register base. The detailed register map is what the driver
> cares about, so no need to explain it in the binding.
>
> Changes in v2:
> - Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"
>
> .../devicetree/bindings/mmc/sdhci-cadence.txt | 30 +++
> drivers/mmc/host/Kconfig | 11 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-cadence.c | 283
> +
> 4 files changed, 325 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> create mode 100644 drivers/mmc/host/sdhci-cadence.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> new file mode 100644
> index 000..750374f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> @@ -0,0 +1,30 @@
> +* Cadence SD/SDIO/eMMC Host Controller
> +
> +Required properties:
> +- compatible: should be "cdns,sd4hc".
Needs SoC specific compatible strings too.
> +- reg: offset and length of the register set for the device.
> +- interrupts: a single interrupt specifier.
> +- clocks: phandle to the input clock.
> +
> +Optional properties:
> +For eMMC configuration, supported speed modes are not indicated by the SDHCI
> +Capabilities Register. Instead, the following properties should be specified
> +if supported. See mmc.txt for details.
> +- mmc-ddr-1_8v
> +- mmc-ddr-1_2v
> +- mmc-hs200-1_8v
> +- mmc-hs200-1_2v
> +- mmc-hs400-1_8v
> +- mmc-hs400-1_2v
There's now a property to override SDHCI capabilities register. Maybe
you should use that instead? I'll defer to Ulf.
> +
> +Example:
> + emmc: sdhci@5a00 {
> + compatible = "cdns,sd4hc";
> + reg = <0x5a00 0x400>;
> + interrupts = <0 78 4>;
> + clocks = <&clk 4>;
> + bus-width = <8>;
> + mmc-ddr-1_8v;
> + mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + };
[PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
Add a driver for the Cadence SD4HC SD/SDIO/eMMC Controller.
For SD, it basically relies on the SDHCI standard code.
For eMMC, this driver provides some callbacks to support the
hardware part that is specific to this IP design.
Signed-off-by: Masahiro Yamada
Acked-by: Adrian Hunter
---
Changes in v5:
- Fix calculation of the center of the tuned window
Changes in v4:
- Override mmc_host_ops.execute_tuning instead of the
.platform_execute_tuning implementation
Changes in v3:
- Remove unneeded explanation about HRS and SRS from DT binding;
the offsets to HRS/SRS are fixed for this hardware and this is
quite normal, like each hardware has a fixed register view except
the register base. The detailed register map is what the driver
cares about, so no need to explain it in the binding.
Changes in v2:
- Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"
.../devicetree/bindings/mmc/sdhci-cadence.txt | 30 +++
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-cadence.c | 283 +
4 files changed, 325 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
create mode 100644 drivers/mmc/host/sdhci-cadence.c
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
new file mode 100644
index 000..750374f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
@@ -0,0 +1,30 @@
+* Cadence SD/SDIO/eMMC Host Controller
+
+Required properties:
+- compatible: should be "cdns,sd4hc".
+- reg: offset and length of the register set for the device.
+- interrupts: a single interrupt specifier.
+- clocks: phandle to the input clock.
+
+Optional properties:
+For eMMC configuration, supported speed modes are not indicated by the SDHCI
+Capabilities Register. Instead, the following properties should be specified
+if supported. See mmc.txt for details.
+- mmc-ddr-1_8v
+- mmc-ddr-1_2v
+- mmc-hs200-1_8v
+- mmc-hs200-1_2v
+- mmc-hs400-1_8v
+- mmc-hs400-1_2v
+
+Example:
+ emmc: sdhci@5a00 {
+ compatible = "cdns,sd4hc";
+ reg = <0x5a00 0x400>;
+ interrupts = <0 78 4>;
+ clocks = <&clk 4>;
+ bus-width = <8>;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index ab9181e..8ac1640 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -164,6 +164,17 @@ config MMC_SDHCI_OF_HLWD
If unsure, say N.
+config MMC_SDHCI_CADENCE
+ tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
+ depends on MMC_SDHCI_PLTFM
+ depends on OF
+ help
+ This selects the Cadence SD/SDIO/eMMC driver.
+
+ If you have a controller with this interface, say Y or M here.
+
+ If unsure, say N.
+
config MMC_SDHCI_CNS3XXX
tristate "SDHCI support on the Cavium Networks CNS3xxx SoC"
depends on ARCH_CNS3XXX
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e49a82a..55f7193 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_MMC_REALTEK_PCI) += rtsx_pci_sdmmc.o
obj-$(CONFIG_MMC_REALTEK_USB) += rtsx_usb_sdmmc.o
obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
+obj-$(CONFIG_MMC_SDHCI_CADENCE)+= sdhci-cadence.o
obj-$(CONFIG_MMC_SDHCI_CNS3XXX)+= sdhci-cns3xxx.o
obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
new file mode 100644
index 000..1501cfd
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -0,0 +1,283 @@
+/*
+ * Copyright (C) 2016 Socionext Inc.
+ * Author: Masahiro Yamada
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "sdhci-pltfm.h"
+
+/* HRS - Host Register Set (specific to Cadence) */
+#define SDHCI_CDNS_HRS04 0x10/* PHY access port */
+#define SDHCI_CDNS_HRS04_ACK BIT(26)
+#define SDHCI_CDNS_HRS04_RD BIT(25)
+#define SDHCI_CDNS_HRS04_WR BIT(24)
+#define SDHCI_CDNS_HRS04_RDATA_SHIFT 1
