Re: [Qemu-devel] VersatetilePB: request for comments on PL031 patch

2007-06-30 Thread Paul Brook
On Friday 15 June 2007, Simone wrote:
 Hello,

 My name is Simone Basso, and I study Computer Science at Politecnico di
 Torino, Italy. In the three past weeks I've been working to a project that
 involves Qemu, as a part of an exam, together with another student, Gaspare
 Scherma.

You implementation seems seriously over-complicated (AFAICS all struct tm bits 
are completely unnecessary) and missing a few other bits, so I rewrote it.

Paul




[Qemu-devel] VersatetilePB: request for comments on PL031 patch

2007-06-15 Thread Simone

Hello,

My name is Simone Basso, and I study Computer Science at Politecnico di Torino,
Italy. In the three past weeks I've been working to a project that involves
Qemu, as a part of an exam, together with another student, Gaspare Scherma.


The requested task was to add to Qemu 0.9.0 support for AMBA ARM PL031 RTC chip,
to avoid to have the emulated system (it was debian 4.0) always starting in
1970-01-01, which may be _very_ annoying :-).

The patch that we attach provides the following features:

  - The chip is recognized by the kernel;

  - It is possible to read the current time from the RTC;

  - It is possible to request an interrupt in the next 24 hours;

  - It is possible to acknowledge such interrupt, to lower the IRQ signal.


To implement such patch we've used the following knowledge sources:

  - The implementation of rtc-pl031 driver in the linux kernel;

  - The emulated device hw/pl011.c in qemu sources;

  - Documentation in the linux kernel.

Needless to say, any error in the patch was certainly introduced by us,
and does not depends on them :-).


The reason why we used linux kernel driver as a source of documentation were 
two:

  - We were not able to find complete documentation of the chip[1];

  - Another part of the exam task was to study Linux's RTC framework, so
we were yet a bit confident with the driver's code.

Having used the Linux kernel as the main source of information, we've also
taken some code from the kernel itself, to implement some bits. Anyway, it
should not be a problem, since the new file hw/pl031.c, added by the patch,
is in the GPL.


I need also to point that, to test linux kernel with our patch, you also need to
patch the kernel itself: infact we've find out a problem in linux's rtc-pl031
interrupt handler. For this reason, a _very_ little patch for linux kernel is
also attached[2].


I write this email to make you, qemu developers, aware of our effert, in the
hope that the patch could be a base for PL031 support with Qemu. If I had missed
some information useful to revise the patch, please tell me.

Regards,
-Simone Basso

--

Footnotes
---

 [1] We've just found www.arm.com/pdfs/DDI0287B_arm926ejs_dev_chip_trm.pdf 
which

 says something about PL031 on page 222.

 [2] The patch is for 2.6.21.3, however I known for sure that it applies 
without

 any problem to 2.6.18 (I guess that it won't create any problem for any
 kernel = 2.6.18, but I can't say anything about less than .18).

 We've yet send via email the patch to the proper driver mantainer, who
 said that it seemed ok and that he'll forward it upstream.

 This happened a week ago. However, at the moment now, no stable kernel 
from

 kernel.org has the patch, therefore it is necessary to apply it.







--- linux-2.6.21.3/drivers/rtc/rtc-pl031.c.orig	2007-05-24 23:22:47.0 +0200
+++ linux-2.6.21.3/drivers/rtc/rtc-pl031.c	2007-06-06 22:07:53.0 +0200
@@ -49,9 +49,14 @@
 
 static irqreturn_t pl031_interrupt(int irq, void *dev_id)
 {
-	struct rtc_device *rtc = dev_id;
+	struct pl031_local *ldata = dev_id;
 
-	rtc_update_irq(rtc-class_dev, 1, RTC_AF);
+	rtc_update_irq(ldata-rtc-class_dev, 1, RTC_AF);
+
+	/* www.arm.com/pdfs/DDI0287B_arm926ejs_dev_chip_trm.pdf at page
+	   222 tells that The interrupt is cleared by writing any data
+   value to the interrupt clear register RTCICR. */
+	__raw_writel(1, ldata-base + RTC_ICR);
 
 	return IRQ_HANDLED;
 }
@@ -173,8 +178,11 @@
 		goto out_no_remap;
 	}
 
-	if (request_irq(adev-irq[0], pl031_interrupt, IRQF_DISABLED,
-			rtc-pl031, ldata-rtc)) {
+	/* Pass ldata to the interrupt handler, so we're able to write
+	   the register that clears the interrupt: we need ldata-base
+	   for that. */
+ 	if (request_irq(adev-irq[0], pl031_interrupt, IRQF_DISABLED,
+			rtc-pl031, ldata)) {
 		ret = -EIO;
 		goto out_no_irq;
 	}
















--- qemu-0.9.0/hw/versatilepb.c.orig	2007-06-06 21:45:40.0 +0200
+++ qemu-0.9.0/hw/versatilepb.c	2007-06-06 21:45:40.0 +0200
@@ -214,6 +214,9 @@
that includes hardware cursor support from the PL111.  */
 pl110_init(ds, 0x1012, pic, 16, 1);
 
+/* Add PL031 Real Time Clock. */
+pl031_init(0x101e8000,pic,10);
+
 /* Memory map for Versatile/PB:  */
 /* 0x1000 System registers.  */
 /* 0x10001000 PCI controller config registers.  */
--- qemu-0.9.0/vl.h.orig	2007-06-06 21:45:40.0 +0200
+++ qemu-0.9.0/vl.h	2007-06-06 21:45:40.0 +0200
@@ -1307,6 +1307,9 @@
 /* smc91c111.c */
 void smc91c111_init(NICInfo *, uint32_t, void *, int);
 
+/* pl031.c */
+void pl031_init(uint32_t base, void * pic, int irq);
+
 /* pl110.c */
 void *pl110_init(DisplayState *ds, uint32_t base, void *pic, int irq, int);
 
--- qemu-0.9.0/Makefile.target.orig	2007-06-06 21:45:40.0 +0200
+++ qemu-0.9.0/Makefile.target	2007-06-06 21:45:40.0 +0200
@@