Re: [RFC PATCH 06/13] switchtec_ntb: initialize hardware for memory windows
From: Logan Gunthorpe
Date: Sat Jun 17 2017 - 12:39:55 EST
On 16/06/17 11:16 PM, Greg Kroah-Hartman wrote:
>> +#ifndef ioread64
>> +#ifdef readq
>> +#define ioread64 readq
>> +#define ioread64 _ioread64
>> +static inline u64 _ioread64(void __iomem *mmio)
>> + u64 low, high;
>> + low = ioread32(mmio);
>> + high = ioread32(mmio + sizeof(u32));
>> + return low | (high << 32);
> Really? Don't we have ioread64 in generic code for all arches? If not,
> that should be fixed, don't hide this in a random driver please. Or
> just restrict your driver to only building on those arches that does
> provide this api.
Yes, I know these are _very_ ugly. Unfortunately, the other ntb drivers
each have the exact same thing. ioread64 is not provided universally at
this time and I did spend a bit of time digging and things are a bit
messy so I wasn't at all sure of the correct solution.
For starters, ioread64 is only defined on 64 bit machines. They are
surrounded by ifdef CONFIG_64BIT and it's not clear to me if the above
wrapper (around two ioread32s) would be acceptable universally.
Second, the x86_64 version doesn't even compile. This is because the
arch doesn't provide any ioread64 implementation anywhere and the macro
in io.h isn't used because CONFIG_GENERIC_IOMAP is defined.
The only arch where I _think_ ioread64 would work is powerpc or any that
define CONFIG_64BIT and not CONFIG_GENERIC_IOMAP.
If you have any guidance on this I'd be happy to try and make a patch or
two for it.