linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] powerpc/time: Rename mftbl() to mftb()
@ 2020-10-01 12:42 Christophe Leroy
  2020-10-01 12:42 ` [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64 Christophe Leroy
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

On PPC64, we have mftb().
On PPC32, we have mftbl() and an #define mftb() mftbl().

mftb() and mftbl() are equivalent, their purpose is to read the
content of SPRN_TRBL, as returned by 'mftb' simplified instruction.

binutils seems to define 'mftbl' instruction as an equivalent
of 'mftb'.

However in both 32 bits and 64 bits documentation, only 'mftb' is
defined, and when performing a disassembly with objdump, the displayed
instruction is 'mftb'

No need to have two ways to do the same thing with different
names, rename mftbl() to have only mftb().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/reg.h  | 5 ++---
 arch/powerpc/include/asm/time.h | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 788058af1d44..c66dcdb47c44 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1439,19 +1439,18 @@ static inline void msr_check_and_clear(unsigned long bits)
 #else /* __powerpc64__ */
 
 #if defined(CONFIG_PPC_8xx)
-#define mftbl()		({unsigned long rval;	\
+#define mftb()		({unsigned long rval;	\
 			asm volatile("mftbl %0" : "=r" (rval)); rval;})
 #define mftbu()		({unsigned long rval;	\
 			asm volatile("mftbu %0" : "=r" (rval)); rval;})
 #else
-#define mftbl()		({unsigned long rval;	\
+#define mftb()		({unsigned long rval;	\
 			asm volatile("mfspr %0, %1" : "=r" (rval) : \
 				"i" (SPRN_TBRL)); rval;})
 #define mftbu()		({unsigned long rval;	\
 			asm volatile("mfspr %0, %1" : "=r" (rval) : \
 				"i" (SPRN_TBRU)); rval;})
 #endif
-#define mftb()		mftbl()
 #endif /* !__powerpc64__ */
 
 #define mttbl(v)	asm volatile("mttbl %0":: "r"(v))
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index a80abf64c8a5..b0fb8456305f 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -50,7 +50,7 @@ struct div_result {
 
 static inline unsigned long get_tbl(void)
 {
-	return mftbl();
+	return mftb();
 }
 
 static inline unsigned int get_tbu(void)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
@ 2020-10-01 12:42 ` Christophe Leroy
  2020-10-01 12:42 ` [PATCH 3/6] powerpc/time: Avoid using get_tbl() and get_tbu() internally Christophe Leroy
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

No need to have two versions that are identical.

CONFIG_PPC_CELL is only selected by PPC64 targets.
CONFIG_E500 is the only PPC64 target selecting CONFIG_FSL_BOOK3E.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/reg.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index c66dcdb47c44..f877a576b338 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1419,8 +1419,7 @@ static inline void msr_check_and_clear(unsigned long bits)
 		__msr_check_and_clear(bits);
 }
 
-#ifdef __powerpc64__
-#if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
+#if defined(CONFIG_PPC_CELL) || defined(CONFIG_E500)
 #define mftb()		({unsigned long rval;				\
 			asm volatile(					\
 				"90:	mfspr %0, %2;\n"		\
@@ -1430,28 +1429,23 @@ static inline void msr_check_and_clear(unsigned long bits)
 			: "=r" (rval) \
 			: "i" (CPU_FTR_CELL_TB_BUG), "i" (SPRN_TBRL) : "cr0"); \
 			rval;})
+#elif defined(CONFIG_PPC_8xx)
+#define mftb()		({unsigned long rval;	\
+			asm volatile("mftbl %0" : "=r" (rval)); rval;})
 #else
 #define mftb()		({unsigned long rval;	\
 			asm volatile("mfspr %0, %1" : \
 				     "=r" (rval) : "i" (SPRN_TBRL)); rval;})
 #endif /* !CONFIG_PPC_CELL */
 
-#else /* __powerpc64__ */
-
 #if defined(CONFIG_PPC_8xx)
-#define mftb()		({unsigned long rval;	\
-			asm volatile("mftbl %0" : "=r" (rval)); rval;})
 #define mftbu()		({unsigned long rval;	\
 			asm volatile("mftbu %0" : "=r" (rval)); rval;})
 #else
-#define mftb()		({unsigned long rval;	\
-			asm volatile("mfspr %0, %1" : "=r" (rval) : \
-				"i" (SPRN_TBRL)); rval;})
 #define mftbu()		({unsigned long rval;	\
 			asm volatile("mfspr %0, %1" : "=r" (rval) : \
 				"i" (SPRN_TBRU)); rval;})
 #endif
-#endif /* !__powerpc64__ */
 
 #define mttbl(v)	asm volatile("mttbl %0":: "r"(v))
 #define mttbu(v)	asm volatile("mttbu %0":: "r"(v))
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] powerpc/time: Avoid using get_tbl() and get_tbu() internally
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
  2020-10-01 12:42 ` [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64 Christophe Leroy
@ 2020-10-01 12:42 ` Christophe Leroy
  2020-10-01 12:42 ` [PATCH 4/6] powerpc/time: Remove get_tbu() Christophe Leroy
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

get_tbl() is confusing as it returns the content of TBL register
on PPC32 but the concatenation of TBL and TBU on PPC64.

Use mftb() instead.

Do the same with get_tbu() for consistency allthough it's name
is less confusing.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/delay.h | 2 +-
 arch/powerpc/include/asm/time.h  | 8 ++++----
 arch/powerpc/kernel/time.c       | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index 66963f7d3e64..51bb8c1476c7 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -54,7 +54,7 @@ extern void udelay(unsigned long usecs);
 ({                                                                             \
 	typeof(condition) __ret;                                               \
 	unsigned long __loops = tb_ticks_per_usec * timeout;                   \
-	unsigned long __start = get_tbl();                                     \
+	unsigned long __start = mftb();                                     \
                                                                                \
 	if (delay) {                                                           \
 		while (!(__ret = (condition)) &&                               \
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index b0fb8456305f..3ef0f4b3299e 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -97,9 +97,9 @@ static inline u64 get_tb(void)
 	unsigned int tbhi, tblo, tbhi2;
 
 	do {
-		tbhi = get_tbu();
-		tblo = get_tbl();
-		tbhi2 = get_tbu();
+		tbhi = mftbu();
+		tblo = mftb();
+		tbhi2 = mftbu();
 	} while (tbhi != tbhi2);
 
 	return ((u64)tbhi << 32) | tblo;
@@ -153,7 +153,7 @@ static inline unsigned long tb_ticks_since(unsigned long tstamp)
 		int delta = get_rtcl() - (unsigned int) tstamp;
 		return delta < 0 ? delta + 1000000000 : delta;
 	}
-	return get_tbl() - tstamp;
+	return mftb() - tstamp;
 }
 
 #define mulhwu(x,y) \
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index f85539ebb513..a9cbd5a61585 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -467,8 +467,8 @@ void __delay(unsigned long loops)
 		 */
 		spin_cpu_relax();
 	} else {
-		start = get_tbl();
-		while (get_tbl() - start < loops)
+		start = mftb();
+		while (mftb() - start < loops)
 			spin_cpu_relax();
 	}
 	spin_end();
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] powerpc/time: Remove get_tbu()
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
  2020-10-01 12:42 ` [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64 Christophe Leroy
  2020-10-01 12:42 ` [PATCH 3/6] powerpc/time: Avoid using get_tbl() and get_tbu() internally Christophe Leroy
@ 2020-10-01 12:42 ` Christophe Leroy
  2020-10-01 12:42 ` [PATCH 5/6] powerpc/time: Make get_tbl() common to PPC32 and PPC64 Christophe Leroy
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

get_tbu() is redundant with mftbu() and is not used anymore.

Remove it.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/time.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 3ef0f4b3299e..c4ea81c966b0 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -52,11 +52,6 @@ static inline unsigned long get_tbl(void)
 {
 	return mftb();
 }
-
-static inline unsigned int get_tbu(void)
-{
-	return mftbu();
-}
 #endif /* !CONFIG_PPC64 */
 
 static inline unsigned int get_rtcl(void)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] powerpc/time: Make get_tbl() common to PPC32 and PPC64
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
                   ` (2 preceding siblings ...)
  2020-10-01 12:42 ` [PATCH 4/6] powerpc/time: Remove get_tbu() Christophe Leroy
@ 2020-10-01 12:42 ` Christophe Leroy
  2020-10-01 12:42 ` [PATCH 6/6] powerpc/time: Make get_tb() " Christophe Leroy
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

On PPC64, get_tbl() is defined as an alias of get_tb() which return
the result of mftb(). That exactly the same as what the PPC32 version
does. We don't need two versions.

Remove the PPC64 definition of get_tbl() and use the PPC32 version
for both.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/time.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index c4ea81c966b0..01b054b9766f 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -41,18 +41,11 @@ struct div_result {
 /* Accessor functions for the timebase (RTC on 601) registers. */
 #define __USE_RTC()	(IS_ENABLED(CONFIG_PPC_BOOK3S_601))
 
-#ifdef CONFIG_PPC64
-
 /* For compatibility, get_tbl() is defined as get_tb() on ppc64 */
-#define get_tbl		get_tb
-
-#else
-
 static inline unsigned long get_tbl(void)
 {
 	return mftb();
 }
-#endif /* !CONFIG_PPC64 */
 
 static inline unsigned int get_rtcl(void)
 {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] powerpc/time: Make get_tb() common to PPC32 and PPC64
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
                   ` (3 preceding siblings ...)
  2020-10-01 12:42 ` [PATCH 5/6] powerpc/time: Make get_tbl() common to PPC32 and PPC64 Christophe Leroy
@ 2020-10-01 12:42 ` Christophe Leroy
  2020-10-01 20:44 ` [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Segher Boessenkool
  2020-10-09  6:03 ` Michael Ellerman
  6 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

mftbu() is always defined now, so the #ifdef can be removed
and replaced by an IS_ENABLED(CONFIG_PPC64) inside the
PPC32 version of get_tb().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/time.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 01b054b9766f..418edaba8bd0 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -74,16 +74,13 @@ static inline u64 get_vtb(void)
 	return 0;
 }
 
-#ifdef CONFIG_PPC64
-static inline u64 get_tb(void)
-{
-	return mftb();
-}
-#else /* CONFIG_PPC64 */
 static inline u64 get_tb(void)
 {
 	unsigned int tbhi, tblo, tbhi2;
 
+	if (IS_ENABLED(CONFIG_PPC64))
+		return mftb();
+
 	do {
 		tbhi = mftbu();
 		tblo = mftb();
@@ -92,7 +89,6 @@ static inline u64 get_tb(void)
 
 	return ((u64)tbhi << 32) | tblo;
 }
-#endif /* !CONFIG_PPC64 */
 
 static inline u64 get_tb_or_rtc(void)
 {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/6] powerpc/time: Rename mftbl() to mftb()
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
                   ` (4 preceding siblings ...)
  2020-10-01 12:42 ` [PATCH 6/6] powerpc/time: Make get_tb() " Christophe Leroy
@ 2020-10-01 20:44 ` Segher Boessenkool
  2020-10-09  6:03 ` Michael Ellerman
  6 siblings, 0 replies; 8+ messages in thread
From: Segher Boessenkool @ 2020-10-01 20:44 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel

On Thu, Oct 01, 2020 at 12:42:39PM +0000, Christophe Leroy wrote:
> On PPC64, we have mftb().
> On PPC32, we have mftbl() and an #define mftb() mftbl().
> 
> mftb() and mftbl() are equivalent, their purpose is to read the
> content of SPRN_TRBL, as returned by 'mftb' simplified instruction.
> 
> binutils seems to define 'mftbl' instruction as an equivalent
> of 'mftb'.
> 
> However in both 32 bits and 64 bits documentation, only 'mftb' is
> defined, and when performing a disassembly with objdump, the displayed
> instruction is 'mftb'
> 
> No need to have two ways to do the same thing with different
> names, rename mftbl() to have only mftb().

There are mttbl and mttbu insns (and no mttb insn); they write a 32-bit
half for the time base.  There is an mftb, and an mftbu.  mftbu reads
the upper half, while mftb reads the *whole* register.  SPR 269 is the
TBU register, while SPR 268 is called both TB and TBL.  Yes, it is
confusing :-)

The "mftb" name is much clearer than "mftbl" (on 64-bit), because it
reads the whole 64-bit register.  On 32-bit mftbl is clearer (but not
defined in the architecture, not officially an insn or even an extended
mnemonic).


Segher

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/6] powerpc/time: Rename mftbl() to mftb()
  2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
                   ` (5 preceding siblings ...)
  2020-10-01 20:44 ` [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Segher Boessenkool
@ 2020-10-09  6:03 ` Michael Ellerman
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2020-10-09  6:03 UTC (permalink / raw)
  To: Paul Mackerras, Benjamin Herrenschmidt, Michael Ellerman,
	Christophe Leroy
  Cc: linuxppc-dev, linux-kernel

On Thu, 1 Oct 2020 12:42:39 +0000 (UTC), Christophe Leroy wrote:
> On PPC64, we have mftb().
> On PPC32, we have mftbl() and an #define mftb() mftbl().
> 
> mftb() and mftbl() are equivalent, their purpose is to read the
> content of SPRN_TRBL, as returned by 'mftb' simplified instruction.
> 
> binutils seems to define 'mftbl' instruction as an equivalent
> of 'mftb'.
> 
> [...]

Applied to powerpc/next.

[1/6] powerpc/time: Rename mftbl() to mftb()
      https://git.kernel.org/powerpc/c/15c102153e722cc6e0729764a7068c209a7469cd
[2/6] powerpc/time: Make mftb() common to PPC32 and PPC64
      https://git.kernel.org/powerpc/c/ff125fbcd45d1706861579dbe66e31f5b3f1e779
[3/6] powerpc/time: Avoid using get_tbl() and get_tbu() internally
      https://git.kernel.org/powerpc/c/942e89115b588b4b5df86930b5302a5c07b820ba
[4/6] powerpc/time: Remove get_tbu()
      https://git.kernel.org/powerpc/c/e8d5bf30eafc37e31ce68bc7ccf1db970fe3cd04
[5/6] powerpc/time: Make get_tbl() common to PPC32 and PPC64
      https://git.kernel.org/powerpc/c/1156a6285cd38e5a6987ddee3758e7954c56cb3d
[6/6] powerpc/time: Make get_tb() common to PPC32 and PPC64
      https://git.kernel.org/powerpc/c/9686e431c683ee7b8aca0f3985c244aee3d9f30d

cheers

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-09  6:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 12:42 [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Christophe Leroy
2020-10-01 12:42 ` [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64 Christophe Leroy
2020-10-01 12:42 ` [PATCH 3/6] powerpc/time: Avoid using get_tbl() and get_tbu() internally Christophe Leroy
2020-10-01 12:42 ` [PATCH 4/6] powerpc/time: Remove get_tbu() Christophe Leroy
2020-10-01 12:42 ` [PATCH 5/6] powerpc/time: Make get_tbl() common to PPC32 and PPC64 Christophe Leroy
2020-10-01 12:42 ` [PATCH 6/6] powerpc/time: Make get_tb() " Christophe Leroy
2020-10-01 20:44 ` [PATCH 1/6] powerpc/time: Rename mftbl() to mftb() Segher Boessenkool
2020-10-09  6:03 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).