lkml.org 
[lkml]   [2011]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [patch v2 24/35] Hexagon: Provide basic implementation and/or stubs for I/O routines.
    Date
    On Tuesday 30 August 2011, Richard Kuo wrote:
    > Changed read/write to use inline assembly.
    >
    > Rearranged the file and put all the ioport stuff at the bottom. For now
    > I'd like to just leave the stubs in there in case they need to be
    > filled out later; plus things don't compile.
    >
    > We still seem to need IO_SPACE_LIMIT to be defined, particularly to something
    > large, as we have drivers that request resources with IORESOURCE_IO that
    > are memory mapped IO. The ioread/iowrites work as they select the correct
    > routine to use.

    Which drivers? If the drivers are buggy, better fix them than working around
    in the architecture.

    I have experimental patches in the tmp/randconfig3 branch of the arm-soc.git
    tree to remove the need for PIO functions in ARM. Please have a look there
    if you are interested.

    > +/*
    > + * We don't have PCI yet.
    > + */
    > +#define IO_SPACE_LIMIT 0xffffffff
    > +#define _IO_BASE 0x0

    If you absolutely insist on defining these, at least make them meaningful,
    e.g. defining _IO_BASE to an unused location in the virtual address space,
    and IO_SPACE_LIMIT small enough to be harmless, at most 0xffff.

    If you ever gain PCI support, you can then map the PCI I/O space into that
    area.

    > +static inline void memcpy_fromio(void *dst, const volatile void __iomem *src,
    > + int count)
    > +{
    > + memcpy(dst, (void *) src, count);
    > +}
    > +
    > +static inline void memcpy_toio(volatile void __iomem *dst, const void *src,
    > + int count)
    > +{
    > + memcpy((void *) dst, src, count);
    > +}

    This will need __force in order to build with sparse.

    > +#define PCI_IO_ADDR volatile void __iomem *

    Better make _IO_BASE have the right type to start with, like

    #define _IO_BASE ((void __iomem *)0xefff0000)
    static inline u8 inb(unsigned long port)
    {
    return readb(_IO_BASE + (port & IO_SPACE_LIMIT));
    }

    static inline void outb(u8 data, unsigned long port)
    {
    writeb(data, _IO_BASE + (port & IO_SPACE_LIMIT));
    }

    > +/* _p means "pause until the I/O completes" */
    > +#define outb_p outb
    > +#define outw_p outw
    > +#define outl_p outl
    > +
    > +#define inb_p inb
    > +#define inw_p inw
    > +#define inl_p inl

    It doesn't actually mean that. It means 'pause for a bit longer'. The inb/outb
    family of functions is already required to wait for the I/O to complete,
    unlike readb/writeb, which only needs to wait for inbound data, while
    outbound writes are posted to the bus without waiting for the data to arrive.

    > +static inline void insb(unsigned long addr, void *buffer, int count)
    > +{
    > + printk(KERN_INFO "insb not implemented\n");
    > +}
    > +
    > +static inline void insw(unsigned long addr, void *buffer, int count)
    > +{
    > + printk(KERN_INFO "insw not implemented\n");
    > +}
    > +
    > +static inline void insl(unsigned long addr, void *buffer, int count)
    > +{
    > + printk(KERN_INFO "insl not implemented\n");
    > +}
    > +
    > +static inline void outsb(unsigned long addr, const void *buffer, int count)
    > +{
    > + printk(KERN_INFO "outsb not implemented\n");
    > +}
    > +
    > +static inline void outsw(unsigned long addr, const void *buffer, int count)
    > +{
    > + printk(KERN_INFO "outsw not implemented\n");
    > +}
    > +
    > +static inline void outsl(unsigned long addr, const void *buffer, int count)
    > +{
    > + printk(KERN_INFO "outsl not implemented\n");
    > +}

    Either just add the obvious implementations adding _IO_BASE as above
    or stub these out to create a link time error in drivers using them.
    If a driver relies on these functions, it won't work anyway, so it's
    better to not even build it.

    > +/* generic versions defined in lib/iomap.c */
    > +extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
    > +extern void ioport_unmap(void __iomem *addr);


    You can also just set CONFIG_NO_IOPORT (CONFIG_NO_IOPORT_MAP once my
    patches went in) to remove support for ioport_map.

    Arnd


    \
     
     \ /
      Last update: 2011-08-31 16:49    [W:2.823 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site