[PATCH] Assign IRQs to HPET Timers

[PATCH] Assign IRQs to HPET Timers

[Balaji Rao]

Assign an IRQ to HPET Timer devices when interrupt enable is requested.
This now makes the HPET userspace API work.

drivers/char/hpet.c  |   31 +++++++++++++++++++++++++++++--
include/linux/hpet.h |    2 +-
2 files changed, 30 insertions(+), 3 deletions(-)

Signed-off-by: Balaji Rao R <balajirrao@gmail.com>

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 4c16778..92bd889 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -390,7 +390,8 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
 	struct hpets *hpetp;
 	int irq;
 	unsigned long g, v, t, m;
-	unsigned long flags, isr;
+	unsigned long flags, isr, irq_bitmap;
+	u64 hpet_config;
 
 	timer = devp->hd_timer;
 	hpet = devp->hd_hpet;
@@ -412,7 +413,29 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
 		devp->hd_flags |= HPET_SHARED_IRQ;
 	spin_unlock_irq(&hpet_lock);
 
-	irq = devp->hd_hdwirq;
+	/* Assign an IRQ to the timer */
+	hpet_config = readq(&timer->hpet_config);
+	irq_bitmap =
+		(hpet_config & Tn_INT_ROUTE_CAP_MASK) >> Tn_INT_ROUTE_CAP_SHIFT;
+	if (!irq_bitmap)
+		irq = 0;	/* No IRQ Assignable */
+	else {
+		irq = find_first_bit(&irq_bitmap, 32);
+		do {
+			hpet_config |= irq << Tn_INT_ROUTE_CNF_SHIFT;
+			writeq(hpet_config, &timer->hpet_config);
+
+			/* Check whether we wrote a valid IRQ
+			 * number by reading back the field
+			 */
+			hpet_config = readq(&timer->hpet_config);
+			if (irq == (hpet_config & Tn_INT_ROUTE_CNF_MASK)
+					>> Tn_INT_ROUTE_CNF_SHIFT) {
+				devp->hd_hdwirq = irq;
+				break;	/* Success */
+			}
+		} while ((irq = (find_next_bit(&irq_bitmap, 32, irq))));
+	}
 
 	if (irq) {
 		unsigned long irq_flags;
@@ -509,6 +532,10 @@ hpet_ioctl_common(struct hpet_dev *devp, int cmd, unsigned long arg, int kernel)
 			break;
 		v = readq(&timer->hpet_config);
 		v &= ~Tn_INT_ENB_CNF_MASK;
+
+		/* Zero out the IRQ field*/
+		v &= ~Tn_INT_ROUTE_CNF_MASK;
+
 		writeq(v, &timer->hpet_config);
 		if (devp->hd_irq) {
 			free_irq(devp->hd_irq, devp);
diff --git a/include/linux/hpet.h b/include/linux/hpet.h
index 707f7cb..e3c0b2a 100644
--- a/include/linux/hpet.h
+++ b/include/linux/hpet.h
@@ -64,7 +64,7 @@ struct hpet {
  */
 
 #define	Tn_INT_ROUTE_CAP_MASK		(0xffffffff00000000ULL)
-#define	Tn_INI_ROUTE_CAP_SHIFT		(32UL)
+#define	Tn_INT_ROUTE_CAP_SHIFT		(32UL)
 #define	Tn_FSB_INT_DELCAP_MASK		(0x8000UL)
 #define	Tn_FSB_INT_DELCAP_SHIFT		(15)
 #define	Tn_FSB_EN_CNF_MASK		(0x4000UL)

Re: [PATCH] Assign IRQs to HPET Timers

[Balbir Singh]

* Balaji Rao <balajirrao@gmail.com> [2008-01-12 00:36:11]:

> Assign an IRQ to HPET Timer devices when interrupt enable is requested.
> This now makes the HPET userspace API work.
>

A more detailed changelog will better help understand the nature and
origin of the problem and how to reproduce it.
 
> drivers/char/hpet.c  |   31 +++++++++++++++++++++++++++++--
> include/linux/hpet.h |    2 +-
> 2 files changed, 30 insertions(+), 3 deletions(-)
> 
> Signed-off-by: Balaji Rao R <balajirrao@gmail.com>
> 
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
> index 4c16778..92bd889 100644
> --- a/drivers/char/hpet.c
> +++ b/drivers/char/hpet.c
> @@ -390,7 +390,8 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
>  	struct hpets *hpetp;
>  	int irq;
>  	unsigned long g, v, t, m;
> -	unsigned long flags, isr;
> +	unsigned long flags, isr, irq_bitmap;
> +	u64 hpet_config;
> 
>  	timer = devp->hd_timer;
>  	hpet = devp->hd_hpet;
> @@ -412,7 +413,29 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
>  		devp->hd_flags |= HPET_SHARED_IRQ;
>  	spin_unlock_irq(&hpet_lock);
> 
> -	irq = devp->hd_hdwirq;
> +	/* Assign an IRQ to the timer */
> +	hpet_config = readq(&timer->hpet_config);
> +	irq_bitmap =
> +		(hpet_config & Tn_INT_ROUTE_CAP_MASK) >> Tn_INT_ROUTE_CAP_SHIFT;

Should we check if the interrupts are being delivered via FSB, prior
to doing this?

> +	if (!irq_bitmap)
> +		irq = 0;	/* No IRQ Assignable */
> +	else {
> +		irq = find_first_bit(&irq_bitmap, 32);

> +		do {
> +			hpet_config |= irq << Tn_INT_ROUTE_CNF_SHIFT;
> +			writeq(hpet_config, &timer->hpet_config);
> +
> +			/* Check whether we wrote a valid IRQ
> +			 * number by reading back the field
> +			 */
> +			hpet_config = readq(&timer->hpet_config);
> +			if (irq == (hpet_config & Tn_INT_ROUTE_CNF_MASK)
> +					>> Tn_INT_ROUTE_CNF_SHIFT) {
> +				devp->hd_hdwirq = irq;
> +				break;	/* Success */
> +			}
> +		} while ((irq = (find_next_bit(&irq_bitmap, 32, irq))));
> +	}

Shouldn't we do this at hpet_alloc() time?


> 
>  	if (irq) {
>  		unsigned long irq_flags;
> @@ -509,6 +532,10 @@ hpet_ioctl_common(struct hpet_dev *devp, int cmd, unsigned long arg, int kernel)
>  			break;
>  		v = readq(&timer->hpet_config);
>  		v &= ~Tn_INT_ENB_CNF_MASK;
> +
> +		/* Zero out the IRQ field*/
> +		v &= ~Tn_INT_ROUTE_CNF_MASK;
> +
>  		writeq(v, &timer->hpet_config);
>  		if (devp->hd_irq) {
>  			free_irq(devp->hd_irq, devp);
> diff --git a/include/linux/hpet.h b/include/linux/hpet.h
> index 707f7cb..e3c0b2a 100644
> --- a/include/linux/hpet.h
> +++ b/include/linux/hpet.h
> @@ -64,7 +64,7 @@ struct hpet {
>   */
> 
>  #define	Tn_INT_ROUTE_CAP_MASK		(0xffffffff00000000ULL)
> -#define	Tn_INI_ROUTE_CAP_SHIFT		(32UL)
> +#define	Tn_INT_ROUTE_CAP_SHIFT		(32UL)
>  #define	Tn_FSB_INT_DELCAP_MASK		(0x8000UL)
>  #define	Tn_FSB_INT_DELCAP_SHIFT		(15)
>  #define	Tn_FSB_EN_CNF_MASK		(0x4000UL)

The patch looks good overall!

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

[PATCH] [RESEND] Assign IRQs to HPET Timers

[Balaji Rao]

The userspace API for the HPET (see Documentation/hpet.txt) did not work. The 
HPET_IE_ON ioctl was failing as there was no IRQ assigned to the timer 
device. This patch fixes it by allocating IRQs to timer blocks in the HPET.

arch/x86/kernel/hpet.c |   13 +++++--------
drivers/char/hpet.c    |   45 ++++++++++++++++++++++++++++++++++++++-------
include/linux/hpet.h   |    2 +-
3 files changed, 44 insertions(+), 16 deletions(-)

Signed-off-by: Balaji Rao <balajirrao@gmail.com>

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 4a86ffd..08ee998 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -116,8 +116,7 @@ int is_hpet_enabled(void)
 static void hpet_reserve_platform_timers(unsigned long id)
 {
 	struct hpet __iomem *hpet = hpet_virt_address;
-	struct hpet_timer __iomem *timer = &hpet->hpet_timers[2];
-	unsigned int nrtimers, i;
+	unsigned int nrtimers;
 	struct hpet_data hd;
 
 	nrtimers = ((id & HPET_ID_NUMBER) >> HPET_ID_NUMBER_SHIFT) + 1;
@@ -132,16 +131,14 @@ static void hpet_reserve_platform_timers(unsigned long 
id)
 #ifdef CONFIG_HPET_EMULATE_RTC
 	hpet_reserve_timer(&hd, 1);
 #endif
-
 	hd.hd_irq[0] = HPET_LEGACY_8254;
 	hd.hd_irq[1] = HPET_LEGACY_RTC;
 
-	for (i = 2; i < nrtimers; timer++, i++)
-		hd.hd_irq[i] = (timer->hpet_config & Tn_INT_ROUTE_CNF_MASK) >>
-			Tn_INT_ROUTE_CNF_SHIFT;
-
+	/*
+	 * IRQs for the other timers are assigned dynamically
+	 * in hpet_alloc
+	 */
 	hpet_alloc(&hd);
-
 }
 #else
 static void hpet_reserve_platform_timers(unsigned long id) { }
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 4c16778..593b32c 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -806,14 +806,14 @@ static unsigned long hpet_calibrate(struct hpets *hpetp)
 
 int hpet_alloc(struct hpet_data *hdp)
 {
-	u64 cap, mcfg;
+	u64 cap, mcfg, hpet_config;
 	struct hpet_dev *devp;
-	u32 i, ntimer;
+	u32 i, ntimer, irq;
 	struct hpets *hpetp;
 	size_t siz;
 	struct hpet __iomem *hpet;
 	static struct hpets *last = NULL;
-	unsigned long period;
+	unsigned long period, irq_bitmap;
 	unsigned long long temp;
 
 	/*
@@ -840,11 +840,41 @@ int hpet_alloc(struct hpet_data *hdp)
 	hpetp->hp_hpet_phys = hdp->hd_phys_address;
 
 	hpetp->hp_ntimer = hdp->hd_nirqs;
+	hpet = hpetp->hp_hpet;
 
-	for (i = 0; i < hdp->hd_nirqs; i++)
-		hpetp->hp_dev[i].hd_hdwirq = hdp->hd_irq[i];
+	/* Assign IRQs statically for legacy devices */
+	hpetp->hp_dev[0].hd_hdwirq = hdp->hd_irq[0];
+	hpetp->hp_dev[1].hd_hdwirq = hdp->hd_irq[1];
 
-	hpet = hpetp->hp_hpet;
+	/* Assign IRQs dynamically for the others */
+	for (i = 2, devp = &hpetp->hp_dev[2]; i < hdp->hd_nirqs; i++, devp++) {
+		struct hpet_timer __iomem *timer;
+
+		timer = &hpet->hpet_timers[devp - hpetp->hp_dev];
+
+		hpet_config = readq(&timer->hpet_config);
+		irq_bitmap = (hpet_config & Tn_INT_ROUTE_CAP_MASK)
+			>> Tn_INT_ROUTE_CAP_SHIFT;
+		if (!irq_bitmap)
+			irq = 0;        /* No valid IRQ Assignable */
+		else {
+			irq = find_first_bit(&irq_bitmap, 32);
+			do {
+				hpet_config |= irq << Tn_INT_ROUTE_CNF_SHIFT;
+				writeq(hpet_config, &timer->hpet_config);
+
+				/*
+				 * Verify whether we have written a valid
+				 * IRQ number by reading it back again
+				 */
+				hpet_config = readq(&timer->hpet_config);
+				if (irq == (hpet_config & Tn_INT_ROUTE_CNF_MASK)
+						>> Tn_INT_ROUTE_CNF_SHIFT)
+					break;  /* Success */
+			} while ((irq = (find_next_bit(&irq_bitmap, 32, irq))));
+		}
+		hpetp->hp_dev[i].hd_hdwirq = irq;
+	}
 
 	cap = readq(&hpet->hpet_cap);
 
@@ -875,7 +905,8 @@ int hpet_alloc(struct hpet_data *hdp)
 		hpetp->hp_which, hdp->hd_phys_address,
 		hpetp->hp_ntimer > 1 ? "s" : "");
 	for (i = 0; i < hpetp->hp_ntimer; i++)
-		printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
+		printk("%s %d", i > 0 ? "," : "",
+				hpetp->hp_dev[i].hd_hdwirq);
 	printk("\n");
 
 	printk(KERN_INFO "hpet%u: %u %d-bit timers, %Lu Hz\n",
diff --git a/include/linux/hpet.h b/include/linux/hpet.h
index 707f7cb..e3c0b2a 100644
--- a/include/linux/hpet.h
+++ b/include/linux/hpet.h
@@ -64,7 +64,7 @@ struct hpet {
  */
 
 #define	Tn_INT_ROUTE_CAP_MASK		(0xffffffff00000000ULL)
-#define	Tn_INI_ROUTE_CAP_SHIFT		(32UL)
+#define	Tn_INT_ROUTE_CAP_SHIFT		(32UL)
 #define	Tn_FSB_INT_DELCAP_MASK		(0x8000UL)
 #define	Tn_FSB_INT_DELCAP_SHIFT		(15)
 #define	Tn_FSB_EN_CNF_MASK		(0x4000UL)
---

Re: [PATCH] [RESEND] Assign IRQs to HPET Timers

[Ingo Molnar]

* Balaji Rao <balajirrao@gmail.com> wrote:

> The userspace API for the HPET (see Documentation/hpet.txt) did not 
> work. The HPET_IE_ON ioctl was failing as there was no IRQ assigned to 
> the timer device. This patch fixes it by allocating IRQs to timer 
> blocks in the HPET.

thanks, applied to x86.git. (sidenote: your patch was linewrapped, fixed 
that up. See Documentation/email-clients.txt)

	Ingo