Re: [-mm patch 5/5] SharpSL: Add new ARM PXA machines Spitz andBorzoi with partial Akita Support

From: Richard Purdie
Date: Thu Sep 08 2005 - 09:53:15 EST


On Thu, 2005-09-08 at 13:23 +0100, Russell King wrote:
> On Tue, Sep 06, 2005 at 12:53:52PM +0100, Richard Purdie wrote:
> > +/*
> > + * MMC/SD Device
> > + *
> > + * The card detect interrupt isn't debounced so we delay it by HZ/4
> > + * to give the card a chance to fully insert/eject.
> > + */
> > +static struct mmc_detect {
> > + struct timer_list detect_timer;
> > + void *devid;
> > +} mmc_detect;
>
> This isn't necessary. The "devid" is in timer_list already - in the "data"
> element. This is passed to the callback function as its only argument.
> Sure, it means a couple of extra casts, but that's an mis-feature we
> all know about in the timer API. It should've been a void *.

This was done because I didn't like to assume:

> +static irqreturn_t spitz_mmc_detect_int(int irq, void *devid, struct pt_regs *regs)
> > +{
> > + mmc_detect.devid=devid;
>
> Also you don't need to set it each time. devid will be a constan

Although if we're happy with that assumption (which I am), the
simplification can be made.

Alternatively, would you accept a patch to add an optional delay option
to mmc_detect_change()? Something like the patch below but with all the
callers updated and the pxa platform data amended to pass the optional
delay. As a case for this, the same debounce timer is needed for corgi,
poodle, hx2750, spitz and tosa.

This also highlights that schedule_delayed_work() with delay=0 isn't
special cased which perhaps it should be?

Index: linux-2.6.13/drivers/mmc/mmc.c
===================================================================
--- linux-2.6.13.orig/drivers/mmc/mmc.c 2005-09-07 22:23:01.000000000 +0100
+++ linux-2.6.13/drivers/mmc/mmc.c 2005-09-07 22:40:03.000000000 +0100
@@ -1067,13 +1067,17 @@
/**
* mmc_detect_change - process change of state on a MMC socket
* @host: host which changed state.
+ * @delay: optional delay to wait before detection (jiffies)
*
* All we know is that card(s) have been inserted or removed
* from the socket(s). We don't know which socket or cards.
*/
-void mmc_detect_change(struct mmc_host *host)
+void mmc_detect_change(struct mmc_host *host, unsigned long delay)
{
- schedule_work(&host->detect);
+ if (delay)
+ schedule_delayed_work(&host->detect, delay);
+ else
+ schedule_work(&host->detect);
}



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/