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