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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <83c58cdc-b96a-40c3-bed1-767e9e832893@app.fastmail.com>
Date:   Wed, 13 Sep 2023 20:01:40 +0200
From:   "Arnd Bergmann" <arnd@...nel.org>
To:     "Ian Abbott" <abbotti@....co.uk>, linux-kernel@...r.kernel.org
Cc:     "Niklas Schnelle" <schnelle@...ux.ibm.com>
Subject: Re: [PATCH 09/13] comedi: ni_mio_common: Conditionally use I/O port or MMIO

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