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

[Jaehoon Chung <jh80.chung@gmail.com>]

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