Re: [PATCH v6 03/10] ASoC: qcom: Add register definition for codec rddma and wrdma

[Srinivas Kandagatla]

On 02/12/2021 10:55, Srinivasa Rao Mandadapu wrote:
> > >   +/* LPAIF RXTX IRQ */
> > > +#define LPAIF_RXTX_IRQ_REG_ADDR(v, addr, port, dai_id) \
> > > +        ((dai_id == LPASS_CDC_DMA_RX0 || dai_id == 
> > > LPASS_CDC_DMA_TX3) ? \
> > > +        (v->rxtx_irq_reg_base + (addr) + v->rxtx_irq_reg_stride * 
> > > (port)) : \
> > > +        (v->va_irq_reg_base + (addr) + v->va_irq_reg_stride * (port)))
> > > +
> > > +#define LPAIF_RXTX_IRQEN_REG(v, port, dai_id) 
> > > LPAIF_RXTX_IRQ_REG_ADDR(v, 0x0, port, dai_id)
> > > +#define LPAIF_RXTX_IRQSTAT_REG(v, port, dai_id) 
> > > LPAIF_RXTX_IRQ_REG_ADDR(v, 0x4, port, dai_id)
> > > +#define LPAIF_RXTX_IRQCLEAR_REG(v, port, dai_id) 
> > > LPAIF_RXTX_IRQ_REG_ADDR(v, 0xC, port, dai_id)
> > > +
> >
> > How about doing like this:
> >
> >
> > /* LPAIF RXTX IRQ */
> > #define LPAIF_RXTX_IRQ_REG_ADDR(v, addr, port) \
> >         (v->rxtx_irq_reg_base + (addr) + v->rxtx_irq_reg_stride * (port))
> >
> > #define LPAIF_RXTX_IRQEN_REG(v, port, dai_id) 
> > LPAIF_RXTX_IRQ_REG_ADDR(v, 0x0, port)
> > #define LPAIF_RXTX_IRQSTAT_REG(v, port, dai_id) 
> > LPAIF_RXTX_IRQ_REG_ADDR(v, 0x4, port)
> > #define LPAIF_RXTX_IRQCLEAR_REG(v, port, dai_id) 
> > LPAIF_RXTX_IRQ_REG_ADDR(v, 0xC, port)
> >
> > /* LPAIF VA IRQ */
> > #define LPAIF_VA_IRQ_REG_ADDR(v, addr, port) \
> >         (v->va_irq_reg_base + (addr) + v->va_irq_reg_stride * (port))
> >
> > #define LPAIF_VA_IRQEN_REG(v, port, dai_id) LPAIF_VA_IRQ_REG_ADDR(v, 
> > 0x0, port)
> > #define LPAIF_VA_IRQSTAT_REG(v, port, dai_id) LPAIF_VA_IRQ_REG_ADDR(v, 
> > 0x4, port)
> > #define LPAIF_VA_IRQCLEAR_REG(v, port, dai_id) 
> > LPAIF_VA_IRQ_REG_ADDR(v, 0xC, port)
> With this we are seeing number macros increasing. How about handling 

Its okay to add new macros, this makes them much clear to the reader and 
inline with rest of the macros in the file.


--srini
> like below.
> > lpass.h:
>
> static inline bool is_rxtx_cdc_dma_port(int dai_id)
> {
>
>      switch (dai_id) {
>          case LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX9:
>          case LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
>              return true;
>          default:
>              return false;
>        }
> }
>
>
> Usage:
>
> #define LPAIF_RXTX_IRQ_REG_ADDR(v, addr, port, dai_id) \
> is_rxtx_cdc_dma_port(dai_id) ? \
> (v->rxtx_irq_reg_base + (addr) + v->rxtx_irq_reg_stride * (port)) : \
> (v->va_irq_reg_base + (addr) + v->va_irq_reg_stride * (port))