linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] MMC: fix sdhci-dove removal
Date: Mon, 15 Oct 2012 11:37:25 +0100	[thread overview]
Message-ID: <20121015103725.GQ21164@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20121015094348.GP21164@n2100.arm.linux.org.uk>

On Mon, Oct 15, 2012 at 10:43:48AM +0100, Russell King - ARM Linux wrote:
> 1. Unregister the device _BEFORE_ taking away any resources it may
>    be using.
> 2. Don't check clks against NULL.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Looking at this driver some more, who the hell came up with the sdhci
registration interface?  It violates one of the most fundamental
principles of kernel driver programming.  You do _NOT_ publish your
driver interfaces _UNTIL_ you have finished setting your device up.
Otherwise, in a preemptible or SMP kernel, your driver can be used
before the initialization has completed.

As this driver calls sdhci_pltfm_register() before it has obtained the
clock for the interface, and this function does:
	sdhci_pltfm_init
	sdhci_add_host
		mmc_add_host
			mmc_start_host
				mmc_power_up
					mmc_set_ios
						sdhci_set_ios

See, we're trying to power up and clock the card _before_ the dove
sdhci driver has even claimed the clock let alone enabled it.  This
is total bollocks.  The sdhci platform interface is total crap for
creating this broken design in the first place.  This is why MMC has
the init + add interfaces, they're there to allow drivers to do stuff
the Right(tm) way and avoid shit like the above.

This should have been picked up at review time before the driver went
into mainline.  In any case, it needs to be fixed.

  reply	other threads:[~2012-10-15 10:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-15  9:43 [PATCH] MMC: fix sdhci-dove removal Russell King - ARM Linux
2012-10-15 10:37 ` Russell King - ARM Linux [this message]
2012-10-15 10:44   ` Russell King - ARM Linux
2012-10-15 14:58     ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-15 15:07       ` Russell King - ARM Linux
2012-10-29 21:10     ` Chris Ball
2012-10-29 21:43       ` Russell King - ARM Linux
2012-10-29 21:49         ` Chris Ball
2012-10-29 22:16           ` Sebastian Hesselbarth
2012-10-29 22:20             ` Chris Ball

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=20121015103725.GQ21164@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.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;
as well as URLs for NNTP newsgroup(s).