> Hello Linus, hello everyone,
> I asked few days ago about correct way to crossplatform accesses to
> PCI MMIO - specifically how to access MMIO with same code on Alpha and
> PPC. Unfortunately, there was only one reply - whether I'm porting
> matroxfb to Alpha. Yes, I'm.
> So again, currently I'm using
> char* virtual = ioremap(pcidev->base_address[x], 8MB);
> and then
> virtual[y*size + x] = pixel;
> for writting or
> pixel = virtual[y*size + x];
> for reading and
> iounmap(virtual);
> at the end for release resources.
> But I was pointed to that this does not work on Alpha - driver must use
> writex and readx macros. Unfortunately, these macros DO NOT work for me:
I haven't been able 2 years ago to make the ncr53c8xx driver work with
MMIO on Alpha even using readX/writeX and have had to not provide these
feature since I haven't access to an Alpha in order to test the thing. If
it now works, it should be interesting to give it a try.
> 1) they probably works on Alpha as expected, so no problem there
> 2) they do not work at all on PPC because of they swap bytes to little
> endian format. While it is (I'm not too much sure) probably correct
I also think this has been a bad choice and I had informed Cort of my
opinion when this has been decided. Seems the goal was to transparently
deal with byte ordering from PCI which is little-endian.
Below is what I suggested in the ncr53c8xx driver to provide from arch
dependencies:
/*
** If the CPU and the NCR use same endian-ness adressing,
** no byte reordering is needed for accessing chip io
** registers. Functions suffixed by '_raw' are assumed
** to access the chip over the PCI without doing byte
** reordering. Functions suffixed by '_l2b' are
** assumed to perform little-endian to big-endian byte
** reordering, those suffixed by '_b2l' blah, blah,
** blah, ...
*/
#if defined(NCR_IOMAPPED)
/*
** IO mapped only input / ouput
*/
#define INB_OFF(o) inb (np->base_io + ncr_offb(o))
#define OUTB_OFF(o, val) outb ((val), np->base_io + ncr_offb(o))
#if defined(__BIG_ENDIAN) && !defined(SCSI_NCR_BIG_ENDIAN)
#define INW_OFF(o) inw_l2b (np->base_io + ncr_offw(o))
#define INL_OFF(o) inl_l2b (np->base_io + (o))
#define OUTW_OFF(o, val) outw_b2l ((val), np->base_io + ncr_offw(o))
#define OUTL_OFF(o, val) outl_b2l ((val), np->base_io + (o))
#elif defined(__LITTLE_ENDIAN) && defined(SCSI_NCR_BIG_ENDIAN)
#define INW_OFF(o) inw_b2l (np->base_io + ncr_offw(o))
#define INL_OFF(o) inl_b2l (np->base_io + (o))
#define OUTW_OFF(o, val) outw_l2b ((val), np->base_io + ncr_offw(o))
#define OUTL_OFF(o, val) outl_l2b ((val), np->base_io + (o))
#else
#define INW_OFF(o) inw_raw (np->base_io + ncr_offw(o))
#define INL_OFF(o) inl_raw (np->base_io + (o))
#define OUTW_OFF(o, val) outw_raw ((val), np->base_io + ncr_offw(o))
#define OUTL_OFF(o, val) outl_raw ((val), np->base_io + (o))
#endif /* ENDIANs */
#else /* defined NCR_IOMAPPED */
/*
** MEMORY mapped IO input / output
*/
#define INB_OFF(o) readb((char *)np->reg + ncr_offb(o))
#define OUTB_OFF(o, val) writeb((val), (char *)np->reg + ncr_offb(o))
#if defined(__BIG_ENDIAN) && !defined(SCSI_NCR_BIG_ENDIAN)
#define INW_OFF(o) readw_l2b((char *)np->reg + ncr_offw(o))
#define INL_OFF(o) readl_l2b((char *)np->reg + (o))
#define OUTW_OFF(o, val) writew_b2l((val), (char *)np->reg + ncr_offw(o))
#define OUTL_OFF(o, val) writel_b2l((val), (char *)np->reg + (o))
#elif defined(__LITTLE_ENDIAN) && defined(SCSI_NCR_BIG_ENDIAN)
#define INW_OFF(o) readw_b2l((char *)np->reg + ncr_offw(o))
#define INL_OFF(o) readl_b2l((char *)np->reg + (o))
#define OUTW_OFF(o, val) writew_l2b((val), (char *)np->reg + ncr_offw(o))
#define OUTL_OFF(o, val) writel_l2b((val), (char *)np->reg + (o))
#else
#define INW_OFF(o) readw_raw((char *)np->reg + ncr_offw(o))
#define INL_OFF(o) readl_raw((char *)np->reg + (o))
#define OUTW_OFF(o, val) writew_raw((val), (char *)np->reg + ncr_offw(o))
#define OUTL_OFF(o, val) writel_raw((val), (char *)np->reg + (o))
#endif
#endif /* defined NCR_IOMAPPED */
#define INB(r) INB_OFF (offsetof(struct ncr_reg,r))
#define INW(r) INW_OFF (offsetof(struct ncr_reg,r))
#define INL(r) INL_OFF (offsetof(struct ncr_reg,r))
#define OUTB(r, val) OUTB_OFF (offsetof(struct ncr_reg,r), (val))
#define OUTW(r, val) OUTW_OFF (offsetof(struct ncr_reg,r), (val))
#define OUTL(r, val) OUTL_OFF (offsetof(struct ncr_reg,r), (val))
[ ... ]
> b) readl/writel should not do anything with address
Old ALphas are only able to perform 32 bit and 64 bit memory accesses and
you have to use sparse space for 16 bit and 8 bit MMIO accesses. It is
hard for 8/16 bit access to do nothing with address on old Alpha.
> c) virt_to_phys and virt_to_bus should walk through pagetables instead
> of anding/oring with something with something
> (or explicitly say that they cannot be used for shared (adaptor)
> memory... after reading up and down through IO-mapping it looks
> clear to me, but...)
> (after this change, you can do DMA I/O directly to/from userspace
> much easier)
Other UNIX systems donnot require the kernel IOable virtual address space
to be physically contiguous and performs virtual to physical/bus address
translations from low-level drivers, for scatter/gather for example.
Anyway, they remap user buffer in the kernel virtual space for physical
IOs and so only provides kernel virtual addresses to drivers in all
situations.
Linux handles this differently. There are probably reasons???
(BTW, that's the reason we don't have raw devices for now)
> d) phys_to_virt, bus_to_virt should be removed, it cannot be done
> in linear time (I'm not aware of such solution (on ia32)) and
> ioremap is already here and should be used instead of this one-time
> translation) (or explicitly say that for shared (adaptor) memory
> it cannot be used)
Yes.
These functions seem to only exist in Linux. I recommend not to use them
at all, but to prefer some reverse table using hashcoding (for example) if
you really need to retrieve the virtal address that corresponds to a bus
physical address returned by a device.
> e) that means:
> 1) clear readl/writel interface that passed address must
> be obtained from ioremap (so first example fro IO-mapping.txt -
> - readl(0xC0000) - is wrong). On ia32, ioremap can return
> virtual address and readl/writel are simple reading/writting (no
> playing with PAGE_OFFSET), on alpha, ioremap can return bus
Indeed.
> address and readl/writel are complicated function as they are
> now.
> 2) same for memcpy_toio, memcpy_fromio, memset_io, so examples for
> these functions in IO-mapping.txt are no longer valid.
> f) it must be done before 2.2, because of otherwise it is not possible
> to have more than 1.8 GB of memory in an Intel box (you can have
> it... but you cannot use it as system memory) (I know that no one
> real user uses ia32 this time, but there are such)
I would agree, but 2.2 is already so late ...
> So unless someone has more correct solution (please, please), I have to use
> #ifdef __alpha__
> #define mga_readl(x) readl(x)
> #else
> #define mga_readl(x) (*(volatile u_int32_t*)(x))
> #endif
> But I do not want to have this crappy code in my driver because of I think
> that it should not be there (I've found only four #ifdef __alpha__ in
> drivers subtree of kernel, so I think that it is not right way to go).
Implementing abstractions at driver level is sometimes the only solution
we have. If they are fine, you will just have to replace them by the
official right implementation when such will be available.
There is always room for improvement, but releases must also go out of the
door.
[ ... ]
Regards,
Gerard.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/