Re: [RFC PATCH v2 02/14] x86/hpet: Expose more functions to read and write registers

2019-04-08 Thread Ricardo Neri
On Tue, Mar 26, 2019 at 10:00:24PM +0100, Thomas Gleixner wrote:
> On Wed, 27 Feb 2019, Ricardo Neri wrote:
> >  struct irq_data;
> > @@ -109,6 +114,11 @@ extern void 
> > hpet_unregister_irq_handler(rtc_irq_handler handler);
> >  static inline int hpet_enable(void) { return 0; }
> >  static inline int is_hpet_enabled(void) { return 0; }
> >  #define hpet_readl(a) 0
> > +#define hpet_writel(d, a)
> 
> What for?
> 
> > +#ifdef CONFIG_X86_64
> > +#define hpet_readq(a) 0
> > +#define hpet_writeq(d, a)
> > +#endif
> 
> Ditto.
> 
> There are no users outside of HPET and your new HPET watchdog code for
> those. And both are not compiled when CONFIG_HPET=n.

I'll remove these unneeded defintions.
> 
> The only reason to have the hpet_readl() define, which btw. should be an
> inline, is to avoid massive ifdeffery in the TSC calibration code.

May I ask what is the problem with the #define hpet_readl()? Commit
bfc0f5947afa("x86: merge tsc calibration") changed it from inline to
#define. Should I change it back?

Thanks and BR,
Ricardo


Re: [RFC PATCH v2 02/14] x86/hpet: Expose more functions to read and write registers

2019-03-26 Thread Thomas Gleixner
On Wed, 27 Feb 2019, Ricardo Neri wrote:
>  struct irq_data;
> @@ -109,6 +114,11 @@ extern void hpet_unregister_irq_handler(rtc_irq_handler 
> handler);
>  static inline int hpet_enable(void) { return 0; }
>  static inline int is_hpet_enabled(void) { return 0; }
>  #define hpet_readl(a) 0
> +#define hpet_writel(d, a)

What for?

> +#ifdef CONFIG_X86_64
> +#define hpet_readq(a) 0
> +#define hpet_writeq(d, a)
> +#endif

Ditto.

There are no users outside of HPET and your new HPET watchdog code for
those. And both are not compiled when CONFIG_HPET=n.

The only reason to have the hpet_readl() define, which btw. should be an
inline, is to avoid massive ifdeffery in the TSC calibration code.

Thanks,

tglx


[RFC PATCH v2 02/14] x86/hpet: Expose more functions to read and write registers

2019-02-27 Thread Ricardo Neri
Some of the registers in the HPET hardware have a width of 64 bits. 64-bit
access functions are needed mostly to read the counter and write the
comparator in a single read or write. Also, 64-bit accesses can be used to
to read parameters located in the higher bits of some registers (such as
the timer period and the IO APIC pins that can be asserted by the timer)
without the need of masking and shifting the register values.

64-bit read and write functions are added. These functions, along with the
existing hpet_writel(), are exposed via the HPET header to be used by other
kernel subsystems.

Thus far, the only consumer of these functions will the HPET-based
hardlockup detector, which will only be available in 64-bit builds. Thus,
the 64-bit access functions are wrapped in CONFIG_X86_64.

Cc: "H. Peter Anvin" 
Cc: Ashok Raj 
Cc: Andi Kleen 
Cc: Tony Luck 
Cc: Philippe Ombredanne 
Cc: Kate Stewart 
Cc: "Rafael J. Wysocki" 
Cc: "Ravi V. Shankar" 
Cc: x...@kernel.org
Signed-off-by: Ricardo Neri 
---
 arch/x86/include/asm/hpet.h | 10 ++
 arch/x86/kernel/hpet.c  | 12 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/hpet.h b/arch/x86/include/asm/hpet.h
index 67385d56d4f4..9e0afde47587 100644
--- a/arch/x86/include/asm/hpet.h
+++ b/arch/x86/include/asm/hpet.h
@@ -72,6 +72,11 @@ extern int is_hpet_enabled(void);
 extern int hpet_enable(void);
 extern void hpet_disable(void);
 extern unsigned int hpet_readl(unsigned int a);
+extern void hpet_writel(unsigned int d, unsigned int a);
+#ifdef CONFIG_X86_64
+extern unsigned long hpet_readq(unsigned int a);
+extern void hpet_writeq(unsigned long d, unsigned int a);
+#endif
 extern void force_hpet_resume(void);
 
 struct irq_data;
@@ -109,6 +114,11 @@ extern void hpet_unregister_irq_handler(rtc_irq_handler 
handler);
 static inline int hpet_enable(void) { return 0; }
 static inline int is_hpet_enabled(void) { return 0; }
 #define hpet_readl(a) 0
+#define hpet_writel(d, a)
+#ifdef CONFIG_X86_64
+#define hpet_readq(a) 0
+#define hpet_writeq(d, a)
+#endif
 #define default_setup_hpet_msi NULL
 
 #endif
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index dfd3aca82c61..ee439d84e83b 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -61,12 +61,22 @@ inline unsigned int hpet_readl(unsigned int a)
return readl(hpet_virt_address + a);
 }
 
-static inline void hpet_writel(unsigned int d, unsigned int a)
+inline void hpet_writel(unsigned int d, unsigned int a)
 {
writel(d, hpet_virt_address + a);
 }
 
 #ifdef CONFIG_X86_64
+inline unsigned long hpet_readq(unsigned int a)
+{
+   return readq(hpet_virt_address + a);
+}
+
+inline void hpet_writeq(unsigned long d, unsigned int a)
+{
+   writeq(d, hpet_virt_address + a);
+}
+
 #include 
 #endif
 
-- 
2.17.1