[U-Boot] [PATCH v2 1/4] mmc: sdhci: Distinguish between base clock and maximum peripheral frequency

Jaehoon Chung jh80.chung at samsung.com
Thu Dec 1 07:42:57 CET 2016


On 11/25/2016 10:59 PM, Stefan Herbrechtsmeier wrote:
> The sdhci controller assumes that the base clock frequency is fully supported by
> the peripheral and doesn't support hardware limitations. The Linux kernel
> distinguishes between base clock (max_clk) of the host controller and maximum
> frequency (f_max) of the card interface. Use the same differentiation and allow
> the platform to constrain the peripheral interface.
> 
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.de>
> ---
> 
> Changes in v2: None
> 
>  drivers/mmc/atmel_sdhci.c    |  7 +++++--
>  drivers/mmc/bcm2835_sdhci.c  |  3 ++-
>  drivers/mmc/ftsdc021_sdhci.c |  3 ++-
>  drivers/mmc/kona_sdhci.c     |  3 ++-
>  drivers/mmc/msm_sdhci.c      |  2 ++
>  drivers/mmc/mv_sdhci.c       |  3 ++-
>  drivers/mmc/pci_mmc.c        |  1 +
>  drivers/mmc/pic32_sdhci.c    |  4 +++-
>  drivers/mmc/rockchip_sdhci.c |  4 ++--
>  drivers/mmc/s5p_sdhci.c      |  5 +++--
>  drivers/mmc/sdhci.c          | 28 +++++++++++++++-------------
>  drivers/mmc/spear_sdhci.c    |  3 ++-
>  drivers/mmc/zynq_sdhci.c     |  4 +++-
>  include/sdhci.h              | 13 +++++++------
>  14 files changed, 51 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> index 6654b54..62cb242 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -35,8 +35,9 @@ int atmel_sdhci_init(void *regbase, u32 id)
>  		free(host);
>  		return -ENODEV;
>  	}
> +	host->max_clk = max_clk;
>  
> -	add_sdhci(host, max_clk, min_clk);
> +	add_sdhci(host, 0, min_clk);
>  
>  	return 0;
>  }
> @@ -95,7 +96,9 @@ static int atmel_sdhci_probe(struct udevice *dev)
>  	if (!max_clk)
>  		return -EINVAL;
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, max_clk, ATMEL_SDHC_MIN_FREQ);
> +	host->max_clk = max_clk;
> +
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0, ATMEL_SDHC_MIN_FREQ);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
> index cb2bd40..29c2a85 100644
> --- a/drivers/mmc/bcm2835_sdhci.c
> +++ b/drivers/mmc/bcm2835_sdhci.c
> @@ -181,10 +181,11 @@ int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
>  	host->ioaddr = (void *)(unsigned long)regbase;
>  	host->quirks = SDHCI_QUIRK_BROKEN_VOLTAGE | SDHCI_QUIRK_BROKEN_R1B |
>  		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_NO_HISPD_BIT;
> +	host->max_clk = emmc_freq;
>  	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  	host->ops = &bcm2835_ops;
>  
> -	add_sdhci(host, emmc_freq, MIN_FREQ);
> +	add_sdhci(host, 0, MIN_FREQ);
>  
>  	return 0;
>  }
> diff --git a/drivers/mmc/ftsdc021_sdhci.c b/drivers/mmc/ftsdc021_sdhci.c
> index 6e9fefa..4940ccb 100644
> --- a/drivers/mmc/ftsdc021_sdhci.c
> +++ b/drivers/mmc/ftsdc021_sdhci.c
> @@ -27,7 +27,8 @@ int ftsdc021_sdhci_init(u32 regbase)
>  	host->name = "FTSDC021";
>  	host->ioaddr = (void __iomem *)regbase;
>  	host->quirks = 0;
> -	add_sdhci(host, freq, 0);
> +	host->max_clk = freq;
> +	add_sdhci(host, 0, 0);
>  
>  	return 0;
>  }
> diff --git a/drivers/mmc/kona_sdhci.c b/drivers/mmc/kona_sdhci.c
> index 549f6bc..ddd821b 100644
> --- a/drivers/mmc/kona_sdhci.c
> +++ b/drivers/mmc/kona_sdhci.c
> @@ -121,12 +121,13 @@ int kona_sdhci_init(int dev_index, u32 min_clk, u32 quirks)
>  	host->name = "kona-sdhci";
>  	host->ioaddr = reg_base;
>  	host->quirks = quirks;
> +	host->max_clk = max_clk;
>  
>  	if (init_kona_mmc_core(host)) {
>  		free(host);
>  		return -EINVAL;
>  	}
>  
> -	add_sdhci(host, max_clk, min_clk);
> +	add_sdhci(host, 0, min_clk);
>  	return ret;
>  }
> diff --git a/drivers/mmc/msm_sdhci.c b/drivers/mmc/msm_sdhci.c
> index f33714b..1db683d 100644
> --- a/drivers/mmc/msm_sdhci.c
> +++ b/drivers/mmc/msm_sdhci.c
> @@ -96,6 +96,8 @@ static int msm_sdc_probe(struct udevice *dev)
>  
>  	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_BROKEN_R1B;
>  
> +	host->max_clk = 0;
> +
>  	/* Init clocks */
>  	ret = msm_sdc_clk_init(dev);
>  	if (ret)
> diff --git a/drivers/mmc/mv_sdhci.c b/drivers/mmc/mv_sdhci.c
> index e388ad1..69aa87b 100644
> --- a/drivers/mmc/mv_sdhci.c
> +++ b/drivers/mmc/mv_sdhci.c
> @@ -77,6 +77,7 @@ int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks)
>  	host->name = MVSDH_NAME;
>  	host->ioaddr = (void *)regbase;
>  	host->quirks = quirks;
> +	host->max_clk = max_clk;
>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>  	memset(&mv_ops, 0, sizeof(struct sdhci_ops));
>  	mv_ops.write_b = mv_sdhci_writeb;
> @@ -88,5 +89,5 @@ int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks)
>  		sdhci_mvebu_mbus_config((void __iomem *)regbase);
>  	}
>  
> -	return add_sdhci(host, max_clk, min_clk);
> +	return add_sdhci(host, 0, min_clk);
>  }
> diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c
> index 3d587cc..e39b476 100644
> --- a/drivers/mmc/pci_mmc.c
> +++ b/drivers/mmc/pci_mmc.c
> @@ -32,6 +32,7 @@ int pci_mmc_init(const char *name, struct pci_device_id *mmc_supported)
>  		dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0, &iobase);
>  		mmc_host->ioaddr = (void *)(ulong)iobase;
>  		mmc_host->quirks = 0;
> +		mmc_host->max_clk = 0;
>  		ret = add_sdhci(mmc_host, 0, 0);
>  		if (ret)
>  			return ret;
> diff --git a/drivers/mmc/pic32_sdhci.c b/drivers/mmc/pic32_sdhci.c
> index 2abf943..c562aec 100644
> --- a/drivers/mmc/pic32_sdhci.c
> +++ b/drivers/mmc/pic32_sdhci.c
> @@ -41,7 +41,9 @@ static int pic32_sdhci_probe(struct udevice *dev)
>  		return ret;
>  	}
>  
> -	ret = add_sdhci(host, f_min_max[1], f_min_max[0]);
> +	host->max_clk   = f_min_max[1];
> +
> +	ret = add_sdhci(host, 0, f_min_max[0]);
>  	if (ret)
>  		return ret;
>  	host->mmc->dev = dev;
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index c56e1a3..4456a02 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -35,9 +35,9 @@ static int arasan_sdhci_probe(struct udevice *dev)
>  	int ret;
>  
>  	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
> +	host->max_clk = CONFIG_ROCKCHIP_SDHCI_MAX_FREQ;
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ,
> -			EMMC_MIN_FREQ);
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);
>  
>  	host->mmc = &plat->mmc;
>  	if (ret)
> diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
> index b329bef..50bd8dc 100644
> --- a/drivers/mmc/s5p_sdhci.c
> +++ b/drivers/mmc/s5p_sdhci.c
> @@ -80,6 +80,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
>  	host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
>  		SDHCI_QUIRK_32BIT_DMA_ADDR |
>  		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_USE_WIDE8;
> +	host->max_clk = 52000000;
>  	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  
>  	host->set_control_reg = &s5p_sdhci_set_control_reg;
> @@ -89,7 +90,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
>  		host->host_caps |= MMC_MODE_8BIT;
>  
>  #ifndef CONFIG_BLK
> -	return add_sdhci(host, 52000000, 400000);
> +	return add_sdhci(host, 0, 400000);
>  #else
>  	return 0;
>  #endif
> @@ -245,7 +246,7 @@ static int s5p_sdhci_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, 52000000, 400000);
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 400000);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 766e9ee..d57c4dc 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -325,7 +325,7 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>  		 */
>  		if (host->clk_mul) {
>  			for (div = 1; div <= 1024; div++) {
> -				if ((mmc->cfg->f_max * host->clk_mul / div)
> +				if ((host->max_clk * host->clk_mul / div)
>  					<= clock)
>  					break;
>  			}
> @@ -338,13 +338,13 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>  			div--;
>  		} else {
>  			/* Version 3.00 divisors must be a multiple of 2. */
> -			if (mmc->cfg->f_max <= clock) {
> +			if (host->max_clk <= clock) {
>  				div = 1;
>  			} else {
>  				for (div = 2;
>  				     div < SDHCI_MAX_DIV_SPEC_300;
>  				     div += 2) {
> -					if ((mmc->cfg->f_max / div) <= clock)
> +					if ((host->max_clk / div) <= clock)
>  						break;
>  				}
>  			}
> @@ -353,7 +353,7 @@ static int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
>  	} else {
>  		/* Version 2.00 divisors must be a power of 2. */
>  		for (div = 1; div < SDHCI_MAX_DIV_SPEC_200; div *= 2) {
> -			if ((mmc->cfg->f_max / div) <= clock)
> +			if ((host->max_clk / div) <= clock)
>  				break;
>  		}
>  		div >>= 1;
> @@ -557,22 +557,24 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,

If my understanding is right, f_max and f_min should be passed.
You changed the argument's names of sdhci_setup_cfg() in include/sdhci.h.
(from max_clk/min_clk to f_max/f_min)

So i think you need to change the argument's name for preventing a confusion.

Best Regards,
Jaehoon Chung

>  #ifndef CONFIG_DM_MMC_OPS
>  	cfg->ops = &sdhci_ops;
>  #endif
> -	if (max_clk)
> -		cfg->f_max = max_clk;
> -	else {
> +	if (host->max_clk == 0) {
>  		if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300)
> -			cfg->f_max = (caps & SDHCI_CLOCK_V3_BASE_MASK) >>
> +			host->max_clk = (caps & SDHCI_CLOCK_V3_BASE_MASK) >>
>  				SDHCI_CLOCK_BASE_SHIFT;
>  		else
> -			cfg->f_max = (caps & SDHCI_CLOCK_BASE_MASK) >>
> +			host->max_clk = (caps & SDHCI_CLOCK_BASE_MASK) >>
>  				SDHCI_CLOCK_BASE_SHIFT;
> -		cfg->f_max *= 1000000;
> +		host->max_clk *= 1000000;
>  	}
> -	if (cfg->f_max == 0) {
> +	if (host->max_clk == 0) {
>  		printf("%s: Hardware doesn't specify base clock frequency\n",
>  		       __func__);
>  		return -EINVAL;
>  	}
> +	if (max_clk && (max_clk < host->max_clk))
> +		cfg->f_max = max_clk;
> +	else
> +		cfg->f_max = host->max_clk;
>  	if (min_clk)
>  		cfg->f_min = min_clk;
>  	else {
> @@ -623,11 +625,11 @@ int sdhci_bind(struct udevice *dev, struct mmc *mmc, struct mmc_config *cfg)
>  	return mmc_bind(dev, mmc, cfg);
>  }
>  #else
> -int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk)
> +int add_sdhci(struct sdhci_host *host, u32 f_max, u32 f_min)
>  {
>  	int ret;
>  
> -	ret = sdhci_setup_cfg(&host->cfg, host, max_clk, min_clk);
> +	ret = sdhci_setup_cfg(&host->cfg, host, f_max, f_min);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/mmc/spear_sdhci.c b/drivers/mmc/spear_sdhci.c
> index 06179cd..d3f8669 100644
> --- a/drivers/mmc/spear_sdhci.c
> +++ b/drivers/mmc/spear_sdhci.c
> @@ -21,7 +21,8 @@ int spear_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
>  	host->name = "sdhci";
>  	host->ioaddr = (void *)regbase;
>  	host->quirks = quirks;
> +	host->max_clk = max_clk;
>  
> -	add_sdhci(host, max_clk, min_clk);
> +	add_sdhci(host, 0, min_clk);
>  	return 0;
>  }
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 3da1385..69efa38 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -36,7 +36,9 @@ static int arasan_sdhci_probe(struct udevice *dev)
>  	host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
>  #endif
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, CONFIG_ZYNQ_SDHCI_MAX_FREQ,
> +	host->max_clk = CONFIG_ZYNQ_SDHCI_MAX_FREQ;
> +
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0,
>  			      CONFIG_ZYNQ_SDHCI_MIN_FREQ);
>  	host->mmc = &plat->mmc;
>  	if (ret)
> diff --git a/include/sdhci.h b/include/sdhci.h
> index 144570f..6c9f7a7 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -243,6 +243,7 @@ struct sdhci_host {
>  	unsigned int quirks;
>  	unsigned int host_caps;
>  	unsigned int version;
> +	unsigned int max_clk;   /* Maximum Base Clock frequency */
>  	unsigned int clk_mul;   /* Clock Multiplier value */
>  	unsigned int clock;
>  	struct mmc *mmc;
> @@ -372,11 +373,11 @@ static inline u8 sdhci_readb(struct sdhci_host *host, int reg)
>   *
>   * @cfg:	Configuration structure to fill in (generally &plat->mmc)
>   * @host:	SDHCI host structure
> - * @max_clk:	Maximum supported clock speed in HZ (0 for default)
> - * @min_clk:	Minimum supported clock speed in HZ (0 for default)
> + * @f_max:	Maximum supported clock frequency in HZ (0 for default)
> + * @f_min:	Minimum supported clock frequency in HZ (0 for default)
>   */
>  int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
> -		    u32 max_clk, u32 min_clk);
> +		    u32 f_max, u32 f_min);
>  
>  /**
>   * sdhci_bind() - Set up a new MMC block device
> @@ -402,11 +403,11 @@ int sdhci_bind(struct udevice *dev, struct mmc *mmc, struct mmc_config *cfg);
>   * This is used when you are not using CONFIG_BLK. Convert your driver over!
>   *
>   * @host:	SDHCI host structure
> - * @max_clk:	Maximum supported clock speed in HZ (0 for default)
> - * @min_clk:	Minimum supported clock speed in HZ (0 for default)
> + * @f_max:	Maximum supported clock frequency in HZ (0 for default)
> + * @f_min:	Minimum supported clock frequency in HZ (0 for default)
>   * @return 0 if OK, -ve on error
>   */
> -int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk);
> +int add_sdhci(struct sdhci_host *host, u32 f_max, u32 f_min);
>  #endif /* !CONFIG_BLK */
>  
>  #ifdef CONFIG_DM_MMC_OPS
> 



More information about the U-Boot mailing list