All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan.Herbrechtsmeier at weidmueller.de <Stefan.Herbrechtsmeier@weidmueller.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 1/4] mmc: sdhci: Distinguish between base clock and maximum peripheral frequency
Date: Thu, 1 Dec 2016 14:03:33 +0000	[thread overview]
Message-ID: <531ADB0D40411F4DAE16FC92133DDBF1010C58D1@SRVDE270.weidmueller.com> (raw)
In-Reply-To: <c0085d1d-327a-9d49-ad15-b932cbfad877@samsung.com>

> -----Urspr?ngliche Nachricht-----
> Von: Jaehoon Chung [mailto:jh80.chung at samsung.com]
> Gesendet: Donnerstag, 1. Dezember 2016 07:43
> An: Herbrechtsmeier, Stefan; u-boot at lists.denx.de
> Cc: Simon Glass; Masahiro Yamada; Stephen Warren; Minkyu Kang; Wenyou
> Yang
> Betreff: Re: [PATCH v2 1/4] mmc: sdhci: Distinguish between base clock
> and maximum peripheral frequency
> 
> 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@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.

You are right. Will update the argument's names.

> 
> >  #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
> >



Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - 
Komplement?rin: Weidm?ller Interface F?hrungsgesellschaft mbH - 
Sitz: Detmold - Amtsgericht Lemgo HRB 3924; 
Gesch?ftsf?hrer: Jos? Carlos ?lvarez Tobar, Elke Eckstein, Dr. Peter K?hler, J?rg Timmermann;
USt-ID-Nr. DE124599660

  reply	other threads:[~2016-12-01 14:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-25 13:59 [U-Boot] [PATCH v2 0/4] mmc: sdhci: Add support for frequency constrained peripheral interfaces Stefan Herbrechtsmeier
2016-11-25 13:59 ` [U-Boot] [PATCH v2 1/4] mmc: sdhci: Distinguish between base clock and maximum peripheral frequency Stefan Herbrechtsmeier
2016-12-01  6:42   ` Jaehoon Chung
2016-12-01 14:03     ` Stefan.Herbrechtsmeier at weidmueller.de [this message]
2016-11-25 13:59 ` [U-Boot] [PATCH v2 2/4] serial: zynq: Remove unused index from get uart clock function Stefan Herbrechtsmeier
2016-11-28  7:36   ` Michal Simek
2016-11-25 13:59 ` [U-Boot] [PATCH v2 3/4] mmc: zynq: Determine base clock frequency via clock framework Stefan Herbrechtsmeier
2016-11-28  7:42   ` Michal Simek
2016-12-27 10:10     ` Stefan Herbrechtsmeier
2017-01-02  7:00       ` Michal Simek
2017-01-02 14:02         ` Stefan Herbrechtsmeier
2017-01-02 14:07           ` Michal Simek
2016-11-25 13:59 ` [U-Boot] [PATCH v2 4/4] mmc: zynq: Add fdt max-frequency support Stefan Herbrechtsmeier
2016-11-28  7:55   ` Michal Simek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=531ADB0D40411F4DAE16FC92133DDBF1010C58D1@SRVDE270.weidmueller.com \
    --to=stefan.herbrechtsmeier@weidmueller.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.