Hi Kevin,

> > --- /dev/null
> > +++ b/arch/arm/mach-davinci/board_dm365_evm_resources.h
> > @@ -0,0 +1,17 @@
> > +#ifndef _DAVINCI_EVM_RESOURCES_H
> > +#define _DAVINCI_EVM_RESOURCES_H
> > +
> > +#include <mach/irqs.h>
> > +#include <mach/edma.h>
> > +
> > +/* IRQs */
> > +#define IRQ_SPI0           IRQ_DM365_SPIINT0_0
> 
> This can be dropped, and IRQ_DM365_SPIINT0_0 used directly.
> 
> > +/* DMA channels */
> > +#define DMA_CHAN_SPI0_TX   16
> > +#define DMA_CHAN_SPI0_RX   17
> 
> These are SoC common, should be in <soc>.h
> 
> > +/* DMA event queues */
> > +#define DMA_EVQ_SPI0               EVENTQ_3
> 
> This can be dropped, and EVNETQ_3 used directly.
> 
> > +#endif /* _DAVINCI_EVM_RESOURCES_H */
> 
> I think it's best to just drop this file and these #defines altogether
> and just use the values directly in the board files.  It will make the
> board files a bit more readable and avoid a level of indirection.

I thought it a good idea to have central point where all those
resources (I am using this term in an informal way here) are defined
in terms of their intended use, making it easier to spot
possible conflicts.

> > diff --git a/arch/arm/mach-davinci/dm365.c 
> b/arch/arm/mach-davinci/dm365.c
> > index ed6c9c7..023d708 100644
> > --- a/arch/arm/mach-davinci/dm365.c
> > +++ b/arch/arm/mach-davinci/dm365.c
> > @@ -616,72 +616,256 @@ EVT_CFG(DM365,       EVT3_ASP_RX,    
>      1,     1,    0,     false)
> >  #endif
> >  };
> >  
> > -static u64 dm365_spi0_dma_mask = DMA_BIT_MASK(32);
> > +static u64 dm365_spi_dma_mask = DMA_BIT_MASK(32);
> >  
> > -static struct davinci_spi_platform_data dm365_spi0_pdata = {
> > -   .version        = SPI_VERSION_1,
> > -   .num_chipselect = 2,
> > -   .clk_internal   = 1,
> > -   .cs_hold        = 1,
> > -   .intr_level     = 0,
> > -   .poll_mode      = 1,    /* 0 -> interrupt mode 1-> 
> polling mode */
> > -   .use_dma        = 1,    /* when 1, value in poll_mode 
> is ignored */
> > -   .c2tdelay       = 0,
> > -   .t2cdelay       = 0,
> > +enum dm365_spi_resource_index {
> > +   spirsrc_iomem,
> > +   spirsrc_irq,
> > +   spirsrc_rxdma,
> > +   spirsrc_txdma,
> > +   spirsrc_evqdma
> >  };
> >
> > -static struct resource dm365_spi0_resources[] = {
> > +
> > +static struct resource dm365_spi_resources[spirsrc_evqdma 
> + 1][5] = {
> >     {
> > -           .start = 0x01c66000,
> > -           .end   = 0x01c667ff,
> > -           .flags = IORESOURCE_MEM,
> > +           [spirsrc_iomem] = {
> > +                   .start  = 0x01c66000,
> > +                   .end    = 0x01c667ff,
> > +                   .flags  = IORESOURCE_MEM,
> > +           },
> > +           [spirsrc_irq] = {
> > +                   .flags  = IORESOURCE_IRQ,
> > +           },
> > +           [spirsrc_rxdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_RX_CHAN,
> > +           },
> > +           [spirsrc_txdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_TX_CHAN,
> > +           },
> > +           [spirsrc_evqdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_EVENT_Q,
> > +           }
> >     },
> >     {
> > -           .start = IRQ_DM365_SPIINT0_0,
> > -           .flags = IORESOURCE_IRQ,
> > +           [spirsrc_iomem] = {
> > +                   .start  = 0x01c66800,
> > +                   .end    = 0x01c66fff,
> > +                   .flags  = IORESOURCE_MEM,
> > +           },
> > +           [spirsrc_irq] = {
> > +                   .flags  = IORESOURCE_IRQ,
> > +           },
> > +           [spirsrc_rxdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_RX_CHAN,
> > +           },
> > +           [spirsrc_txdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_TX_CHAN,
> > +           },
> > +           [spirsrc_evqdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_EVENT_Q,
> > +           }
> >     },
> >     {
> > -           .start = 17,
> > -           .flags = IORESOURCE_DMA | IORESOURCE_DMA_RX_CHAN,
> > +           [spirsrc_iomem] = {
> > +                   .start  = 0x01c67800,
> > +                   .end    = 0x01c67fff,
> > +                   .flags  = IORESOURCE_MEM,
> > +           },
> > +           [spirsrc_irq] = {
> > +                   .flags  = IORESOURCE_IRQ,
> > +           },
> > +           [spirsrc_rxdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_RX_CHAN,
> > +           },
> > +           [spirsrc_txdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_TX_CHAN,
> > +           },
> > +           [spirsrc_evqdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_EVENT_Q,
> > +           }
> >     },
> >     {
> > -           .start = 16,
> > -           .flags = IORESOURCE_DMA | IORESOURCE_DMA_TX_CHAN,
> > +           [spirsrc_iomem] = {
> > +                   .start  = 0x01c68000,
> > +                   .end    = 0x01c687ff,
> > +                   .flags  = IORESOURCE_MEM,
> > +           },
> > +           [spirsrc_irq] = {
> > +                   .flags  = IORESOURCE_IRQ,
> > +           },
> > +           [spirsrc_rxdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_RX_CHAN,
> > +           },
> > +           [spirsrc_txdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_TX_CHAN,
> > +           },
> > +           [spirsrc_evqdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_EVENT_Q,
> > +           }
> >     },
> >     {
> > -           .start = EVENTQ_3,
> > -           .flags = IORESOURCE_DMA | IORESOURCE_DMA_EVENT_Q,
> > +           [spirsrc_iomem] = {
> > +                   .start  = 0x01c23000,
> > +                   .end    = 0x01c237ff,
> > +                   .flags  = IORESOURCE_MEM,
> > +           },
> > +           [spirsrc_irq] = {
> > +                   .flags  = IORESOURCE_IRQ,
> > +           },
> > +           [spirsrc_rxdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_RX_CHAN,
> > +           },
> > +           [spirsrc_txdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_TX_CHAN,
> > +           },
> > +           [spirsrc_evqdma] = {
> > +                   .flags  = IORESOURCE_DMA | 
> IORESOURCE_DMA_EVENT_Q,
> > +           }
> > +   }
> > +};
> 
> Since most boards will not be using all of the above, Rather than
> statically allocate the IRQ and DMA resources, why not allocate them
> on demand during _init_spi()?

Don't think this is worthwile. You save a couple of bytes, but have
to add code that does memory allocation and associated error handling.

thanks,
Thomas
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to