Re: [PATCH 07/28] nios2: I/O Mapping

[Ley Foon Tan]

On Thu, Apr 24, 2014 at 3:43 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Thursday 24 April 2014 14:02:47 Ley Foon Tan wrote:

>> >> +#define IO_SPACE_LIMIT 0xffffffff
>> >
>> > Please use 0xffff here, this should work for almost any PCI bus.
>> Ah, CONFIG_PCI is not enable in nios2. So, I think IO_SPACE_LIMIT
>> should set to 0.
>
> Yes, possibly with a comment added regarding PCI.
Sure.

>
>> >> +/*
>> >> + *   make the short names macros so specific devices
>> >> + *   can override them as required
>> >> + */
>> >> +#define inb(addr)            readb(addr)
>> >> +#define inw(addr)            readw(addr)
>> >> +#define inl(addr)            readl(addr)
>> >> +#define outb(x, addr)                ((void) writeb(x, addr))
>> >> +#define outw(x, addr)                ((void) writew(x, addr))
>> >> +#define outl(x, addr)                ((void) writel(x, addr))
>> >
>> > This makes no sense: the legacy PC I/O accessors take a completely
>> > different argument type: I would recommend to make both inline
>> > functions so you can ensure that readl() gets passed an __iomem
>> > pointer, while inl() gets an integer number.
>> >
>> > Please see the asm-generic version for an example. You should also
>> > define a non-NULL PCI_IOBASE to which the PCI I/O space gets mapped.
>> I think these macros can be removed if PCI is not enabled in Nios2.
>
> We have a bunch of ISA device drivers that use these interfaces but
> are not built conditionally through a Kconfig dependency. If you
> want 'allyesconfig' to build successfully, you have to provide some
> dummy implementation.
Okay, will provide empty write and read with return 0 dummy implementation here.

>
>> >> +#define ioport_map(port, nr) ioremap(port, nr)
>> >> +#define ioport_unmap(port)   iounmap(port)
>> >
>> > Use this one instead:
>> >
>> > #ifdef CONFIG_PCI
>> > static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
>> > {
>> >         return PCI_IOBASE + port;
>> > }
>> >
>> > static inline void __iomem *ioport_unmap(void __iomem *)
>> > {
>> > }
>> > #endif
>> These also can be removed.
>
> For this, there is a Kconfig symbol. If you don't provide ioport_map,
> you should set CONFIG_NO_IOPORT_MAP. Uwe Kleine-KÃnig has recently
> started some work in this area. Ideally we would also have a CONFIG_HAS_IOPORT
> option that is only set by architectures that actually implement inb/outb.
> Getting there still needs work, but any help is appreciated.
Will set CONFIG_NO_IOPORT_MAP. I noticed that there is
CONFIG_HAS_IOPORT_MAP, is it same as CONFIG_HAS_IOPORT you refer to?
But, seem only arch/sh is using it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/