Proposed changes to io.h

14 messages, 5 authors, 2004-04-01 · open the first message on its own page

Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 15:44:25

I've made few changes to include/asm-ppc/io.h that might want to be
incorporated into the mainline tree.  These changes include:

1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
cache-coherent allocations (consistent_alloc(), etc.), just subracting
KERNELBASE from the virtual address is no longer sufficient.  Because
of this, I have modified virt_to_phys and phys_to_virt to look like:

extern inline unsigned long virt_to_phys(volatile void * address)
{
     unsigned long phys_addr;
     unsigned long virt_addr = (unsigned long) address;

#ifndef CONFIG_APUS
     if ((virt_addr >= KERNELBASE) && (virt_addr < (unsigned long)
high_memory))
         phys_addr = (virt_addr - KERNELBASE);
     else
         phys_addr = iopa (virt_addr);
#else
     phys_addr = iopa (virt_addr);
#endif

     return phys_addr;
}

extern inline void * phys_to_virt(unsigned long address)
{
#ifndef CONFIG_APUS
     return (void *) (address + KERNELBASE);
#else
     return (void*) mm_ptov (address);
#endif
}

And I have modified virt_to_bus and bus_to_virt to use those inlined
functions:

extern inline unsigned long virt_to_bus(volatile void * address)
{
     if (address == (void *) 0)
         return 0;

     return virt_to_phys (address) + PCI_DRAM_OFFSET;
}

extern inline void * bus_to_virt(unsigned long address)
{
     if (address == 0)
         return 0;

     return phys_to_virt (address - PCI_DRAM_OFFSET);
}

This simplifies the "bus" routines, and makes sure that they use the
standard virtual/physical translations.


2. I'd like to add 64-bit __raw_readll and __raw_writell routines to
io.h, done using floating-point registers.  Currently, modules such as
MTD (when writing to 64-bit buses) perform two 32-bit, non-atomic
writes, which can cause problems.  Using a floating-point register to
guarantee a 64-bit write is ugly, but it works.  Code for these inlined
routines is as follows:

/*
  * For reading and writing 64-bit values, we need to use the floating
point
  * registers.  The code will enable MSR_FP in the MSR register, use
FPR1 to
  * read from and write to memory, and then restore everything to the
  * previous values.
  */
#define __raw_readll __raw_readll
static inline unsigned long long __raw_readll (int addr)
{
     unsigned long flags;
     unsigned long msr;
     unsigned long msr_save;
     unsigned long long result;
     unsigned long long fp_save;

     local_irq_save (flags);
     asm volatile ("sync\n"
                   "mfmsr    %0\n"
                   "ori      %1,%0,0x2000\n"
                   "mtmsr    %1\n"
                   "isync\n"
                   "stfdx    1,0,%2\n"
                   "lfdx     1,0,%4\n"
                   "stfdx    1,0,%3\n"
                   "sync\n"
                   "lfdx     1,0,%2\n"
                   "mtmsr    %0\n"
                   "sync\n"
                   "isync\n"
                   : "=&r" (msr_save), "=&r" (msr)
                   : "r" (&fp_save), "r" (&result), "r" (addr)
                   : "memory" );
     local_irq_restore (flags);

     return result;
}

#define __raw_writell __raw_writell
static inline void __raw_writell (unsigned long long value, int addr)
{
     unsigned long flags;
     unsigned long msr;
     unsigned long msr_save;
     unsigned long long fp_save;

     local_irq_save (flags);
     asm volatile ("sync\n"
                   "mfmsr    %0\n"
                   "ori      %1,%0,0x2000\n"
                   "mtmsr    %1\n"
                   "isync\n"
                   "stfdx    1,0,%2\n"
                   "lfdx     1,0,%3\n"
                   "stfdx    1,0,%4\n"
                   "sync\n"
                   "lfdx     1,0,%2\n"
                   "mtmsr    %0\n"
                   "sync\n"
                   "isync\n"
                   : "=&r" (msr_save), "=&r" (msr)
                   : "r" (&fp_save), "r" (&value), "r" (addr)
                   : "memory" );
     local_irq_restore (flags);
     return;
}


Questions, comments, or flames?

John Whitney

Re: Proposed changes to io.h

From: Eugene Surovegin <hidden>
Date: 2004-03-31 16:44:23

On Wed, Mar 31, 2004 at 10:44:25AM -0500, John Whitney wrote:
2. I'd like to add 64-bit __raw_readll and __raw_writell routines to
io.h, done using floating-point registers.  Currently, modules such as
MTD (when writing to 64-bit buses) perform two 32-bit, non-atomic
writes, which can cause problems.  Using a floating-point register to
guarantee a 64-bit write is ugly, but it works.  Code for these inlined
routines is as follows:
[snip]

I wonder will it work on 4xx CPUs which don't have floating point unit?

Eugene.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Dan Malek <hidden>
Date: 2004-03-31 16:58:27

Eugene Surovegin wrote:
I wonder will it work on 4xx CPUs which don't have floating point unit?
It won't work on anything, will it?  Floating point isn't enabled for
the kernel, unless I missed that update.


	-- Dan


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 17:32:47

On Mar 31, 2004, at 11:58 AM, Dan Malek wrote:
Eugene Surovegin wrote:
quoted
I wonder will it work on 4xx CPUs which don't have floating point
unit?
It won't work on anything, will it?  Floating point isn't enabled for
the kernel, unless I missed that update.
This code specifically handles this by enabling the FP bit in the MSR,
bypassing the standard floating point handling.  It saves the contents
of FP0, uses it, restores the contents, restores the MSR, and exits.

John


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Dan Malek <hidden>
Date: 2004-03-31 17:40:16

John Whitney wrote:
This code specifically handles this .....

OK, already, enough of the e-mails, especially the personal ones. :-P

I was stupid and didn't read the code or comments, just the e-mail
explanation.  I don't normally read code unless it is in a patch
that can be compared to something so I can see what has changed.

Thanks.


	-- Dan


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Christoph Hellwig <hch@lst.de>
Date: 2004-03-31 18:18:52

On Wed, Mar 31, 2004 at 10:44:25AM -0500, John Whitney wrote:
1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
These are all obsolete interface and you're not supposed to use them
for new plattforms or drivers at all.
2. I'd like to add 64-bit __raw_readll and __raw_writell routines to
io.h, done using floating-point registers.  Currently, modules such as
MTD (when writing to 64-bit buses) perform two 32-bit, non-atomic
writes, which can cause problems.  Using a floating-point register to
guarantee a 64-bit write is ugly, but it works.  Code for these inlined
routines is as follows:
Standard-naming for those are readq/writeq afaik


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 18:40:15

quoted
1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
These are all obsolete interface and you're not supposed to use them
for new plattforms or drivers at all.
Just out of curiosity, are there replacements or are you just supposed
to do good-practice things like store the dma address returned by
pci_alloc_consistent/dma_alloc_coherent?
Standard-naming for those are readq/writeq afaik
Specifically, the MTD code looks for __raw_readll/writell, which is why
I used the convention I did.  I'll be happy to update the names if
desired.

John


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Christoph Hellwig <hch@lst.de>
Date: 2004-03-31 18:43:53

On Wed, Mar 31, 2004 at 01:40:15PM -0500, John Whitney wrote:
Just out of curiosity, are there replacements or are you just supposed
to do good-practice things like store the dma address returned by
pci_alloc_consistent/dma_alloc_coherent?
The latter.  (or pci_map* / dma_map*)
quoted
Standard-naming for those are readq/writeq afaik
Specifically, the MTD code looks for __raw_readll/writell, which is why
I used the convention I did.  I'll be happy to update the names if
desired.
What mtd code?

hch@bird:/repo/repo/linux-2.5$ grep __raw_readll include/asm-*/io.h | wc -l
0
hch@bird:/repo/repo/linux-2.5$ grep __raw_readq include/asm-*/io.h | wc -l
22


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 18:50:43

What mtd code?

hch@bird:/repo/repo/linux-2.5$ grep __raw_readll include/asm-*/io.h |
wc -l
0
hch@bird:/repo/repo/linux-2.5$ grep __raw_readq include/asm-*/io.h |
wc -l
22
Specifically, include/linux/mtd/cfi.h and include/linux/mtd/map.h.  If
__raw_readll/writell aren't defined (as macros), they define them as a
direct read/write from a volatile u64 pointer.  I'll copy this over to
an MTD developer as well, perhaps we can fix MTD to use the standard
__raw_readq/writeq instead.

John


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 21:09:13

quoted
1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
These are all obsolete interface and you're not supposed to use them
for new plattforms or drivers at all.
pci_map_single() (include/asm-ppc/pci.h) and dma_map_single()
(include/asm-ppc/dma-mapping.h) use virt_to_bus().  Shouldn't some care
be taken to ensure that ANY virtual address passed to these routines
maps to the correct physical (bus) address?

John


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Dan Malek <hidden>
Date: 2004-03-31 21:49:20

John Whitney wrote:

 > ..... Shouldn't some care
be taken to ensure that ANY virtual address passed to these routines
maps to the correct physical (bus) address?
Don't go there.  This has been discussed over and over in the past
and as Matt said, this is not the place.  Read the archives if
you want to see these discussions.

The code works according to the documentation, and the implementation
of the pci_*/dma_* functions allows the flexibility we need to support
systems where the mapping is more challenging.  The kernel and drivers
should not be directly using virt_to_bus() and all of its cousins, they
should use the DMA mapping functions documented.


	-- Dan


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Eugene Surovegin <hidden>
Date: 2004-03-31 21:52:48

On Wed, Mar 31, 2004 at 04:09:13PM -0500, John Whitney wrote:
quoted
quoted
1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
These are all obsolete interface and you're not supposed to use them
for new plattforms or drivers at all.
pci_map_single() (include/asm-ppc/pci.h) and dma_map_single()
(include/asm-ppc/dma-mapping.h) use virt_to_bus().  Shouldn't some care
be taken to ensure that ANY virtual address passed to these routines
maps to the correct physical (bus) address?
Please, read DMA-mapping.txt, it's quite clear on what memory is considered
DMA'able.

Eugene.

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: John Whitney <hidden>
Date: 2004-03-31 22:07:38

Please, read DMA-mapping.txt, it's quite clear on what memory is
considered
DMA'able.
Sorry, Eugene.  It just seemed odd that dma_map_single(), called with
the virtual address produced by dma_alloc_coherent(), would return the
wrong address.  Thanks for the insight.

John


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Proposed changes to io.h

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2004-04-01 02:52:23

On Thu, 2004-04-01 at 01:44, John Whitney wrote:
I've made few changes to include/asm-ppc/io.h that might want to be
incorporated into the mainline tree.  These changes include:

1. Modifications to virt_to_bus, bus_to_virt, virt_to_phys, and
phys_to_virt.  With the use of fully virtual addresses for
cache-coherent allocations (consistent_alloc(), etc.), just subracting
KERNELBASE from the virtual address is no longer sufficient.  Because
of this, I have modified virt_to_phys and phys_to_virt to look like:
Those functions are deprecated actually (the _bus_ ones at least,
the _phys_ one can still be used by some internal arch code, though
it's well known that they will only work with the linear mapping,
thus a simple substraction is enough).
This simplifies the "bus" routines, and makes sure that they use the
standard virtual/physical translations.
The proper simplification is to kill them
2. I'd like to add 64-bit __raw_readll and __raw_writell routines to
io.h, done using floating-point registers.  Currently, modules such as
MTD (when writing to 64-bit buses) perform two 32-bit, non-atomic
writes, which can cause problems.  Using a floating-point register to
guarantee a 64-bit write is ugly, but it works.  Code for these inlined
routines is as follows:
First, the proper name is readq/writeq ;) Then, most machines may
not have a 64 bits IO bus anyway, I don't think we need to provide
those functions for ppc32 and the MSR munging will cost you more
than the benefit of doing a 64 bits access.

So unless you have a specific need for those, I don't think we
need that in the kernel. Note about your implementation: you could
probably clear MSR:EE at the same time as you set MSR:FP instead
of using local_irq_save outside of the asm block ;)

Ben.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help