lkml.org 
[lkml]   [2014]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 07/28] nios2: I/O Mapping
    Date
    On Friday 18 April 2014, Ley Foon Tan wrote:

    > +
    > +#include <asm/pgtable-bits.h>
    > +
    > +#define IO_SPACE_LIMIT 0xffffffff

    Please use 0xffff here, this should work for almost any PCI bus.

    > +
    > +#ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
    > +# define __IO_USE_DUFFS
    > +#endif
    > +
    > +#ifdef __IO_USE_DUFFS
    > +
    > +/* Use "Duff's Device" to unroll the loops. */
    > +#define __IO_OUT_LOOP(a, b, l) \
    > + do { \
    > + if (l > 0) { \
    > + int _n = (l + 7) / 8; \
    > + switch (l % 8) { \
    > + case 0: \
    > + do { \
    > + *a = *b++; \

    I would recommend just doing all of this in out-of-line implementations
    rather than macros and inline functions.

    > +
    > +/*
    > + * 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.

    > +#define virt_to_bus virt_to_phys
    > +#define bus_to_virt phys_to_virt

    Please drop these, and the CONFIG_VIRT_TO_BUS Kconfig option,
    and fix all drivers that rely on it.

    > +#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

    > +/* Macros used for smc91x.c driver */
    > +#define readsb(p, d, l) insb(p, d, l)
    > +#define readsw(p, d, l) insw(p, d, l)
    > +#define readsl(p, d, l) insl(p, d, l)
    > +#define writesb(p, d, l) outsb(p, d, l)
    > +#define writesw(p, d, l) outsw(p, d, l)
    > +#define writesl(p, d, l) outsl(p, d, l)

    These should of course not fall back to the PCI I/O space functions.
    You can do it the other way round.

    Arnd


    \
     
     \ /
      Last update: 2014-04-22 17:21    [W:2.324 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site