mips: PMC MSP71xx mips common
Quoted 25 lines · Ralf Baechle
On Wed, Mar 07, 2007 at 12:01:34PM -0600, Marc St-Jean wrote:
Looks fine ... except this one:
include/asm-mips/regops.h line 2
Quoted 8 lines
include/asm-mips/regops.h new file +2
-0@@ -0,0 +1,166 @@ +/* +* VPE/SMP-safe functions to access registers. They use ll/sc instructions, so
include/asm-mips/regops.h line 3
Quoted 1 line
include/asm-mips/regops.h new file +1
-0+* it is your responsibility to ensure these are available on your platform
include/asm-mips/regops.h line 6
Quoted 3 lines
include/asm-mips/regops.h new file +3
-0+* before including this file. +* +* In addition, there is a bug on the R10000 chips which has a workaround. If
include/asm-mips/regops.h line 8
Quoted 2 lines
include/asm-mips/regops.h new file +2
-0+* you are affected by this bug, make sure to define the symbol +* 'R10000_LLSC_WAR' to be non-zero. If you are using this header from within
include/asm-mips/regops.h line 9
Quoted 1 line
include/asm-mips/regops.h new file +1
-0+* linux, you may include <asm/war.h> before including this file to have this
Quoted 1 line
include/asm-mips/regops.h new file +1
-0+* defined appropriately for you.
[...]
Quoted 11 lines
include/asm-mips/regops.h
Quoted 8 lines
+ */
include/asm-mips/regops.h new file +7
-0+ +#ifndef __ASM_REGOPS_H__ +#define __ASM_REGOPS_H__ + +#ifndef R10000_LLSC_WAR +#define R10000_LLSC_WAR 0 +#endif Better include <
asm/war.h> orR10000_LLSC_WARmight have an unexpected value.
Thanks, will do.
Quoted 5 lines
Quoted 5 lines
include/asm-mips/regops.h new file +5
-0+#if R10000_LLSC_WAR ==1 +#define __beqz "beqzl " +#else +#define __beqz "beqz " +#endif
Quoted 8 lines
include/asm-mips/regops.h
Quoted 4 lines
include/asm-mips/regops.h new file +4
-0+ +#ifndef _LINUX_TYPES_H +typedef unsigned int uint32_t ; +#endif Again, include <
linux/types.h>. Including this file first then <linux/types.h>
will result in a duplicate definition ...
OK.
Quoted 73 lines
Quoted 73 lines
include/asm-mips/regops.h new file +73
-0+/* +* Sets all the masked bits to the corresponding value bits +*/ +static inline void set_value_reg32 (volatile uint32_t *const addr, +uint32_t const mask, +uint32_t const value ) +{ +uint32_t temp; + + __asm__ __volatile__( +" .set mips3 \n" +"1: ll %0, %1 # set_value_reg32 \n" +" and %0, %2 \n" +" or %0, %3 \n" +" sc %0, %1 \n" +" " __beqz"%0, 1b \n" +" .set mips0 \n" + :"=&r" (temp),"=m" (*addr) + :"ir" (~mask),"ir" (value),"m" (*addr) ); +} + +/* +* Sets all the masked bits to '1' +*/ +static inline void set_reg32 (volatile uint32_t *const addr, +uint32_t const mask ) +{ +uint32_t temp; + + __asm__ __volatile__( +" .set mips3 \n" +"1: ll %0, %1 # set_reg32 \n" +" or %0, %2 \n" +" sc %0, %1 \n" +" " __beqz"%0, 1b \n" +" .set mips0 \n" + :"=&r" (temp),"=m" (*addr) + :"ir" (mask),"m" (*addr) ); +} + +/* +* Sets all the masked bits to '0' +*/ +static inline void clear_reg32 (volatile uint32_t *const addr, +uint32_t const mask ) +{ +uint32_t temp; + + __asm__ __volatile__( +" .set mips3 \n" +"1: ll %0, %1 # clear_reg32 \n" +" and %0, %2 \n" +" sc %0, %1 \n" +" " __beqz"%0, 1b \n" +" .set mips0 \n" + :"=&r" (temp),"=m" (*addr) + :"ir" (~mask),"m" (*addr) ); +} + +/* +* Toggles all masked bits from '0' to '1' and '1' to '0' +*/ +static inline void toggle_reg32 (volatile uint32_t *const addr, +uint32_t const mask ) +{ +uint32_t temp; + + __asm__ __volatile__( +" .set mips3 \n" +"1: ll %0, %1 # toggle_reg32 \n" +" xor %0, %2 \n" +" sc %0, %1 \n" + " "__beqz"%0, 1b \n"
Quoted 8 lines
include/asm-mips/regops.h
Quoted 4 lines
include/asm-mips/regops.h new file +4
-0+ " .set mips0 \n" + : "=&r" (temp), "=m" (*addr) + : "ir" (mask), "m" (*addr) ); +}As I understand you're using the preceeding 4 functions with the address pointing to device registers, that is uncached memory? The operation of ll/sc on uncached memory is undefined.
I have been searching for information on this in various 34k manuals as well as Dominic's See MIPS Run 2nd Ed. and have not found this limitation stated anywhere.
Programming the MIPS32 34K Core Family (MD00427) Section 3.5 states:
"The MIPS MT ASE requires that ll/sc also work between concurrent threads on an MT CPU. Each TC is equipped with a CP0 register called LLAddr, which remembers the physical address (at least to the enclosing 32-byte block) of the target location of any ll/sc sequence. The 34K core detects possible non-atomicity by checking every write made by any thread against the LLAddr of all other TCs."
Doesn't the fact that a physical address is being compared imply any logical address type can be used?
Quoted 10 lines
So to fix this you'd have to use something like this:
unsigned long flags; spinlock_t lock; spin_lock_irqsave(&lock, flags); writel(readl(addr) | mask); spin_unlock_restore(&lock, flags);Which of course is considerably slower and heavier weight.
I don't believe we can use a linux lock as the code running on the other TCs is not necessarily a linux driver/thread/process. It does not share linux's lock implementation.
Quoted 47 lines
Quoted 3 lines
+/* For special strange cases only:
+ *
+ * If you need custom processing within a ll/sc loop, use thefollowing macros
Quoted 3 lines
+ * VERY CAREFULLY:
+ *
+ *uint32_ttmp; <-- Define a variable tohold the data
Quoted 2 lines
+ *
+ *custom_reg32_start(address, tmp); <-- Reads the addressand puts the value
Quoted 1 line
+ * in the 'tmp' variable
given
Quoted 2 lines
+ *
+ * < From here on out, you are (basicly) atomic, so don't doanything too
Quoted 3 lines
+ * < fancy! + * < Also, this code may loop if the end of this block fails to write + * < everything back safely due do the other CPU, so do NOT doanything
Quoted 3 lines
+ * < with side-effects!
+ *
+ *custom_reg32_stop(address, tmp); <-- Writes back 'tmp'safely.
Quoted 10 lines
+ *
+ */include/asm-mips/regops.h new file +8
-0+#define custom_reg32_read (address, tmp) \ + __asm__ __volatile__( \ + " .set mips3 \n" \ + "1: ll %0, %1 #custom_reg32_read \n" \ + " .set mips0 \n" \ + : "=r" (tmp), "=m" (*address) \ + : "m" (*address) ) +include/asm-mips/regops.h
Quoted 10 lines
include/asm-mips/regops.h new file +10
-0+#define custom_reg32_write (address, tmp) \ + __asm__ __volatile__( \ + " .set mips3 \n" \ + " sc %0, %1 #custom_reg32_write \n" \ + " "__beqz"%0, 1b \n" \ + " .set mips0 \n" \ + : "=&r" (tmp), "=m" (*address) \ + : "0" (tmp), "m" (*address) ) + +#endif /* __ASM_REGOPS_H__ */ Same comment as above applies for uncached accesses.
Quoted 8 lines
I used to have functions that expand into ll/sc in the kernel ages ago - gcc did generate interesting (for some definition of interesting) but not terribly useful code from it. The chance that something like this is happening has grown even further thanks to the more agressive optimizer of modern gcc.
A valid ll/sc sequence also requires that there may not be another load or store in between these two instructions. So for example:
MIPS32 34K Processor Core Family Software User's Manual (MD00534) Store Conditional Word instruction states:
"If either of the following events occurs between the execution of LL and SC, the SC may succeed or it may fail; the success or failure is not predictable. Portable programs should not cause one of these events.
-A memory access instruction (load, store, or prefetch) is executed on
the processor executing the LL/SC.
-The instructions executed starting with the LL and ending with the SC
do not lie in a 2048-byte contiguous region of virtual memory. (The region does not have to be aligned, other than the alignment required for instruction words.)"
For the first of these points, if a failure occurs because of activity on another TC, the loop should repeat until success.
For the second point, the code between custom_reg32_read and custom_reg32_write is meant to be very simple. We can live with a 2k limit.
Quoted 9 lines
(quoted diff) +8
-0+ custom_reg32_read(MSP_GPIO_DATA_REGISTER[gpio], + tmpdata); + if (tmpdata & EXTENDED_CLR(gpio)) + tmpdata = EXTENDED_CLR(gpio); + else + tmpdata = EXTENDED_SET(gpio); + custom_reg32_write(MSP_GPIO_DATA_REGISTER[gpio], + tmpdata);
Quoted 5 lines
What if gcc just because it feels like it puts tmpdata or gpio into the stackframe? In this particular case there is little register pressure so it's unlikely but be afraid, be very afraid ...
Ralf
We've been using this for a year with different versions of gcc and have not encountered any problems. We could declare tmpdata as "register" and increase the odds of it being allocated into a register.
Marc
Quoted 11 lines · Marc St-Jean
I have been searching for information on this in various 34k manuals as well as Dominic's See MIPS Run 2nd Ed. and have not found this limitation stated anywhere.
Programming the MIPS32 34K Core Family (MD00427) Section 3.5 states:
"The MIPS MT ASE requires that ll/sc also work between concurrent threads on an MT CPU. Each TC is equipped with a CP0 register called LLAddr, which remembers the physical address (at least to the enclosing 32-byte block) of the target location of any ll/sc sequence. The 34K core detects possible non-atomicity by checking every write made by any thread against the LLAddr of all other TCs."
These are implementation details for the 34K. The 34K manual and MIPS MT ASE documentation are extensions to the MIPS32 Revision 2 architecture.
Quoted 2 lines
Doesn't the fact that a physical address is being compared imply any logical address type can be used?
You would be relying on an implementation detail, not the guaranteed semantics of the instruction here.
Now, for a uniprocessor a _likely_ implementation is based on just the comparing either the virtual or physical address. I say likely because it's the simplest implementation. And yes, there is at least one MIPS processor implemetation that does it differently, and where SC will always fail even on uniprocessors when using uncached memory.
Quoted 9 lines
Quoted 9 lines
So to fix this you'd have to use something like this:
unsigned long flags; spinlock_t lock; spin_lock_irqsave(&lock, flags); writel(readl(addr) | mask); spin_unlock_restore(&lock, flags);
Quoted 5 lines
Quoted 1 line
Which of course is considerably slower and heavier weight.
I don't believe we can use a linux lock as the code running on the other TCs is not necessarily a linux driver/thread/process. It does not share linux's lock implementation.
In which case I suggest you move this file into an platform-specific directory and add a big and fat comment to ensure nobody reuses the code.
Quoted 11 lines
MIPS32 34K Processor Core Family Software User's Manual (MD00534) Store Conditional Word instruction states:
"If either of the following events occurs between the execution of LL and SC, the SC may succeed or it may fail; the success or failure is not predictable. Portable programs should not cause one of these events.
-A memory access instruction (load, store, or prefetch) is executed on
the processor executing the LL/SC.
-The instructions executed starting with the LL and ending with the SC
do not lie in a 2048-byte contiguous region of virtual memory. (The region does not have to be aligned, other than the alignment required for instruction words.)"
The MIPS32 spec says for the SC instruction:
[...]
If either of the following events occurs between the execution of LL and SC, the SC fails:
• A coherent store is completed by another processor or coherent I/O module into the block of synchronizable physical memory containing the word. The size and alignment of the block is implementation dependent, but it is at least one word and at most the minimum page size.
• An ERET instruction is executed.
[...]
Atomic RMW is provided only for synchronizable memory locations. A synchronizable memory location is one that is associated with the state and logic necessary to implement the LL/SC semantics. Whether a memory location is synchronizable depends on the processor and system configurations, and on the memory access type used for the location:
• Uniprocessor atomicity: To provide atomic RMW on a single processor, all accesses to the location must be made with memory access type of either cached noncoherent or cached coherent. All accesses must be to one or the other access type, and they may not be mixed.
• MP atomicity: To provide atomic RMW among multiple processors, all accesses to the location must be made with a memory access type of cached coherent.
• I/O System: To provide atomic RMW with a coherent I/O system, all accesses to the location must be made with a memory access type of cached coherent. If the I/O system does not use coherent memory operations, then atomic RMW cannot be provided with respect to the I/O reads and writes.
Restrictions:
The addressed location must have a memory access type of cached noncoherent or cached coherent; if it does not, the result is UNPREDICTABLE.
The effective address must be naturally-aligned. If either of the 2 least-significant bits of the address is non-zero, an Address Error exception occurs.
[...]
Quoted 6 lines
For the first of these points, if a failure occurs because of activity on another TC, the loop should repeat until success.
For the second point, the code between
custom_reg32_readandcustom_reg32_writeis meant to be very simple. We can live with a 2k limit.
The 2k limit did show up in the MIPS architecture I think for MIPS IV, so either the R8000 or R10000 over 10 years ago. I'm not aware what the technical reason for this restriction is. I was wondering if it was just future proofing.
Ralf