Hi

Comments below, and updated patch attached.

On Mon, Aug 30, 2010 at 3:29 PM, Anton Vorontsov [off-list ref] wrote:

quoted

quoted

+static int of_mmc_spi_init(struct device *dev,

+                          irqreturn_t (*irqhandler)(int, void *), void *mmc)
+{
+       struct of_mmc_spi *oms = to_of_mmc_spi(dev);

Please add an empty line here.

Ok.

quoted

+       return request_threaded_irq(
+               oms->detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc);

I'd write it this way:

return request_threaded_irq(oms->detect_irq, NULL, irqhandler,
                           0, dev_name(dev), mmc);

But that's a matter of taste.

Fine with me.

quoted

+}
+
+static void of_mmc_spi_exit(struct device *dev, void *mmc)
+{
+       struct of_mmc_spi *oms = to_of_mmc_spi(dev);

Empty line.

Ok.

quoted

+       free_irq(oms->detect_irq, mmc);
+}
+
 struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
 {
        struct device *dev = &spi->dev;

@@ -121,8 +136,14 @@ struct mmc_spi_platform_data

*mmc_spi_get_pdata(struct spi_device *spi)
        if (gpio_is_valid(oms->gpios[WP_GPIO]))
                oms->pdata.get_ro = of_mmc_spi_get_ro;

-       /* We don't support interrupts yet, let's poll. */
-       oms->pdata.caps |= MMC_CAP_NEEDS_POLL;
+       oms->detect_irq = irq_of_parse_and_map(np, 0);
+       if (oms->detect_irq != NO_IRQ) {

I'd write "if (oms->detect_irq)", which is a bit more natural
(and still correct, 0 is the only invalid VIRQ number).

Most other architectures has NO_IRQ defined to -1, so I will stick
with the NO_IRQ comparsion.
Hopefully, arm users will soon enjoy this driver/wrapper soon also.

quoted

+               oms->pdata.init = of_mmc_spi_init;
+               oms->pdata.exit = of_mmc_spi_exit;
+       }
+       else {

} else {

Done.

Plus, please add an appropriate interrupts = <> bindings into
Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.

Done.

And on the next resend, be sure to add Andrew Morton
[off-list ref], David Brownell
[off-list ref], and linux-mmc@vger.kernel.org
the Cc list.

Should be there now.

/Esben