[PATCH 0/2] mmc: Add support for F_SDH30_E51

[PATCH 0/2] mmc: Add support for F_SDH30_E51

[Kunihiko Hayashi]

This series adds a new quirk "SUPPORT_SINGLE" for single transaction to
sdhci framework and Socionext F_SDH30_E51 IP support to f_sdh30 driver.

Kunihiko Hayashi (2):
  mmc: sdhci: Add new quirks for SUPPORT_SINGLE
  mmc: f_sdh30: Add support for F_SDH30_E51

 drivers/mmc/Kconfig   |  4 +--
 drivers/mmc/f_sdh30.c | 64 +++++++++++++++++++++++++++++++++++++++++--
 drivers/mmc/sdhci.c   |  8 ++++--
 include/sdhci.h       |  1 +
 4 files changed, 70 insertions(+), 7 deletions(-)

-- 
2.17.1

[PATCH 1/2] mmc: sdhci: Add new quirks for SUPPORT_SINGLE

[Kunihiko Hayashi]

This patch defines a quirk to disable the block count
for single block transactions.

This is similar to Linux kernel commit d3fc5d71ac4d
("mmc: sdhci: add a quirk for single block transactions").

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/mmc/sdhci.c | 8 +++++---
 include/sdhci.h     | 1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index bf989a594f7e..a80ad8329a38 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -211,7 +211,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 	unsigned int stat = 0;
 	int ret = 0;
 	int trans_bytes = 0, is_aligned = 1;
-	u32 mask, flags, mode;
+	u32 mask, flags, mode = 0;
 	unsigned int time = 0;
 	int mmc_dev = mmc_get_blk_desc(mmc)->devnum;
 	ulong start = get_timer(0);
@@ -273,10 +273,12 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 	/* Set Transfer mode regarding to data flag */
 	if (data) {
 		sdhci_writeb(host, 0xe, SDHCI_TIMEOUT_CONTROL);
-		mode = SDHCI_TRNS_BLK_CNT_EN;
+
+		if (!(host->quirks & SDHCI_QUIRK_SUPPORT_SINGLE))
+			mode = SDHCI_TRNS_BLK_CNT_EN;
 		trans_bytes = data->blocks * data->blocksize;
 		if (data->blocks > 1)
-			mode |= SDHCI_TRNS_MULTI;
+			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_BLK_CNT_EN;
 
 		if (data->flags == MMC_DATA_READ)
 			mode |= SDHCI_TRNS_READ;
diff --git a/include/sdhci.h b/include/sdhci.h
index 88f1917480b6..24b4599b857d 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -247,6 +247,7 @@
 #define SDHCI_QUIRK_WAIT_SEND_CMD	(1 << 6)
 #define SDHCI_QUIRK_USE_WIDE8		(1 << 8)
 #define SDHCI_QUIRK_NO_1_8_V		(1 << 9)
+#define SDHCI_QUIRK_SUPPORT_SINGLE	(1 << 10)
 
 /* to make gcc happy */
 struct sdhci_host;
-- 
2.17.1

[PATCH 2/2] mmc: f_sdh30: Add support for F_SDH30_E51

[Kunihiko Hayashi]

Add Socionext F_SDH30_E51 IP support. The features of this IP includes
CMD/DAT line delay and force card insertion mode for non-removable cards.
And the IP needs to add some quirks.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/mmc/Kconfig   |  4 +--
 drivers/mmc/f_sdh30.c | 64 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 0dcec8adcee8..c30f20cba5f2 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -577,12 +577,12 @@ config MMC_SDHCI_IPROC
 	  If unsure, say N.
 
 config MMC_SDHCI_F_SDH30
-	bool "SDHCI support for Fujitsu Semiconductor F_SDH30"
+	bool "SDHCI support for Fujitsu Semiconductor/Socionext F_SDH30"
 	depends on BLK && DM_MMC
 	depends on MMC_SDHCI
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
-	  Needed by some Fujitsu SoC for MMC / SD / SDIO support.
+	  Needed by some Fujitsu/Socionext SoC for MMC / SD / SDIO support.
 	  If you have a controller with this interface, say Y or M here.
 	  If unsure, say N.
 
diff --git a/drivers/mmc/f_sdh30.c b/drivers/mmc/f_sdh30.c
index 3a85d9e348ab..6b950edab74e 100644
--- a/drivers/mmc/f_sdh30.c
+++ b/drivers/mmc/f_sdh30.c
@@ -11,13 +11,46 @@
 #include <malloc.h>
 #include <sdhci.h>
 
+#define F_SDH30_ESD_CONTROL		0x124
+#define F_SDH30_CMD_DAT_DELAY		BIT(9)
+
+#define F_SDH30_TEST			0x158
+#define F_SDH30_FORCE_CARD_INSERT	BIT(6)
+
+struct f_sdh30_data {
+	void (*init)(struct udevice *dev);
+	u32 quirks;
+};
+
 struct f_sdh30_plat {
 	struct mmc_config cfg;
 	struct mmc mmc;
+
+	bool enable_cmd_dat_delay;
+	const struct f_sdh30_data *data;
 };
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static void f_sdh30_e51_init(struct udevice *dev)
+{
+	struct f_sdh30_plat *plat = dev_get_plat(dev);
+	struct sdhci_host *host = dev_get_priv(dev);
+	u32 val;
+
+	if (plat->enable_cmd_dat_delay) {
+		val = sdhci_readl(host, F_SDH30_ESD_CONTROL);
+		val |= F_SDH30_CMD_DAT_DELAY;
+		sdhci_writel(host, val, F_SDH30_ESD_CONTROL);
+	}
+
+	if (plat->cfg.host_caps & MMC_CAP_NONREMOVABLE) {
+		val = sdhci_readl(host, F_SDH30_TEST);
+		val |= F_SDH30_FORCE_CARD_INSERT;
+		sdhci_writel(host, val, F_SDH30_TEST);
+	}
+}
+
 static int f_sdh30_sdhci_probe(struct udevice *dev)
 {
 	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
@@ -25,6 +58,8 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
 	struct sdhci_host *host = dev_get_priv(dev);
 	int ret;
 
+	plat->data = (const struct f_sdh30_data *)dev_get_driver_data(dev);
+
 	ret = mmc_of_parse(dev, &plat->cfg);
 	if (ret)
 		return ret;
@@ -33,6 +68,9 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
 	host->mmc->dev = dev;
 	host->mmc->priv = host;
 
+	if (plat->data && plat->data->quirks)
+		host->quirks |= plat->data->quirks;
+
 	ret = sdhci_setup_cfg(&plat->cfg, host, 200000000, 400000);
 	if (ret)
 		return ret;
@@ -41,18 +79,29 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
 
 	mmc_set_clock(host->mmc, host->mmc->cfg->f_min, MMC_CLK_ENABLE);
 
-	return sdhci_probe(dev);
+	ret = sdhci_probe(dev);
+	if (ret)
+		return ret;
+
+	if (plat->data && plat->data->init)
+		plat->data->init(dev);
+
+	return 0;
 }
 
 static int f_sdh30_of_to_plat(struct udevice *dev)
 {
 	struct sdhci_host *host = dev_get_priv(dev);
+	struct f_sdh30_plat *plat = dev_get_plat(dev);
 
 	host->name = strdup(dev->name);
 	host->ioaddr = dev_read_addr_ptr(dev);
 	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
 	host->index = dev_read_u32_default(dev, "index", 0);
 
+	plat->enable_cmd_dat_delay =
+		dev_read_bool(dev, "socionext,enable-cmd-dat-delay");
+
 	return 0;
 }
 
@@ -63,8 +112,19 @@ static int f_sdh30_bind(struct udevice *dev)
 	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
 }
 
+static const struct f_sdh30_data f_sdh30_e51_data = {
+	.init = f_sdh30_e51_init,
+	.quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_SUPPORT_SINGLE,
+};
+
 static const struct udevice_id f_sdh30_mmc_ids[] = {
-	{ .compatible = "fujitsu,mb86s70-sdhci-3.0" },
+	{
+		.compatible = "fujitsu,mb86s70-sdhci-3.0",
+	},
+	{
+		.compatible = "socionext,f-sdh30-e51-mmc",
+		.data = (ulong)&f_sdh30_e51_data,
+	},
 	{ }
 };
 
-- 
2.17.1

Re: [PATCH 1/2] mmc: sdhci: Add new quirks for SUPPORT_SINGLE

[Jaehoon Chung]

Hi,

On 9/6/22 09:39, Kunihiko Hayashi wrote:
> This patch defines a quirk to disable the block count
> for single block transactions.
> 
> This is similar to Linux kernel commit d3fc5d71ac4d
> ("mmc: sdhci: add a quirk for single block transactions").
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/sdhci.c | 8 +++++---
>  include/sdhci.h     | 1 +
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index bf989a594f7e..a80ad8329a38 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -211,7 +211,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
>  	unsigned int stat = 0;
>  	int ret = 0;
>  	int trans_bytes = 0, is_aligned = 1;
> -	u32 mask, flags, mode;
> +	u32 mask, flags, mode = 0;
>  	unsigned int time = 0;
>  	int mmc_dev = mmc_get_blk_desc(mmc)->devnum;
>  	ulong start = get_timer(0);
> @@ -273,10 +273,12 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
>  	/* Set Transfer mode regarding to data flag */
>  	if (data) {
>  		sdhci_writeb(host, 0xe, SDHCI_TIMEOUT_CONTROL);
> -		mode = SDHCI_TRNS_BLK_CNT_EN;
> +
> +		if (!(host->quirks & SDHCI_QUIRK_SUPPORT_SINGLE))
> +			mode = SDHCI_TRNS_BLK_CNT_EN;
>  		trans_bytes = data->blocks * data->blocksize;
>  		if (data->blocks > 1)
> -			mode |= SDHCI_TRNS_MULTI;
> +			mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_BLK_CNT_EN;
>  
>  		if (data->flags == MMC_DATA_READ)
>  			mode |= SDHCI_TRNS_READ;
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 88f1917480b6..24b4599b857d 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -247,6 +247,7 @@
>  #define SDHCI_QUIRK_WAIT_SEND_CMD	(1 << 6)
>  #define SDHCI_QUIRK_USE_WIDE8		(1 << 8)
>  #define SDHCI_QUIRK_NO_1_8_V		(1 << 9)
> +#define SDHCI_QUIRK_SUPPORT_SINGLE	(1 << 10)
>  
>  /* to make gcc happy */
>  struct sdhci_host;

Re: [PATCH 2/2] mmc: f_sdh30: Add support for F_SDH30_E51

[Jaehoon Chung]

On 9/6/22 09:39, Kunihiko Hayashi wrote:
> Add Socionext F_SDH30_E51 IP support. The features of this IP includes
> CMD/DAT line delay and force card insertion mode for non-removable cards.
> And the IP needs to add some quirks.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>  drivers/mmc/Kconfig   |  4 +--
>  drivers/mmc/f_sdh30.c | 64 +++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 0dcec8adcee8..c30f20cba5f2 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -577,12 +577,12 @@ config MMC_SDHCI_IPROC
>  	  If unsure, say N.
>  
>  config MMC_SDHCI_F_SDH30
> -	bool "SDHCI support for Fujitsu Semiconductor F_SDH30"
> +	bool "SDHCI support for Fujitsu Semiconductor/Socionext F_SDH30"
>  	depends on BLK && DM_MMC
>  	depends on MMC_SDHCI
>  	help
>  	  This selects the Secure Digital Host Controller Interface (SDHCI)
> -	  Needed by some Fujitsu SoC for MMC / SD / SDIO support.
> +	  Needed by some Fujitsu/Socionext SoC for MMC / SD / SDIO support.
>  	  If you have a controller with this interface, say Y or M here.
>  	  If unsure, say N.
>  
> diff --git a/drivers/mmc/f_sdh30.c b/drivers/mmc/f_sdh30.c
> index 3a85d9e348ab..6b950edab74e 100644
> --- a/drivers/mmc/f_sdh30.c
> +++ b/drivers/mmc/f_sdh30.c
> @@ -11,13 +11,46 @@
>  #include <malloc.h>
>  #include <sdhci.h>
>  
> +#define F_SDH30_ESD_CONTROL		0x124
> +#define F_SDH30_CMD_DAT_DELAY		BIT(9)
> +
> +#define F_SDH30_TEST			0x158
> +#define F_SDH30_FORCE_CARD_INSERT	BIT(6)
> +
> +struct f_sdh30_data {
> +	void (*init)(struct udevice *dev);
> +	u32 quirks;
> +};
> +
>  struct f_sdh30_plat {
>  	struct mmc_config cfg;
>  	struct mmc mmc;
> +
> +	bool enable_cmd_dat_delay;
> +	const struct f_sdh30_data *data;
>  };
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +static void f_sdh30_e51_init(struct udevice *dev)
> +{
> +	struct f_sdh30_plat *plat = dev_get_plat(dev);
> +	struct sdhci_host *host = dev_get_priv(dev);
> +	u32 val;
> +
> +	if (plat->enable_cmd_dat_delay) {
> +		val = sdhci_readl(host, F_SDH30_ESD_CONTROL);
> +		val |= F_SDH30_CMD_DAT_DELAY;

Is there a case to set its regardless of enable_cmd_dat_delay?

how about below?

if (plat->enable_cmd_dat_delay)
	val |= F_SDH30_CMD_DAT_DELAY;
else
	val &= ~F_SDH30_CMD_DAT_DELAY;


> +		sdhci_writel(host, val, F_SDH30_ESD_CONTROL);
> +	}
> +
> +	if (plat->cfg.host_caps & MMC_CAP_NONREMOVABLE) {
> +		val = sdhci_readl(host, F_SDH30_TEST);
> +		val |= F_SDH30_FORCE_CARD_INSERT;

Ditto.

> +		sdhci_writel(host, val, F_SDH30_TEST);
> +	}
> +}
> +
>  static int f_sdh30_sdhci_probe(struct udevice *dev)
>  {
>  	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> @@ -25,6 +58,8 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
>  	struct sdhci_host *host = dev_get_priv(dev);
>  	int ret;
>  
> +	plat->data = (const struct f_sdh30_data *)dev_get_driver_data(dev);
> +
>  	ret = mmc_of_parse(dev, &plat->cfg);
>  	if (ret)
>  		return ret;
> @@ -33,6 +68,9 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
>  	host->mmc->dev = dev;
>  	host->mmc->priv = host;
>  
> +	if (plat->data && plat->data->quirks)
> +		host->quirks |= plat->data->quirks;

Doesn't need to use "|" ? 

Best Regards,
Jaehoon Chung


> +
>  	ret = sdhci_setup_cfg(&plat->cfg, host, 200000000, 400000);
>  	if (ret)
>  		return ret;
> @@ -41,18 +79,29 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
>  
>  	mmc_set_clock(host->mmc, host->mmc->cfg->f_min, MMC_CLK_ENABLE);
>  
> -	return sdhci_probe(dev);
> +	ret = sdhci_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (plat->data && plat->data->init)
> +		plat->data->init(dev);
> +
> +	return 0;
>  }
>  
>  static int f_sdh30_of_to_plat(struct udevice *dev)
>  {
>  	struct sdhci_host *host = dev_get_priv(dev);
> +	struct f_sdh30_plat *plat = dev_get_plat(dev);
>  
>  	host->name = strdup(dev->name);
>  	host->ioaddr = dev_read_addr_ptr(dev);
>  	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
>  	host->index = dev_read_u32_default(dev, "index", 0);
>  
> +	plat->enable_cmd_dat_delay =
> +		dev_read_bool(dev, "socionext,enable-cmd-dat-delay");
> +
>  	return 0;
>  }
>  
> @@ -63,8 +112,19 @@ static int f_sdh30_bind(struct udevice *dev)
>  	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
>  }
>  
> +static const struct f_sdh30_data f_sdh30_e51_data = {
> +	.init = f_sdh30_e51_init,
> +	.quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_SUPPORT_SINGLE,
> +};
> +
>  static const struct udevice_id f_sdh30_mmc_ids[] = {
> -	{ .compatible = "fujitsu,mb86s70-sdhci-3.0" },
> +	{
> +		.compatible = "fujitsu,mb86s70-sdhci-3.0",
> +	},
> +	{
> +		.compatible = "socionext,f-sdh30-e51-mmc",
> +		.data = (ulong)&f_sdh30_e51_data,
> +	},
>  	{ }
>  };
>  

Re: [PATCH 2/2] mmc: f_sdh30: Add support for F_SDH30_E51

[Kunihiko Hayashi]

Hi Jaehoon,
Thank you for checking.

On 2022/09/08 20:35, Jaehoon Chung wrote:
> 
> 
> On 9/6/22 09:39, Kunihiko Hayashi wrote:
>> Add Socionext F_SDH30_E51 IP support. The features of this IP includes
>> CMD/DAT line delay and force card insertion mode for non-removable cards.
>> And the IP needs to add some quirks.
>>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>> ---
>>   drivers/mmc/Kconfig   |  4 +--
>>   drivers/mmc/f_sdh30.c | 64 +++++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 64 insertions(+), 4 deletions(-)

(snip)

>> +static void f_sdh30_e51_init(struct udevice *dev)
>> +{
>> +	struct f_sdh30_plat *plat = dev_get_plat(dev);
>> +	struct sdhci_host *host = dev_get_priv(dev);
>> +	u32 val;
>> +
>> +	if (plat->enable_cmd_dat_delay) {
>> +		val = sdhci_readl(host, F_SDH30_ESD_CONTROL);
>> +		val |= F_SDH30_CMD_DAT_DELAY;
> 
> Is there a case to set its regardless of enable_cmd_dat_delay?
> 
> how about below?
> 
> if (plat->enable_cmd_dat_delay)
> 	val |= F_SDH30_CMD_DAT_DELAY;
> else
> 	val &= ~F_SDH30_CMD_DAT_DELAY;

Yes, I forgot the false case. Need to add it.

>> +		sdhci_writel(host, val, F_SDH30_ESD_CONTROL);
>> +	}
>> +
>> +	if (plat->cfg.host_caps & MMC_CAP_NONREMOVABLE) {
>> +		val = sdhci_readl(host, F_SDH30_TEST);
>> +		val |= F_SDH30_FORCE_CARD_INSERT;
> 
> Ditto.

I'll fix it too.

>> +		sdhci_writel(host, val, F_SDH30_TEST);
>> +	}
>> +}
>> +
>>   static int f_sdh30_sdhci_probe(struct udevice *dev)
>>   {
>>   	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> @@ -25,6 +58,8 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
>>   	struct sdhci_host *host = dev_get_priv(dev);
>>   	int ret;
>>   
>> +	plat->data = (const struct f_sdh30_data *)dev_get_driver_data(dev);
>> +
>>   	ret = mmc_of_parse(dev, &plat->cfg);
>>   	if (ret)
>>   		return ret;
>> @@ -33,6 +68,9 @@ static int f_sdh30_sdhci_probe(struct udevice *dev)
>>   	host->mmc->dev = dev;
>>   	host->mmc->priv = host;
>>   
>> +	if (plat->data && plat->data->quirks)
>> +		host->quirks |= plat->data->quirks;
> 
> Doesn't need to use "|" ?

I thought there was some premise in "host", however, this is a private
structure allocated by calloc() before probing the driver.
I can remove "|".

Thank you,

---
Best Regards
Kunihiko Hayashi