linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 1/3] arm64: zynqmp: dt: Add support for setting SD tap delays
@ 2018-06-07 12:11 Manish Narani
  2018-06-07 12:11 ` [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details Manish Narani
  2018-06-07 12:11 ` [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT Manish Narani
  0 siblings, 2 replies; 11+ messages in thread
From: Manish Narani @ 2018-06-07 12:11 UTC (permalink / raw)
  To: robh+dt, mark.rutland, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, adrian.hunter, michal.simek, ulf.hansson
  Cc: Manish Narani

This patch adds support for setting SD tap delays from Device Tree.
Earlier, these tap values were made static via macros in the driver.
So changing the tap values in the device tree makes the driver free
from handling different tap values inside it.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index a091e6f..696aac8 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -491,6 +491,26 @@
                        interrupts = <0 48 4>;
                        reg = <0x0 0xff160000 0x0 0x1000>;
                        clock-names = "clk_xin", "clk_ahb";
+                       xlnx,itap_delay_sd_hsd = <0x15>;
+                       xlnx,otap_delay_sd_hsd = <0x5>;
+                       xlnx,itap_delay_sdr25 = <0x15>;
+                       xlnx,otap_delay_sdr25 = <0x5>;
+                       xlnx,itap_delay_sdr50 = <0>;
+                       xlnx,otap_delay_sdr50 = <0x3>;
+                       xlnx,itap_delay_sd_ddr50 = <0x3D>;
+                       xlnx,otap_delay_sd_ddr50 = <0x4>;
+                       xlnx,itap_delay_mmc_hsd = <0x15>;
+                       xlnx,otap_delay_mmc_hsd = <0x6>;
+                       xlnx,itap_delay_mmc_ddr50 = <0x12>;
+                       xlnx,otap_delay_mmc_ddr50 = <0x6>;
+                       xlnx,itap_delay_sdr104_b0 = <0>;
+                       xlnx,otap_delay_sdr104_b0 = <0x3>;
+                       xlnx,itap_delay_sdr104_b2 = <0>;
+                       xlnx,otap_delay_sdr104_b2 = <0x2>;
+                       xlnx,itap_delay_mmc_hs200_b0 = <0>;
+                       xlnx,otap_delay_mmc_hs200_b0 = <0x3>;
+                       xlnx,itap_delay_mmc_hs200_b2 = <0>;
+                       xlnx,otap_delay_mmc_hs200_b2 = <0x2>;
                };

                sdhci1: sdhci@ff170000 {
@@ -500,6 +520,26 @@
                        interrupts = <0 49 4>;
                        reg = <0x0 0xff170000 0x0 0x1000>;
                        clock-names = "clk_xin", "clk_ahb";
+                       xlnx,itap_delay_sd_hsd = <0x15>;
+                       xlnx,otap_delay_sd_hsd = <0x5>;
+                       xlnx,itap_delay_sdr25 = <0x15>;
+                       xlnx,otap_delay_sdr25 = <0x5>;
+                       xlnx,itap_delay_sdr50 = <0>;
+                       xlnx,otap_delay_sdr50 = <0x3>;
+                       xlnx,itap_delay_sd_ddr50 = <0x3D>;
+                       xlnx,otap_delay_sd_ddr50 = <0x4>;
+                       xlnx,itap_delay_mmc_hsd = <0x15>;
+                       xlnx,otap_delay_mmc_hsd = <0x6>;
+                       xlnx,itap_delay_mmc_ddr50 = <0x12>;
+                       xlnx,otap_delay_mmc_ddr50 = <0x6>;
+                       xlnx,itap_delay_sdr104_b0 = <0>;
+                       xlnx,otap_delay_sdr104_b0 = <0x3>;
+                       xlnx,itap_delay_sdr104_b2 = <0>;
+                       xlnx,otap_delay_sdr104_b2 = <0x2>;
+                       xlnx,itap_delay_mmc_hs200_b0 = <0>;
+                       xlnx,otap_delay_mmc_hs200_b0 = <0x3>;
+                       xlnx,itap_delay_mmc_hs200_b2 = <0>;
+                       xlnx,otap_delay_mmc_hs200_b2 = <0x2>;
                };

                smmu: smmu@fd800000 {
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

* [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details
  2018-06-07 12:11 [RFC PATCH 1/3] arm64: zynqmp: dt: Add support for setting SD tap delays Manish Narani
@ 2018-06-07 12:11 ` Manish Narani
  2018-06-07 12:47   ` Mark Rutland
  2018-06-07 12:11 ` [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT Manish Narani
  1 sibling, 1 reply; 11+ messages in thread
From: Manish Narani @ 2018-06-07 12:11 UTC (permalink / raw)
  To: robh+dt, mark.rutland, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, adrian.hunter, michal.simek, ulf.hansson
  Cc: Manish Narani

This patch adds details of SD tap value properties in device tree.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 .../devicetree/bindings/mmc/arasan,sdhci.txt       | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 60481bf..0e08877 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -15,6 +15,8 @@ Required Properties:
     - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
     - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
       For this device it is strongly suggested to include arasan,soc-ctl-syscon.
+    - "xlnx,zynqmp-8.9a": Xilinx ZynqMP Arasan SDHCI 8.9a PHY
+      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
   - reg: From mmc bindings: Register location and length.
   - clocks: From clock bindings: Handles to clock inputs.
   - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
@@ -26,6 +28,30 @@ Required Properties for "arasan,sdhci-5.1":
   - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
   - phy-names:  MUST be "phy_arasan".

+Required Properties for "xlnx,zynqmp-8.9a":
+  - xlnx,mio_bank: The value will be 0/1/2 depending on MIO bank selection.
+  - xlnx,device_id: Unique Id of the device, value will be 0/1.
+  - xlnx,itap_delay_sd_hsd: Input Tap Delay for SD HS.
+  - xlnx,itap_delay_sdr25: Input Tap Delay for SDR25.
+  - xlnx,itap_delay_sdr50: Input Tap Delay for SDR50.
+  - xlnx,itap_delay_sdr104_b0: Input Tap Delay for SDR104.
+  - xlnx,itap_delay_sdr104_b2: Input Tap Delay for SDR104.
+  - xlnx,itap_delay_sd_ddr50: Input Tap Delay for SD DDR50.
+  - xlnx,itap_delay_mmc_hsd: Input Tap Delay for MMC HS.
+  - xlnx,itap_delay_mmc_ddr50: Input Tap Delay for MMC DDR50.
+  - xlnx,itap_delay_mmc_hs200_b0: Input Tap Delay for MMC HS200.
+  - xlnx,itap_delay_mmc_hs200_b2: Input Tap Delay for MMC HS200.
+  - xlnx,otap_delay_sd_hsd: Output Tap Delay for SD HS.
+  - xlnx,otap_delay_sdr25: Output Tap Delay for SDR25.
+  - xlnx,otap_delay_sdr50: Output Tap Delay for SDR50.
+  - xlnx,otap_delay_sdr104_b0: Output Tap Delay for SDR104.
+  - xlnx,otap_delay_sdr104_b2: Output Tap Delay for SDR104.
+  - xlnx,otap_delay_sd_ddr50: Output Tap Delay for DDR50.
+  - xlnx,otap_delay_mmc_hsd: Output Tap Delay for MMC HS.
+  - xlnx,otap_delay_mmc_ddr50: Output Tap Delay for MMC DDR50.
+  - xlnx,otap_delay_mmc_hs200_b0: Output Tap Delay for MMC HS200.
+  - xlnx,otap_delay_mmc_hs200_b2: Output Tap Delay for MMC HS200.
+
 Optional Properties:
   - arasan,soc-ctl-syscon: A phandle to a syscon device (see ../mfd/syscon.txt)
     used to access core corecfg registers.  Offsets of registers in this
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

* [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-07 12:11 [RFC PATCH 1/3] arm64: zynqmp: dt: Add support for setting SD tap delays Manish Narani
  2018-06-07 12:11 ` [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details Manish Narani
@ 2018-06-07 12:11 ` Manish Narani
  2018-06-14  5:38   ` Manish Narani
  2018-07-10  8:31   ` Ulf Hansson
  1 sibling, 2 replies; 11+ messages in thread
From: Manish Narani @ 2018-06-07 12:11 UTC (permalink / raw)
  To: robh+dt, mark.rutland, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, adrian.hunter, michal.simek, ulf.hansson
  Cc: Manish Narani

This patch adds support for reading Tap Delay values from Device Tree
and write them via eemi calls. The macros containing these tap delay
values are removed from the driver.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 131 +++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index e3332a5..fc0fd01 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -36,6 +36,8 @@

 #define PHY_CLK_TOO_SLOW_HZ            400000

+#define MMC_BANK2              0x2
+
 /*
  * On some SoCs the syscon area has a feature where the upper 16-bits of
  * each 32-bit register act as a write mask for the lower 16-bits.  This allows
@@ -90,6 +92,10 @@ struct sdhci_arasan_data {
        struct sdhci_host *host;
        struct clk      *clk_ahb;
        struct phy      *phy;
+       u32 mio_bank;
+       u32 device_id;
+       u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
+       u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
        bool            is_phy_on;

        bool            has_cqe;
@@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct sdhci_host *host,
        return ret;
 }

+/**
+ * arasan_zynqmp_set_tap_delay - Program the tap delays.
+ * @deviceid:          Unique Id of device
+ * @itap_delay:                Input Tap Delay
+ * @oitap_delay:       Output Tap Delay
+ */
+static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay, u8 otap_delay)
+{
+       const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+       u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
+
+       if (!eemi_ops || !eemi_ops->ioctl)
+               return;
+
+       if (itap_delay)
+               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
+                               PM_TAPDELAY_INPUT, itap_delay, NULL);
+
+       if (otap_delay)
+               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
+                               PM_TAPDELAY_OUTPUT, otap_delay, NULL);
+}
+
 static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
 {
        struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
        struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
        bool ctrl_phy = false;
+       u8 itap_delay;
+       u8 otap_delay;

        if (!IS_ERR(sdhci_arasan->phy)) {
                if (!sdhci_arasan->is_phy_on && clock <= PHY_CLK_TOO_SLOW_HZ) {
@@ -200,6 +231,16 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
                }
        }

+       if (host->version >= SDHCI_SPEC_300) {
+               if ((host->timing != MMC_TIMING_LEGACY) &&
+                       (host->timing != MMC_TIMING_UHS_SDR12)) {
+                       itap_delay = sdhci_arasan->itapdly[host->timing];
+                       otap_delay = sdhci_arasan->otapdly[host->timing];
+                       arasan_zynqmp_set_tap_delay(sdhci_arasan->device_id,
+                                                   itap_delay, otap_delay);
+               }
+       }
+
        if (ctrl_phy && sdhci_arasan->is_phy_on) {
                phy_power_off(sdhci_arasan->phy);
                sdhci_arasan->is_phy_on = false;
@@ -456,6 +497,7 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
        { .compatible = "arasan,sdhci-8.9a" },
        { .compatible = "arasan,sdhci-5.1" },
        { .compatible = "arasan,sdhci-4.9a" },
+       { .compatible = "xlnx,zynqmp-8.9a" },

        { /* sentinel */ }
 };
@@ -641,6 +683,74 @@ static void sdhci_arasan_unregister_sdclk(struct device *dev)
        of_clk_del_provider(dev->of_node);
 }

+/**
+ * arasan_zynqmp_dt_parse_tap_delays - Read Tap Delay values from DT
+ *
+ * Called at initialization to parse the values of Tap Delays.
+ *
+ * @dev:               Pointer to our struct device.
+ */
+static void arasan_zynqmp_dt_parse_tap_delays(struct device *dev)
+{
+       struct platform_device *pdev = to_platform_device(dev);
+       struct sdhci_host *host = platform_get_drvdata(pdev);
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+       struct device_node *np = dev->of_node;
+
+       of_property_read_u32(np, "xlnx,itap_delay_sd_hsd",
+                            &sdhci_arasan->itapdly[MMC_TIMING_SD_HS]);
+       of_property_read_u32(np, "xlnx,otap_delay_sd_hsd",
+                            &sdhci_arasan->otapdly[MMC_TIMING_SD_HS]);
+       of_property_read_u32(np, "xlnx,itap_delay_sdr25",
+                            &sdhci_arasan->itapdly[MMC_TIMING_UHS_SDR25]);
+       of_property_read_u32(np, "xlnx,otap_delay_sdr25",
+                            &sdhci_arasan->otapdly[MMC_TIMING_UHS_SDR25]);
+       of_property_read_u32(np, "xlnx,itap_delay_sdr50",
+                            &sdhci_arasan->itapdly[MMC_TIMING_UHS_SDR50]);
+       of_property_read_u32(np, "xlnx,otap_delay_sdr50",
+                            &sdhci_arasan->otapdly[MMC_TIMING_UHS_SDR50]);
+       of_property_read_u32(np, "xlnx,itap_delay_sd_ddr50",
+                            &sdhci_arasan->itapdly[MMC_TIMING_UHS_DDR50]);
+       of_property_read_u32(np, "xlnx,otap_delay_sd_ddr50",
+                            &sdhci_arasan->otapdly[MMC_TIMING_UHS_DDR50]);
+       of_property_read_u32(np, "xlnx,itap_delay_mmc_hsd",
+                            &sdhci_arasan->itapdly[MMC_TIMING_MMC_HS]);
+       of_property_read_u32(np, "xlnx,otap_delay_mmc_hsd",
+                            &sdhci_arasan->otapdly[MMC_TIMING_MMC_HS]);
+       of_property_read_u32(np, "xlnx,itap_delay_mmc_ddr50",
+                            &sdhci_arasan->itapdly[MMC_TIMING_MMC_DDR52]);
+       of_property_read_u32(np, "xlnx,otap_delay_mmc_ddr50",
+                            &sdhci_arasan->otapdly[MMC_TIMING_MMC_DDR52]);
+       if (sdhci_arasan->mio_bank == MMC_BANK2) {
+               of_property_read_u32(np,
+                                    "xlnx,itap_delay_sdr104_b2",
+                               &sdhci_arasan->itapdly[MMC_TIMING_UHS_SDR104]);
+               of_property_read_u32(np,
+                                    "xlnx,otap_delay_sdr104_b2",
+                               &sdhci_arasan->otapdly[MMC_TIMING_UHS_SDR104]);
+               of_property_read_u32(np,
+                                    "xlnx,itap_delay_mmc_hs200_b2",
+                               &sdhci_arasan->itapdly[MMC_TIMING_MMC_HS200]);
+               of_property_read_u32(np,
+                                    "xlnx,otap_delay_mmc_hs200_b2",
+                               &sdhci_arasan->otapdly[MMC_TIMING_MMC_HS200]);
+       } else {
+               of_property_read_u32(np,
+                                    "xlnx,itap_delay_sdr104_b0",
+                               &sdhci_arasan->itapdly[MMC_TIMING_UHS_SDR104]);
+               of_property_read_u32(np,
+                                    "xlnx,otap_delay_sdr104_b0",
+                               &sdhci_arasan->otapdly[MMC_TIMING_UHS_SDR104]);
+               of_property_read_u32(np,
+                                    "xlnx,itap_delay_mmc_hs200_b0",
+                               &sdhci_arasan->itapdly[MMC_TIMING_MMC_HS200]);
+               of_property_read_u32(np,
+                                    "xlnx,otap_delay_mmc_hs200_b0",
+                               &sdhci_arasan->otapdly[MMC_TIMING_MMC_HS200]);
+       }
+}
+
 static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)
 {
        struct sdhci_host *host = sdhci_arasan->host;
@@ -776,6 +886,27 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
                goto unreg_clk;
        }

+       if (of_device_is_compatible(pdev->dev.of_node,
+                                   "xlnx,zynqmp-8.9a")) {
+               ret = of_property_read_u32(pdev->dev.of_node,
+                                          "xlnx,mio_bank",
+                                          &sdhci_arasan->mio_bank);
+               if (ret < 0) {
+                       dev_err(&pdev->dev,
+                               "\"xlnx,mio_bank \" property is missing.\n");
+                       goto clk_disable_all;
+               }
+               ret = of_property_read_u32(pdev->dev.of_node,
+                                          "xlnx,device_id",
+                                          &sdhci_arasan->device_id);
+               if (ret < 0) {
+                       dev_err(&pdev->dev,
+                               "\"xlnx,device_id \" property is missing.\n");
+                       goto clk_disable_all;
+               }
+               arasan_zynqmp_dt_parse_tap_delays(&pdev->dev);
+       }
+
        sdhci_arasan->phy = ERR_PTR(-ENODEV);
        if (of_device_is_compatible(pdev->dev.of_node,
                                    "arasan,sdhci-5.1")) {
--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

* Re: [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details
  2018-06-07 12:11 ` [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details Manish Narani
@ 2018-06-07 12:47   ` Mark Rutland
  2018-06-21 12:41     ` Manish Narani
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2018-06-07 12:47 UTC (permalink / raw)
  To: Manish Narani
  Cc: robh+dt, catalin.marinas, will.deacon, mdf, stefan.krsmanovic,
	linux-arm-kernel, linux-kernel, linux-mmc, devicetree,
	adrian.hunter, michal.simek, ulf.hansson

On Thu, Jun 07, 2018 at 05:41:39PM +0530, Manish Narani wrote:
> This patch adds details of SD tap value properties in device tree.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  .../devicetree/bindings/mmc/arasan,sdhci.txt       | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 60481bf..0e08877 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -15,6 +15,8 @@ Required Properties:
>      - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
>      - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
>        For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> +    - "xlnx,zynqmp-8.9a": Xilinx ZynqMP Arasan SDHCI 8.9a PHY
> +      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
>    - reg: From mmc bindings: Register location and length.
>    - clocks: From clock bindings: Handles to clock inputs.
>    - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> @@ -26,6 +28,30 @@ Required Properties for "arasan,sdhci-5.1":
>    - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
>    - phy-names:  MUST be "phy_arasan".
> 
> +Required Properties for "xlnx,zynqmp-8.9a":
> +  - xlnx,mio_bank: The value will be 0/1/2 depending on MIO bank selection.

For all of these properties, please s/_/-/, folowing the usual property
name conventions.

It's not clear to me why you need this property. The code in patch 3
only seems to use this to determine which properties to read, choosing
between <prop>_b0 or <prop>_b2. I don't see why you dont have the base
<prop> alone...

Is this a HW detail, or configuration that you prefer?

> +  - xlnx,device_id: Unique Id of the device, value will be 0/1.

What's this used for?

> +  - xlnx,itap_delay_sd_hsd: Input Tap Delay for SD HS.

What unit at hese delays in?

Please follow the conventions in
Documentation/devicetree/bindings/property-units.txt.

> +  - xlnx,itap_delay_sdr25: Input Tap Delay for SDR25.
> +  - xlnx,itap_delay_sdr50: Input Tap Delay for SDR50.
> +  - xlnx,itap_delay_sdr104_b0: Input Tap Delay for SDR104.
> +  - xlnx,itap_delay_sdr104_b2: Input Tap Delay for SDR104.

As above, Given you have to specify the bank, I don't see why you need
multiple properties.

Thanks,
Mark.

> +  - xlnx,itap_delay_sd_ddr50: Input Tap Delay for SD DDR50.
> +  - xlnx,itap_delay_mmc_hsd: Input Tap Delay for MMC HS.
> +  - xlnx,itap_delay_mmc_ddr50: Input Tap Delay for MMC DDR50.
> +  - xlnx,itap_delay_mmc_hs200_b0: Input Tap Delay for MMC HS200.
> +  - xlnx,itap_delay_mmc_hs200_b2: Input Tap Delay for MMC HS200.
> +  - xlnx,otap_delay_sd_hsd: Output Tap Delay for SD HS.
> +  - xlnx,otap_delay_sdr25: Output Tap Delay for SDR25.
> +  - xlnx,otap_delay_sdr50: Output Tap Delay for SDR50.
> +  - xlnx,otap_delay_sdr104_b0: Output Tap Delay for SDR104.
> +  - xlnx,otap_delay_sdr104_b2: Output Tap Delay for SDR104.
> +  - xlnx,otap_delay_sd_ddr50: Output Tap Delay for DDR50.
> +  - xlnx,otap_delay_mmc_hsd: Output Tap Delay for MMC HS.
> +  - xlnx,otap_delay_mmc_ddr50: Output Tap Delay for MMC DDR50.
> +  - xlnx,otap_delay_mmc_hs200_b0: Output Tap Delay for MMC HS200.
> +  - xlnx,otap_delay_mmc_hs200_b2: Output Tap Delay for MMC HS200.
> +
>  Optional Properties:
>    - arasan,soc-ctl-syscon: A phandle to a syscon device (see ../mfd/syscon.txt)
>      used to access core corecfg registers.  Offsets of registers in this
> --
> 2.7.4
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

* RE: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-07 12:11 ` [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT Manish Narani
@ 2018-06-14  5:38   ` Manish Narani
  2018-06-19 11:38     ` Adrian Hunter
  2018-07-10  8:31   ` Ulf Hansson
  1 sibling, 1 reply; 11+ messages in thread
From: Manish Narani @ 2018-06-14  5:38 UTC (permalink / raw)
  To: Manish Narani, robh+dt, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, adrian.hunter, michal.simek, ulf.hansson
  Cc: Srinivas Goud, Anirudha Sarangi

Ping for RFC

> -----Original Message-----
> From: Manish Narani [mailto:manish.narani@xilinx.com]
> Sent: Thursday, June 7, 2018 5:42 PM
> To: robh+dt@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com;
> will.deacon@arm.com; mdf@kernel.org; stefan.krsmanovic@aggios.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mmc@vger.kernel.org; devicetree@vger.kernel.org;
> adrian.hunter@intel.com; michal.simek@xilinx.com; ulf.hansson@linaro.org
> Cc: Manish Narani <MNARANI@xilinx.com>
> Subject: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values
> from DT
> 
> This patch adds support for reading Tap Delay values from Device Tree and
> write them via eemi calls. The macros containing these tap delay values are
> removed from the driver.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 131
> +++++++++++++++++++++++++++++++++++++
>  1 file changed, 131 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-
> of-arasan.c
> index e3332a5..fc0fd01 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -36,6 +36,8 @@
> 
>  #define PHY_CLK_TOO_SLOW_HZ		400000
> 
> +#define MMC_BANK2		0x2
> +
>  /*
>   * On some SoCs the syscon area has a feature where the upper 16-bits of
>   * each 32-bit register act as a write mask for the lower 16-bits.  This allows
> @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
>  	struct sdhci_host *host;
>  	struct clk	*clk_ahb;
>  	struct phy	*phy;
> +	u32 mio_bank;
> +	u32 device_id;
> +	u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
> +	u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
>  	bool		is_phy_on;
> 
>  	bool		has_cqe;
> @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct
> sdhci_host *host,
>  	return ret;
>  }
> 
> +/**
> + * arasan_zynqmp_set_tap_delay - Program the tap delays.
> + * @deviceid:		Unique Id of device
> + * @itap_delay:		Input Tap Delay
> + * @oitap_delay:	Output Tap Delay
> + */
> +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay, u8
> +otap_delay) {
> +	const struct zynqmp_eemi_ops *eemi_ops =
> zynqmp_pm_get_eemi_ops();
> +	u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
> +
> +	if (!eemi_ops || !eemi_ops->ioctl)
> +		return;
> +
> +	if (itap_delay)
> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> +				PM_TAPDELAY_INPUT, itap_delay, NULL);
> +
> +	if (otap_delay)
> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> +				PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> }
> +
>  static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int
> clock)  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_arasan_data *sdhci_arasan =
> sdhci_pltfm_priv(pltfm_host);
>  	bool ctrl_phy = false;
> +	u8 itap_delay;
> +	u8 otap_delay;
> 
>  	if (!IS_ERR(sdhci_arasan->phy)) {
>  		if (!sdhci_arasan->is_phy_on && clock <=
> PHY_CLK_TOO_SLOW_HZ) { @@ -200,6 +231,16 @@ static void
> sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
>  		}
>  	}
> 
> +	if (host->version >= SDHCI_SPEC_300) {
> +		if ((host->timing != MMC_TIMING_LEGACY) &&
> +			(host->timing != MMC_TIMING_UHS_SDR12)) {
> +			itap_delay = sdhci_arasan->itapdly[host->timing];
> +			otap_delay = sdhci_arasan->otapdly[host->timing];
> +			arasan_zynqmp_set_tap_delay(sdhci_arasan-
> >device_id,
> +						    itap_delay, otap_delay);
> +		}
> +	}
> +
>  	if (ctrl_phy && sdhci_arasan->is_phy_on) {
>  		phy_power_off(sdhci_arasan->phy);
>  		sdhci_arasan->is_phy_on = false;
> @@ -456,6 +497,7 @@ static const struct of_device_id
> sdhci_arasan_of_match[] = {
>  	{ .compatible = "arasan,sdhci-8.9a" },
>  	{ .compatible = "arasan,sdhci-5.1" },
>  	{ .compatible = "arasan,sdhci-4.9a" },
> +	{ .compatible = "xlnx,zynqmp-8.9a" },
> 
>  	{ /* sentinel */ }
>  };
> @@ -641,6 +683,74 @@ static void sdhci_arasan_unregister_sdclk(struct
> device *dev)
>  	of_clk_del_provider(dev->of_node);
>  }
> 
> +/**
> + * arasan_zynqmp_dt_parse_tap_delays - Read Tap Delay values from DT
> + *
> + * Called at initialization to parse the values of Tap Delays.
> + *
> + * @dev:		Pointer to our struct device.
> + */
> +static void arasan_zynqmp_dt_parse_tap_delays(struct device *dev) {
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_arasan_data *sdhci_arasan =
> sdhci_pltfm_priv(pltfm_host);
> +	struct device_node *np = dev->of_node;
> +
> +	of_property_read_u32(np, "xlnx,itap_delay_sd_hsd",
> +			     &sdhci_arasan->itapdly[MMC_TIMING_SD_HS]);
> +	of_property_read_u32(np, "xlnx,otap_delay_sd_hsd",
> +			     &sdhci_arasan->otapdly[MMC_TIMING_SD_HS]);
> +	of_property_read_u32(np, "xlnx,itap_delay_sdr25",
> +			     &sdhci_arasan-
> >itapdly[MMC_TIMING_UHS_SDR25]);
> +	of_property_read_u32(np, "xlnx,otap_delay_sdr25",
> +			     &sdhci_arasan-
> >otapdly[MMC_TIMING_UHS_SDR25]);
> +	of_property_read_u32(np, "xlnx,itap_delay_sdr50",
> +			     &sdhci_arasan-
> >itapdly[MMC_TIMING_UHS_SDR50]);
> +	of_property_read_u32(np, "xlnx,otap_delay_sdr50",
> +			     &sdhci_arasan-
> >otapdly[MMC_TIMING_UHS_SDR50]);
> +	of_property_read_u32(np, "xlnx,itap_delay_sd_ddr50",
> +			     &sdhci_arasan-
> >itapdly[MMC_TIMING_UHS_DDR50]);
> +	of_property_read_u32(np, "xlnx,otap_delay_sd_ddr50",
> +			     &sdhci_arasan-
> >otapdly[MMC_TIMING_UHS_DDR50]);
> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_hsd",
> +			     &sdhci_arasan-
> >itapdly[MMC_TIMING_MMC_HS]);
> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_hsd",
> +			     &sdhci_arasan-
> >otapdly[MMC_TIMING_MMC_HS]);
> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_ddr50",
> +			     &sdhci_arasan-
> >itapdly[MMC_TIMING_MMC_DDR52]);
> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_ddr50",
> +			     &sdhci_arasan-
> >otapdly[MMC_TIMING_MMC_DDR52]);
> +	if (sdhci_arasan->mio_bank == MMC_BANK2) {
> +		of_property_read_u32(np,
> +				     "xlnx,itap_delay_sdr104_b2",
> +				&sdhci_arasan-
> >itapdly[MMC_TIMING_UHS_SDR104]);
> +		of_property_read_u32(np,
> +				     "xlnx,otap_delay_sdr104_b2",
> +				&sdhci_arasan-
> >otapdly[MMC_TIMING_UHS_SDR104]);
> +		of_property_read_u32(np,
> +				     "xlnx,itap_delay_mmc_hs200_b2",
> +				&sdhci_arasan-
> >itapdly[MMC_TIMING_MMC_HS200]);
> +		of_property_read_u32(np,
> +				     "xlnx,otap_delay_mmc_hs200_b2",
> +				&sdhci_arasan-
> >otapdly[MMC_TIMING_MMC_HS200]);
> +	} else {
> +		of_property_read_u32(np,
> +				     "xlnx,itap_delay_sdr104_b0",
> +				&sdhci_arasan-
> >itapdly[MMC_TIMING_UHS_SDR104]);
> +		of_property_read_u32(np,
> +				     "xlnx,otap_delay_sdr104_b0",
> +				&sdhci_arasan-
> >otapdly[MMC_TIMING_UHS_SDR104]);
> +		of_property_read_u32(np,
> +				     "xlnx,itap_delay_mmc_hs200_b0",
> +				&sdhci_arasan-
> >itapdly[MMC_TIMING_MMC_HS200]);
> +		of_property_read_u32(np,
> +				     "xlnx,otap_delay_mmc_hs200_b0",
> +				&sdhci_arasan-
> >otapdly[MMC_TIMING_MMC_HS200]);
> +	}
> +}
> +
>  static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)  {
>  	struct sdhci_host *host = sdhci_arasan->host; @@ -776,6 +886,27
> @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>  		goto unreg_clk;
>  	}
> 
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "xlnx,zynqmp-8.9a")) {
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "xlnx,mio_bank",
> +					   &sdhci_arasan->mio_bank);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev,
> +				"\"xlnx,mio_bank \" property is missing.\n");
> +			goto clk_disable_all;
> +		}
> +		ret = of_property_read_u32(pdev->dev.of_node,
> +					   "xlnx,device_id",
> +					   &sdhci_arasan->device_id);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev,
> +				"\"xlnx,device_id \" property is missing.\n");
> +			goto clk_disable_all;
> +		}
> +		arasan_zynqmp_dt_parse_tap_delays(&pdev->dev);
> +	}
> +
>  	sdhci_arasan->phy = ERR_PTR(-ENODEV);
>  	if (of_device_is_compatible(pdev->dev.of_node,
>  				    "arasan,sdhci-5.1")) {
> --
> 2.7.4


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

* Re: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-14  5:38   ` Manish Narani
@ 2018-06-19 11:38     ` Adrian Hunter
  2018-06-21 12:54       ` Manish Narani
  2018-07-09 10:38       ` Manish Narani
  0 siblings, 2 replies; 11+ messages in thread
From: Adrian Hunter @ 2018-06-19 11:38 UTC (permalink / raw)
  To: Manish Narani, robh+dt, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, michal.simek, ulf.hansson
  Cc: Srinivas Goud, Anirudha Sarangi

On 14/06/18 08:38, Manish Narani wrote:
> Ping for RFC

What is eemi?  Why aren't there patches for that?

> 
>> -----Original Message-----
>> From: Manish Narani [mailto:manish.narani@xilinx.com]
>> Sent: Thursday, June 7, 2018 5:42 PM
>> To: robh+dt@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com;
>> will.deacon@arm.com; mdf@kernel.org; stefan.krsmanovic@aggios.com;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>> mmc@vger.kernel.org; devicetree@vger.kernel.org;
>> adrian.hunter@intel.com; michal.simek@xilinx.com; ulf.hansson@linaro.org
>> Cc: Manish Narani <MNARANI@xilinx.com>
>> Subject: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values
>> from DT
>>
>> This patch adds support for reading Tap Delay values from Device Tree and
>> write them via eemi calls. The macros containing these tap delay values are
>> removed from the driver.
>>
>> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
>> ---
>>  drivers/mmc/host/sdhci-of-arasan.c | 131
>> +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 131 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-
>> of-arasan.c
>> index e3332a5..fc0fd01 100644
>> --- a/drivers/mmc/host/sdhci-of-arasan.c
>> +++ b/drivers/mmc/host/sdhci-of-arasan.c
>> @@ -36,6 +36,8 @@
>>
>>  #define PHY_CLK_TOO_SLOW_HZ		400000
>>
>> +#define MMC_BANK2		0x2
>> +
>>  /*
>>   * On some SoCs the syscon area has a feature where the upper 16-bits of
>>   * each 32-bit register act as a write mask for the lower 16-bits.  This allows
>> @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
>>  	struct sdhci_host *host;
>>  	struct clk	*clk_ahb;
>>  	struct phy	*phy;
>> +	u32 mio_bank;
>> +	u32 device_id;
>> +	u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
>> +	u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
>>  	bool		is_phy_on;
>>
>>  	bool		has_cqe;
>> @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct
>> sdhci_host *host,
>>  	return ret;
>>  }
>>
>> +/**
>> + * arasan_zynqmp_set_tap_delay - Program the tap delays.
>> + * @deviceid:		Unique Id of device
>> + * @itap_delay:		Input Tap Delay
>> + * @oitap_delay:	Output Tap Delay
>> + */
>> +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay, u8
>> +otap_delay) {
>> +	const struct zynqmp_eemi_ops *eemi_ops =
>> zynqmp_pm_get_eemi_ops();
>> +	u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
>> +
>> +	if (!eemi_ops || !eemi_ops->ioctl)
>> +		return;
>> +
>> +	if (itap_delay)
>> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
>> +				PM_TAPDELAY_INPUT, itap_delay, NULL);
>> +
>> +	if (otap_delay)
>> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
>> +				PM_TAPDELAY_OUTPUT, otap_delay, NULL);
>> }
>> +
>>  static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int
>> clock)  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>  	struct sdhci_arasan_data *sdhci_arasan =
>> sdhci_pltfm_priv(pltfm_host);
>>  	bool ctrl_phy = false;
>> +	u8 itap_delay;
>> +	u8 otap_delay;
>>
>>  	if (!IS_ERR(sdhci_arasan->phy)) {
>>  		if (!sdhci_arasan->is_phy_on && clock <=
>> PHY_CLK_TOO_SLOW_HZ) { @@ -200,6 +231,16 @@ static void
>> sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
>>  		}
>>  	}
>>
>> +	if (host->version >= SDHCI_SPEC_300) {
>> +		if ((host->timing != MMC_TIMING_LEGACY) &&
>> +			(host->timing != MMC_TIMING_UHS_SDR12)) {
>> +			itap_delay = sdhci_arasan->itapdly[host->timing];
>> +			otap_delay = sdhci_arasan->otapdly[host->timing];
>> +			arasan_zynqmp_set_tap_delay(sdhci_arasan-
>>> device_id,
>> +						    itap_delay, otap_delay);
>> +		}
>> +	}
>> +
>>  	if (ctrl_phy && sdhci_arasan->is_phy_on) {
>>  		phy_power_off(sdhci_arasan->phy);
>>  		sdhci_arasan->is_phy_on = false;
>> @@ -456,6 +497,7 @@ static const struct of_device_id
>> sdhci_arasan_of_match[] = {
>>  	{ .compatible = "arasan,sdhci-8.9a" },
>>  	{ .compatible = "arasan,sdhci-5.1" },
>>  	{ .compatible = "arasan,sdhci-4.9a" },
>> +	{ .compatible = "xlnx,zynqmp-8.9a" },
>>
>>  	{ /* sentinel */ }
>>  };
>> @@ -641,6 +683,74 @@ static void sdhci_arasan_unregister_sdclk(struct
>> device *dev)
>>  	of_clk_del_provider(dev->of_node);
>>  }
>>
>> +/**
>> + * arasan_zynqmp_dt_parse_tap_delays - Read Tap Delay values from DT
>> + *
>> + * Called at initialization to parse the values of Tap Delays.
>> + *
>> + * @dev:		Pointer to our struct device.
>> + */
>> +static void arasan_zynqmp_dt_parse_tap_delays(struct device *dev) {
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_arasan_data *sdhci_arasan =
>> sdhci_pltfm_priv(pltfm_host);
>> +	struct device_node *np = dev->of_node;
>> +
>> +	of_property_read_u32(np, "xlnx,itap_delay_sd_hsd",
>> +			     &sdhci_arasan->itapdly[MMC_TIMING_SD_HS]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_sd_hsd",
>> +			     &sdhci_arasan->otapdly[MMC_TIMING_SD_HS]);
>> +	of_property_read_u32(np, "xlnx,itap_delay_sdr25",
>> +			     &sdhci_arasan-
>>> itapdly[MMC_TIMING_UHS_SDR25]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_sdr25",
>> +			     &sdhci_arasan-
>>> otapdly[MMC_TIMING_UHS_SDR25]);
>> +	of_property_read_u32(np, "xlnx,itap_delay_sdr50",
>> +			     &sdhci_arasan-
>>> itapdly[MMC_TIMING_UHS_SDR50]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_sdr50",
>> +			     &sdhci_arasan-
>>> otapdly[MMC_TIMING_UHS_SDR50]);
>> +	of_property_read_u32(np, "xlnx,itap_delay_sd_ddr50",
>> +			     &sdhci_arasan-
>>> itapdly[MMC_TIMING_UHS_DDR50]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_sd_ddr50",
>> +			     &sdhci_arasan-
>>> otapdly[MMC_TIMING_UHS_DDR50]);
>> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_hsd",
>> +			     &sdhci_arasan-
>>> itapdly[MMC_TIMING_MMC_HS]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_hsd",
>> +			     &sdhci_arasan-
>>> otapdly[MMC_TIMING_MMC_HS]);
>> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_ddr50",
>> +			     &sdhci_arasan-
>>> itapdly[MMC_TIMING_MMC_DDR52]);
>> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_ddr50",
>> +			     &sdhci_arasan-
>>> otapdly[MMC_TIMING_MMC_DDR52]);
>> +	if (sdhci_arasan->mio_bank == MMC_BANK2) {
>> +		of_property_read_u32(np,
>> +				     "xlnx,itap_delay_sdr104_b2",
>> +				&sdhci_arasan-
>>> itapdly[MMC_TIMING_UHS_SDR104]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,otap_delay_sdr104_b2",
>> +				&sdhci_arasan-
>>> otapdly[MMC_TIMING_UHS_SDR104]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,itap_delay_mmc_hs200_b2",
>> +				&sdhci_arasan-
>>> itapdly[MMC_TIMING_MMC_HS200]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,otap_delay_mmc_hs200_b2",
>> +				&sdhci_arasan-
>>> otapdly[MMC_TIMING_MMC_HS200]);
>> +	} else {
>> +		of_property_read_u32(np,
>> +				     "xlnx,itap_delay_sdr104_b0",
>> +				&sdhci_arasan-
>>> itapdly[MMC_TIMING_UHS_SDR104]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,otap_delay_sdr104_b0",
>> +				&sdhci_arasan-
>>> otapdly[MMC_TIMING_UHS_SDR104]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,itap_delay_mmc_hs200_b0",
>> +				&sdhci_arasan-
>>> itapdly[MMC_TIMING_MMC_HS200]);
>> +		of_property_read_u32(np,
>> +				     "xlnx,otap_delay_mmc_hs200_b0",
>> +				&sdhci_arasan-
>>> otapdly[MMC_TIMING_MMC_HS200]);
>> +	}
>> +}
>> +
>>  static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)  {
>>  	struct sdhci_host *host = sdhci_arasan->host; @@ -776,6 +886,27
>> @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>>  		goto unreg_clk;
>>  	}
>>
>> +	if (of_device_is_compatible(pdev->dev.of_node,
>> +				    "xlnx,zynqmp-8.9a")) {
>> +		ret = of_property_read_u32(pdev->dev.of_node,
>> +					   "xlnx,mio_bank",
>> +					   &sdhci_arasan->mio_bank);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev,
>> +				"\"xlnx,mio_bank \" property is missing.\n");
>> +			goto clk_disable_all;
>> +		}
>> +		ret = of_property_read_u32(pdev->dev.of_node,
>> +					   "xlnx,device_id",
>> +					   &sdhci_arasan->device_id);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev,
>> +				"\"xlnx,device_id \" property is missing.\n");
>> +			goto clk_disable_all;
>> +		}
>> +		arasan_zynqmp_dt_parse_tap_delays(&pdev->dev);
>> +	}
>> +
>>  	sdhci_arasan->phy = ERR_PTR(-ENODEV);
>>  	if (of_device_is_compatible(pdev->dev.of_node,
>>  				    "arasan,sdhci-5.1")) {
>> --
>> 2.7.4
> 
> 


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

* RE: [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details
  2018-06-07 12:47   ` Mark Rutland
@ 2018-06-21 12:41     ` Manish Narani
  0 siblings, 0 replies; 11+ messages in thread
From: Manish Narani @ 2018-06-21 12:41 UTC (permalink / raw)
  To: Mark Rutland
  Cc: robh+dt, catalin.marinas, will.deacon, mdf, stefan.krsmanovic,
	linux-arm-kernel, linux-kernel, linux-mmc, devicetree,
	adrian.hunter, Michal Simek, ulf.hansson

Hi Mark,

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: Thursday, June 7, 2018 6:18 PM
> To: Manish Narani <MNARANI@xilinx.com>
> Cc: robh+dt@kernel.org; catalin.marinas@arm.com; will.deacon@arm.com;
> mdf@kernel.org; stefan.krsmanovic@aggios.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mmc@vger.kernel.org; devicetree@vger.kernel.org; adrian.hunter@intel.com;
> michal.simek@xilinx.com; ulf.hansson@linaro.org
> Subject: Re: [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details
> 
> On Thu, Jun 07, 2018 at 05:41:39PM +0530, Manish Narani wrote:
> > This patch adds details of SD tap value properties in device tree.
> >
> > Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> > ---
> >  .../devicetree/bindings/mmc/arasan,sdhci.txt       | 26
> ++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > index 60481bf..0e08877 100644
> > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > @@ -15,6 +15,8 @@ Required Properties:
> >      - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
> >      - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
> >        For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> > +    - "xlnx,zynqmp-8.9a": Xilinx ZynqMP Arasan SDHCI 8.9a PHY
> > +      For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> >    - reg: From mmc bindings: Register location and length.
> >    - clocks: From clock bindings: Handles to clock inputs.
> >    - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> > @@ -26,6 +28,30 @@ Required Properties for "arasan,sdhci-5.1":
> >    - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
> >    - phy-names:  MUST be "phy_arasan".
> >
> > +Required Properties for "xlnx,zynqmp-8.9a":
> > +  - xlnx,mio_bank: The value will be 0/1/2 depending on MIO bank selection.
> 
> For all of these properties, please s/_/-/, folowing the usual property name
> conventions.
I will correct this in the next version.
> 
> It's not clear to me why you need this property. The code in patch 3 only seems
> to use this to determine which properties to read, choosing between <prop>_b0
> or <prop>_b2. I don't see why you dont have the base <prop> alone...
The property 'xlnx,mio_bank' will be different for various ZynqMP varients. So different ZynqMP dts files may have different values for 'xlnx,mio_bank'. That's why I am maintaining _b0 and _b2 as different values
> 
> Is this a HW detail, or configuration that you prefer?
These are SD tap values which are generally used for configuring taps in SD. Keeping it in device tree makes it User Configurable without changing the driver code.
> 
> > +  - xlnx,device_id: Unique Id of the device, value will be 0/1.
> 
> What's this used for?
This used to identify the controller ID between two SD controllers present on ZynqMP.
> 
> > +  - xlnx,itap_delay_sd_hsd: Input Tap Delay for SD HS.
> 
> What unit at hese delays in?
The tap values don't have specific units. They are generally a fraction of the clock cycle.
For the SD frequency of -
200 MHz: 30 Input taps are available
100 MHz: 60 Input taps are available
50 MHz: 120 Input taps are available
200 MHz: 8 Output taps are available
100 MHz: 15 Output taps are available
50 MHz: 30 Output taps are available

Thanks,
Manish
> 
> Please follow the conventions in
> Documentation/devicetree/bindings/property-units.txt.
> 
> > +  - xlnx,itap_delay_sdr25: Input Tap Delay for SDR25.
> > +  - xlnx,itap_delay_sdr50: Input Tap Delay for SDR50.
> > +  - xlnx,itap_delay_sdr104_b0: Input Tap Delay for SDR104.
> > +  - xlnx,itap_delay_sdr104_b2: Input Tap Delay for SDR104.
> 
> As above, Given you have to specify the bank, I don't see why you need multiple
> properties.
> 
> Thanks,
> Mark.
> 
> > +  - xlnx,itap_delay_sd_ddr50: Input Tap Delay for SD DDR50.
> > +  - xlnx,itap_delay_mmc_hsd: Input Tap Delay for MMC HS.
> > +  - xlnx,itap_delay_mmc_ddr50: Input Tap Delay for MMC DDR50.
> > +  - xlnx,itap_delay_mmc_hs200_b0: Input Tap Delay for MMC HS200.
> > +  - xlnx,itap_delay_mmc_hs200_b2: Input Tap Delay for MMC HS200.
> > +  - xlnx,otap_delay_sd_hsd: Output Tap Delay for SD HS.
> > +  - xlnx,otap_delay_sdr25: Output Tap Delay for SDR25.
> > +  - xlnx,otap_delay_sdr50: Output Tap Delay for SDR50.
> > +  - xlnx,otap_delay_sdr104_b0: Output Tap Delay for SDR104.
> > +  - xlnx,otap_delay_sdr104_b2: Output Tap Delay for SDR104.
> > +  - xlnx,otap_delay_sd_ddr50: Output Tap Delay for DDR50.
> > +  - xlnx,otap_delay_mmc_hsd: Output Tap Delay for MMC HS.
> > +  - xlnx,otap_delay_mmc_ddr50: Output Tap Delay for MMC DDR50.
> > +  - xlnx,otap_delay_mmc_hs200_b0: Output Tap Delay for MMC HS200.
> > +  - xlnx,otap_delay_mmc_hs200_b2: Output Tap Delay for MMC HS200.
> > +
> >  Optional Properties:
> >    - arasan,soc-ctl-syscon: A phandle to a syscon device (see ../mfd/syscon.txt)
> >      used to access core corecfg registers.  Offsets of registers in
> > this
> > --
> > 2.7.4


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

* RE: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-19 11:38     ` Adrian Hunter
@ 2018-06-21 12:54       ` Manish Narani
  2018-07-09 10:38       ` Manish Narani
  1 sibling, 0 replies; 11+ messages in thread
From: Manish Narani @ 2018-06-21 12:54 UTC (permalink / raw)
  To: Adrian Hunter, robh+dt, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, Michal Simek, ulf.hansson
  Cc: Srinivas Goud, Anirudha Sarangi

Hi Adrian,

> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> Sent: Tuesday, June 19, 2018 5:08 PM
> To: Manish Narani <MNARANI@xilinx.com>; robh+dt@kernel.org;
> catalin.marinas@arm.com; will.deacon@arm.com; mdf@kernel.org;
> stefan.krsmanovic@aggios.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-mmc@vger.kernel.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> ulf.hansson@linaro.org
> Cc: Srinivas Goud <sgoud@xilinx.com>; Anirudha Sarangi
> <anirudh@xilinx.com>
> Subject: Re: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay
> values from DT
> 
> On 14/06/18 08:38, Manish Narani wrote:
> > Ping for RFC
> 
> What is eemi?  Why aren't there patches for that?
Eemi(Extensible Energy Management Interface) is a power management interface for ZynqMP core. The patches for the same are already in process of mainlining. 
https://lkml.org/lkml/2018/6/20/823

Thanks,
Manish
> 
> >
> >> -----Original Message-----
> >> From: Manish Narani [mailto:manish.narani@xilinx.com]
> >> Sent: Thursday, June 7, 2018 5:42 PM
> >> To: robh+dt@kernel.org; mark.rutland@arm.com;
> >> catalin.marinas@arm.com; will.deacon@arm.com; mdf@kernel.org;
> >> stefan.krsmanovic@aggios.com; linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org; linux- mmc@vger.kernel.org;
> >> devicetree@vger.kernel.org; adrian.hunter@intel.com;
> >> michal.simek@xilinx.com; ulf.hansson@linaro.org
> >> Cc: Manish Narani <MNARANI@xilinx.com>
> >> Subject: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay
> >> values from DT
> >>
> >> This patch adds support for reading Tap Delay values from Device Tree
> >> and write them via eemi calls. The macros containing these tap delay
> >> values are removed from the driver.
> >>
> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> >> ---
> >>  drivers/mmc/host/sdhci-of-arasan.c | 131
> >> +++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 131 insertions(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> >> b/drivers/mmc/host/sdhci- of-arasan.c index e3332a5..fc0fd01 100644
> >> --- a/drivers/mmc/host/sdhci-of-arasan.c
> >> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> >> @@ -36,6 +36,8 @@
> >>
> >>  #define PHY_CLK_TOO_SLOW_HZ		400000
> >>
> >> +#define MMC_BANK2		0x2
> >> +
> >>  /*
> >>   * On some SoCs the syscon area has a feature where the upper 16-bits of
> >>   * each 32-bit register act as a write mask for the lower 16-bits.
> >> This allows @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
> >>  	struct sdhci_host *host;
> >>  	struct clk	*clk_ahb;
> >>  	struct phy	*phy;
> >> +	u32 mio_bank;
> >> +	u32 device_id;
> >> +	u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
> >> +	u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
> >>  	bool		is_phy_on;
> >>
> >>  	bool		has_cqe;
> >> @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct
> >> sdhci_host *host,
> >>  	return ret;
> >>  }
> >>
> >> +/**
> >> + * arasan_zynqmp_set_tap_delay - Program the tap delays.
> >> + * @deviceid:		Unique Id of device
> >> + * @itap_delay:		Input Tap Delay
> >> + * @oitap_delay:	Output Tap Delay
> >> + */
> >> +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay,
> >> +u8
> >> +otap_delay) {
> >> +	const struct zynqmp_eemi_ops *eemi_ops =
> >> zynqmp_pm_get_eemi_ops();
> >> +	u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
> >> +
> >> +	if (!eemi_ops || !eemi_ops->ioctl)
> >> +		return;
> >> +
> >> +	if (itap_delay)
> >> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> >> +				PM_TAPDELAY_INPUT, itap_delay, NULL);
> >> +
> >> +	if (otap_delay)
> >> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> >> +				PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> >> }
> >> +
> >>  static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned
> >> int
> >> clock)  {
> >>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >>  	struct sdhci_arasan_data *sdhci_arasan =
> >> sdhci_pltfm_priv(pltfm_host);
> >>  	bool ctrl_phy = false;
> >> +	u8 itap_delay;
> >> +	u8 otap_delay;
> >>
> >>  	if (!IS_ERR(sdhci_arasan->phy)) {
> >>  		if (!sdhci_arasan->is_phy_on && clock <=
> >> PHY_CLK_TOO_SLOW_HZ) { @@ -200,6 +231,16 @@ static void
> >> sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
> >>  		}
> >>  	}
> >>
> >> +	if (host->version >= SDHCI_SPEC_300) {
> >> +		if ((host->timing != MMC_TIMING_LEGACY) &&
> >> +			(host->timing != MMC_TIMING_UHS_SDR12)) {
> >> +			itap_delay = sdhci_arasan->itapdly[host->timing];
> >> +			otap_delay = sdhci_arasan->otapdly[host->timing];
> >> +			arasan_zynqmp_set_tap_delay(sdhci_arasan-
> >>> device_id,
> >> +						    itap_delay, otap_delay);
> >> +		}
> >> +	}
> >> +
> >>  	if (ctrl_phy && sdhci_arasan->is_phy_on) {
> >>  		phy_power_off(sdhci_arasan->phy);
> >>  		sdhci_arasan->is_phy_on = false;
> >> @@ -456,6 +497,7 @@ static const struct of_device_id
> >> sdhci_arasan_of_match[] = {
> >>  	{ .compatible = "arasan,sdhci-8.9a" },
> >>  	{ .compatible = "arasan,sdhci-5.1" },
> >>  	{ .compatible = "arasan,sdhci-4.9a" },
> >> +	{ .compatible = "xlnx,zynqmp-8.9a" },
> >>
> >>  	{ /* sentinel */ }
> >>  };
> >> @@ -641,6 +683,74 @@ static void sdhci_arasan_unregister_sdclk(struct
> >> device *dev)
> >>  	of_clk_del_provider(dev->of_node);
> >>  }
> >>
> >> +/**
> >> + * arasan_zynqmp_dt_parse_tap_delays - Read Tap Delay values from DT
> >> + *
> >> + * Called at initialization to parse the values of Tap Delays.
> >> + *
> >> + * @dev:		Pointer to our struct device.
> >> + */
> >> +static void arasan_zynqmp_dt_parse_tap_delays(struct device *dev) {
> >> +	struct platform_device *pdev = to_platform_device(dev);
> >> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> >> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >> +	struct sdhci_arasan_data *sdhci_arasan =
> >> sdhci_pltfm_priv(pltfm_host);
> >> +	struct device_node *np = dev->of_node;
> >> +
> >> +	of_property_read_u32(np, "xlnx,itap_delay_sd_hsd",
> >> +			     &sdhci_arasan->itapdly[MMC_TIMING_SD_HS]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_sd_hsd",
> >> +			     &sdhci_arasan->otapdly[MMC_TIMING_SD_HS]);
> >> +	of_property_read_u32(np, "xlnx,itap_delay_sdr25",
> >> +			     &sdhci_arasan-
> >>> itapdly[MMC_TIMING_UHS_SDR25]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_sdr25",
> >> +			     &sdhci_arasan-
> >>> otapdly[MMC_TIMING_UHS_SDR25]);
> >> +	of_property_read_u32(np, "xlnx,itap_delay_sdr50",
> >> +			     &sdhci_arasan-
> >>> itapdly[MMC_TIMING_UHS_SDR50]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_sdr50",
> >> +			     &sdhci_arasan-
> >>> otapdly[MMC_TIMING_UHS_SDR50]);
> >> +	of_property_read_u32(np, "xlnx,itap_delay_sd_ddr50",
> >> +			     &sdhci_arasan-
> >>> itapdly[MMC_TIMING_UHS_DDR50]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_sd_ddr50",
> >> +			     &sdhci_arasan-
> >>> otapdly[MMC_TIMING_UHS_DDR50]);
> >> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_hsd",
> >> +			     &sdhci_arasan-
> >>> itapdly[MMC_TIMING_MMC_HS]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_hsd",
> >> +			     &sdhci_arasan-
> >>> otapdly[MMC_TIMING_MMC_HS]);
> >> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_ddr50",
> >> +			     &sdhci_arasan-
> >>> itapdly[MMC_TIMING_MMC_DDR52]);
> >> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_ddr50",
> >> +			     &sdhci_arasan-
> >>> otapdly[MMC_TIMING_MMC_DDR52]);
> >> +	if (sdhci_arasan->mio_bank == MMC_BANK2) {
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,itap_delay_sdr104_b2",
> >> +				&sdhci_arasan-
> >>> itapdly[MMC_TIMING_UHS_SDR104]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,otap_delay_sdr104_b2",
> >> +				&sdhci_arasan-
> >>> otapdly[MMC_TIMING_UHS_SDR104]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,itap_delay_mmc_hs200_b2",
> >> +				&sdhci_arasan-
> >>> itapdly[MMC_TIMING_MMC_HS200]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,otap_delay_mmc_hs200_b2",
> >> +				&sdhci_arasan-
> >>> otapdly[MMC_TIMING_MMC_HS200]);
> >> +	} else {
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,itap_delay_sdr104_b0",
> >> +				&sdhci_arasan-
> >>> itapdly[MMC_TIMING_UHS_SDR104]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,otap_delay_sdr104_b0",
> >> +				&sdhci_arasan-
> >>> otapdly[MMC_TIMING_UHS_SDR104]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,itap_delay_mmc_hs200_b0",
> >> +				&sdhci_arasan-
> >>> itapdly[MMC_TIMING_MMC_HS200]);
> >> +		of_property_read_u32(np,
> >> +				     "xlnx,otap_delay_mmc_hs200_b0",
> >> +				&sdhci_arasan-
> >>> otapdly[MMC_TIMING_MMC_HS200]);
> >> +	}
> >> +}
> >> +
> >>  static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)  {
> >>  	struct sdhci_host *host = sdhci_arasan->host; @@ -776,6 +886,27 @@
> >> static int sdhci_arasan_probe(struct platform_device *pdev)
> >>  		goto unreg_clk;
> >>  	}
> >>
> >> +	if (of_device_is_compatible(pdev->dev.of_node,
> >> +				    "xlnx,zynqmp-8.9a")) {
> >> +		ret = of_property_read_u32(pdev->dev.of_node,
> >> +					   "xlnx,mio_bank",
> >> +					   &sdhci_arasan->mio_bank);
> >> +		if (ret < 0) {
> >> +			dev_err(&pdev->dev,
> >> +				"\"xlnx,mio_bank \" property is missing.\n");
> >> +			goto clk_disable_all;
> >> +		}
> >> +		ret = of_property_read_u32(pdev->dev.of_node,
> >> +					   "xlnx,device_id",
> >> +					   &sdhci_arasan->device_id);
> >> +		if (ret < 0) {
> >> +			dev_err(&pdev->dev,
> >> +				"\"xlnx,device_id \" property is missing.\n");
> >> +			goto clk_disable_all;
> >> +		}
> >> +		arasan_zynqmp_dt_parse_tap_delays(&pdev->dev);
> >> +	}
> >> +
> >>  	sdhci_arasan->phy = ERR_PTR(-ENODEV);
> >>  	if (of_device_is_compatible(pdev->dev.of_node,
> >>  				    "arasan,sdhci-5.1")) {
> >> --
> >> 2.7.4
> >
> >


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

* RE: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-19 11:38     ` Adrian Hunter
  2018-06-21 12:54       ` Manish Narani
@ 2018-07-09 10:38       ` Manish Narani
  1 sibling, 0 replies; 11+ messages in thread
From: Manish Narani @ 2018-07-09 10:38 UTC (permalink / raw)
  To: Adrian Hunter, robh+dt, catalin.marinas, will.deacon, mdf,
	stefan.krsmanovic, linux-arm-kernel, linux-kernel, linux-mmc,
	devicetree, Michal Simek, ulf.hansson
  Cc: Srinivas Goud, Anirudha Sarangi

Ping

> -----Original Message-----
> From: Manish Narani
> Sent: Thursday, June 21, 2018 6:25 PM
> To: Adrian Hunter <adrian.hunter@intel.com>; robh+dt@kernel.org;
> catalin.marinas@arm.com; will.deacon@arm.com; mdf@kernel.org;
> stefan.krsmanovic@aggios.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-mmc@vger.kernel.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> ulf.hansson@linaro.org
> Cc: Srinivas Goud <sgoud@xilinx.com>; Anirudha Sarangi
> <anirudh@xilinx.com>
> Subject: RE: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay
> values from DT
> 
> Hi Adrian,
> 
> > -----Original Message-----
> > From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> > Sent: Tuesday, June 19, 2018 5:08 PM
> > To: Manish Narani <MNARANI@xilinx.com>; robh+dt@kernel.org;
> > catalin.marinas@arm.com; will.deacon@arm.com; mdf@kernel.org;
> > stefan.krsmanovic@aggios.com; linux-arm-kernel@lists.infradead.org;
> > linux- kernel@vger.kernel.org; linux-mmc@vger.kernel.org;
> > devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> > ulf.hansson@linaro.org
> > Cc: Srinivas Goud <sgoud@xilinx.com>; Anirudha Sarangi
> > <anirudh@xilinx.com>
> > Subject: Re: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap
> > Delay values from DT
> >
> > On 14/06/18 08:38, Manish Narani wrote:
> > > Ping for RFC
> >
> > What is eemi?  Why aren't there patches for that?
> Eemi(Extensible Energy Management Interface) is a power management
> interface for ZynqMP core. The patches for the same are already in process of
> mainlining.
> https://lkml.org/lkml/2018/6/20/823
> 
> Thanks,
> Manish
> >
> > >
> > >> -----Original Message-----
> > >> From: Manish Narani [mailto:manish.narani@xilinx.com]
> > >> Sent: Thursday, June 7, 2018 5:42 PM
> > >> To: robh+dt@kernel.org; mark.rutland@arm.com;
> > >> catalin.marinas@arm.com; will.deacon@arm.com; mdf@kernel.org;
> > >> stefan.krsmanovic@aggios.com; linux-arm-kernel@lists.infradead.org;
> > >> linux-kernel@vger.kernel.org; linux- mmc@vger.kernel.org;
> > >> devicetree@vger.kernel.org; adrian.hunter@intel.com;
> > >> michal.simek@xilinx.com; ulf.hansson@linaro.org
> > >> Cc: Manish Narani <MNARANI@xilinx.com>
> > >> Subject: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap
> > >> Delay values from DT
> > >>
> > >> This patch adds support for reading Tap Delay values from Device
> > >> Tree and write them via eemi calls. The macros containing these tap
> > >> delay values are removed from the driver.
> > >>
> > >> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> > >> ---
> > >>  drivers/mmc/host/sdhci-of-arasan.c | 131
> > >> +++++++++++++++++++++++++++++++++++++
> > >>  1 file changed, 131 insertions(+)
> > >>
> > >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> > >> b/drivers/mmc/host/sdhci- of-arasan.c index e3332a5..fc0fd01 100644
> > >> --- a/drivers/mmc/host/sdhci-of-arasan.c
> > >> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> > >> @@ -36,6 +36,8 @@
> > >>
> > >>  #define PHY_CLK_TOO_SLOW_HZ		400000
> > >>
> > >> +#define MMC_BANK2		0x2
> > >> +
> > >>  /*
> > >>   * On some SoCs the syscon area has a feature where the upper 16-bits of
> > >>   * each 32-bit register act as a write mask for the lower 16-bits.
> > >> This allows @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
> > >>  	struct sdhci_host *host;
> > >>  	struct clk	*clk_ahb;
> > >>  	struct phy	*phy;
> > >> +	u32 mio_bank;
> > >> +	u32 device_id;
> > >> +	u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
> > >> +	u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
> > >>  	bool		is_phy_on;
> > >>
> > >>  	bool		has_cqe;
> > >> @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct
> > >> sdhci_host *host,
> > >>  	return ret;
> > >>  }
> > >>
> > >> +/**
> > >> + * arasan_zynqmp_set_tap_delay - Program the tap delays.
> > >> + * @deviceid:		Unique Id of device
> > >> + * @itap_delay:		Input Tap Delay
> > >> + * @oitap_delay:	Output Tap Delay
> > >> + */
> > >> +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8
> > >> +itap_delay,
> > >> +u8
> > >> +otap_delay) {
> > >> +	const struct zynqmp_eemi_ops *eemi_ops =
> > >> zynqmp_pm_get_eemi_ops();
> > >> +	u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
> > >> +
> > >> +	if (!eemi_ops || !eemi_ops->ioctl)
> > >> +		return;
> > >> +
> > >> +	if (itap_delay)
> > >> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> > >> +				PM_TAPDELAY_INPUT, itap_delay, NULL);
> > >> +
> > >> +	if (otap_delay)
> > >> +		eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> > >> +				PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> > >> }
> > >> +
> > >>  static void sdhci_arasan_set_clock(struct sdhci_host *host,
> > >> unsigned int
> > >> clock)  {
> > >>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > >>  	struct sdhci_arasan_data *sdhci_arasan =
> > >> sdhci_pltfm_priv(pltfm_host);
> > >>  	bool ctrl_phy = false;
> > >> +	u8 itap_delay;
> > >> +	u8 otap_delay;
> > >>
> > >>  	if (!IS_ERR(sdhci_arasan->phy)) {
> > >>  		if (!sdhci_arasan->is_phy_on && clock <=
> > >> PHY_CLK_TOO_SLOW_HZ) { @@ -200,6 +231,16 @@ static void
> > >> sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)
> > >>  		}
> > >>  	}
> > >>
> > >> +	if (host->version >= SDHCI_SPEC_300) {
> > >> +		if ((host->timing != MMC_TIMING_LEGACY) &&
> > >> +			(host->timing != MMC_TIMING_UHS_SDR12)) {
> > >> +			itap_delay = sdhci_arasan->itapdly[host->timing];
> > >> +			otap_delay = sdhci_arasan->otapdly[host->timing];
> > >> +			arasan_zynqmp_set_tap_delay(sdhci_arasan-
> > >>> device_id,
> > >> +						    itap_delay, otap_delay);
> > >> +		}
> > >> +	}
> > >> +
> > >>  	if (ctrl_phy && sdhci_arasan->is_phy_on) {
> > >>  		phy_power_off(sdhci_arasan->phy);
> > >>  		sdhci_arasan->is_phy_on = false; @@ -456,6 +497,7 @@ static
> > >> const struct of_device_id sdhci_arasan_of_match[] = {
> > >>  	{ .compatible = "arasan,sdhci-8.9a" },
> > >>  	{ .compatible = "arasan,sdhci-5.1" },
> > >>  	{ .compatible = "arasan,sdhci-4.9a" },
> > >> +	{ .compatible = "xlnx,zynqmp-8.9a" },
> > >>
> > >>  	{ /* sentinel */ }
> > >>  };
> > >> @@ -641,6 +683,74 @@ static void
> > >> sdhci_arasan_unregister_sdclk(struct
> > >> device *dev)
> > >>  	of_clk_del_provider(dev->of_node);
> > >>  }
> > >>
> > >> +/**
> > >> + * arasan_zynqmp_dt_parse_tap_delays - Read Tap Delay values from
> > >> +DT
> > >> + *
> > >> + * Called at initialization to parse the values of Tap Delays.
> > >> + *
> > >> + * @dev:		Pointer to our struct device.
> > >> + */
> > >> +static void arasan_zynqmp_dt_parse_tap_delays(struct device *dev) {
> > >> +	struct platform_device *pdev = to_platform_device(dev);
> > >> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> > >> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > >> +	struct sdhci_arasan_data *sdhci_arasan =
> > >> sdhci_pltfm_priv(pltfm_host);
> > >> +	struct device_node *np = dev->of_node;
> > >> +
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_sd_hsd",
> > >> +			     &sdhci_arasan->itapdly[MMC_TIMING_SD_HS]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_sd_hsd",
> > >> +			     &sdhci_arasan->otapdly[MMC_TIMING_SD_HS]);
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_sdr25",
> > >> +			     &sdhci_arasan-
> > >>> itapdly[MMC_TIMING_UHS_SDR25]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_sdr25",
> > >> +			     &sdhci_arasan-
> > >>> otapdly[MMC_TIMING_UHS_SDR25]);
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_sdr50",
> > >> +			     &sdhci_arasan-
> > >>> itapdly[MMC_TIMING_UHS_SDR50]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_sdr50",
> > >> +			     &sdhci_arasan-
> > >>> otapdly[MMC_TIMING_UHS_SDR50]);
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_sd_ddr50",
> > >> +			     &sdhci_arasan-
> > >>> itapdly[MMC_TIMING_UHS_DDR50]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_sd_ddr50",
> > >> +			     &sdhci_arasan-
> > >>> otapdly[MMC_TIMING_UHS_DDR50]);
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_hsd",
> > >> +			     &sdhci_arasan-
> > >>> itapdly[MMC_TIMING_MMC_HS]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_hsd",
> > >> +			     &sdhci_arasan-
> > >>> otapdly[MMC_TIMING_MMC_HS]);
> > >> +	of_property_read_u32(np, "xlnx,itap_delay_mmc_ddr50",
> > >> +			     &sdhci_arasan-
> > >>> itapdly[MMC_TIMING_MMC_DDR52]);
> > >> +	of_property_read_u32(np, "xlnx,otap_delay_mmc_ddr50",
> > >> +			     &sdhci_arasan-
> > >>> otapdly[MMC_TIMING_MMC_DDR52]);
> > >> +	if (sdhci_arasan->mio_bank == MMC_BANK2) {
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,itap_delay_sdr104_b2",
> > >> +				&sdhci_arasan-
> > >>> itapdly[MMC_TIMING_UHS_SDR104]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,otap_delay_sdr104_b2",
> > >> +				&sdhci_arasan-
> > >>> otapdly[MMC_TIMING_UHS_SDR104]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,itap_delay_mmc_hs200_b2",
> > >> +				&sdhci_arasan-
> > >>> itapdly[MMC_TIMING_MMC_HS200]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,otap_delay_mmc_hs200_b2",
> > >> +				&sdhci_arasan-
> > >>> otapdly[MMC_TIMING_MMC_HS200]);
> > >> +	} else {
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,itap_delay_sdr104_b0",
> > >> +				&sdhci_arasan-
> > >>> itapdly[MMC_TIMING_UHS_SDR104]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,otap_delay_sdr104_b0",
> > >> +				&sdhci_arasan-
> > >>> otapdly[MMC_TIMING_UHS_SDR104]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,itap_delay_mmc_hs200_b0",
> > >> +				&sdhci_arasan-
> > >>> itapdly[MMC_TIMING_MMC_HS200]);
> > >> +		of_property_read_u32(np,
> > >> +				     "xlnx,otap_delay_mmc_hs200_b0",
> > >> +				&sdhci_arasan-
> > >>> otapdly[MMC_TIMING_MMC_HS200]);
> > >> +	}
> > >> +}
> > >> +
> > >>  static int sdhci_arasan_add_host(struct sdhci_arasan_data *sdhci_arasan)
> {
> > >>  	struct sdhci_host *host = sdhci_arasan->host; @@ -776,6 +886,27
> > >> @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> > >>  		goto unreg_clk;
> > >>  	}
> > >>
> > >> +	if (of_device_is_compatible(pdev->dev.of_node,
> > >> +				    "xlnx,zynqmp-8.9a")) {
> > >> +		ret = of_property_read_u32(pdev->dev.of_node,
> > >> +					   "xlnx,mio_bank",
> > >> +					   &sdhci_arasan->mio_bank);
> > >> +		if (ret < 0) {
> > >> +			dev_err(&pdev->dev,
> > >> +				"\"xlnx,mio_bank \" property is missing.\n");
> > >> +			goto clk_disable_all;
> > >> +		}
> > >> +		ret = of_property_read_u32(pdev->dev.of_node,
> > >> +					   "xlnx,device_id",
> > >> +					   &sdhci_arasan->device_id);
> > >> +		if (ret < 0) {
> > >> +			dev_err(&pdev->dev,
> > >> +				"\"xlnx,device_id \" property is missing.\n");
> > >> +			goto clk_disable_all;
> > >> +		}
> > >> +		arasan_zynqmp_dt_parse_tap_delays(&pdev->dev);
> > >> +	}
> > >> +
> > >>  	sdhci_arasan->phy = ERR_PTR(-ENODEV);
> > >>  	if (of_device_is_compatible(pdev->dev.of_node,
> > >>  				    "arasan,sdhci-5.1")) {
> > >> --
> > >> 2.7.4
> > >
> > >


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

* Re: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-06-07 12:11 ` [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT Manish Narani
  2018-06-14  5:38   ` Manish Narani
@ 2018-07-10  8:31   ` Ulf Hansson
  2018-07-20  6:23     ` Manish Narani
  1 sibling, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2018-07-10  8:31 UTC (permalink / raw)
  To: Manish Narani
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Moritz Fischer, stefan.krsmanovic, Linux ARM,
	Linux Kernel Mailing List, linux-mmc, devicetree, Adrian Hunter,
	Michal Simek

On 7 June 2018 at 14:11, Manish Narani <manish.narani@xilinx.com> wrote:
> This patch adds support for reading Tap Delay values from Device Tree
> and write them via eemi calls. The macros containing these tap delay
> values are removed from the driver.
>
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 131 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 131 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index e3332a5..fc0fd01 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -36,6 +36,8 @@
>
>  #define PHY_CLK_TOO_SLOW_HZ            400000
>
> +#define MMC_BANK2              0x2
> +
>  /*
>   * On some SoCs the syscon area has a feature where the upper 16-bits of
>   * each 32-bit register act as a write mask for the lower 16-bits.  This allows
> @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
>         struct sdhci_host *host;
>         struct clk      *clk_ahb;
>         struct phy      *phy;
> +       u32 mio_bank;
> +       u32 device_id;
> +       u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
> +       u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
>         bool            is_phy_on;
>
>         bool            has_cqe;
> @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct sdhci_host *host,
>         return ret;
>  }
>
> +/**
> + * arasan_zynqmp_set_tap_delay - Program the tap delays.
> + * @deviceid:          Unique Id of device
> + * @itap_delay:                Input Tap Delay
> + * @oitap_delay:       Output Tap Delay
> + */
> +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay, u8 otap_delay)
> +{
> +       const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();

No thanks!

Isn't there a more generic framework we can use to change the tap
values, rather than calling SoC specific functions from the driver?

BTW, what is a tap value, more exactly?

What does changing a tap value mean and where does the property belong, really?

Of course this doesn't even compile, as you have a dependency to
another series. Next time, please clarify that in a cover-letter
(maybe you did, but I can't find it).

> +       u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
> +
> +       if (!eemi_ops || !eemi_ops->ioctl)
> +               return;
> +
> +       if (itap_delay)
> +               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> +                               PM_TAPDELAY_INPUT, itap_delay, NULL);
> +
> +       if (otap_delay)
> +               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> +                               PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> +}

Another overall comment for the series.

I would recommend to change the order of the patches in the series.
Let the DT doc change come first, next the driver change and finally
the change to the DTS file(s). This makes it easier to follow and
review.

[...]

Kind regards
Uffe

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

* RE: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT
  2018-07-10  8:31   ` Ulf Hansson
@ 2018-07-20  6:23     ` Manish Narani
  0 siblings, 0 replies; 11+ messages in thread
From: Manish Narani @ 2018-07-20  6:23 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Moritz Fischer, stefan.krsmanovic, Linux ARM,
	Linux Kernel Mailing List, linux-mmc, devicetree, Adrian Hunter,
	Michal Simek

Hi Uffe,

> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: Tuesday, July 10, 2018 2:02 PM
> To: Manish Narani <MNARANI@xilinx.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Mark Rutland
> <mark.rutland@arm.com>; Catalin Marinas <catalin.marinas@arm.com>; Will
> Deacon <will.deacon@arm.com>; Moritz Fischer <mdf@kernel.org>;
> stefan.krsmanovic@aggios.com; Linux ARM <linux-arm-
> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; linux-mmc@vger.kernel.org;
> devicetree@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>;
> Michal Simek <michals@xilinx.com>
> Subject: Re: [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay
> values from DT
> 
> On 7 June 2018 at 14:11, Manish Narani <manish.narani@xilinx.com> wrote:
> > This patch adds support for reading Tap Delay values from Device Tree
> > and write them via eemi calls. The macros containing these tap delay
> > values are removed from the driver.
> >
> > Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> > ---
> >  drivers/mmc/host/sdhci-of-arasan.c | 131
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 131 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> > b/drivers/mmc/host/sdhci-of-arasan.c
> > index e3332a5..fc0fd01 100644
> > --- a/drivers/mmc/host/sdhci-of-arasan.c
> > +++ b/drivers/mmc/host/sdhci-of-arasan.c
> > @@ -36,6 +36,8 @@
> >
> >  #define PHY_CLK_TOO_SLOW_HZ            400000
> >
> > +#define MMC_BANK2              0x2
> > +
> >  /*
> >   * On some SoCs the syscon area has a feature where the upper 16-bits of
> >   * each 32-bit register act as a write mask for the lower 16-bits.
> > This allows @@ -90,6 +92,10 @@ struct sdhci_arasan_data {
> >         struct sdhci_host *host;
> >         struct clk      *clk_ahb;
> >         struct phy      *phy;
> > +       u32 mio_bank;
> > +       u32 device_id;
> > +       u32 itapdly[MMC_TIMING_MMC_HS400 + 1];
> > +       u32 otapdly[MMC_TIMING_MMC_HS400 + 1];
> >         bool            is_phy_on;
> >
> >         bool            has_cqe;
> > @@ -160,11 +166,36 @@ static int sdhci_arasan_syscon_write(struct
> sdhci_host *host,
> >         return ret;
> >  }
> >
> > +/**
> > + * arasan_zynqmp_set_tap_delay - Program the tap delays.
> > + * @deviceid:          Unique Id of device
> > + * @itap_delay:                Input Tap Delay
> > + * @oitap_delay:       Output Tap Delay
> > + */
> > +static void arasan_zynqmp_set_tap_delay(u8 deviceid, u8 itap_delay,
> > +u8 otap_delay) {
> > +       const struct zynqmp_eemi_ops *eemi_ops =
> > +zynqmp_pm_get_eemi_ops();
> 
> No thanks!
> 
> Isn't there a more generic framework we can use to change the tap values,
> rather than calling SoC specific functions from the driver?
Yes, Thanks for your suggestion. I will work on the generic framework which will be used to change tap values
Via SoC drivers.
> 
> BTW, what is a tap value, more exactly?
> What does changing a tap value mean and where does the property belong,
> really?
Tap Value is the delay of clock phase which is used to adjust phase to the working value. The auto tuning
process generally sets tap values internally in controller in SD UHS modes. But for other modes where
auto tuning is applicable, we are determining tap values via trial & error method for specific SoC.
> > 
> Of course this doesn't even compile, as you have a dependency to another
> series. Next time, please clarify that in a cover-letter (maybe you did, but I
> can't find it).
> 
> > +       u32 node_id = (deviceid == 0) ? NODE_SD_0 : NODE_SD_1;
> > +
> > +       if (!eemi_ops || !eemi_ops->ioctl)
> > +               return;
> > +
> > +       if (itap_delay)
> > +               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> > +                               PM_TAPDELAY_INPUT, itap_delay, NULL);
> > +
> > +       if (otap_delay)
> > +               eemi_ops->ioctl(node_id, IOCTL_SET_SD_TAPDELAY,
> > +                               PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> > +}
> 
> Another overall comment for the series.
> 
> I would recommend to change the order of the patches in the series.
> Let the DT doc change come first, next the driver change and finally the change
> to the DTS file(s). This makes it easier to follow and review.
Sure, I will do that.

Thanks & Regards,
Manish

> 
> [...]
> 
> Kind regards
> Uffe

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

end of thread, other threads:[~2018-07-20  6:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 12:11 [RFC PATCH 1/3] arm64: zynqmp: dt: Add support for setting SD tap delays Manish Narani
2018-06-07 12:11 ` [RFC PATCH 2/3] dt: bindings: Add SD tap value properties details Manish Narani
2018-06-07 12:47   ` Mark Rutland
2018-06-21 12:41     ` Manish Narani
2018-06-07 12:11 ` [RFC PATCH 3/3] sdhci: arasan: Add support to read Tap Delay values from DT Manish Narani
2018-06-14  5:38   ` Manish Narani
2018-06-19 11:38     ` Adrian Hunter
2018-06-21 12:54       ` Manish Narani
2018-07-09 10:38       ` Manish Narani
2018-07-10  8:31   ` Ulf Hansson
2018-07-20  6:23     ` Manish Narani

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