[PATCH fpga 4/9] fpga zynq: Check for errors after completing DMA

Jason Gunthorpe jgunthorpe at obsidianresearch.com
Thu Nov 17 10:28:12 PST 2016


On Wed, Nov 16, 2016 at 10:10:35PM -0800, Moritz Fischer wrote:
> >   /* FPGA programmed */
> >  #define IXR_PCFG_DONE_MASK             BIT(2)
> > -#define IXR_ERROR_FLAGS_MASK           0x00F0F860
> > +#define IXR_ERROR_FLAGS_MASK           0x00F0C860
> 
> True. Ouch.

Yeah, this only works at all by accident..


> > -static void zynq_fpga_mask_irqs(struct zynq_fpga_priv *priv)
> > +static inline void zynq_fpga_set_irq_mask(struct zynq_fpga_priv *priv,
> > +                                         u32 enable)
> >  {
> > -       u32 intr_mask;
> > -
> > -       intr_mask = zynq_fpga_read(priv, INT_MASK_OFFSET);
> > -       zynq_fpga_write(priv, INT_MASK_OFFSET,
> > -                       intr_mask | IXR_DMA_DONE_MASK | IXR_ERROR_FLAGS_MASK);
> > -}
> > +       zynq_fpga_write(priv, INT_MASK_OFFSET, ~enable);
> 
> Not a fan of this ~enable here. Function name indicates you're doing
> the opposite

Lets call it zynq_fpga_set_irq then

The ~ is best placed here because it is after the compiler has coerced
the argument into u32 so the ~ will always do the right
thing

I've been reading the IXR_*_MASK as indicating it is a bit pattern for
the INT_MASK register not as actually meaning literal masking.

> > +out_clk:
> >         clk_disable(priv->clk);
> > -
> 
> Personally found it more readable with newline.

sure

Jason



More information about the linux-arm-kernel mailing list