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

["Alexander Shiyan" <shc_work@mail.ru>]

Суббота, 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);
}

---