Messages in this thread | | | Date | Thu, 8 Sep 2005 13:23:40 +0100 | From | Russell King <> | Subject | Re: [-mm patch 5/5] SharpSL: Add new ARM PXA machines Spitz and Borzoi with partial Akita Support |
| |
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 *.
> +static void mmc_detect_callback(unsigned long data) > +{ > + mmc_detect_change(mmc_detect.devid); > +} > + > +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 constant.
> + mod_timer(&mmc_detect.detect_timer, jiffies + HZ/4); > + printk(KERN_WARNING "MMC detect callback\n");
Do you really want this obvious debugging noise here?
> + return IRQ_HANDLED; > +}
Hence, this becomes:
static void mmc_detect_callback(unsigned long devid) { mmc_detect_change((void *)devid); }
static irqreturn_t spitz_mmc_detect_int(int irq, void *devid, struct pt_regs *regs) { struct timer_list *timer = devid; mod_timer(timer, jiffies + HZ/4); return IRQ_HANDLED; }
> + > +static int spitz_mci_init(struct device *dev, irqreturn_t (*unused_detect_int)(int, void *, struct pt_regs *), void *data) > +{ > + int err; > + > + /* setup GPIO for PXA27x MMC controller */ > + pxa_gpio_mode(GPIO32_MMCCLK_MD); > + pxa_gpio_mode(GPIO112_MMCCMD_MD); > + pxa_gpio_mode(GPIO92_MMCDAT0_MD); > + pxa_gpio_mode(GPIO109_MMCDAT1_MD); > + pxa_gpio_mode(GPIO110_MMCDAT2_MD); > + pxa_gpio_mode(GPIO111_MMCDAT3_MD); > + pxa_gpio_mode(SPITZ_GPIO_nSD_DETECT | GPIO_IN); > + pxa_gpio_mode(SPITZ_GPIO_nSD_WP | GPIO_IN); > + > + init_timer(&mmc_detect.detect_timer); > + mmc_detect.detect_timer.function = mmc_detect_callback; > + mmc_detect.detect_timer.data = (unsigned long) &mmc_detect;
init_timer(&mmc_detect_timer); mmc_detect_timer.function = mmc_detect_callback; mmc_detect_timer.data = (unsigned long)data;
> + > + err = request_irq(SPITZ_IRQ_GPIO_nSD_DETECT, spitz_mmc_detect_int, SA_INTERRUPT, > + "MMC card detect", data);
err = request_irq(SPITZ_IRQ_GPIO_nSD_DETECT, spitz_mmc_detect_int, SA_INTERRUPT, "MMC card detect", &mmc_detect_timer);
> + if (err) { > + printk(KERN_ERR "spitz_mci_init: MMC/SD: can't request MMC card detect IRQ\n"); > + return -1; > + } > + > + set_irq_type(SPITZ_IRQ_GPIO_nSD_DETECT, IRQT_BOTHEDGE); > + > + return 0; > +} > + > +/* Power control is shared with one of the CF slots so we have a mess */ > +static void spitz_mci_setpower(struct device *dev, unsigned int vdd) > +{ > + struct pxamci_platform_data* p_d = dev->platform_data; > + > + unsigned short cpr = read_scoop_reg(&spitzscoop_device.dev, SCOOP_CPR); > + > + if (( 1 << vdd) & p_d->ocr_mask) { > + /* printk(KERN_DEBUG "%s: on\n", __FUNCTION__); */ > + set_scoop_gpio(&spitzscoop_device.dev, SPITZ_SCP_CF_POWER); > + mdelay(2); > + write_scoop_reg(&spitzscoop_device.dev, SCOOP_CPR, cpr | 0x04); > + } else { > + /* printk(KERN_DEBUG "%s: off\n", __FUNCTION__); */ > + write_scoop_reg(&spitzscoop_device.dev, SCOOP_CPR, cpr & ~0x04); > + > + if (!(cpr | 0x02)) { > + mdelay(1); > + reset_scoop_gpio(&spitzscoop_device.dev, SPITZ_SCP_CF_POWER); > + } > + }
Hmm, this actually highlights an API issue - we need to call the setpower() function with an additional argument which says whether we want the power on or off - a vdd value of zero is used for both "off" and the lowest voltage.
> + .pxafb_backlight_power = NULL,
We don't normally include NULL initialisers.
> +#endif > \ No newline at end of file
This should be fixed.
-- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core - 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/
| |