From: Stafford Horne > Sent: 19 July 2022 13:24 > > On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote: > > On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@xxxxxxxxx> wrote: > > > > > diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c > > > index 700982464c53..508421809128 100644 > > > --- a/drivers/comedi/drivers/comedi_isadma.c > > > +++ b/drivers/comedi/drivers/comedi_isadma.c > > > @@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma) > > > > > > flags = claim_dma_lock(); > > > clear_dma_ff(desc->chan); > > > +#ifdef CONFIG_X86_32 > > > if (!isa_dma_bridge_buggy) > > > disable_dma(desc->chan); > > > +#endif > > > > There is a logic mistake here: if we are on something other than x86-32, > > this always needs to call the disable_dma()/enable_dma(). > > Oops, thats right. Sorry, I should have noticed that. > > > Not sure how to best express this in a readable way, something like this > > would work: > > Option 1: > > > #ifdef CONFIG_X86_32 > > if (!isa_dma_bridge_buggy) > > #endif > > disable_dma(desc->chan); > > > > > > or possibly at the start of this file, a > > Option 2: > > > #ifndef CONFIG_X86_32 > > #define isa_dma_bridge_buggy 0 > > #endif > > Option 3: > > > Or we could try to keep the generic definition in a global header > > like linux/isa-dma.h. > > Perhaps option 3 makes the whole patch the most clean. Isn't there a define that can be used inside an if? So you could do: if (!IS_CONFIG_X86_32 || !isa_dma_bridge_buggy) disable_dma(); (but I can't remember the name!) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
