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

[Richard Purdie <rpurdie@rpsys.net>]

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/