From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Fabio Estevam <festevam@gmail.com>
Cc: Holger Schurig <holgerschurig@gmail.com>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
Chris Ball <chris@printf.net>,
Ulf Hansson <ulf.hansson@linaro.org>,
open list <linux-kernel@vger.kernel.org>,
Dong Aisheng <b29396@freescale.com>,
Sascha Hauer <kernel@pengutronix.de>
Subject: Re: SDHCI: mdelay() in hot path in esdhc_pltfm_set_clock looses CAN (!) frames
Date: Tue, 7 Jul 2015 21:52:43 +0100 [thread overview]
Message-ID: <20150707205243.GK7557@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <CAOMZO5D02tarn9QrOuS3i1ekn=1Jupngo6RBOfyVm9QASPA23A@mail.gmail.com>
On Tue, Jul 07, 2015 at 03:17:08PM -0300, Fabio Estevam wrote:
> On Tue, Jun 30, 2015 at 10:43 AM, Holger Schurig
> <holgerschurig@gmail.com> wrote:
> > So it seems that esdhc_pltfm_set_clock() somehow waits. And really
> > there is an mdelay(1) there. So it waits a whopping millisecond there!
> >
> > What's worse: I put a printk() into this function, just before the
> > mdelay(). And it get's called hundreds of times when I boot, or when I
> > update via rsync. And I believe that for each call the mdelay() kills
> > preemption.
The whole SDHCI thing remains a total trainwreck. Sorry guys, that's
precisely what it is. That damned driver needs rewriting to be a
library, the quirks need to be killed, and handled in specific driver
code for the variants with a library of core functions.
Then, the whole set_ios() thing in MMC needs killing and replaced with
a functional interface instead - "turn power on" "initialise/turn clock
on" "set clock speed" etc.
I suspect if that were to happen, then we'd get rid of a lot of those
mdelay()s you're seeing, because the problem sdhci has today is that
it's virtually impossible to work out what each set_ios() call into
the code is trying to achieve (well, we can work that out, what can't
be worked out is whether splitting the code is going to break some
random sdhci host, so the risk of causing breakage is high - hence
sdhci in its current form is basically unmaintainable.)
I've been saying this for quite some time - I put some work into it a
couple of years ago to try and clean up some of the crap there, but
since then people have continued with their same old ideas, adding
yet more quirk bits to those damned quirk variables, adding intercepts
for register accesses which all makes the issue worse, rather than
addressing the fundamental problem.
It needs a huge amount of effort put into it...
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2015-07-07 20:52 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-30 13:43 SDHCI: mdelay() in hot path in esdhc_pltfm_set_clock looses CAN (!) frames Holger Schurig
2015-07-07 18:17 ` Fabio Estevam
2015-07-07 20:52 ` Russell King - ARM Linux [this message]
2015-07-08 6:22 ` Holger Schurig
2015-07-08 6:43 ` Sascha Hauer
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=20150707205243.GK7557@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=b29396@freescale.com \
--cc=chris@printf.net \
--cc=festevam@gmail.com \
--cc=holgerschurig@gmail.com \
--cc=kernel@pengutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
/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