Re: [Qemu-devel] [PATCH 16/18] hw: add QEMU model for Faraday RTCtimer
Sorry for the inconveniences, our e-mail server has a limitation on the rate of
sending mails.
It prevents the git-send-email to do that for me. I'll try to use gmail later
when I get the QOM issues fixed.
Best Regards
Dante Su
-Original Message-
From: qemu-devel-bounces+dantesu=faraday-tech@nongnu.org
[mailto:qemu-devel-bounces+dantesu=faraday-tech@nongnu.org] On Behalf Of
Andreas Farber
Sent: Friday, January 18, 2013 6:43 PM
To: Dante Kuo-Jung Su(蘇國榮)
Cc: peter.mayd...@linaro.org; p...@codesourcery.com; Paolo Bonzini;
qemu-devel@nongnu.org; KONRAD Frédéric
Subject: Re: [Qemu-devel] [PATCH 16/18] hw: add QEMU model for Faraday RTCtimer
Kuo-Jung, please thread your messages together (e.g., using
git-send-email) and prepend a cover letter, right now this is a badly
reviewable mess of individual patches on the list.
Am 18.01.2013 09:44, schrieb KONRAD Frédéric:
> On 18/01/2013 07:32, Dante wrote:
>> Signed-off-by: Kuo-Jung Su
>> ---
>> hw/ftrtc011.c | 308
>> +
>> 1 file changed, 308 insertions(+)
>> create mode 100644 hw/ftrtc011.c
>>
>> diff --git a/hw/ftrtc011.c b/hw/ftrtc011.c new file mode 100644 index
>> 000..466cbb6
>> --- /dev/null
>> +++ b/hw/ftrtc011.c
>> @@ -0,0 +1,308 @@
>> +/*
>> + * QEMU model of the FTRTC011 RTC Timer
>> + *
>> + * Copyright (C) 2012 Faraday Technology
>> + * Copyright (C) 2012 Dante Su
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a copy
>> + * of this software and associated documentation files (the
>> "Software"), to deal
>> + * in the Software without restriction, including without limitation
>> the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense,
>> and/or sell
>> + * copies of the Software, and to permit persons to whom the
>> + Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "sysbus.h"
>> +#include "sysemu/sysemu.h"
>> +#include "qemu/timer.h"
>> +
>> +/* Hardware registers */
>> +#define REG_SEC0x00
>> +#define REG_MIN0x04
>> +#define REG_HOUR0x08
>> +#define REG_DAY0x0C
>> +
>> +#define REG_ALARM_SEC0x10
>> +#define REG_ALARM_MIN0x14
>> +#define REG_ALARM_HOUR0x18
>> +
>> +#define REG_CR0x20
>> +#define REG_WSEC0x24
>> +#define REG_WMIN0x28
>> +#define REG_WHOUR0x2C
>> +#define REG_WDAY0x30
>> +#define REG_ISR0x34
>> +
>> +#define REG_REV0x3C
>> +#define REG_CURRENT0x44
You would be well advised to put these constants into their own header file so
that they can be reused for qtest test cases. Please take a look at the
existing rtc code and test cases.
>> +
>> +enum ftrtc011_irqpin {
>> +IRQ_ALARM_LEVEL = 0,
>> +IRQ_ALARM_EDGE,
>> +IRQ_SEC,
>> +IRQ_MIN,
>> +IRQ_HOUR,
>> +IRQ_DAY,
>> +};
>> +
>> +typedef struct {
Please name the struct, usually like the typedef.
>> +SysBusDevice busdev;
parent_obj please and please separate from the remaining fields.
>> +MemoryRegion mmio;
>> +
>> +qemu_irq irq[6];
>> +
>> +QEMUTimer *qtimer;
>> +
>> +uint8_t sec;
>> +uint8_t min;
>> +uint8_t hr;
>> +uint32_t day;
>> +
>> +uint8_t alarm_sec;
>> +uint8_t alarm_min;
>> +uint8_t alarm_hr;
>> +
>> +uint32_t cr;
>> +uint32_t isr;
>> +
>> +} ftrtc011_state;
CamelCase please.
These comments may apply to other patches in the series as well, please check
on your own.
>> +
>> +/* Update interrupts. */
>
Re: [Qemu-devel] [PATCH 16/18] hw: add QEMU model for Faraday RTCtimer
Hi Konrad:
Thanks for the information, I'm now studying the QOM.
And when I finished the reading. I'll send out new patches later.
Best Regards
Dante Su
-Original Message-
From: qemu-devel-bounces+dantesu=faraday-tech@nongnu.org
[mailto:qemu-devel-bounces+dantesu=faraday-tech@nongnu.org] On Behalf Of
KONRAD Frederic
Sent: Friday, January 18, 2013 4:44 PM
To: Dante Kuo-Jung Su(蘇國榮)
Cc: peter.mayd...@linaro.org; qemu-devel@nongnu.org; Andreas Färber;
p...@codesourcery.com
Subject: Re: [Qemu-devel] [PATCH 16/18] hw: add QEMU model for Faraday RTCtimer
On 18/01/2013 07:32, Dante wrote:
> Signed-off-by: Kuo-Jung Su
> ---
> hw/ftrtc011.c | 308
> +
> 1 file changed, 308 insertions(+)
> create mode 100644 hw/ftrtc011.c
>
> diff --git a/hw/ftrtc011.c b/hw/ftrtc011.c new file mode 100644 index
> 000..466cbb6
> --- /dev/null
> +++ b/hw/ftrtc011.c
> @@ -0,0 +1,308 @@
> +/*
> + * QEMU model of the FTRTC011 RTC Timer
> + *
> + * Copyright (C) 2012 Faraday Technology
> + * Copyright (C) 2012 Dante Su
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a copy
> + * of this software and associated documentation files (the
> +"Software"), to deal
> + * in the Software without restriction, including without limitation
> +the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense,
> +and/or sell
> + * copies of the Software, and to permit persons to whom the Software
> +is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> +OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "sysbus.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/timer.h"
> +
> +/* Hardware registers */
> +#define REG_SEC0x00
> +#define REG_MIN0x04
> +#define REG_HOUR0x08
> +#define REG_DAY0x0C
> +
> +#define REG_ALARM_SEC0x10
> +#define REG_ALARM_MIN0x14
> +#define REG_ALARM_HOUR0x18
> +
> +#define REG_CR0x20
> +#define REG_WSEC0x24
> +#define REG_WMIN0x28
> +#define REG_WHOUR0x2C
> +#define REG_WDAY0x30
> +#define REG_ISR0x34
> +
> +#define REG_REV0x3C
> +#define REG_CURRENT0x44
> +
> +enum ftrtc011_irqpin {
> +IRQ_ALARM_LEVEL = 0,
> +IRQ_ALARM_EDGE,
> +IRQ_SEC,
> +IRQ_MIN,
> +IRQ_HOUR,
> +IRQ_DAY,
> +};
> +
> +typedef struct {
> +SysBusDevice busdev;
> +MemoryRegion mmio;
> +
> +qemu_irq irq[6];
> +
> +QEMUTimer *qtimer;
> +
> +uint8_t sec;
> +uint8_t min;
> +uint8_t hr;
> +uint32_t day;
> +
> +uint8_t alarm_sec;
> +uint8_t alarm_min;
> +uint8_t alarm_hr;
> +
> +uint32_t cr;
> +uint32_t isr;
> +
> +} ftrtc011_state;
> +
> +/* Update interrupts. */
> +static inline void ftrtc011_update_irq(ftrtc011_state *s) {
> +uint32_t mask = ((s->cr >> 1) & 0x1f) & s->isr;
> +
> +qemu_set_irq(s->irq[IRQ_ALARM_LEVEL], (mask & 0x10) ? 1 : 0);
> +
> +if (mask) {
> +if (mask & 0x01)
> +qemu_irq_pulse(s->irq[IRQ_SEC]);
> +if (mask & 0x02)
> +qemu_irq_pulse(s->irq[IRQ_MIN]);
> +if (mask & 0x04)
> +qemu_irq_pulse(s->irq[IRQ_HOUR]);
> +if (mask & 0x08)
> +qemu_irq_pulse(s->irq[IRQ_DAY]);
> +if (mask & 0x10)
> +qemu_irq_pulse(s->irq[IRQ_ALARM_EDGE]);
> +}
> +}
> +
> +static uint64_t ftrtc011_mem_read(void *opaque, hwaddr addr, unsigned
> +int size) {
> +ftrtc011_state *s = opaque;
> +uint32_t rc = 0;
> +
> +switch (addr) {
> +case REG_SEC:
> +return s->sec;
> +case REG_MIN:
> +return s->min;
> +case REG_HOUR:
> +return s->hr;
> +case REG_DAY:
> +return s->
