Re: [PATCH 09/13] comedi: ni_mio_common: Conditionally use I/O port or MMIO

[Arnd Bergmann]

On Wed, Sep 13, 2023, at 17:45, Ian Abbott wrote:
> On 13/09/2023 13:17, Arnd Bergmann wrote:
>> On Wed, Sep 13, 2023, at 13:20, Ian Abbott wrote:
>>>
>>> The changes make it possible to build the ni_pcimio module even if the
>>> port I/O functions have not been declared.  (The ni_atmio and ni_mio_cs
>>> modules do still require the port I/O functions to be declared.)
>> 
>> I think this all works, but there is probably a simpler way to
>> achieve the same:
>>   
>>> +#ifdef PCIDMA
>>> +
>>>   static void ni_writel(struct comedi_device *dev, unsigned int data, int reg)
>>>   {
>>> -	if (dev->mmio)
>>> -		writel(data, dev->mmio + reg);
>>> -	else
>>> -		outl(data, dev->iobase + reg);
>>> +	writel(data, dev->mmio + reg);
>>>   }
>>>
>>>
>>> +#else /* PCIDMA */
>>> +
>>> +static void ni_writel(struct comedi_device *dev, unsigned int data, int reg)
>>> +{
>>> +	outl(data, dev->iobase + reg);
>>> +}
>> 
>> We already have an abstraction for this using iowrite32(),
>> which turns into either writel() or outl() depending on the
>> argument, so you could just use pci_iomap() or ioport_map()
>> to turn port numbers into tokens suitable for the common
>> helper.
>
> It would affect three modules, and there are similar changes that could 
> be made to other modules, so I think I'll put that suggestion on hold 
> for now.  I'd also like to get rid of the '#include "ni_mio_common.c"' 
> nonsense at some point, but that's a big job!

Sure, that's fine, I just thought I'd mention it in case you
were looking for a nicer implementation. I wouldn't have done
the current series to start with since there is not a huge amount
of value in running these drivers on obscure architectures
without HAS_IOPORT, but your changes are otherwise completely
sensible.

  Arnd