The following patches update phy configurations for AM65x as given in
the latest data manual.
The patches depend on my fixes series posted just before this:
https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
Device tree patch updating the actual otap values will be posted
separately.
Tested with Am65x-evm and J721e-evm.
Faiz Abbas (3):
dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
mmc: sdhci_am654: Update OTAPDLY writes
mmc: sdhci_am654: Enable DLL only for some speed modes
.../devicetree/bindings/mmc/sdhci-am654.txt | 21 +-
drivers/mmc/host/sdhci_am654.c | 247 ++++++++++++------
include/linux/mmc/host.h | 2 +
3 files changed, 192 insertions(+), 78 deletions(-)
--
2.19.2
Its recommended that DLL must only be enabled for SDR50, DDR50, DDR52,
SDR104, HS200 and HS400 speed modes. Move DLL configuration to its own
function and call it only in the above speed modes.
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++----------------
1 file changed, 68 insertions(+), 60 deletions(-)
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index bb977de43f7d..575bbab1a6ed 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -119,16 +119,80 @@ static const struct timing_data td[] = {
[MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
};
+static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+ int sel50, sel100, freqsel;
+ u32 mask, val;
+ int ret;
+
+ if (sdhci_am654->flags & FREQSEL_2_BIT) {
+ switch (clock) {
+ case 200000000:
+ sel50 = 0;
+ sel100 = 0;
+ break;
+ case 100000000:
+ sel50 = 0;
+ sel100 = 1;
+ break;
+ default:
+ sel50 = 1;
+ sel100 = 0;
+ }
+
+ /* Configure PHY DLL frequency */
+ mask = SEL50_MASK | SEL100_MASK;
+ val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val);
+
+ } else {
+ switch (clock) {
+ case 200000000:
+ freqsel = 0x0;
+ break;
+ default:
+ freqsel = 0x4;
+ }
+
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL5, FREQSEL_MASK,
+ freqsel << FREQSEL_SHIFT);
+ }
+ /* Configure DLL TRIM */
+ mask = DLL_TRIM_ICP_MASK;
+ val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
+
+ /* Configure DLL driver strength */
+ mask |= DR_TY_MASK;
+ val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
+
+ /* Enable DLL */
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
+ 0x1 << ENDLL_SHIFT);
+ /*
+ * Poll for DLL ready. Use a one second timeout.
+ * Works in all experiments done so far
+ */
+ ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, val,
+ val & DLLRDY_MASK, 1000, 1000000);
+ if (ret) {
+ dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
+ return;
+ }
+
+ sdhci_am654->dll_on = true;
+}
+
static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
unsigned char timing = host->mmc->ios.timing;
- int sel50, sel100, freqsel;
u32 otap_del_sel;
u32 otap_del_ena;
u32 mask, val;
- int ret;
if (sdhci_am654->dll_on) {
regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0);
@@ -163,64 +227,8 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
- if (sdhci_am654->flags & FREQSEL_2_BIT) {
- switch (clock) {
- case 200000000:
- sel50 = 0;
- sel100 = 0;
- break;
- case 100000000:
- sel50 = 0;
- sel100 = 1;
- break;
- default:
- sel50 = 1;
- sel100 = 0;
- }
-
- /* Configure PHY DLL frequency */
- mask = SEL50_MASK | SEL100_MASK;
- val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
- regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask,
- val);
- } else {
- switch (clock) {
- case 200000000:
- freqsel = 0x0;
- break;
- default:
- freqsel = 0x4;
- }
-
- regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
- FREQSEL_MASK,
- freqsel << FREQSEL_SHIFT);
- }
-
- /* Configure DLL TRIM */
- mask = DLL_TRIM_ICP_MASK;
- val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
-
- /* Configure DLL driver strength */
- mask |= DR_TY_MASK;
- val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
- regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
- /* Enable DLL */
- regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
- 0x1 << ENDLL_SHIFT);
- /*
- * Poll for DLL ready. Use a one second timeout.
- * Works in all experiments done so far
- */
- ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
- val, val & DLLRDY_MASK, 1000,
- 1000000);
- if (ret) {
- dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
- return;
- }
-
- sdhci_am654->dll_on = true;
+ if (timing > MMC_TIMING_UHS_SDR25)
+ sdhci_am654_setup_dll(host, clock);
}
}
--
2.19.2
According to latest AM65x Data Manual[1], a different output tap delay
value is recommended for all speed modes. Therefore, replace the
ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
mode.
[1] http://www.ti.com/lit/gpn/am6526
Signed-off-by: Faiz Abbas <[email protected]>
---
.../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
index 50e87df47971..c6ccecb9ae5a 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
@@ -18,7 +18,20 @@ Required Properties:
- clocks: Handles to the clock inputs.
- clock-names: Tuple including "clk_xin" and "clk_ahb"
- interrupts: Interrupt specifiers
- - ti,otap-del-sel: Output Tap Delay select
+ Output tap delay for each speed mode:
+ - ti,otap-del-sel-legacy
+ - ti,otap-del-sel-mmc-hs
+ - ti,otap-del-sel-sd-hs
+ - ti,otap-del-sel-sdr12
+ - ti,otap-del-sel-sdr25
+ - ti,otap-del-sel-sdr50
+ - ti,otap-del-sel-sdr104
+ - ti,otap-del-sel-ddr50
+ - ti,otap-del-sel-ddr52
+ - ti,otap-del-sel-hs200
+ - ti,otap-del-sel-hs400
+ These bindings must be provided otherwise the driver will disable the
+ corresponding speed mode (i.e. all nodes must provide at least -legacy)
Optional Properties (Required for ti,am654-sdhci-5.1 and ti,j721e-sdhci-8bit):
- ti,trm-icp: DLL trim select
@@ -38,6 +51,10 @@ Example:
interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
sdhci-caps-mask = <0x80000007 0x0>;
mmc-ddr-1_8v;
- ti,otap-del-sel = <0x2>;
+ ti,otap-del-sel-legacy = <0x0>;
+ ti,otap-del-sel-mmc-hs = <0x0>;
+ ti,otap-del-sel-ddr52 = <0x5>;
+ ti,otap-del-sel-hs200 = <0x5>;
+ ti,otap-del-sel-hs400 = <0x0>;
ti,trm-icp = <0x8>;
};
--
2.19.2
According to the latest AM65x Data Manual[1], a different output tap
delay value is optimum for a given speed mode. Therefore, deprecate the
ti,otap-del-sel binding and introduce a new binding for each of the
possible MMC/SD speed modes. If the legacy mode is not found, fall back
to old binding to maintain dts compatibility.
[1] http://www.ti.com/lit/gpn/am6526
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/mmc/host/sdhci_am654.c | 123 ++++++++++++++++++++++++++++-----
include/linux/mmc/host.h | 2 +
2 files changed, 107 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index b8fe94fd9525..bb977de43f7d 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -81,7 +81,8 @@ static struct regmap_config sdhci_am654_regmap_config = {
struct sdhci_am654_data {
struct regmap *base;
- int otap_del_sel;
+ bool legacy_otapdly;
+ int otap_del_sel[11];
int trm_icp;
int drv_strength;
bool dll_on;
@@ -98,11 +99,34 @@ struct sdhci_am654_driver_data {
#define DLL_PRESENT (1 << 3)
};
+struct timing_data {
+ const char *binding;
+ u32 capability;
+};
+
+static const struct timing_data td[] = {
+ [MMC_TIMING_LEGACY] = {"ti,otap-del-sel-legacy", 0},
+ [MMC_TIMING_MMC_HS] = {"ti,otap-del-sel-mmc-hs", MMC_CAP_MMC_HIGHSPEED},
+ [MMC_TIMING_SD_HS] = {"ti,otap-del-sel-sd-hs", MMC_CAP_SD_HIGHSPEED},
+ [MMC_TIMING_UHS_SDR12] = {"ti,otap-del-sel-sdr12", MMC_CAP_UHS_SDR12},
+ [MMC_TIMING_UHS_SDR25] = {"ti,otap-del-sel-sdr25", MMC_CAP_UHS_SDR25},
+ [MMC_TIMING_UHS_SDR50] = {"ti,otap-del-sel-sdr50", MMC_CAP_UHS_SDR50},
+ [MMC_TIMING_UHS_SDR104] = {"ti,otap-del-sel-sdr104",
+ MMC_CAP_UHS_SDR104},
+ [MMC_TIMING_UHS_DDR50] = {"ti,otap-del-sel-ddr50", MMC_CAP_UHS_DDR50},
+ [MMC_TIMING_MMC_DDR52] = {"ti,otap-del-sel-ddr52", MMC_CAP_DDR},
+ [MMC_TIMING_MMC_HS200] = {"ti,otap-del-sel-hs200", MMC_CAP2_HS200},
+ [MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
+};
+
static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+ unsigned char timing = host->mmc->ios.timing;
int sel50, sel100, freqsel;
+ u32 otap_del_sel;
+ u32 otap_del_ena;
u32 mask, val;
int ret;
@@ -116,22 +140,29 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
if (clock > CLOCK_TOO_SLOW_HZ) {
/* Setup DLL Output TAP delay */
+ if (sdhci_am654->legacy_otapdly)
+ otap_del_sel = sdhci_am654->otap_del_sel[0];
+ else
+ otap_del_sel = sdhci_am654->otap_del_sel[timing];
+
+ otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
+
mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
- val = (1 << OTAPDLYENA_SHIFT) |
- (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
- regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
+ val = (otap_del_ena << OTAPDLYENA_SHIFT) |
+ (otap_del_sel << OTAPDLYSEL_SHIFT);
+
/* Write to STRBSEL for HS400 speed mode */
- if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
+ if (timing == MMC_TIMING_MMC_HS400) {
if (sdhci_am654->flags & STRBSEL_4_BIT)
- mask = STRBSEL_4BIT_MASK;
+ mask |= STRBSEL_4BIT_MASK;
else
- mask = STRBSEL_8BIT_MASK;
+ mask |= STRBSEL_8BIT_MASK;
- regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask,
- sdhci_am654->strb_sel <<
- STRBSEL_SHIFT);
+ val |= sdhci_am654->strb_sel << STRBSEL_SHIFT;
}
+ regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
+
if (sdhci_am654->flags & FREQSEL_2_BIT) {
switch (clock) {
case 200000000:
@@ -198,11 +229,19 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
- int val, mask;
+ unsigned char timing = host->mmc->ios.timing;
+ u32 otap_del_sel;
+ u32 mask, val;
+
+ /* Setup DLL Output TAP delay */
+ if (sdhci_am654->legacy_otapdly)
+ otap_del_sel = sdhci_am654->otap_del_sel[0];
+ else
+ otap_del_sel = sdhci_am654->otap_del_sel[timing];
mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
- val = (1 << OTAPDLYENA_SHIFT) |
- (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
+ val = (0x1 << OTAPDLYENA_SHIFT) |
+ (otap_del_sel << OTAPDLYSEL_SHIFT);
regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
sdhci_set_clock(host, clock);
@@ -371,6 +410,55 @@ static int sdhci_am654_cqe_add_host(struct sdhci_host *host)
return ret;
}
+static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
+ struct sdhci_am654_data *sdhci_am654)
+{
+ struct device *dev = mmc_dev(host->mmc);
+ int i;
+ int ret;
+
+ ret = device_property_read_u32(dev, td[MMC_TIMING_LEGACY].binding,
+ &sdhci_am654->otap_del_sel[MMC_TIMING_LEGACY]);
+ if (ret) {
+ /*
+ * ti,otap-del-sel-legacy is mandatory, look for old binding
+ * if not found.
+ */
+ ret = device_property_read_u32(dev, "ti,otap-del-sel",
+ &sdhci_am654->otap_del_sel[0]);
+ if (ret) {
+ dev_err(dev, "Couldn't find otap-del-sel\n");
+
+ return ret;
+ }
+
+ dev_info(dev, "Using legacy binding ti,otap-del-sel\n");
+ sdhci_am654->legacy_otapdly = true;
+
+ return 0;
+ }
+
+ for (i = MMC_TIMING_MMC_HS; i <= MMC_TIMING_MMC_HS400; i++) {
+
+ ret = device_property_read_u32(dev, td[i].binding,
+ &sdhci_am654->otap_del_sel[i]);
+ if (ret) {
+ dev_dbg(dev, "Couldn't find %s\n",
+ td[i].binding);
+ /*
+ * Remove the corresponding capability
+ * if an otap-del-sel value is not found
+ */
+ if (i <= MMC_TIMING_MMC_DDR52)
+ host->mmc->caps &= ~td[i].capability;
+ else
+ host->mmc->caps2 &= ~td[i].capability;
+ }
+ }
+
+ return 0;
+}
+
static int sdhci_am654_init(struct sdhci_host *host)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -419,6 +507,10 @@ static int sdhci_am654_init(struct sdhci_host *host)
if (ret)
goto err_cleanup_host;
+ ret = sdhci_am654_get_otap_delay(host, sdhci_am654);
+ if (ret)
+ goto err_cleanup_host;
+
ret = __sdhci_add_host(host);
if (ret)
goto err_cleanup_host;
@@ -437,11 +529,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
int drv_strength;
int ret;
- ret = device_property_read_u32(dev, "ti,otap-del-sel",
- &sdhci_am654->otap_del_sel);
- if (ret)
- return ret;
-
if (sdhci_am654->flags & DLL_PRESENT) {
ret = device_property_read_u32(dev, "ti,trm-icp",
&sdhci_am654->trm_icp);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ba703384bea0..a22a10456c62 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -322,6 +322,8 @@ struct mmc_host {
#define MMC_CAP_3_3V_DDR (1 << 11) /* Host supports eMMC DDR 3.3V */
#define MMC_CAP_1_8V_DDR (1 << 12) /* Host supports eMMC DDR 1.8V */
#define MMC_CAP_1_2V_DDR (1 << 13) /* Host supports eMMC DDR 1.2V */
+#define MMC_CAP_DDR (MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
+ MMC_CAP_1_2V_DDR)
#define MMC_CAP_POWER_OFF_CARD (1 << 14) /* Can power off after boot */
#define MMC_CAP_BUS_WIDTH_TEST (1 << 15) /* CMD14/CMD19 bus width ok */
#define MMC_CAP_UHS_SDR12 (1 << 16) /* Host supports UHS SDR12 mode */
--
2.19.2
On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
> According to latest AM65x Data Manual[1], a different output tap delay
> value is recommended for all speed modes. Therefore, replace the
> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
> mode.
>
> [1] http://www.ti.com/lit/gpn/am6526
>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> index 50e87df47971..c6ccecb9ae5a 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
> @@ -18,7 +18,20 @@ Required Properties:
> - clocks: Handles to the clock inputs.
> - clock-names: Tuple including "clk_xin" and "clk_ahb"
> - interrupts: Interrupt specifiers
> - - ti,otap-del-sel: Output Tap Delay select
> + Output tap delay for each speed mode:
> + - ti,otap-del-sel-legacy
> + - ti,otap-del-sel-mmc-hs
> + - ti,otap-del-sel-sd-hs
> + - ti,otap-del-sel-sdr12
> + - ti,otap-del-sel-sdr25
> + - ti,otap-del-sel-sdr50
> + - ti,otap-del-sel-sdr104
> + - ti,otap-del-sel-ddr50
> + - ti,otap-del-sel-ddr52
> + - ti,otap-del-sel-hs200
> + - ti,otap-del-sel-hs400
> + These bindings must be provided otherwise the driver will disable the
> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
Why not just extend the existing property to be an array. We already
have properties to enable/disable speed modes.
>
> Optional Properties (Required for ti,am654-sdhci-5.1 and ti,j721e-sdhci-8bit):
> - ti,trm-icp: DLL trim select
> @@ -38,6 +51,10 @@ Example:
> interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> sdhci-caps-mask = <0x80000007 0x0>;
> mmc-ddr-1_8v;
> - ti,otap-del-sel = <0x2>;
> + ti,otap-del-sel-legacy = <0x0>;
> + ti,otap-del-sel-mmc-hs = <0x0>;
> + ti,otap-del-sel-ddr52 = <0x5>;
> + ti,otap-del-sel-hs200 = <0x5>;
> + ti,otap-del-sel-hs400 = <0x0>;
> ti,trm-icp = <0x8>;
> };
> --
> 2.19.2
>
Hi Rob,
On 15/01/20 7:20 am, Rob Herring wrote:
> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>> According to latest AM65x Data Manual[1], a different output tap delay
>> value is recommended for all speed modes. Therefore, replace the
>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>> mode.
>>
>> [1] http://www.ti.com/lit/gpn/am6526
>>
>> Signed-off-by: Faiz Abbas <[email protected]>
>> ---
>> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> index 50e87df47971..c6ccecb9ae5a 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>> @@ -18,7 +18,20 @@ Required Properties:
>> - clocks: Handles to the clock inputs.
>> - clock-names: Tuple including "clk_xin" and "clk_ahb"
>> - interrupts: Interrupt specifiers
>> - - ti,otap-del-sel: Output Tap Delay select
>> + Output tap delay for each speed mode:
>> + - ti,otap-del-sel-legacy
>> + - ti,otap-del-sel-mmc-hs
>> + - ti,otap-del-sel-sd-hs
>> + - ti,otap-del-sel-sdr12
>> + - ti,otap-del-sel-sdr25
>> + - ti,otap-del-sel-sdr50
>> + - ti,otap-del-sel-sdr104
>> + - ti,otap-del-sel-ddr50
>> + - ti,otap-del-sel-ddr52
>> + - ti,otap-del-sel-hs200
>> + - ti,otap-del-sel-hs400
>> + These bindings must be provided otherwise the driver will disable the
>> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
>
> Why not just extend the existing property to be an array. We already
> have properties to enable/disable speed modes.
>
Its hard to keep track of which modes have values and which don't when
you add an array. This scheme is just easier on anyone adding new values
or updating old values.
We already disable speed modes based on platform specific properties in
other drivers. In sdhci-omap.c, the driver disables the corresponding
speed mode if the corresponding pinmux and iodelay values are not populated.
Thanks,
Faiz
Thanks,
Faiz
On 8/01/20 5:09 pm, Faiz Abbas wrote:
> According to the latest AM65x Data Manual[1], a different output tap
> delay value is optimum for a given speed mode. Therefore, deprecate the
> ti,otap-del-sel binding and introduce a new binding for each of the
> possible MMC/SD speed modes. If the legacy mode is not found, fall back
> to old binding to maintain dts compatibility.
>
> [1] http://www.ti.com/lit/gpn/am6526
>
> Signed-off-by: Faiz Abbas <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci_am654.c | 123 ++++++++++++++++++++++++++++-----
> include/linux/mmc/host.h | 2 +
> 2 files changed, 107 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index b8fe94fd9525..bb977de43f7d 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -81,7 +81,8 @@ static struct regmap_config sdhci_am654_regmap_config = {
>
> struct sdhci_am654_data {
> struct regmap *base;
> - int otap_del_sel;
> + bool legacy_otapdly;
> + int otap_del_sel[11];
> int trm_icp;
> int drv_strength;
> bool dll_on;
> @@ -98,11 +99,34 @@ struct sdhci_am654_driver_data {
> #define DLL_PRESENT (1 << 3)
> };
>
> +struct timing_data {
> + const char *binding;
> + u32 capability;
> +};
> +
> +static const struct timing_data td[] = {
> + [MMC_TIMING_LEGACY] = {"ti,otap-del-sel-legacy", 0},
> + [MMC_TIMING_MMC_HS] = {"ti,otap-del-sel-mmc-hs", MMC_CAP_MMC_HIGHSPEED},
> + [MMC_TIMING_SD_HS] = {"ti,otap-del-sel-sd-hs", MMC_CAP_SD_HIGHSPEED},
> + [MMC_TIMING_UHS_SDR12] = {"ti,otap-del-sel-sdr12", MMC_CAP_UHS_SDR12},
> + [MMC_TIMING_UHS_SDR25] = {"ti,otap-del-sel-sdr25", MMC_CAP_UHS_SDR25},
> + [MMC_TIMING_UHS_SDR50] = {"ti,otap-del-sel-sdr50", MMC_CAP_UHS_SDR50},
> + [MMC_TIMING_UHS_SDR104] = {"ti,otap-del-sel-sdr104",
> + MMC_CAP_UHS_SDR104},
> + [MMC_TIMING_UHS_DDR50] = {"ti,otap-del-sel-ddr50", MMC_CAP_UHS_DDR50},
> + [MMC_TIMING_MMC_DDR52] = {"ti,otap-del-sel-ddr52", MMC_CAP_DDR},
> + [MMC_TIMING_MMC_HS200] = {"ti,otap-del-sel-hs200", MMC_CAP2_HS200},
> + [MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
> +};
> +
> static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> + unsigned char timing = host->mmc->ios.timing;
> int sel50, sel100, freqsel;
> + u32 otap_del_sel;
> + u32 otap_del_ena;
> u32 mask, val;
> int ret;
>
> @@ -116,22 +140,29 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>
> if (clock > CLOCK_TOO_SLOW_HZ) {
> /* Setup DLL Output TAP delay */
> + if (sdhci_am654->legacy_otapdly)
> + otap_del_sel = sdhci_am654->otap_del_sel[0];
> + else
> + otap_del_sel = sdhci_am654->otap_del_sel[timing];
> +
> + otap_del_ena = (timing > MMC_TIMING_UHS_SDR25) ? 1 : 0;
> +
> mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> - val = (1 << OTAPDLYENA_SHIFT) |
> - (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
> + val = (otap_del_ena << OTAPDLYENA_SHIFT) |
> + (otap_del_sel << OTAPDLYSEL_SHIFT);
> +
> /* Write to STRBSEL for HS400 speed mode */
> - if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400) {
> + if (timing == MMC_TIMING_MMC_HS400) {
> if (sdhci_am654->flags & STRBSEL_4_BIT)
> - mask = STRBSEL_4BIT_MASK;
> + mask |= STRBSEL_4BIT_MASK;
> else
> - mask = STRBSEL_8BIT_MASK;
> + mask |= STRBSEL_8BIT_MASK;
>
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask,
> - sdhci_am654->strb_sel <<
> - STRBSEL_SHIFT);
> + val |= sdhci_am654->strb_sel << STRBSEL_SHIFT;
> }
>
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
> +
> if (sdhci_am654->flags & FREQSEL_2_BIT) {
> switch (clock) {
> case 200000000:
> @@ -198,11 +229,19 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> - int val, mask;
> + unsigned char timing = host->mmc->ios.timing;
> + u32 otap_del_sel;
> + u32 mask, val;
> +
> + /* Setup DLL Output TAP delay */
> + if (sdhci_am654->legacy_otapdly)
> + otap_del_sel = sdhci_am654->otap_del_sel[0];
> + else
> + otap_del_sel = sdhci_am654->otap_del_sel[timing];
>
> mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> - val = (1 << OTAPDLYENA_SHIFT) |
> - (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT);
> + val = (0x1 << OTAPDLYENA_SHIFT) |
> + (otap_del_sel << OTAPDLYSEL_SHIFT);
> regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
>
> sdhci_set_clock(host, clock);
> @@ -371,6 +410,55 @@ static int sdhci_am654_cqe_add_host(struct sdhci_host *host)
> return ret;
> }
>
> +static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
> + struct sdhci_am654_data *sdhci_am654)
> +{
> + struct device *dev = mmc_dev(host->mmc);
> + int i;
> + int ret;
> +
> + ret = device_property_read_u32(dev, td[MMC_TIMING_LEGACY].binding,
> + &sdhci_am654->otap_del_sel[MMC_TIMING_LEGACY]);
> + if (ret) {
> + /*
> + * ti,otap-del-sel-legacy is mandatory, look for old binding
> + * if not found.
> + */
> + ret = device_property_read_u32(dev, "ti,otap-del-sel",
> + &sdhci_am654->otap_del_sel[0]);
> + if (ret) {
> + dev_err(dev, "Couldn't find otap-del-sel\n");
> +
> + return ret;
> + }
> +
> + dev_info(dev, "Using legacy binding ti,otap-del-sel\n");
> + sdhci_am654->legacy_otapdly = true;
> +
> + return 0;
> + }
> +
> + for (i = MMC_TIMING_MMC_HS; i <= MMC_TIMING_MMC_HS400; i++) {
> +
> + ret = device_property_read_u32(dev, td[i].binding,
> + &sdhci_am654->otap_del_sel[i]);
> + if (ret) {
> + dev_dbg(dev, "Couldn't find %s\n",
> + td[i].binding);
> + /*
> + * Remove the corresponding capability
> + * if an otap-del-sel value is not found
> + */
> + if (i <= MMC_TIMING_MMC_DDR52)
> + host->mmc->caps &= ~td[i].capability;
> + else
> + host->mmc->caps2 &= ~td[i].capability;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int sdhci_am654_init(struct sdhci_host *host)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -419,6 +507,10 @@ static int sdhci_am654_init(struct sdhci_host *host)
> if (ret)
> goto err_cleanup_host;
>
> + ret = sdhci_am654_get_otap_delay(host, sdhci_am654);
> + if (ret)
> + goto err_cleanup_host;
> +
> ret = __sdhci_add_host(host);
> if (ret)
> goto err_cleanup_host;
> @@ -437,11 +529,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
> int drv_strength;
> int ret;
>
> - ret = device_property_read_u32(dev, "ti,otap-del-sel",
> - &sdhci_am654->otap_del_sel);
> - if (ret)
> - return ret;
> -
> if (sdhci_am654->flags & DLL_PRESENT) {
> ret = device_property_read_u32(dev, "ti,trm-icp",
> &sdhci_am654->trm_icp);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index ba703384bea0..a22a10456c62 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -322,6 +322,8 @@ struct mmc_host {
> #define MMC_CAP_3_3V_DDR (1 << 11) /* Host supports eMMC DDR 3.3V */
> #define MMC_CAP_1_8V_DDR (1 << 12) /* Host supports eMMC DDR 1.8V */
> #define MMC_CAP_1_2V_DDR (1 << 13) /* Host supports eMMC DDR 1.2V */
> +#define MMC_CAP_DDR (MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
> + MMC_CAP_1_2V_DDR)
> #define MMC_CAP_POWER_OFF_CARD (1 << 14) /* Can power off after boot */
> #define MMC_CAP_BUS_WIDTH_TEST (1 << 15) /* CMD14/CMD19 bus width ok */
> #define MMC_CAP_UHS_SDR12 (1 << 16) /* Host supports UHS SDR12 mode */
>
On 8/01/20 5:09 pm, Faiz Abbas wrote:
> Its recommended that DLL must only be enabled for SDR50, DDR50, DDR52,
> SDR104, HS200 and HS400 speed modes. Move DLL configuration to its own
> function and call it only in the above speed modes.
>
> Signed-off-by: Faiz Abbas <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci_am654.c | 128 +++++++++++++++++----------------
> 1 file changed, 68 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index bb977de43f7d..575bbab1a6ed 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -119,16 +119,80 @@ static const struct timing_data td[] = {
> [MMC_TIMING_MMC_HS400] = {"ti,otap-del-sel-hs400", MMC_CAP2_HS400},
> };
>
> +static void sdhci_am654_setup_dll(struct sdhci_host *host, unsigned int clock)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> + int sel50, sel100, freqsel;
> + u32 mask, val;
> + int ret;
> +
> + if (sdhci_am654->flags & FREQSEL_2_BIT) {
> + switch (clock) {
> + case 200000000:
> + sel50 = 0;
> + sel100 = 0;
> + break;
> + case 100000000:
> + sel50 = 0;
> + sel100 = 1;
> + break;
> + default:
> + sel50 = 1;
> + sel100 = 0;
> + }
> +
> + /* Configure PHY DLL frequency */
> + mask = SEL50_MASK | SEL100_MASK;
> + val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val);
> +
> + } else {
> + switch (clock) {
> + case 200000000:
> + freqsel = 0x0;
> + break;
> + default:
> + freqsel = 0x4;
> + }
> +
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL5, FREQSEL_MASK,
> + freqsel << FREQSEL_SHIFT);
> + }
> + /* Configure DLL TRIM */
> + mask = DLL_TRIM_ICP_MASK;
> + val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
> +
> + /* Configure DLL driver strength */
> + mask |= DR_TY_MASK;
> + val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
> +
> + /* Enable DLL */
> + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
> + 0x1 << ENDLL_SHIFT);
> + /*
> + * Poll for DLL ready. Use a one second timeout.
> + * Works in all experiments done so far
> + */
> + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, val,
> + val & DLLRDY_MASK, 1000, 1000000);
> + if (ret) {
> + dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
> + return;
> + }
> +
> + sdhci_am654->dll_on = true;
> +}
> +
> static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> unsigned char timing = host->mmc->ios.timing;
> - int sel50, sel100, freqsel;
> u32 otap_del_sel;
> u32 otap_del_ena;
> u32 mask, val;
> - int ret;
>
> if (sdhci_am654->dll_on) {
> regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0);
> @@ -163,64 +227,8 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock)
>
> regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val);
>
> - if (sdhci_am654->flags & FREQSEL_2_BIT) {
> - switch (clock) {
> - case 200000000:
> - sel50 = 0;
> - sel100 = 0;
> - break;
> - case 100000000:
> - sel50 = 0;
> - sel100 = 1;
> - break;
> - default:
> - sel50 = 1;
> - sel100 = 0;
> - }
> -
> - /* Configure PHY DLL frequency */
> - mask = SEL50_MASK | SEL100_MASK;
> - val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT);
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask,
> - val);
> - } else {
> - switch (clock) {
> - case 200000000:
> - freqsel = 0x0;
> - break;
> - default:
> - freqsel = 0x4;
> - }
> -
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL5,
> - FREQSEL_MASK,
> - freqsel << FREQSEL_SHIFT);
> - }
> -
> - /* Configure DLL TRIM */
> - mask = DLL_TRIM_ICP_MASK;
> - val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT;
> -
> - /* Configure DLL driver strength */
> - mask |= DR_TY_MASK;
> - val |= sdhci_am654->drv_strength << DR_TY_SHIFT;
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val);
> - /* Enable DLL */
> - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK,
> - 0x1 << ENDLL_SHIFT);
> - /*
> - * Poll for DLL ready. Use a one second timeout.
> - * Works in all experiments done so far
> - */
> - ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1,
> - val, val & DLLRDY_MASK, 1000,
> - 1000000);
> - if (ret) {
> - dev_err(mmc_dev(host->mmc), "DLL failed to relock\n");
> - return;
> - }
> -
> - sdhci_am654->dll_on = true;
> + if (timing > MMC_TIMING_UHS_SDR25)
> + sdhci_am654_setup_dll(host, clock);
> }
> }
>
>
Rob,
On 20/01/20 11:00 am, Faiz Abbas wrote:
> Hi Rob,
>
> On 15/01/20 7:20 am, Rob Herring wrote:
>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>> According to latest AM65x Data Manual[1], a different output tap delay
>>> value is recommended for all speed modes. Therefore, replace the
>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>> mode.
>>>
>>> [1] http://www.ti.com/lit/gpn/am6526
>>>
>>> Signed-off-by: Faiz Abbas <[email protected]>
>>> ---
>>> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
>>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> index 50e87df47971..c6ccecb9ae5a 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>> @@ -18,7 +18,20 @@ Required Properties:
>>> - clocks: Handles to the clock inputs.
>>> - clock-names: Tuple including "clk_xin" and "clk_ahb"
>>> - interrupts: Interrupt specifiers
>>> - - ti,otap-del-sel: Output Tap Delay select
>>> + Output tap delay for each speed mode:
>>> + - ti,otap-del-sel-legacy
>>> + - ti,otap-del-sel-mmc-hs
>>> + - ti,otap-del-sel-sd-hs
>>> + - ti,otap-del-sel-sdr12
>>> + - ti,otap-del-sel-sdr25
>>> + - ti,otap-del-sel-sdr50
>>> + - ti,otap-del-sel-sdr104
>>> + - ti,otap-del-sel-ddr50
>>> + - ti,otap-del-sel-ddr52
>>> + - ti,otap-del-sel-hs200
>>> + - ti,otap-del-sel-hs400
>>> + These bindings must be provided otherwise the driver will disable the
>>> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>
>> Why not just extend the existing property to be an array. We already
>> have properties to enable/disable speed modes.
>>
>
> Its hard to keep track of which modes have values and which don't when
> you add an array. This scheme is just easier on anyone adding new values
> or updating old values.
>
> We already disable speed modes based on platform specific properties in
> other drivers. In sdhci-omap.c, the driver disables the corresponding
> speed mode if the corresponding pinmux and iodelay values are not populated.
>
Do you agree on above?
Thanks,
Faiz
Rob,
On 07/02/20 3:07 pm, Faiz Abbas wrote:
> Rob,
>
> On 20/01/20 11:00 am, Faiz Abbas wrote:
>> Hi Rob,
>>
>> On 15/01/20 7:20 am, Rob Herring wrote:
>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>> value is recommended for all speed modes. Therefore, replace the
>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>> mode.
>>>>
>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>
>>>> Signed-off-by: Faiz Abbas <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
>>>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>> - clocks: Handles to the clock inputs.
>>>> - clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>> - interrupts: Interrupt specifiers
>>>> - - ti,otap-del-sel: Output Tap Delay select
>>>> + Output tap delay for each speed mode:
>>>> + - ti,otap-del-sel-legacy
>>>> + - ti,otap-del-sel-mmc-hs
>>>> + - ti,otap-del-sel-sd-hs
>>>> + - ti,otap-del-sel-sdr12
>>>> + - ti,otap-del-sel-sdr25
>>>> + - ti,otap-del-sel-sdr50
>>>> + - ti,otap-del-sel-sdr104
>>>> + - ti,otap-del-sel-ddr50
>>>> + - ti,otap-del-sel-ddr52
>>>> + - ti,otap-del-sel-hs200
>>>> + - ti,otap-del-sel-hs400
>>>> + These bindings must be provided otherwise the driver will disable the
>>>> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>
>>> Why not just extend the existing property to be an array. We already
>>> have properties to enable/disable speed modes.
>>>
>>
>> Its hard to keep track of which modes have values and which don't when
>> you add an array. This scheme is just easier on anyone adding new values
>> or updating old values.
>>
>> We already disable speed modes based on platform specific properties in
>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>
>
> Do you agree on above?
>
Gentle ping.
Thanks,
Faiz
Rob,
On 14/02/20 4:28 pm, Faiz Abbas wrote:
> Rob,
>
> On 07/02/20 3:07 pm, Faiz Abbas wrote:
>> Rob,
>>
>> On 20/01/20 11:00 am, Faiz Abbas wrote:
>>> Hi Rob,
>>>
>>> On 15/01/20 7:20 am, Rob Herring wrote:
>>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>>> value is recommended for all speed modes. Therefore, replace the
>>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>>> mode.
>>>>>
>>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>>
>>>>> Signed-off-by: Faiz Abbas <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
>>>>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>>> - clocks: Handles to the clock inputs.
>>>>> - clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>>> - interrupts: Interrupt specifiers
>>>>> - - ti,otap-del-sel: Output Tap Delay select
>>>>> + Output tap delay for each speed mode:
>>>>> + - ti,otap-del-sel-legacy
>>>>> + - ti,otap-del-sel-mmc-hs
>>>>> + - ti,otap-del-sel-sd-hs
>>>>> + - ti,otap-del-sel-sdr12
>>>>> + - ti,otap-del-sel-sdr25
>>>>> + - ti,otap-del-sel-sdr50
>>>>> + - ti,otap-del-sel-sdr104
>>>>> + - ti,otap-del-sel-ddr50
>>>>> + - ti,otap-del-sel-ddr52
>>>>> + - ti,otap-del-sel-hs200
>>>>> + - ti,otap-del-sel-hs400
>>>>> + These bindings must be provided otherwise the driver will disable the
>>>>> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>>
>>>> Why not just extend the existing property to be an array. We already
>>>> have properties to enable/disable speed modes.
>>>>
>>>
>>> Its hard to keep track of which modes have values and which don't when
>>> you add an array. This scheme is just easier on anyone adding new values
>>> or updating old values.
>>>
>>> We already disable speed modes based on platform specific properties in
>>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>>
>>
>> Do you agree on above?
>>
>
> Gentle ping.
>
Ping.
Thanks,
Faiz
Uffe,
On 08/01/20 8:39 pm, Faiz Abbas wrote:
> The following patches update phy configurations for AM65x as given in
> the latest data manual.
>
> The patches depend on my fixes series posted just before this:
> https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
>
> Device tree patch updating the actual otap values will be posted
> separately.
>
> Tested with Am65x-evm and J721e-evm.
>
> Faiz Abbas (3):
> dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
> mmc: sdhci_am654: Update OTAPDLY writes
> mmc: sdhci_am654: Enable DLL only for some speed modes
>
> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +-
> drivers/mmc/host/sdhci_am654.c | 247 ++++++++++++------
> include/linux/mmc/host.h | 2 +
> 3 files changed, 192 insertions(+), 78 deletions(-)
>
Can you help merge this?
Thanks,
Faiz
On Mon, 2 Mar 2020 at 20:11, Faiz Abbas <[email protected]> wrote:
>
> Uffe,
>
> On 08/01/20 8:39 pm, Faiz Abbas wrote:
> > The following patches update phy configurations for AM65x as given in
> > the latest data manual.
> >
> > The patches depend on my fixes series posted just before this:
> > https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
> >
> > Device tree patch updating the actual otap values will be posted
> > separately.
> >
> > Tested with Am65x-evm and J721e-evm.
> >
> > Faiz Abbas (3):
> > dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
> > mmc: sdhci_am654: Update OTAPDLY writes
> > mmc: sdhci_am654: Enable DLL only for some speed modes
> >
> > .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +-
> > drivers/mmc/host/sdhci_am654.c | 247 ++++++++++++------
> > include/linux/mmc/host.h | 2 +
> > 3 files changed, 192 insertions(+), 78 deletions(-)
> >
>
> Can you help merge this?
Apologize with the delay, still focused on fixing various regressions in v5.6.
I start catching up on my mmc backlog as of tomorrow. Thanks for pinging me.
Kind regards
Uffe
On Tue, 3 Mar 2020 at 21:53, Ulf Hansson <[email protected]> wrote:
>
> On Mon, 2 Mar 2020 at 20:11, Faiz Abbas <[email protected]> wrote:
> >
> > Uffe,
> >
> > On 08/01/20 8:39 pm, Faiz Abbas wrote:
> > > The following patches update phy configurations for AM65x as given in
> > > the latest data manual.
> > >
> > > The patches depend on my fixes series posted just before this:
> > > https://patchwork.kernel.org/project/linux-mmc/list/?series=225425
> > >
> > > Device tree patch updating the actual otap values will be posted
> > > separately.
> > >
> > > Tested with Am65x-evm and J721e-evm.
> > >
> > > Faiz Abbas (3):
> > > dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
> > > mmc: sdhci_am654: Update OTAPDLY writes
> > > mmc: sdhci_am654: Enable DLL only for some speed modes
> > >
> > > .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +-
> > > drivers/mmc/host/sdhci_am654.c | 247 ++++++++++++------
> > > include/linux/mmc/host.h | 2 +
> > > 3 files changed, 192 insertions(+), 78 deletions(-)
> > >
> >
> > Can you help merge this?
>
> Apologize with the delay, still focused on fixing various regressions in v5.6.
>
> I start catching up on my mmc backlog as of tomorrow. Thanks for pinging me.
Applied for next, thanks!
Kind regards
Uffe