public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Pierre Ossman <pierre@ossman.eu>
Cc: akpm@linux-foundation.org, drzeus@drzeus.cx, ian@mnementh.co.uk,
	matt@console-pimps.org, roberto.foglietta@gmail.com,
	linux-kernel@vger.kernel.org, sdhci-devel@lists.ossman.eu
Subject: [PATCH] sdhci: Be more strict with get_min_clock() usage
Date: Tue, 4 Aug 2009 21:20:55 +0400	[thread overview]
Message-ID: <20090804172055.GA9271@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <20090801100443.7a5c2f86@mjolnir.ossman.eu>

get_min_clock() makes sense only with NONSTANDARD_CLOCK quirk and when
set_clock() callback is specified.

The patch should cause no functional changes, it just makes the code
self-documented and avoids any possible misuse of get_min_clock().

Suggested-by: Pierre Ossman <pierre@ossman.eu>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

On Sat, Aug 01, 2009 at 10:04:43AM +0200, Pierre Ossman wrote:
[...]
> Hmm... I don't think this is quite right. Right now you kill the
> warning without verifying that there is a mechanism to go lower.
[...]
> > -	mmc->f_min = host->max_clk / 256;
> > +	if (host->ops->get_min_clock)
> > +		mmc->f_min = host->ops->get_min_clock(host);
> > +	else
> > +		mmc->f_min = host->max_clk / 256;
> >  	mmc->f_max = host->max_clk;
> >  	mmc->caps = MMC_CAP_SDIO_IRQ;
> 
> You should be checking ops->set_clock and SDHCI_QUIRK_NONSTANDARD_CLOCK
> here.

Well, it makes no sense to specify .get_min_clock() w/o .set_clock().

But okay, to make things self documented and to avoid misuses, we'd
better add these checks.

Thanks Pierre!

 drivers/mmc/host/sdhci.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fc96f8c..7f7f45b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1772,7 +1772,8 @@ int sdhci_add_host(struct sdhci_host *host)
 	 * Set host parameters.
 	 */
 	mmc->ops = &sdhci_ops;
-	if (host->ops->get_min_clock)
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK &&
+			host->ops->set_clock && host->ops->get_min_clock)
 		mmc->f_min = host->ops->get_min_clock(host);
 	else
 		mmc->f_min = host->max_clk / 256;
-- 
1.6.3.3


           reply	other threads:[~2009-08-04 17:20 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <20090801100443.7a5c2f86@mjolnir.ossman.eu>]

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=20090804172055.GA9271@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=akpm@linux-foundation.org \
    --cc=drzeus@drzeus.cx \
    --cc=ian@mnementh.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@console-pimps.org \
    --cc=pierre@ossman.eu \
    --cc=roberto.foglietta@gmail.com \
    --cc=sdhci-devel@lists.ossman.eu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox