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. -Stafford
