Re: [PATCH 1/2] mmc: mxcmmc: Convert to devm-* API

Alexander Shiyan
2014-02-22T16:03:19Z

Суббота, 22 февраля 2014, 16:37 +01:00 от Arnd Bergmann <arnd@arndb.de>:
> On Saturday 22 February 2014 18:46:11 Alexander Shiyan wrote:
> > Replace existing resource handling in the driver with managed
> > device resource, this ensures more consistent error values and
> > simplifies error paths.
> >
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>
> Nice cleanup!
...
> > +++ b/drivers/mmc/host/mxcmmc.c
> > @@ -124,9 +124,8 @@ enum mxcmci_type {
> >
> > struct mxcmci_host {
> > struct mmc_host *mmc;
> > - struct resource *res;
> > void __iomem *base;
> > - int irq;
> > + dma_addr_t phys_base;
> > int detect_irq;
> > struct dma_chan *dma;
> > struct dma_async_tx_descriptor *desc;
>
> Just nitpicking, but I think phys_base should be either phys_addr_t or
> resource_size_t. dma_addr_t is what you get out of the dma-mapping API,
> not what you use for ioremap. In theory they may be different, but I
> don't know how we'd handle that case for the dmaengine API, since we
> don't currently have a way to convert between the two, other than for
> doing DMA on memory pages.

Maybe I'll think of something in the subsequent cleanup of the driver.
Currently, many things look in the driver just scary, here for example:

static inline void mxcmci_writel(struct mxcmci_host *host, u32 val, int reg)
{
if (IS_ENABLED(CONFIG_PPC_MPC512x))
iowrite32be(val, host->base + reg);
else
writel(val, host->base + reg);
}

---