Re: [PATCH 1/1] MMC: SDIO driver for Intel Moorestown platform

[Alan Cox]

> +       /*
> +        * Read the common CIS tuples.
> +        */
> +       err = sdio_read_cccr(card);
> +       if (err)
> +               goto remove;
> +
> +#ifdef CONFIG_MRST_MMC_WR
> +       /* restricting to 24MHz for Langwell A0 */
> +       if (card->cis.max_dtr > 24000000)
> +               card->cis.max_dtr = 24000000;
> +#endif

What if you have an SDIO device on a PCI card plugged into the system ?
You need some kind of card specific "fix up" call as a lot of other
drivers have I suspect ?


>
>         for (i = 0;i < host->card->sdio_funcs;i++) {
>                 if (host->card->sdio_func[i]) {
> +#ifdef CONFIG_SDIO_SUSPEND
> +                       sdio_remove_sysfs_file(host->card->sdio_func[i]);
> +#endif

Would be cleaner to always define sdio_remove_sysfs_file() and arrange in
the header files that this is

#define sdio_remove_sysfs_file(x)  do {} while (0)

>  static const struct mmc_bus_ops mmc_sdio_ops = {
>         .remove = mmc_sdio_remove,
>         .detect = mmc_sdio_detect,
> +       .suspend = mmc_sdio_suspend,
> +       .resume = mmc_sdio_resume,
>  };

You always expose suspend/resume yet you ifdef other stuff ?

> 
> @@ -323,6 +722,10 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
>                 goto err;
>         }
> 
> +#ifdef CONFIG_SDIO_SUSPEND
> +       mutex_init(&card->pm_mutex);
> +#endif

Again this and the create_sysfs_file one could be hidden more nicely in
the header

>
--
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/