* [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
@ 2015-04-25 8:18 ` Alistair Francis
2015-04-25 18:34 ` Peter Crosthwaite
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer Alistair Francis
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:18 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Cleanup the individual DeviceState and SysBusDevice
variables to re-use the same variable for each
device.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
hw/arm/stm32f205_soc.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 0f3bdc7..63893f3 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -59,8 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
{
STM32F205State *s = STM32F205_SOC(dev_soc);
- DeviceState *syscfgdev, *usartdev, *timerdev;
- SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
+ DeviceState *dev;
+ SysBusDevice *busdev;
qemu_irq *pic;
Error *err = NULL;
int i;
@@ -92,41 +92,41 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
s->kernel_filename, s->cpu_model);
/* System configuration controller */
- syscfgdev = DEVICE(&s->syscfg);
+ dev = DEVICE(&s->syscfg);
object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
if (err != NULL) {
error_propagate(errp, err);
return;
}
- syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
- sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
- sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, 0x40013800);
+ sysbus_connect_irq(busdev, 0, pic[71]);
/* Attach UART (uses USART registers) and USART controllers */
for (i = 0; i < STM_NUM_USARTS; i++) {
- usartdev = DEVICE(&(s->usart[i]));
+ dev = DEVICE(&(s->usart[i]));
object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
if (err != NULL) {
error_propagate(errp, err);
return;
}
- usartbusdev = SYS_BUS_DEVICE(usartdev);
- sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
- sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, usart_addr[i]);
+ sysbus_connect_irq(busdev, 0, pic[usart_irq[i]]);
}
/* Timer 2 to 5 */
for (i = 0; i < STM_NUM_TIMERS; i++) {
- timerdev = DEVICE(&(s->timer[i]));
- qdev_prop_set_uint64(timerdev, "clock-frequency", 1000000000);
+ dev = DEVICE(&(s->timer[i]));
+ qdev_prop_set_uint64(dev, "clock-frequency", 1000000000);
object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
if (err != NULL) {
error_propagate(errp, err);
return;
}
- timerbusdev = SYS_BUS_DEVICE(timerdev);
- sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
- sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, timer_addr[i]);
+ sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
}
}
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables Alistair Francis
@ 2015-04-25 18:34 ` Peter Crosthwaite
2015-04-26 1:57 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:34 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Cleanup the individual DeviceState and SysBusDevice
> variables to re-use the same variable for each
> device.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
> hw/arm/stm32f205_soc.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 0f3bdc7..63893f3 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -59,8 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> {
> STM32F205State *s = STM32F205_SOC(dev_soc);
> - DeviceState *syscfgdev, *usartdev, *timerdev;
> - SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
> + DeviceState *dev;
> + SysBusDevice *busdev;
> qemu_irq *pic;
> Error *err = NULL;
> int i;
> @@ -92,41 +92,41 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> s->kernel_filename, s->cpu_model);
>
> /* System configuration controller */
> - syscfgdev = DEVICE(&s->syscfg);
> + dev = DEVICE(&s->syscfg);
> object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
> if (err != NULL) {
> error_propagate(errp, err);
> return;
> }
> - syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
> - sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
> - sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, 0x40013800);
> + sysbus_connect_irq(busdev, 0, pic[71]);
>
> /* Attach UART (uses USART registers) and USART controllers */
> for (i = 0; i < STM_NUM_USARTS; i++) {
> - usartdev = DEVICE(&(s->usart[i]));
> + dev = DEVICE(&(s->usart[i]));
> object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
> if (err != NULL) {
> error_propagate(errp, err);
> return;
> }
> - usartbusdev = SYS_BUS_DEVICE(usartdev);
> - sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
> - sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, usart_addr[i]);
> + sysbus_connect_irq(busdev, 0, pic[usart_irq[i]]);
> }
>
> /* Timer 2 to 5 */
> for (i = 0; i < STM_NUM_TIMERS; i++) {
> - timerdev = DEVICE(&(s->timer[i]));
> - qdev_prop_set_uint64(timerdev, "clock-frequency", 1000000000);
> + dev = DEVICE(&(s->timer[i]));
> + qdev_prop_set_uint64(dev, "clock-frequency", 1000000000);
> object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> if (err != NULL) {
> error_propagate(errp, err);
> return;
> }
> - timerbusdev = SYS_BUS_DEVICE(timerdev);
> - sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
> - sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, timer_addr[i]);
> + sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
> }
> }
>
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables
2015-04-25 18:34 ` Peter Crosthwaite
@ 2015-04-26 1:57 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-04-26 1:57 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:34 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Cleanup the individual DeviceState and SysBusDevice
>> variables to re-use the same variable for each
>> device.
>>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Thanks
Alistair
>
>> ---
>>
>> hw/arm/stm32f205_soc.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> index 0f3bdc7..63893f3 100644
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -59,8 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
>> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> {
>> STM32F205State *s = STM32F205_SOC(dev_soc);
>> - DeviceState *syscfgdev, *usartdev, *timerdev;
>> - SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
>> + DeviceState *dev;
>> + SysBusDevice *busdev;
>> qemu_irq *pic;
>> Error *err = NULL;
>> int i;
>> @@ -92,41 +92,41 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> s->kernel_filename, s->cpu_model);
>>
>> /* System configuration controller */
>> - syscfgdev = DEVICE(&s->syscfg);
>> + dev = DEVICE(&s->syscfg);
>> object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
>> if (err != NULL) {
>> error_propagate(errp, err);
>> return;
>> }
>> - syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
>> - sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
>> - sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
>> + busdev = SYS_BUS_DEVICE(dev);
>> + sysbus_mmio_map(busdev, 0, 0x40013800);
>> + sysbus_connect_irq(busdev, 0, pic[71]);
>>
>> /* Attach UART (uses USART registers) and USART controllers */
>> for (i = 0; i < STM_NUM_USARTS; i++) {
>> - usartdev = DEVICE(&(s->usart[i]));
>> + dev = DEVICE(&(s->usart[i]));
>> object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
>> if (err != NULL) {
>> error_propagate(errp, err);
>> return;
>> }
>> - usartbusdev = SYS_BUS_DEVICE(usartdev);
>> - sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
>> - sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
>> + busdev = SYS_BUS_DEVICE(dev);
>> + sysbus_mmio_map(busdev, 0, usart_addr[i]);
>> + sysbus_connect_irq(busdev, 0, pic[usart_irq[i]]);
>> }
>>
>> /* Timer 2 to 5 */
>> for (i = 0; i < STM_NUM_TIMERS; i++) {
>> - timerdev = DEVICE(&(s->timer[i]));
>> - qdev_prop_set_uint64(timerdev, "clock-frequency", 1000000000);
>> + dev = DEVICE(&(s->timer[i]));
>> + qdev_prop_set_uint64(dev, "clock-frequency", 1000000000);
>> object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
>> if (err != NULL) {
>> error_propagate(errp, err);
>> return;
>> }
>> - timerbusdev = SYS_BUS_DEVICE(timerdev);
>> - sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
>> - sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
>> + busdev = SYS_BUS_DEVICE(dev);
>> + sysbus_mmio_map(busdev, 0, timer_addr[i]);
>> + sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
>> }
>> }
>>
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables Alistair Francis
@ 2015-04-25 8:18 ` Alistair Francis
2015-04-25 18:35 ` Peter Crosthwaite
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device Alistair Francis
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:18 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
If correctly configured allow the STM32F2xx timer to print
out the PWM duty cycle information.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
hw/timer/stm32f2xx_timer.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index ecadf9d..6f463e8 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -49,6 +49,16 @@ static void stm32f2xx_timer_interrupt(void *opaque)
qemu_irq_pulse(s->irq);
stm32f2xx_timer_set_alarm(s, s->hit_time);
}
+
+ if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 + TIM_CCMR1_OC2M1) &&
+ !(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
+ (s->tim_ccmr1 & TIM_CCMR1_OC2PE) &&
+ s->tim_ccer & TIM_CCER_CC2E) {
+ /* PWM 2 - Mode 1 */
+ DB_PRINT("Duty Cycle: %d%%\n",
+ s->tim_ccr2 / (100 * (s->tim_psc + 1)));
+ stm32f2xx_timer_set_alarm(s, s->hit_time);
+ }
}
static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer Alistair Francis
@ 2015-04-25 18:35 ` Peter Crosthwaite
2015-04-26 3:05 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:35 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
> If correctly configured allow the STM32F2xx timer to print
> out the PWM duty cycle information.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>
> hw/timer/stm32f2xx_timer.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index ecadf9d..6f463e8 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -49,6 +49,16 @@ static void stm32f2xx_timer_interrupt(void *opaque)
> qemu_irq_pulse(s->irq);
> stm32f2xx_timer_set_alarm(s, s->hit_time);
> }
> +
> + if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 + TIM_CCMR1_OC2M1) &&
> + !(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
> + (s->tim_ccmr1 & TIM_CCMR1_OC2PE) &&
Bracing looks inconsistent. Can you drop this set of () to make it
consistent with one below?
> + s->tim_ccer & TIM_CCER_CC2E) {
> + /* PWM 2 - Mode 1 */
> + DB_PRINT("Duty Cycle: %d%%\n",
> + s->tim_ccr2 / (100 * (s->tim_psc + 1)));
> + stm32f2xx_timer_set_alarm(s, s->hit_time);
This looks like a functional change - why do you need to set the alarm
for PWM? Commit messages suggests this is supposed to just add debug
printfs.
Regards,
Peter
> + }
> }
>
> static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer
2015-04-25 18:35 ` Peter Crosthwaite
@ 2015-04-26 3:05 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-04-26 3:05 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:35 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> If correctly configured allow the STM32F2xx timer to print
>> out the PWM duty cycle information.
>>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>> ---
>>
>> hw/timer/stm32f2xx_timer.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
>> index ecadf9d..6f463e8 100644
>> --- a/hw/timer/stm32f2xx_timer.c
>> +++ b/hw/timer/stm32f2xx_timer.c
>> @@ -49,6 +49,16 @@ static void stm32f2xx_timer_interrupt(void *opaque)
>> qemu_irq_pulse(s->irq);
>> stm32f2xx_timer_set_alarm(s, s->hit_time);
>> }
>> +
>> + if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 + TIM_CCMR1_OC2M1) &&
>> + !(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
>> + (s->tim_ccmr1 & TIM_CCMR1_OC2PE) &&
>
> Bracing looks inconsistent. Can you drop this set of () to make it
> consistent with one below?
Good catch, I'll remove the braces.
>
>> + s->tim_ccer & TIM_CCER_CC2E) {
>> + /* PWM 2 - Mode 1 */
>> + DB_PRINT("Duty Cycle: %d%%\n",
>> + s->tim_ccr2 / (100 * (s->tim_psc + 1)));
>> + stm32f2xx_timer_set_alarm(s, s->hit_time);
>
> This looks like a functional change - why do you need to set the alarm
> for PWM? Commit messages suggests this is supposed to just add debug
> printfs.
You're right, this shouldn't be required. It must have been left over
from testing. I will remove.
Thanks,
Alistair
>
> Regards,
> Peter
>
>> + }
>> }
>>
>> static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 1/7] STM32F205: Remove the individual device variables Alistair Francis
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 2/7] STM32F2xx: Display PWM duty cycle from timer Alistair Francis
@ 2015-04-25 8:18 ` Alistair Francis
2015-04-25 18:43 ` Peter Crosthwaite
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device Alistair Francis
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:18 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Add the STM32F2xx ADC device. This device randomly
generates values on each read.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
default-configs/arm-softmmu.mak | 1 +
hw/misc/Makefile.objs | 1 +
hw/misc/stm32f2xx_adc.c | 332 ++++++++++++++++++++++++++++++++++++++++
include/hw/misc/stm32f2xx_adc.h | 96 ++++++++++++
4 files changed, 430 insertions(+)
create mode 100644 hw/misc/stm32f2xx_adc.c
create mode 100644 include/hw/misc/stm32f2xx_adc.h
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index a767e4b..2b16590 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -84,6 +84,7 @@ CONFIG_ZYNQ=y
CONFIG_STM32F2XX_TIMER=y
CONFIG_STM32F2XX_USART=y
CONFIG_STM32F2XX_SYSCFG=y
+CONFIG_STM32F2XX_ADC=y
CONFIG_STM32F205_SOC=y
CONFIG_VERSATILE_PCI=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4aa76ff..b65ec3d 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -37,6 +37,7 @@ obj-$(CONFIG_OMAP) += omap_tap.o
obj-$(CONFIG_SLAVIO) += slavio_misc.o
obj-$(CONFIG_ZYNQ) += zynq_slcr.o
obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
+obj-$(CONFIG_STM32F2XX_ADC) += stm32f2xx_adc.o
obj-$(CONFIG_PVPANIC) += pvpanic.o
obj-$(CONFIG_EDU) += edu.o
diff --git a/hw/misc/stm32f2xx_adc.c b/hw/misc/stm32f2xx_adc.c
new file mode 100644
index 0000000..659f14f
--- /dev/null
+++ b/hw/misc/stm32f2xx_adc.c
@@ -0,0 +1,332 @@
+/*
+ * STM32F2XX ADC
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ *
+ * 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 "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/misc/stm32f2xx_adc.h"
+
+#ifndef STM_ADC_ERR_DEBUG
+#define STM_ADC_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+ if (STM_ADC_ERR_DEBUG >= lvl) { \
+ qemu_log("%s: " fmt, __func__, ## args); \
+ } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void stm32f2xx_adc_reset(DeviceState *dev)
+{
+ STM32F2XXAdcState *s = STM32F2XX_ADC(dev);
+
+ s->adc_sr = 0x00000000;
+ s->adc_cr1 = 0x00000000;
+ s->adc_cr2 = 0x00000000;
+ s->adc_smpr1 = 0x00000000;
+ s->adc_smpr2 = 0x00000000;
+ s->adc_jofr1 = 0x00000000;
+ s->adc_jofr2 = 0x00000000;
+ s->adc_jofr3 = 0x00000000;
+ s->adc_jofr4 = 0x00000000;
+ s->adc_htr = 0x00000FFF;
+ s->adc_ltr = 0x00000000;
+ s->adc_sqr1 = 0x00000000;
+ s->adc_sqr2 = 0x00000000;
+ s->adc_sqr3 = 0x00000000;
+ s->adc_jsqr = 0x00000000;
+ s->adc_jdr1 = 0x00000000;
+ s->adc_jdr2 = 0x00000000;
+ s->adc_jdr3 = 0x00000000;
+ s->adc_jdr4 = 0x00000000;
+ s->adc_dr = 0x00000000;
+}
+
+static uint32_t stm32f2xx_adc_generate_value(STM32F2XXAdcState *s)
+{
+ /* Attempts to fake some ADC values */
+ #ifdef RAND_AVALIABLE
+ s->adc_dr = s->adc_dr + rand();
+ #else
+ s->adc_dr = s->adc_dr + 7;
+ #endif
+
+ if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 0) {
+ /* 12-bit */
+ s->adc_dr = s->adc_dr & 0xFFF;
+ } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 1) {
+ /* 10-bit */
+ s->adc_dr = s->adc_dr & 0x3FF;
+ } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 2) {
+ /* 8-bit */
+ s->adc_dr = s->adc_dr & 0xFF;
+ } else {
+ /* 6-bit */
+ s->adc_dr = s->adc_dr & 0x3F;
+ }
+
+ if (s->adc_cr2 & ADC_CR2_ALIGN) {
+ return (s->adc_dr << 1) & 0xFFF0;
+ } else {
+ return s->adc_dr;
+ }
+}
+
+static uint64_t stm32f2xx_adc_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ STM32F2XXAdcState *s = opaque;
+
+ DB_PRINT("%s: Address: 0x%"HWADDR_PRIx"\n", __func__, addr);
+
+ if (addr >= 0x100) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: ADC Common Register Unsupported\n", __func__);
+ }
+
+ switch (addr) {
+ case ADC_SR:
+ return s->adc_sr;
+ case ADC_CR1:
+ return s->adc_cr1;
+ case ADC_CR2:
+ return s->adc_cr2 & 0xFFFFFFF;
+ case ADC_SMPR1:
+ return s->adc_smpr1;
+ case ADC_SMPR2:
+ return s->adc_smpr2;
+ case ADC_JOFR1:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jofr1;
+ case ADC_JOFR2:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jofr2;
+ case ADC_JOFR3:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jofr3;
+ case ADC_JOFR4:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jofr4;
+ case ADC_HTR:
+ return s->adc_htr;
+ case ADC_LTR:
+ return s->adc_ltr;
+ case ADC_SQR1:
+ return s->adc_sqr1;
+ case ADC_SQR2:
+ return s->adc_sqr2;
+ case ADC_SQR3:
+ return s->adc_sqr3;
+ case ADC_JSQR:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jsqr;
+ case ADC_JDR1:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jdr1 - s->adc_jofr1;
+ case ADC_JDR2:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jdr2 - s->adc_jofr2;
+ case ADC_JDR3:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jdr3 - s->adc_jofr3;
+ case ADC_JDR4:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ return s->adc_jdr4 - s->adc_jofr4;
+ case ADC_DR:
+ if ((s->adc_cr2 & ADC_CR2_ADON) && (s->adc_cr2 & ADC_CR2_SWSTART)) {
+ s->adc_cr2 ^= ADC_CR2_SWSTART;
+ return stm32f2xx_adc_generate_value(s);
+ } else {
+ return 0x00000000;
+ }
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+ }
+
+ return 0;
+}
+
+static void stm32f2xx_adc_write(void *opaque, hwaddr addr,
+ uint64_t val64, unsigned int size)
+{
+ STM32F2XXAdcState *s = opaque;
+ uint32_t value = (uint32_t) val64;
+
+ DB_PRINT("%s: Address: 0x%"HWADDR_PRIx", Value: 0x%x\n",
+ __func__, addr, value);
+
+ if (addr >= 0x100) {
+ qemu_log_mask(LOG_UNIMP,
+ "%s: ADC Common Register Unsupported\n", __func__);
+ }
+
+ switch (addr) {
+ case ADC_SR:
+ s->adc_sr &= (value & 0x3F);
+ break;
+ case ADC_CR1:
+ s->adc_cr1 = value;
+ break;
+ case ADC_CR2:
+ s->adc_cr2 = value;
+ break;
+ case ADC_SMPR1:
+ s->adc_smpr1 = value;
+ break;
+ case ADC_SMPR2:
+ s->adc_smpr2 = value;
+ break;
+ case ADC_JOFR1:
+ s->adc_jofr1 = (value & 0xFFF);
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JOFR2:
+ s->adc_jofr2 = (value & 0xFFF);
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JOFR3:
+ s->adc_jofr3 = (value & 0xFFF);
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JOFR4:
+ s->adc_jofr4 = (value & 0xFFF);
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_HTR:
+ s->adc_htr = value;
+ break;
+ case ADC_LTR:
+ s->adc_ltr = value;
+ break;
+ case ADC_SQR1:
+ s->adc_sqr1 = value;
+ break;
+ case ADC_SQR2:
+ s->adc_sqr2 = value;
+ break;
+ case ADC_SQR3:
+ s->adc_sqr3 = value;
+ break;
+ case ADC_JSQR:
+ s->adc_jsqr = value;
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JDR1:
+ s->adc_jdr1 = value;
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JDR2:
+ s->adc_jdr2 = value;
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JDR3:
+ s->adc_jdr3 = value;
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ case ADC_JDR4:
+ s->adc_jdr4 = value;
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Injection ADC is not implemented, the registers are " \
+ "includded for compatability\n", __func__);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+ }
+}
+
+static const MemoryRegionOps stm32f2xx_adc_ops = {
+ .read = stm32f2xx_adc_read,
+ .write = stm32f2xx_adc_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void stm32f2xx_adc_init(Object *obj)
+{
+ STM32F2XXAdcState *s = STM32F2XX_ADC(obj);
+
+ sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+ memory_region_init_io(&s->mmio, obj, &stm32f2xx_adc_ops, s,
+ TYPE_STM32F2XX_ADC, 0xFF);
+ sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+}
+
+static void stm32f2xx_adc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->reset = stm32f2xx_adc_reset;
+}
+
+static const TypeInfo stm32f2xx_adc_info = {
+ .name = TYPE_STM32F2XX_ADC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(STM32F2XXAdcState),
+ .instance_init = stm32f2xx_adc_init,
+ .class_init = stm32f2xx_adc_class_init,
+};
+
+static void stm32f2xx_adc_register_types(void)
+{
+ type_register_static(&stm32f2xx_adc_info);
+}
+
+type_init(stm32f2xx_adc_register_types)
diff --git a/include/hw/misc/stm32f2xx_adc.h b/include/hw/misc/stm32f2xx_adc.h
new file mode 100644
index 0000000..e0ec1d7
--- /dev/null
+++ b/include/hw/misc/stm32f2xx_adc.h
@@ -0,0 +1,96 @@
+/*
+ * STM32F2XX ADC
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ *
+ * 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.
+ */
+
+#ifndef HW_STM32F2XX_ADC_H
+#define HW_STM32F2XX_ADC_H
+
+#define ADC_SR 0x00
+#define ADC_CR1 0x04
+#define ADC_CR2 0x08
+#define ADC_SMPR1 0x0C
+#define ADC_SMPR2 0x10
+#define ADC_JOFR1 0x14
+#define ADC_JOFR2 0x18
+#define ADC_JOFR3 0x1C
+#define ADC_JOFR4 0x20
+#define ADC_HTR 0x24
+#define ADC_LTR 0x28
+#define ADC_SQR1 0x2C
+#define ADC_SQR2 0x30
+#define ADC_SQR3 0x34
+#define ADC_JSQR 0x38
+#define ADC_JDR1 0x3C
+#define ADC_JDR2 0x40
+#define ADC_JDR3 0x44
+#define ADC_JDR4 0x48
+#define ADC_DR 0x4C
+
+#define ADC_CR2_ADON 0x01
+#define ADC_CR2_CONT 0x02
+#define ADC_CR2_ALIGN 0x800
+#define ADC_CR2_SWSTART 0x40000000
+
+#define ADC_CR1_RES 0x3000000
+
+#define TYPE_STM32F2XX_ADC "stm32f2xx-adc"
+#define STM32F2XX_ADC(obj) \
+ OBJECT_CHECK(STM32F2XXAdcState, (obj), TYPE_STM32F2XX_ADC)
+
+#ifdef RAND_MAX
+/* The rand() function is avaliable */
+#define RAND_AVALIABLE
+#undef RAND_MAX
+#define RAND_MAX 0xFF
+#endif /* RAND_MAX */
+
+typedef struct {
+ SysBusDevice parent_obj;
+
+ MemoryRegion mmio;
+
+ uint32_t adc_sr;
+ uint32_t adc_cr1;
+ uint32_t adc_cr2;
+ uint32_t adc_smpr1;
+ uint32_t adc_smpr2;
+ uint32_t adc_jofr1;
+ uint32_t adc_jofr2;
+ uint32_t adc_jofr3;
+ uint32_t adc_jofr4;
+ uint32_t adc_htr;
+ uint32_t adc_ltr;
+ uint32_t adc_sqr1;
+ uint32_t adc_sqr2;
+ uint32_t adc_sqr3;
+ uint32_t adc_jsqr;
+ uint32_t adc_jdr1;
+ uint32_t adc_jdr2;
+ uint32_t adc_jdr3;
+ uint32_t adc_jdr4;
+ uint32_t adc_dr;
+
+ qemu_irq irq;
+} STM32F2XXAdcState;
+
+#endif /* HW_STM32F2XX_ADC_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device Alistair Francis
@ 2015-04-25 18:43 ` Peter Crosthwaite
2015-04-27 12:02 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:43 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Add the STM32F2xx ADC device. This device randomly
> generates values on each read.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>
> default-configs/arm-softmmu.mak | 1 +
> hw/misc/Makefile.objs | 1 +
> hw/misc/stm32f2xx_adc.c | 332 ++++++++++++++++++++++++++++++++++++++++
> include/hw/misc/stm32f2xx_adc.h | 96 ++++++++++++
> 4 files changed, 430 insertions(+)
> create mode 100644 hw/misc/stm32f2xx_adc.c
> create mode 100644 include/hw/misc/stm32f2xx_adc.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index a767e4b..2b16590 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -84,6 +84,7 @@ CONFIG_ZYNQ=y
> CONFIG_STM32F2XX_TIMER=y
> CONFIG_STM32F2XX_USART=y
> CONFIG_STM32F2XX_SYSCFG=y
> +CONFIG_STM32F2XX_ADC=y
> CONFIG_STM32F205_SOC=y
>
> CONFIG_VERSATILE_PCI=y
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 4aa76ff..b65ec3d 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -37,6 +37,7 @@ obj-$(CONFIG_OMAP) += omap_tap.o
> obj-$(CONFIG_SLAVIO) += slavio_misc.o
> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
> +obj-$(CONFIG_STM32F2XX_ADC) += stm32f2xx_adc.o
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> obj-$(CONFIG_EDU) += edu.o
> diff --git a/hw/misc/stm32f2xx_adc.c b/hw/misc/stm32f2xx_adc.c
> new file mode 100644
> index 0000000..659f14f
> --- /dev/null
> +++ b/hw/misc/stm32f2xx_adc.c
> @@ -0,0 +1,332 @@
> +/*
> + * STM32F2XX ADC
> + *
> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
> + *
> + * 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 "hw/sysbus.h"
> +#include "hw/hw.h"
> +#include "hw/misc/stm32f2xx_adc.h"
> +
Misc is starting to pile up in random features whereas ADC seems to be
a nice self contained category. Can we create hw/adc? Are there any
other ADCs in misc?
> +#ifndef STM_ADC_ERR_DEBUG
> +#define STM_ADC_ERR_DEBUG 0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> + if (STM_ADC_ERR_DEBUG >= lvl) { \
> + qemu_log("%s: " fmt, __func__, ## args); \
> + } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +static void stm32f2xx_adc_reset(DeviceState *dev)
> +{
> + STM32F2XXAdcState *s = STM32F2XX_ADC(dev);
> +
> + s->adc_sr = 0x00000000;
> + s->adc_cr1 = 0x00000000;
> + s->adc_cr2 = 0x00000000;
> + s->adc_smpr1 = 0x00000000;
> + s->adc_smpr2 = 0x00000000;
> + s->adc_jofr1 = 0x00000000;
> + s->adc_jofr2 = 0x00000000;
> + s->adc_jofr3 = 0x00000000;
> + s->adc_jofr4 = 0x00000000;
> + s->adc_htr = 0x00000FFF;
> + s->adc_ltr = 0x00000000;
> + s->adc_sqr1 = 0x00000000;
> + s->adc_sqr2 = 0x00000000;
> + s->adc_sqr3 = 0x00000000;
> + s->adc_jsqr = 0x00000000;
> + s->adc_jdr1 = 0x00000000;
> + s->adc_jdr2 = 0x00000000;
> + s->adc_jdr3 = 0x00000000;
> + s->adc_jdr4 = 0x00000000;
> + s->adc_dr = 0x00000000;
I'm guessing the documentation prefixes all the register symbolic
names with "ADC". In these cases where the doc adds such prefixes I
usually drop them from variables names as the doc is usually
documenting them as part of a bigger register block. In this case the
fact that it is adc is implicit so can the prefix be dropped and save
some text?
> +}
> +
> +static uint32_t stm32f2xx_adc_generate_value(STM32F2XXAdcState *s)
> +{
> + /* Attempts to fake some ADC values */
> + #ifdef RAND_AVALIABLE
#ifdef should be non-indented.
Although I have been thinking about this, and perhaps it is better to
make the value (or its randomization factor) runtime controllable. Can
you propertyify something and then you can set the ADC value from the
monitor using a property setter? Even if it cannot be set from the
monitor it might help to at least be start-time configurable using
-global.
> + s->adc_dr = s->adc_dr + rand();
> + #else
> + s->adc_dr = s->adc_dr + 7;
> + #endif
> +
> + if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 0) {
extract to get field value.
switch (s->adc_cr1 & ADC_CR1_RES) >> 24
But could be handled with:
bit_width = 12 - 2 * extract32(a->adc_cr1, ADC_CR1_RES_SHIFT,
ADC_CR1_RES_LENGTH);
s->adc_dr &= (1 << bit_width) - 1;
> + /* 12-bit */
> + s->adc_dr = s->adc_dr & 0xFFF;
&=
> + } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 1) {
> + /* 10-bit */
> + s->adc_dr = s->adc_dr & 0x3FF;
> + } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 2) {
> + /* 8-bit */
> + s->adc_dr = s->adc_dr & 0xFF;
> + } else {
> + /* 6-bit */
> + s->adc_dr = s->adc_dr & 0x3F;
> + }
> +
> + if (s->adc_cr2 & ADC_CR2_ALIGN) {
> + return (s->adc_dr << 1) & 0xFFF0;
> + } else {
> + return s->adc_dr;
> + }
> +}
> +
> +static uint64_t stm32f2xx_adc_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + STM32F2XXAdcState *s = opaque;
> +
> + DB_PRINT("%s: Address: 0x%"HWADDR_PRIx"\n", __func__, addr);
> +
> + if (addr >= 0x100) {
Macro for register offset (even if unimplemented)
> + qemu_log_mask(LOG_UNIMP,
> + "%s: ADC Common Register Unsupported\n", __func__);
> + }
> +
> + switch (addr) {
> + case ADC_SR:
> + return s->adc_sr;
> + case ADC_CR1:
> + return s->adc_cr1;
> + case ADC_CR2:
> + return s->adc_cr2 & 0xFFFFFFF;
> + case ADC_SMPR1:
> + return s->adc_smpr1;
> + case ADC_SMPR2:
> + return s->adc_smpr2;
> + case ADC_JOFR1:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
"included"
> + return s->adc_jofr1;
> + case ADC_JOFR2:
Can you use an array for the JOFRs as they are consecutive registers
with the same semantics. Then the 4X repitition can be removed.
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jofr2;
> + case ADC_JOFR3:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jofr3;
> + case ADC_JOFR4:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jofr4;
> + case ADC_HTR:
> + return s->adc_htr;
> + case ADC_LTR:
> + return s->adc_ltr;
> + case ADC_SQR1:
Same for SQRs
> + return s->adc_sqr1;
> + case ADC_SQR2:
> + return s->adc_sqr2;
> + case ADC_SQR3:
> + return s->adc_sqr3;
> + case ADC_JSQR:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
"Included".
> + return s->adc_jsqr;
> + case ADC_JDR1:
Array here too.
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jdr1 - s->adc_jofr1;
> + case ADC_JDR2:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jdr2 - s->adc_jofr2;
> + case ADC_JDR3:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jdr3 - s->adc_jofr3;
> + case ADC_JDR4:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + return s->adc_jdr4 - s->adc_jofr4;
> + case ADC_DR:
> + if ((s->adc_cr2 & ADC_CR2_ADON) && (s->adc_cr2 & ADC_CR2_SWSTART)) {
> + s->adc_cr2 ^= ADC_CR2_SWSTART;
> + return stm32f2xx_adc_generate_value(s);
> + } else {
> + return 0x00000000;
> + }
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
> + }
> +
> + return 0;
> +}
> +
> +static void stm32f2xx_adc_write(void *opaque, hwaddr addr,
> + uint64_t val64, unsigned int size)
> +{
> + STM32F2XXAdcState *s = opaque;
> + uint32_t value = (uint32_t) val64;
> +
> + DB_PRINT("%s: Address: 0x%"HWADDR_PRIx", Value: 0x%x\n",
> + __func__, addr, value);
> +
> + if (addr >= 0x100) {
> + qemu_log_mask(LOG_UNIMP,
> + "%s: ADC Common Register Unsupported\n", __func__);
> + }
> +
> + switch (addr) {
> + case ADC_SR:
> + s->adc_sr &= (value & 0x3F);
> + break;
> + case ADC_CR1:
> + s->adc_cr1 = value;
> + break;
> + case ADC_CR2:
> + s->adc_cr2 = value;
> + break;
> + case ADC_SMPR1:
> + s->adc_smpr1 = value;
> + break;
> + case ADC_SMPR2:
> + s->adc_smpr2 = value;
> + break;
> + case ADC_JOFR1:
> + s->adc_jofr1 = (value & 0xFFF);
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JOFR2:
> + s->adc_jofr2 = (value & 0xFFF);
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JOFR3:
> + s->adc_jofr3 = (value & 0xFFF);
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JOFR4:
> + s->adc_jofr4 = (value & 0xFFF);
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_HTR:
> + s->adc_htr = value;
> + break;
> + case ADC_LTR:
> + s->adc_ltr = value;
> + break;
> + case ADC_SQR1:
> + s->adc_sqr1 = value;
> + break;
> + case ADC_SQR2:
> + s->adc_sqr2 = value;
> + break;
> + case ADC_SQR3:
> + s->adc_sqr3 = value;
> + break;
> + case ADC_JSQR:
> + s->adc_jsqr = value;
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JDR1:
> + s->adc_jdr1 = value;
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JDR2:
> + s->adc_jdr2 = value;
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JDR3:
> + s->adc_jdr3 = value;
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + case ADC_JDR4:
> + s->adc_jdr4 = value;
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Injection ADC is not implemented, the registers are " \
> + "includded for compatability\n", __func__);
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
> + }
> +}
> +
> +static const MemoryRegionOps stm32f2xx_adc_ops = {
> + .read = stm32f2xx_adc_read,
> + .write = stm32f2xx_adc_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void stm32f2xx_adc_init(Object *obj)
> +{
> + STM32F2XXAdcState *s = STM32F2XX_ADC(obj);
> +
> + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> +
> + memory_region_init_io(&s->mmio, obj, &stm32f2xx_adc_ops, s,
> + TYPE_STM32F2XX_ADC, 0xFF);
> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +}
> +
> +static void stm32f2xx_adc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->reset = stm32f2xx_adc_reset;
> +}
> +
> +static const TypeInfo stm32f2xx_adc_info = {
> + .name = TYPE_STM32F2XX_ADC,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(STM32F2XXAdcState),
> + .instance_init = stm32f2xx_adc_init,
> + .class_init = stm32f2xx_adc_class_init,
> +};
> +
> +static void stm32f2xx_adc_register_types(void)
> +{
> + type_register_static(&stm32f2xx_adc_info);
> +}
> +
> +type_init(stm32f2xx_adc_register_types)
> diff --git a/include/hw/misc/stm32f2xx_adc.h b/include/hw/misc/stm32f2xx_adc.h
> new file mode 100644
> index 0000000..e0ec1d7
> --- /dev/null
> +++ b/include/hw/misc/stm32f2xx_adc.h
> @@ -0,0 +1,96 @@
> +/*
> + * STM32F2XX ADC
> + *
> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
> + *
> + * 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.
> + */
> +
> +#ifndef HW_STM32F2XX_ADC_H
> +#define HW_STM32F2XX_ADC_H
> +
> +#define ADC_SR 0x00
> +#define ADC_CR1 0x04
> +#define ADC_CR2 0x08
> +#define ADC_SMPR1 0x0C
> +#define ADC_SMPR2 0x10
> +#define ADC_JOFR1 0x14
> +#define ADC_JOFR2 0x18
> +#define ADC_JOFR3 0x1C
> +#define ADC_JOFR4 0x20
> +#define ADC_HTR 0x24
> +#define ADC_LTR 0x28
> +#define ADC_SQR1 0x2C
> +#define ADC_SQR2 0x30
> +#define ADC_SQR3 0x34
> +#define ADC_JSQR 0x38
> +#define ADC_JDR1 0x3C
> +#define ADC_JDR2 0x40
> +#define ADC_JDR3 0x44
> +#define ADC_JDR4 0x48
> +#define ADC_DR 0x4C
> +
> +#define ADC_CR2_ADON 0x01
> +#define ADC_CR2_CONT 0x02
> +#define ADC_CR2_ALIGN 0x800
> +#define ADC_CR2_SWSTART 0x40000000
> +
> +#define ADC_CR1_RES 0x3000000
> +
I'm acutally starting to see a big problem with having the reg defs in
the common header, as it is not namespace safe. If you have two
periphs defining a reg with the same name in the same soc you get a
collision. The only safe way to do it would be to preface every symbol
with the full type name. This would be super verbose.
On the other side if we don't headerify them they are not available to
test code.
Out of scope so don't need to do anything for now but maybe we need to
start a top lvl thread.
> +#define TYPE_STM32F2XX_ADC "stm32f2xx-adc"
> +#define STM32F2XX_ADC(obj) \
> + OBJECT_CHECK(STM32F2XXAdcState, (obj), TYPE_STM32F2XX_ADC)
> +
> +#ifdef RAND_MAX
> +/* The rand() function is avaliable */
> +#define RAND_AVALIABLE
RAND_AVAILABLE
> +#undef RAND_MAX
> +#define RAND_MAX 0xFF
> +#endif /* RAND_MAX */
> +
> +typedef struct {
/* <private> */
> + SysBusDevice parent_obj;
> +
/* <public> */
> + MemoryRegion mmio;
> +
> + uint32_t adc_sr;
> + uint32_t adc_cr1;
> + uint32_t adc_cr2;
> + uint32_t adc_smpr1;
> + uint32_t adc_smpr2;
> + uint32_t adc_jofr1;
> + uint32_t adc_jofr2;
> + uint32_t adc_jofr3;
> + uint32_t adc_jofr4;
> + uint32_t adc_htr;
> + uint32_t adc_ltr;
> + uint32_t adc_sqr1;
> + uint32_t adc_sqr2;
> + uint32_t adc_sqr3;
> + uint32_t adc_jsqr;
> + uint32_t adc_jdr1;
> + uint32_t adc_jdr2;
> + uint32_t adc_jdr3;
> + uint32_t adc_jdr4;
> + uint32_t adc_dr;
> +
> + qemu_irq irq;
> +} STM32F2XXAdcState;
STM32F2XXADCState
Regards,
Peter
> +
> +#endif /* HW_STM32F2XX_ADC_H */
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device
2015-04-25 18:43 ` Peter Crosthwaite
@ 2015-04-27 12:02 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-04-27 12:02 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:43 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Add the STM32F2xx ADC device. This device randomly
>> generates values on each read.
>>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>> ---
>>
>> default-configs/arm-softmmu.mak | 1 +
>> hw/misc/Makefile.objs | 1 +
>> hw/misc/stm32f2xx_adc.c | 332 ++++++++++++++++++++++++++++++++++++++++
>> include/hw/misc/stm32f2xx_adc.h | 96 ++++++++++++
>> 4 files changed, 430 insertions(+)
>> create mode 100644 hw/misc/stm32f2xx_adc.c
>> create mode 100644 include/hw/misc/stm32f2xx_adc.h
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index a767e4b..2b16590 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -84,6 +84,7 @@ CONFIG_ZYNQ=y
>> CONFIG_STM32F2XX_TIMER=y
>> CONFIG_STM32F2XX_USART=y
>> CONFIG_STM32F2XX_SYSCFG=y
>> +CONFIG_STM32F2XX_ADC=y
>> CONFIG_STM32F205_SOC=y
>>
>> CONFIG_VERSATILE_PCI=y
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 4aa76ff..b65ec3d 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -37,6 +37,7 @@ obj-$(CONFIG_OMAP) += omap_tap.o
>> obj-$(CONFIG_SLAVIO) += slavio_misc.o
>> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>> obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
>> +obj-$(CONFIG_STM32F2XX_ADC) += stm32f2xx_adc.o
>>
>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>> obj-$(CONFIG_EDU) += edu.o
>> diff --git a/hw/misc/stm32f2xx_adc.c b/hw/misc/stm32f2xx_adc.c
>> new file mode 100644
>> index 0000000..659f14f
>> --- /dev/null
>> +++ b/hw/misc/stm32f2xx_adc.c
>> @@ -0,0 +1,332 @@
>> +/*
>> + * STM32F2XX ADC
>> + *
>> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
>> + *
>> + * 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 "hw/sysbus.h"
>> +#include "hw/hw.h"
>> +#include "hw/misc/stm32f2xx_adc.h"
>> +
>
> Misc is starting to pile up in random features whereas ADC seems to be
> a nice self contained category. Can we create hw/adc? Are there any
> other ADCs in misc?
There are a few, but normally they are already included in another file (like
the machine that is using ADC). I do agree with you that the misc folder is
filling up, so I will create and ADC one (unless anyone has any objections?)
>
>> +#ifndef STM_ADC_ERR_DEBUG
>> +#define STM_ADC_ERR_DEBUG 0
>> +#endif
>> +
>> +#define DB_PRINT_L(lvl, fmt, args...) do { \
>> + if (STM_ADC_ERR_DEBUG >= lvl) { \
>> + qemu_log("%s: " fmt, __func__, ## args); \
>> + } \
>> +} while (0);
>> +
>> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>> +
>> +static void stm32f2xx_adc_reset(DeviceState *dev)
>> +{
>> + STM32F2XXAdcState *s = STM32F2XX_ADC(dev);
>> +
>> + s->adc_sr = 0x00000000;
>> + s->adc_cr1 = 0x00000000;
>> + s->adc_cr2 = 0x00000000;
>> + s->adc_smpr1 = 0x00000000;
>> + s->adc_smpr2 = 0x00000000;
>> + s->adc_jofr1 = 0x00000000;
>> + s->adc_jofr2 = 0x00000000;
>> + s->adc_jofr3 = 0x00000000;
>> + s->adc_jofr4 = 0x00000000;
>> + s->adc_htr = 0x00000FFF;
>> + s->adc_ltr = 0x00000000;
>> + s->adc_sqr1 = 0x00000000;
>> + s->adc_sqr2 = 0x00000000;
>> + s->adc_sqr3 = 0x00000000;
>> + s->adc_jsqr = 0x00000000;
>> + s->adc_jdr1 = 0x00000000;
>> + s->adc_jdr2 = 0x00000000;
>> + s->adc_jdr3 = 0x00000000;
>> + s->adc_jdr4 = 0x00000000;
>> + s->adc_dr = 0x00000000;
>
> I'm guessing the documentation prefixes all the register symbolic
> names with "ADC". In these cases where the doc adds such prefixes I
> usually drop them from variables names as the doc is usually
> documenting them as part of a bigger register block. In this case the
> fact that it is adc is implicit so can the prefix be dropped and save
> some text?
Yes you're right, the documentation does prefix the register names
with adc_*. I don't see any issue with it though. It's not very long
and otherwise some of the register names would only be two letters
long, which I think is too short.
>
>> +}
>> +
>> +static uint32_t stm32f2xx_adc_generate_value(STM32F2XXAdcState *s)
>> +{
>> + /* Attempts to fake some ADC values */
>> + #ifdef RAND_AVALIABLE
>
> #ifdef should be non-indented.
Good catch, will fix.
>
> Although I have been thinking about this, and perhaps it is better to
> make the value (or its randomization factor) runtime controllable. Can
> you propertyify something and then you can set the ADC value from the
> monitor using a property setter? Even if it cannot be set from the
> monitor it might help to at least be start-time configurable using
> -global.
Hmmm... I like the idea of setting the property from the monitor, that
would be useful. I don't see any need to set it via the command-line
though, I can't think of any realistic use cases (maybe the
multiplier, but it still doesn't seem nessesary).
In saying that, at the moment I don't have time to look into it. It
would have to be something that I do after this (and a few more) patch
series get committed.
>
>> + s->adc_dr = s->adc_dr + rand();
>> + #else
>> + s->adc_dr = s->adc_dr + 7;
>> + #endif
>> +
>> + if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 0) {
>
> extract to get field value.
>
> switch (s->adc_cr1 & ADC_CR1_RES) >> 24
>
> But could be handled with:
>
> bit_width = 12 - 2 * extract32(a->adc_cr1, ADC_CR1_RES_SHIFT,
> ADC_CR1_RES_LENGTH);
> s->adc_dr &= (1 << bit_width) - 1;
I think the switch statement is easier to read so I'll swap it over to that.
The bit_width method makes it a little harder to understand what's going on.
>
>> + /* 12-bit */
>> + s->adc_dr = s->adc_dr & 0xFFF;
>
> &=
Will fix.
>
>> + } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 1) {
>> + /* 10-bit */
>> + s->adc_dr = s->adc_dr & 0x3FF;
>> + } else if (((s->adc_cr1 & ADC_CR1_RES) >> 24) == 2) {
>> + /* 8-bit */
>> + s->adc_dr = s->adc_dr & 0xFF;
>> + } else {
>> + /* 6-bit */
>> + s->adc_dr = s->adc_dr & 0x3F;
>> + }
>> +
>> + if (s->adc_cr2 & ADC_CR2_ALIGN) {
>> + return (s->adc_dr << 1) & 0xFFF0;
>> + } else {
>> + return s->adc_dr;
>> + }
>> +}
>> +
>> +static uint64_t stm32f2xx_adc_read(void *opaque, hwaddr addr,
>> + unsigned int size)
>> +{
>> + STM32F2XXAdcState *s = opaque;
>> +
>> + DB_PRINT("%s: Address: 0x%"HWADDR_PRIx"\n", __func__, addr);
>> +
>> + if (addr >= 0x100) {
>
> Macro for register offset (even if unimplemented)
Will fix
>
>> + qemu_log_mask(LOG_UNIMP,
>> + "%s: ADC Common Register Unsupported\n", __func__);
>> + }
>> +
>> + switch (addr) {
>> + case ADC_SR:
>> + return s->adc_sr;
>> + case ADC_CR1:
>> + return s->adc_cr1;
>> + case ADC_CR2:
>> + return s->adc_cr2 & 0xFFFFFFF;
>> + case ADC_SMPR1:
>> + return s->adc_smpr1;
>> + case ADC_SMPR2:
>> + return s->adc_smpr2;
>> + case ADC_JOFR1:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>
> "included"
Good catch, will fix them all.
>
>> + return s->adc_jofr1;
>> + case ADC_JOFR2:
>
> Can you use an array for the JOFRs as they are consecutive registers
> with the same semantics. Then the 4X repitition can be removed.
Yeah, I can do that
>
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jofr2;
>> + case ADC_JOFR3:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jofr3;
>> + case ADC_JOFR4:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jofr4;
>> + case ADC_HTR:
>> + return s->adc_htr;
>> + case ADC_LTR:
>> + return s->adc_ltr;
>> + case ADC_SQR1:
>
> Same for SQRs
They don't have the long unimplemented messages, so I don't think I will bother.
>
>> + return s->adc_sqr1;
>> + case ADC_SQR2:
>> + return s->adc_sqr2;
>> + case ADC_SQR3:
>> + return s->adc_sqr3;
>> + case ADC_JSQR:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>
> "Included".
>
>> + return s->adc_jsqr;
>> + case ADC_JDR1:
>
> Array here too.
Will arraryify these though.
>
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jdr1 - s->adc_jofr1;
>> + case ADC_JDR2:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jdr2 - s->adc_jofr2;
>> + case ADC_JDR3:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jdr3 - s->adc_jofr3;
>> + case ADC_JDR4:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + return s->adc_jdr4 - s->adc_jofr4;
>> + case ADC_DR:
>> + if ((s->adc_cr2 & ADC_CR2_ADON) && (s->adc_cr2 & ADC_CR2_SWSTART)) {
>> + s->adc_cr2 ^= ADC_CR2_SWSTART;
>> + return stm32f2xx_adc_generate_value(s);
>> + } else {
>> + return 0x00000000;
>> + }
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void stm32f2xx_adc_write(void *opaque, hwaddr addr,
>> + uint64_t val64, unsigned int size)
>> +{
>> + STM32F2XXAdcState *s = opaque;
>> + uint32_t value = (uint32_t) val64;
>> +
>> + DB_PRINT("%s: Address: 0x%"HWADDR_PRIx", Value: 0x%x\n",
>> + __func__, addr, value);
>> +
>> + if (addr >= 0x100) {
>> + qemu_log_mask(LOG_UNIMP,
>> + "%s: ADC Common Register Unsupported\n", __func__);
>> + }
>> +
>> + switch (addr) {
>> + case ADC_SR:
>> + s->adc_sr &= (value & 0x3F);
>> + break;
>> + case ADC_CR1:
>> + s->adc_cr1 = value;
>> + break;
>> + case ADC_CR2:
>> + s->adc_cr2 = value;
>> + break;
>> + case ADC_SMPR1:
>> + s->adc_smpr1 = value;
>> + break;
>> + case ADC_SMPR2:
>> + s->adc_smpr2 = value;
>> + break;
>> + case ADC_JOFR1:
>> + s->adc_jofr1 = (value & 0xFFF);
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JOFR2:
>> + s->adc_jofr2 = (value & 0xFFF);
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JOFR3:
>> + s->adc_jofr3 = (value & 0xFFF);
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JOFR4:
>> + s->adc_jofr4 = (value & 0xFFF);
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_HTR:
>> + s->adc_htr = value;
>> + break;
>> + case ADC_LTR:
>> + s->adc_ltr = value;
>> + break;
>> + case ADC_SQR1:
>> + s->adc_sqr1 = value;
>> + break;
>> + case ADC_SQR2:
>> + s->adc_sqr2 = value;
>> + break;
>> + case ADC_SQR3:
>> + s->adc_sqr3 = value;
>> + break;
>> + case ADC_JSQR:
>> + s->adc_jsqr = value;
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JDR1:
>> + s->adc_jdr1 = value;
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JDR2:
>> + s->adc_jdr2 = value;
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JDR3:
>> + s->adc_jdr3 = value;
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + case ADC_JDR4:
>> + s->adc_jdr4 = value;
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Injection ADC is not implemented, the registers are " \
>> + "includded for compatability\n", __func__);
>> + break;
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
>> + }
>> +}
>> +
>> +static const MemoryRegionOps stm32f2xx_adc_ops = {
>> + .read = stm32f2xx_adc_read,
>> + .write = stm32f2xx_adc_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void stm32f2xx_adc_init(Object *obj)
>> +{
>> + STM32F2XXAdcState *s = STM32F2XX_ADC(obj);
>> +
>> + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
>> +
>> + memory_region_init_io(&s->mmio, obj, &stm32f2xx_adc_ops, s,
>> + TYPE_STM32F2XX_ADC, 0xFF);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>> +}
>> +
>> +static void stm32f2xx_adc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->reset = stm32f2xx_adc_reset;
>> +}
>> +
>> +static const TypeInfo stm32f2xx_adc_info = {
>> + .name = TYPE_STM32F2XX_ADC,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(STM32F2XXAdcState),
>> + .instance_init = stm32f2xx_adc_init,
>> + .class_init = stm32f2xx_adc_class_init,
>> +};
>> +
>> +static void stm32f2xx_adc_register_types(void)
>> +{
>> + type_register_static(&stm32f2xx_adc_info);
>> +}
>> +
>> +type_init(stm32f2xx_adc_register_types)
>> diff --git a/include/hw/misc/stm32f2xx_adc.h b/include/hw/misc/stm32f2xx_adc.h
>> new file mode 100644
>> index 0000000..e0ec1d7
>> --- /dev/null
>> +++ b/include/hw/misc/stm32f2xx_adc.h
>> @@ -0,0 +1,96 @@
>> +/*
>> + * STM32F2XX ADC
>> + *
>> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef HW_STM32F2XX_ADC_H
>> +#define HW_STM32F2XX_ADC_H
>> +
>> +#define ADC_SR 0x00
>> +#define ADC_CR1 0x04
>> +#define ADC_CR2 0x08
>> +#define ADC_SMPR1 0x0C
>> +#define ADC_SMPR2 0x10
>> +#define ADC_JOFR1 0x14
>> +#define ADC_JOFR2 0x18
>> +#define ADC_JOFR3 0x1C
>> +#define ADC_JOFR4 0x20
>> +#define ADC_HTR 0x24
>> +#define ADC_LTR 0x28
>> +#define ADC_SQR1 0x2C
>> +#define ADC_SQR2 0x30
>> +#define ADC_SQR3 0x34
>> +#define ADC_JSQR 0x38
>> +#define ADC_JDR1 0x3C
>> +#define ADC_JDR2 0x40
>> +#define ADC_JDR3 0x44
>> +#define ADC_JDR4 0x48
>> +#define ADC_DR 0x4C
>> +
>> +#define ADC_CR2_ADON 0x01
>> +#define ADC_CR2_CONT 0x02
>> +#define ADC_CR2_ALIGN 0x800
>> +#define ADC_CR2_SWSTART 0x40000000
>> +
>> +#define ADC_CR1_RES 0x3000000
>> +
>
> I'm acutally starting to see a big problem with having the reg defs in
> the common header, as it is not namespace safe. If you have two
> periphs defining a reg with the same name in the same soc you get a
> collision. The only safe way to do it would be to preface every symbol
> with the full type name. This would be super verbose.
>
> On the other side if we don't headerify them they are not available to
> test code.
>
> Out of scope so don't need to do anything for now but maybe we need to
> start a top lvl thread.
I see your point, it's a hard problem to solve though.
>
>> +#define TYPE_STM32F2XX_ADC "stm32f2xx-adc"
>> +#define STM32F2XX_ADC(obj) \
>> + OBJECT_CHECK(STM32F2XXAdcState, (obj), TYPE_STM32F2XX_ADC)
>> +
>> +#ifdef RAND_MAX
>> +/* The rand() function is avaliable */
>> +#define RAND_AVALIABLE
>
> RAND_AVAILABLE
Will fix
>
>> +#undef RAND_MAX
>> +#define RAND_MAX 0xFF
>> +#endif /* RAND_MAX */
>> +
>> +typedef struct {
>
> /* <private> */
>
>> + SysBusDevice parent_obj;
>> +
>
> /* <public> */
Will add.
>
>> + MemoryRegion mmio;
>> +
>> + uint32_t adc_sr;
>> + uint32_t adc_cr1;
>> + uint32_t adc_cr2;
>> + uint32_t adc_smpr1;
>> + uint32_t adc_smpr2;
>> + uint32_t adc_jofr1;
>> + uint32_t adc_jofr2;
>> + uint32_t adc_jofr3;
>> + uint32_t adc_jofr4;
>> + uint32_t adc_htr;
>> + uint32_t adc_ltr;
>> + uint32_t adc_sqr1;
>> + uint32_t adc_sqr2;
>> + uint32_t adc_sqr3;
>> + uint32_t adc_jsqr;
>> + uint32_t adc_jdr1;
>> + uint32_t adc_jdr2;
>> + uint32_t adc_jdr3;
>> + uint32_t adc_jdr4;
>> + uint32_t adc_dr;
>> +
>> + qemu_irq irq;
>> +} STM32F2XXAdcState;
>
> STM32F2XXADCState
Ok, will fix
Thanks,
Alistair
>
> Regards,
> Peter
>
>> +
>> +#endif /* HW_STM32F2XX_ADC_H */
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
` (2 preceding siblings ...)
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 3/7] STM32F2xx: Add the ADC device Alistair Francis
@ 2015-04-25 8:18 ` Alistair Francis
2015-04-25 18:43 ` Peter Crosthwaite
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device Alistair Francis
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:18 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Add the STM32F2xx SPI device.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
default-configs/arm-softmmu.mak | 1 +
hw/ssi/Makefile.objs | 1 +
hw/ssi/stm32f2xx_spi.c | 211 ++++++++++++++++++++++++++++++++++++++++
include/hw/ssi/stm32f2xx_spi.h | 74 ++++++++++++++
4 files changed, 287 insertions(+)
create mode 100644 hw/ssi/stm32f2xx_spi.c
create mode 100644 include/hw/ssi/stm32f2xx_spi.h
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 2b16590..580fac3 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -85,6 +85,7 @@ CONFIG_STM32F2XX_TIMER=y
CONFIG_STM32F2XX_USART=y
CONFIG_STM32F2XX_SYSCFG=y
CONFIG_STM32F2XX_ADC=y
+CONFIG_STM32F2XX_SPI=y
CONFIG_STM32F205_SOC=y
CONFIG_VERSATILE_PCI=y
diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 9555825..c674247 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -2,5 +2,6 @@ common-obj-$(CONFIG_PL022) += pl022.o
common-obj-$(CONFIG_SSI) += ssi.o
common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
+common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
obj-$(CONFIG_OMAP) += omap_spi.o
diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
new file mode 100644
index 0000000..11bbdef
--- /dev/null
+++ b/hw/ssi/stm32f2xx_spi.c
@@ -0,0 +1,211 @@
+/*
+ * STM32F405 SPI
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ *
+ * 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 "hw/ssi/stm32f2xx_spi.h"
+
+#ifndef STM_SPI_ERR_DEBUG
+#define STM_SPI_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+ if (STM_SPI_ERR_DEBUG >= lvl) { \
+ qemu_log("%s: " fmt, __func__, ## args); \
+ } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void stm32f2xx_spi_reset(DeviceState *dev)
+{
+ STM32F2XXSpiState *s = STM32F2XX_SPI(dev);
+
+ s->spi_cr1 = 0x00000000;
+ s->spi_cr2 = 0x00000000;
+ s->spi_sr = 0x0000000A;
+ s->spi_dr = 0x0000000C;
+ s->spi_crcpr = 0x00000007;
+ s->spi_rxcrcr = 0x00000000;
+ s->spi_txcrcr = 0x00000000;
+ s->spi_i2scfgr = 0x00000000;
+ s->spi_i2spr = 0x00000002;
+}
+
+static void stm32f2xx_spi_transfer(STM32F2XXSpiState *s)
+{
+ DB_PRINT("Data to send: 0x%x\n", s->spi_dr);
+
+ s->spi_dr = ssi_transfer(s->ssi, s->spi_dr);
+ s->spi_sr |= STM_SPI_SR_RXNE;
+
+ DB_PRINT("Data recieved: 0x%x\n", s->spi_dr);
+}
+
+static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ STM32F2XXSpiState *s = opaque;
+ uint32_t retval;
+
+ DB_PRINT("Read 0x%"HWADDR_PRIx"\n", addr);
+
+ switch (addr) {
+ case STM_SPI_CR1:
+ return s->spi_cr1;
+ case STM_SPI_CR2:
+ qemu_log_mask(LOG_UNIMP, "%s: Interrupts and DMA are not implemented\n",
+ __func__);
+ return s->spi_cr2;
+ case STM_SPI_SR:
+ retval = s->spi_sr;
+ return retval;
+ case STM_SPI_DR:
+ s->spi_sr |= STM_SPI_SR_BSY;
+ stm32f2xx_spi_transfer(s);
+ s->spi_sr &= ~STM_SPI_SR_RXNE;
+ s->spi_sr &= ~STM_SPI_SR_BSY;
+ return s->spi_dr;
+ case STM_SPI_CRCPR:
+ qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+ "are includded for compatability\n", __func__);
+ return s->spi_crcpr;
+ case STM_SPI_RXCRCR:
+ qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+ "are includded for compatability\n", __func__);
+ return s->spi_rxcrcr;
+ case STM_SPI_TXCRCR:
+ qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+ "are includded for compatability\n", __func__);
+ return s->spi_txcrcr;
+ case STM_SPI_I2SCFGR:
+ qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
+ "are includded for compatability\n", __func__);
+ return s->spi_i2scfgr;
+ case STM_SPI_I2SPR:
+ qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
+ "are includded for compatability\n", __func__);
+ return s->spi_i2spr;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n",
+ __func__, addr);
+ }
+
+ return 0;
+}
+
+static void stm32f2xx_spi_write(void *opaque, hwaddr addr,
+ uint64_t val64, unsigned int size)
+{
+ STM32F2XXSpiState *s = opaque;
+ uint32_t value = val64;
+
+ DB_PRINT("Write 0x%x at 0x%"HWADDR_PRIx"\n", value, addr);
+
+ switch (addr) {
+ case STM_SPI_CR1:
+ s->spi_cr1 = value;
+ return;
+ case STM_SPI_CR2:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "Interrupts and DMA are not implemented\n", __func__);
+ s->spi_cr2 = value;
+ return;
+ case STM_SPI_SR:
+ /* Read only register, except for clearing the CRCERR bit, which
+ * is not supported
+ */
+ return;
+ case STM_SPI_DR:
+ s->spi_sr |= STM_SPI_SR_BSY;
+ s->spi_sr &= ~STM_SPI_SR_TXE;
+ s->spi_dr = value;
+ stm32f2xx_spi_transfer(s);
+ s->spi_sr |= STM_SPI_SR_TXE;
+ s->spi_sr &= ~STM_SPI_SR_BSY;
+ return;
+ case STM_SPI_CRCPR:
+ qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented\n", __func__);
+ return;
+ case STM_SPI_RXCRCR:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
+ "0x%"HWADDR_PRIx"\n", __func__, addr);
+ return;
+ case STM_SPI_TXCRCR:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
+ "0x%"HWADDR_PRIx"\n", __func__, addr);
+ return;
+ case STM_SPI_I2SCFGR:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "I2S is not implemented\n", __func__);
+ return;
+ case STM_SPI_I2SPR:
+ qemu_log_mask(LOG_UNIMP, "%s: " \
+ "I2S is not implemented\n", __func__);
+ return;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
+ }
+}
+
+static const MemoryRegionOps stm32f2xx_spi_ops = {
+ .read = stm32f2xx_spi_read,
+ .write = stm32f2xx_spi_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void stm32f2xx_spi_init(Object *obj)
+{
+ STM32F2XXSpiState *s = STM32F2XX_SPI(obj);
+ DeviceState *dev = DEVICE(obj);
+
+ memory_region_init_io(&s->mmio, obj, &stm32f2xx_spi_ops, s,
+ TYPE_STM32F2XX_SPI, 0x400);
+ sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+
+ sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+ s->ssi = ssi_create_bus(dev, "ssi");
+}
+
+static void stm32f2xx_spi_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->reset = stm32f2xx_spi_reset;
+}
+
+static const TypeInfo stm32f2xx_spi_info = {
+ .name = TYPE_STM32F2XX_SPI,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(STM32F2XXSpiState),
+ .instance_init = stm32f2xx_spi_init,
+ .class_init = stm32f2xx_spi_class_init,
+};
+
+static void stm32f2xx_spi_register_types(void)
+{
+ type_register_static(&stm32f2xx_spi_info);
+}
+
+type_init(stm32f2xx_spi_register_types)
diff --git a/include/hw/ssi/stm32f2xx_spi.h b/include/hw/ssi/stm32f2xx_spi.h
new file mode 100644
index 0000000..75dd6a8
--- /dev/null
+++ b/include/hw/ssi/stm32f2xx_spi.h
@@ -0,0 +1,74 @@
+/*
+ * STM32F2XX SPI
+ *
+ * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
+ *
+ * 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.
+ */
+
+#ifndef HW_STM32F2XX_SPI_H
+#define HW_STM32F2XX_SPI_H
+
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/ssi.h"
+
+#define STM_SPI_CR1 0x00
+#define STM_SPI_CR2 0x04
+#define STM_SPI_SR 0x08
+#define STM_SPI_DR 0x0C
+#define STM_SPI_CRCPR 0x10
+#define STM_SPI_RXCRCR 0x14
+#define STM_SPI_TXCRCR 0x18
+#define STM_SPI_I2SCFGR 0x1C
+#define STM_SPI_I2SPR 0x20
+
+#define STM_SPI_CR1_SPE (1 << 6)
+#define STM_SPI_CR1_MSTR (1 << 2)
+
+#define STM_SPI_SR_BSY (1 << 7)
+#define STM_SPI_SR_TXE (1 << 1)
+#define STM_SPI_SR_RXNE 1
+
+#define TYPE_STM32F2XX_SPI "stm32f2xx-spi"
+#define STM32F2XX_SPI(obj) \
+ OBJECT_CHECK(STM32F2XXSpiState, (obj), TYPE_STM32F2XX_SPI)
+
+typedef struct {
+ /* <private> */
+ SysBusDevice parent_obj;
+
+ /* <public> */
+ MemoryRegion mmio;
+
+ uint32_t spi_cr1;
+ uint32_t spi_cr2;
+ uint32_t spi_sr;
+ uint32_t spi_dr;
+ uint32_t spi_crcpr;
+ uint32_t spi_rxcrcr;
+ uint32_t spi_txcrcr;
+ uint32_t spi_i2scfgr;
+ uint32_t spi_i2spr;
+
+ qemu_irq irq;
+ SSIBus *ssi;
+} STM32F2XXSpiState;
+
+#endif /* HW_STM32F2XX_SPI_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device Alistair Francis
@ 2015-04-25 18:43 ` Peter Crosthwaite
2015-05-14 10:36 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:43 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Add the STM32F2xx SPI device.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>
> default-configs/arm-softmmu.mak | 1 +
> hw/ssi/Makefile.objs | 1 +
> hw/ssi/stm32f2xx_spi.c | 211 ++++++++++++++++++++++++++++++++++++++++
> include/hw/ssi/stm32f2xx_spi.h | 74 ++++++++++++++
> 4 files changed, 287 insertions(+)
> create mode 100644 hw/ssi/stm32f2xx_spi.c
> create mode 100644 include/hw/ssi/stm32f2xx_spi.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 2b16590..580fac3 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -85,6 +85,7 @@ CONFIG_STM32F2XX_TIMER=y
> CONFIG_STM32F2XX_USART=y
> CONFIG_STM32F2XX_SYSCFG=y
> CONFIG_STM32F2XX_ADC=y
> +CONFIG_STM32F2XX_SPI=y
> CONFIG_STM32F205_SOC=y
>
> CONFIG_VERSATILE_PCI=y
> diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
> index 9555825..c674247 100644
> --- a/hw/ssi/Makefile.objs
> +++ b/hw/ssi/Makefile.objs
> @@ -2,5 +2,6 @@ common-obj-$(CONFIG_PL022) += pl022.o
> common-obj-$(CONFIG_SSI) += ssi.o
> common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
> common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
> +common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
>
> obj-$(CONFIG_OMAP) += omap_spi.o
> diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
> new file mode 100644
> index 0000000..11bbdef
> --- /dev/null
> +++ b/hw/ssi/stm32f2xx_spi.c
> @@ -0,0 +1,211 @@
> +/*
> + * STM32F405 SPI
> + *
> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
> + *
> + * 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 "hw/ssi/stm32f2xx_spi.h"
> +
> +#ifndef STM_SPI_ERR_DEBUG
> +#define STM_SPI_ERR_DEBUG 0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> + if (STM_SPI_ERR_DEBUG >= lvl) { \
> + qemu_log("%s: " fmt, __func__, ## args); \
> + } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +static void stm32f2xx_spi_reset(DeviceState *dev)
> +{
> + STM32F2XXSpiState *s = STM32F2XX_SPI(dev);
> +
> + s->spi_cr1 = 0x00000000;
> + s->spi_cr2 = 0x00000000;
> + s->spi_sr = 0x0000000A;
> + s->spi_dr = 0x0000000C;
> + s->spi_crcpr = 0x00000007;
> + s->spi_rxcrcr = 0x00000000;
> + s->spi_txcrcr = 0x00000000;
> + s->spi_i2scfgr = 0x00000000;
> + s->spi_i2spr = 0x00000002;
> +}
> +
> +static void stm32f2xx_spi_transfer(STM32F2XXSpiState *s)
> +{
> + DB_PRINT("Data to send: 0x%x\n", s->spi_dr);
> +
> + s->spi_dr = ssi_transfer(s->ssi, s->spi_dr);
> + s->spi_sr |= STM_SPI_SR_RXNE;
> +
> + DB_PRINT("Data recieved: 0x%x\n", s->spi_dr);
"received". I usually just us "tx" and "rx" in the strings though.
Using a %s __func__ you could probably dropthe sentance altogether.
> +}
> +
> +static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + STM32F2XXSpiState *s = opaque;
> + uint32_t retval;
> +
> + DB_PRINT("Read 0x%"HWADDR_PRIx"\n", addr);
__func__
Makes it sef contained and also consistent with ADC (and other devs).
> +
> + switch (addr) {
> + case STM_SPI_CR1:
> + return s->spi_cr1;
> + case STM_SPI_CR2:
> + qemu_log_mask(LOG_UNIMP, "%s: Interrupts and DMA are not implemented\n",
> + __func__);
> + return s->spi_cr2;
> + case STM_SPI_SR:
> + retval = s->spi_sr;
> + return retval;
> + case STM_SPI_DR:
> + s->spi_sr |= STM_SPI_SR_BSY;
> + stm32f2xx_spi_transfer(s);
This seems unusual to trigger the actual SPI transfer using the read
txn. This would mean that this would be a very long bus transaction as
you need to wait for the actual SPI byte txrx to happen during a
single MMIO read. This can be very slow. Does the transfer occur on
the write instead?
> + s->spi_sr &= ~STM_SPI_SR_RXNE;
> + s->spi_sr &= ~STM_SPI_SR_BSY;
This (and the |= of same above) should be unneeded as it will never be
visible to the guest.
> + return s->spi_dr;
> + case STM_SPI_CRCPR:
> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
> + "are includded for compatability\n", __func__);
"included"
> + return s->spi_crcpr;
> + case STM_SPI_RXCRCR:
> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
> + "are includded for compatability\n", __func__);
> + return s->spi_rxcrcr;
> + case STM_SPI_TXCRCR:
> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
> + "are includded for compatability\n", __func__);
You could perhaps factor out a lot of this string into a const
variable to avoid the repetition.
> + return s->spi_txcrcr;
> + case STM_SPI_I2SCFGR:
> + qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
> + "are includded for compatability\n", __func__);
> + return s->spi_i2scfgr;
> + case STM_SPI_I2SPR:
> + qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
> + "are includded for compatability\n", __func__);
> + return s->spi_i2spr;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n",
> + __func__, addr);
> + }
> +
> + return 0;
> +}
> +
> +static void stm32f2xx_spi_write(void *opaque, hwaddr addr,
> + uint64_t val64, unsigned int size)
> +{
> + STM32F2XXSpiState *s = opaque;
> + uint32_t value = val64;
> +
> + DB_PRINT("Write 0x%x at 0x%"HWADDR_PRIx"\n", value, addr);
> +
> + switch (addr) {
> + case STM_SPI_CR1:
> + s->spi_cr1 = value;
> + return;
> + case STM_SPI_CR2:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "Interrupts and DMA are not implemented\n", __func__);
> + s->spi_cr2 = value;
> + return;
> + case STM_SPI_SR:
> + /* Read only register, except for clearing the CRCERR bit, which
> + * is not supported
> + */
> + return;
> + case STM_SPI_DR:
> + s->spi_sr |= STM_SPI_SR_BSY;
> + s->spi_sr &= ~STM_SPI_SR_TXE;
> + s->spi_dr = value;
> + stm32f2xx_spi_transfer(s);
So it does a transfer on the write as well. This means there are
actually two transfers per write/read pair. One will be read only, the
other write only. It is possible to do concurrent read and write with
this IP?
> + s->spi_sr |= STM_SPI_SR_TXE;
> + s->spi_sr &= ~STM_SPI_SR_BSY;
> + return;
> + case STM_SPI_CRCPR:
> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented\n", __func__);
> + return;
> + case STM_SPI_RXCRCR:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
> + "0x%"HWADDR_PRIx"\n", __func__, addr);
> + return;
> + case STM_SPI_TXCRCR:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
> + "0x%"HWADDR_PRIx"\n", __func__, addr);
> + return;
> + case STM_SPI_I2SCFGR:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "I2S is not implemented\n", __func__);
> + return;
> + case STM_SPI_I2SPR:
> + qemu_log_mask(LOG_UNIMP, "%s: " \
> + "I2S is not implemented\n", __func__);
> + return;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
> + }
> +}
> +
> +static const MemoryRegionOps stm32f2xx_spi_ops = {
> + .read = stm32f2xx_spi_read,
> + .write = stm32f2xx_spi_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void stm32f2xx_spi_init(Object *obj)
> +{
> + STM32F2XXSpiState *s = STM32F2XX_SPI(obj);
> + DeviceState *dev = DEVICE(obj);
> +
> + memory_region_init_io(&s->mmio, obj, &stm32f2xx_spi_ops, s,
> + TYPE_STM32F2XX_SPI, 0x400);
> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +
> + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> +
> + s->ssi = ssi_create_bus(dev, "ssi");
> +}
> +
> +static void stm32f2xx_spi_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->reset = stm32f2xx_spi_reset;
> +}
> +
> +static const TypeInfo stm32f2xx_spi_info = {
> + .name = TYPE_STM32F2XX_SPI,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(STM32F2XXSpiState),
> + .instance_init = stm32f2xx_spi_init,
> + .class_init = stm32f2xx_spi_class_init,
> +};
> +
> +static void stm32f2xx_spi_register_types(void)
> +{
> + type_register_static(&stm32f2xx_spi_info);
> +}
> +
> +type_init(stm32f2xx_spi_register_types)
> diff --git a/include/hw/ssi/stm32f2xx_spi.h b/include/hw/ssi/stm32f2xx_spi.h
> new file mode 100644
> index 0000000..75dd6a8
> --- /dev/null
> +++ b/include/hw/ssi/stm32f2xx_spi.h
> @@ -0,0 +1,74 @@
> +/*
> + * STM32F2XX SPI
> + *
> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
> + *
> + * 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.
> + */
> +
> +#ifndef HW_STM32F2XX_SPI_H
> +#define HW_STM32F2XX_SPI_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/hw.h"
> +#include "hw/ssi.h"
> +
> +#define STM_SPI_CR1 0x00
> +#define STM_SPI_CR2 0x04
> +#define STM_SPI_SR 0x08
> +#define STM_SPI_DR 0x0C
> +#define STM_SPI_CRCPR 0x10
> +#define STM_SPI_RXCRCR 0x14
> +#define STM_SPI_TXCRCR 0x18
> +#define STM_SPI_I2SCFGR 0x1C
> +#define STM_SPI_I2SPR 0x20
> +
> +#define STM_SPI_CR1_SPE (1 << 6)
> +#define STM_SPI_CR1_MSTR (1 << 2)
> +
> +#define STM_SPI_SR_BSY (1 << 7)
> +#define STM_SPI_SR_TXE (1 << 1)
> +#define STM_SPI_SR_RXNE 1
> +
> +#define TYPE_STM32F2XX_SPI "stm32f2xx-spi"
> +#define STM32F2XX_SPI(obj) \
> + OBJECT_CHECK(STM32F2XXSpiState, (obj), TYPE_STM32F2XX_SPI)
> +
> +typedef struct {
> + /* <private> */
> + SysBusDevice parent_obj;
> +
> + /* <public> */
> + MemoryRegion mmio;
> +
> + uint32_t spi_cr1;
> + uint32_t spi_cr2;
> + uint32_t spi_sr;
> + uint32_t spi_dr;
> + uint32_t spi_crcpr;
> + uint32_t spi_rxcrcr;
> + uint32_t spi_txcrcr;
> + uint32_t spi_i2scfgr;
> + uint32_t spi_i2spr;
> +
> + qemu_irq irq;
> + SSIBus *ssi;
> +} STM32F2XXSpiState;
STM32F2XXSPIState
Regards,
Peter
> +
> +#endif /* HW_STM32F2XX_SPI_H */
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device
2015-04-25 18:43 ` Peter Crosthwaite
@ 2015-05-14 10:36 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-05-14 10:36 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:43 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Add the STM32F2xx SPI device.
>>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>> ---
>>
>> default-configs/arm-softmmu.mak | 1 +
>> hw/ssi/Makefile.objs | 1 +
>> hw/ssi/stm32f2xx_spi.c | 211 ++++++++++++++++++++++++++++++++++++++++
>> include/hw/ssi/stm32f2xx_spi.h | 74 ++++++++++++++
>> 4 files changed, 287 insertions(+)
>> create mode 100644 hw/ssi/stm32f2xx_spi.c
>> create mode 100644 include/hw/ssi/stm32f2xx_spi.h
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 2b16590..580fac3 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -85,6 +85,7 @@ CONFIG_STM32F2XX_TIMER=y
>> CONFIG_STM32F2XX_USART=y
>> CONFIG_STM32F2XX_SYSCFG=y
>> CONFIG_STM32F2XX_ADC=y
>> +CONFIG_STM32F2XX_SPI=y
>> CONFIG_STM32F205_SOC=y
>>
>> CONFIG_VERSATILE_PCI=y
>> diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
>> index 9555825..c674247 100644
>> --- a/hw/ssi/Makefile.objs
>> +++ b/hw/ssi/Makefile.objs
>> @@ -2,5 +2,6 @@ common-obj-$(CONFIG_PL022) += pl022.o
>> common-obj-$(CONFIG_SSI) += ssi.o
>> common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
>> common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
>> +common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
>>
>> obj-$(CONFIG_OMAP) += omap_spi.o
>> diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
>> new file mode 100644
>> index 0000000..11bbdef
>> --- /dev/null
>> +++ b/hw/ssi/stm32f2xx_spi.c
>> @@ -0,0 +1,211 @@
>> +/*
>> + * STM32F405 SPI
>> + *
>> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
>> + *
>> + * 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 "hw/ssi/stm32f2xx_spi.h"
>> +
>> +#ifndef STM_SPI_ERR_DEBUG
>> +#define STM_SPI_ERR_DEBUG 0
>> +#endif
>> +
>> +#define DB_PRINT_L(lvl, fmt, args...) do { \
>> + if (STM_SPI_ERR_DEBUG >= lvl) { \
>> + qemu_log("%s: " fmt, __func__, ## args); \
>> + } \
>> +} while (0);
>> +
>> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>> +
>> +static void stm32f2xx_spi_reset(DeviceState *dev)
>> +{
>> + STM32F2XXSpiState *s = STM32F2XX_SPI(dev);
>> +
>> + s->spi_cr1 = 0x00000000;
>> + s->spi_cr2 = 0x00000000;
>> + s->spi_sr = 0x0000000A;
>> + s->spi_dr = 0x0000000C;
>> + s->spi_crcpr = 0x00000007;
>> + s->spi_rxcrcr = 0x00000000;
>> + s->spi_txcrcr = 0x00000000;
>> + s->spi_i2scfgr = 0x00000000;
>> + s->spi_i2spr = 0x00000002;
>> +}
>> +
>> +static void stm32f2xx_spi_transfer(STM32F2XXSpiState *s)
>> +{
>> + DB_PRINT("Data to send: 0x%x\n", s->spi_dr);
>> +
>> + s->spi_dr = ssi_transfer(s->ssi, s->spi_dr);
>> + s->spi_sr |= STM_SPI_SR_RXNE;
>> +
>> + DB_PRINT("Data recieved: 0x%x\n", s->spi_dr);
>
> "received". I usually just us "tx" and "rx" in the strings though.
Good catch. I prefer using the full string as it explicitly describes what
is going on. Plus it's not that long.
> Using a %s __func__ you could probably dropthe sentance altogether.
The %s __func__ is included in the DB_PRINT() macro. Although as mentioned
I still like including the string, especially to indicate the direction.
>
>> +}
>> +
>> +static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
>> + unsigned int size)
>> +{
>> + STM32F2XXSpiState *s = opaque;
>> + uint32_t retval;
>> +
>> + DB_PRINT("Read 0x%"HWADDR_PRIx"\n", addr);
>
> __func__
>
> Makes it sef contained and also consistent with ADC (and other devs).
Ok, I'll make it the same as the ADC, which is actually a little different
to some of the others :\
>
>> +
>> + switch (addr) {
>> + case STM_SPI_CR1:
>> + return s->spi_cr1;
>> + case STM_SPI_CR2:
>> + qemu_log_mask(LOG_UNIMP, "%s: Interrupts and DMA are not implemented\n",
>> + __func__);
>> + return s->spi_cr2;
>> + case STM_SPI_SR:
>> + retval = s->spi_sr;
>> + return retval;
>> + case STM_SPI_DR:
>> + s->spi_sr |= STM_SPI_SR_BSY;
>> + stm32f2xx_spi_transfer(s);
>
> This seems unusual to trigger the actual SPI transfer using the read
> txn. This would mean that this would be a very long bus transaction as
> you need to wait for the actual SPI byte txrx to happen during a
> single MMIO read. This can be very slow. Does the transfer occur on
> the write instead?
>From the data sheet it looks like the SPI transfer occurs on both reads/writes,
as there is only one register.
>From the data sheet:
"Data received or to be transmitted.
The data register is split into 2 buffers - one for writing (Transmit
Buffer) and another one for
reading (Receive buffer). A write to the data register will write into
the Tx buffer and a read
from the data register will return the value held in the Rx buffer."
>
>> + s->spi_sr &= ~STM_SPI_SR_RXNE;
>> + s->spi_sr &= ~STM_SPI_SR_BSY;
>
> This (and the |= of same above) should be unneeded as it will never be
> visible to the guest.
True, will remove.
>
>> + return s->spi_dr;
>> + case STM_SPI_CRCPR:
>> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
>> + "are includded for compatability\n", __func__);
>
> "included"
Good catch, will fix them all.
>
>> + return s->spi_crcpr;
>> + case STM_SPI_RXCRCR:
>> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
>> + "are includded for compatability\n", __func__);
>> + return s->spi_rxcrcr;
>> + case STM_SPI_TXCRCR:
>> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
>> + "are includded for compatability\n", __func__);
>
> You could perhaps factor out a lot of this string into a const
> variable to avoid the repetition.
That's true, but I don't think it's nesesary. It's only in the code a few times.
>
>> + return s->spi_txcrcr;
>> + case STM_SPI_I2SCFGR:
>> + qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
>> + "are includded for compatability\n", __func__);
>> + return s->spi_i2scfgr;
>> + case STM_SPI_I2SPR:
>> + qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
>> + "are includded for compatability\n", __func__);
>> + return s->spi_i2spr;
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n",
>> + __func__, addr);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void stm32f2xx_spi_write(void *opaque, hwaddr addr,
>> + uint64_t val64, unsigned int size)
>> +{
>> + STM32F2XXSpiState *s = opaque;
>> + uint32_t value = val64;
>> +
>> + DB_PRINT("Write 0x%x at 0x%"HWADDR_PRIx"\n", value, addr);
>> +
>> + switch (addr) {
>> + case STM_SPI_CR1:
>> + s->spi_cr1 = value;
>> + return;
>> + case STM_SPI_CR2:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "Interrupts and DMA are not implemented\n", __func__);
>> + s->spi_cr2 = value;
>> + return;
>> + case STM_SPI_SR:
>> + /* Read only register, except for clearing the CRCERR bit, which
>> + * is not supported
>> + */
>> + return;
>> + case STM_SPI_DR:
>> + s->spi_sr |= STM_SPI_SR_BSY;
>> + s->spi_sr &= ~STM_SPI_SR_TXE;
>> + s->spi_dr = value;
>> + stm32f2xx_spi_transfer(s);
>
> So it does a transfer on the write as well. This means there are
> actually two transfers per write/read pair. One will be read only, the
> other write only. It is possible to do concurrent read and write with
> this IP?
>From what I get from the data sheet I don't think so. See the quote from
above.
>
>> + s->spi_sr |= STM_SPI_SR_TXE;
>> + s->spi_sr &= ~STM_SPI_SR_BSY;
>> + return;
>> + case STM_SPI_CRCPR:
>> + qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented\n", __func__);
>> + return;
>> + case STM_SPI_RXCRCR:
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
>> + "0x%"HWADDR_PRIx"\n", __func__, addr);
>> + return;
>> + case STM_SPI_TXCRCR:
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Read only register: " \
>> + "0x%"HWADDR_PRIx"\n", __func__, addr);
>> + return;
>> + case STM_SPI_I2SCFGR:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "I2S is not implemented\n", __func__);
>> + return;
>> + case STM_SPI_I2SPR:
>> + qemu_log_mask(LOG_UNIMP, "%s: " \
>> + "I2S is not implemented\n", __func__);
>> + return;
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
>> + }
>> +}
>> +
>> +static const MemoryRegionOps stm32f2xx_spi_ops = {
>> + .read = stm32f2xx_spi_read,
>> + .write = stm32f2xx_spi_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void stm32f2xx_spi_init(Object *obj)
>> +{
>> + STM32F2XXSpiState *s = STM32F2XX_SPI(obj);
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + memory_region_init_io(&s->mmio, obj, &stm32f2xx_spi_ops, s,
>> + TYPE_STM32F2XX_SPI, 0x400);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>> +
>> + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
>> +
>> + s->ssi = ssi_create_bus(dev, "ssi");
>> +}
>> +
>> +static void stm32f2xx_spi_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->reset = stm32f2xx_spi_reset;
>> +}
>> +
>> +static const TypeInfo stm32f2xx_spi_info = {
>> + .name = TYPE_STM32F2XX_SPI,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(STM32F2XXSpiState),
>> + .instance_init = stm32f2xx_spi_init,
>> + .class_init = stm32f2xx_spi_class_init,
>> +};
>> +
>> +static void stm32f2xx_spi_register_types(void)
>> +{
>> + type_register_static(&stm32f2xx_spi_info);
>> +}
>> +
>> +type_init(stm32f2xx_spi_register_types)
>> diff --git a/include/hw/ssi/stm32f2xx_spi.h b/include/hw/ssi/stm32f2xx_spi.h
>> new file mode 100644
>> index 0000000..75dd6a8
>> --- /dev/null
>> +++ b/include/hw/ssi/stm32f2xx_spi.h
>> @@ -0,0 +1,74 @@
>> +/*
>> + * STM32F2XX SPI
>> + *
>> + * Copyright (c) 2014 Alistair Francis <alistair@alistair23.me>
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef HW_STM32F2XX_SPI_H
>> +#define HW_STM32F2XX_SPI_H
>> +
>> +#include "hw/sysbus.h"
>> +#include "hw/hw.h"
>> +#include "hw/ssi.h"
>> +
>> +#define STM_SPI_CR1 0x00
>> +#define STM_SPI_CR2 0x04
>> +#define STM_SPI_SR 0x08
>> +#define STM_SPI_DR 0x0C
>> +#define STM_SPI_CRCPR 0x10
>> +#define STM_SPI_RXCRCR 0x14
>> +#define STM_SPI_TXCRCR 0x18
>> +#define STM_SPI_I2SCFGR 0x1C
>> +#define STM_SPI_I2SPR 0x20
>> +
>> +#define STM_SPI_CR1_SPE (1 << 6)
>> +#define STM_SPI_CR1_MSTR (1 << 2)
>> +
>> +#define STM_SPI_SR_BSY (1 << 7)
>> +#define STM_SPI_SR_TXE (1 << 1)
>> +#define STM_SPI_SR_RXNE 1
>> +
>> +#define TYPE_STM32F2XX_SPI "stm32f2xx-spi"
>> +#define STM32F2XX_SPI(obj) \
>> + OBJECT_CHECK(STM32F2XXSpiState, (obj), TYPE_STM32F2XX_SPI)
>> +
>> +typedef struct {
>> + /* <private> */
>> + SysBusDevice parent_obj;
>> +
>> + /* <public> */
>> + MemoryRegion mmio;
>> +
>> + uint32_t spi_cr1;
>> + uint32_t spi_cr2;
>> + uint32_t spi_sr;
>> + uint32_t spi_dr;
>> + uint32_t spi_crcpr;
>> + uint32_t spi_rxcrcr;
>> + uint32_t spi_txcrcr;
>> + uint32_t spi_i2scfgr;
>> + uint32_t spi_i2spr;
>> +
>> + qemu_irq irq;
>> + SSIBus *ssi;
>> +} STM32F2XXSpiState;
>
> STM32F2XXSPIState
Ok, will fix.
Thanks,
Alistair
>
> Regards,
> Peter
>
>> +
>> +#endif /* HW_STM32F2XX_SPI_H */
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
` (3 preceding siblings ...)
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 4/7] STM32F2xx: Add the SPI device Alistair Francis
@ 2015-04-25 8:18 ` Alistair Francis
2015-04-25 18:32 ` Peter Crosthwaite
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 6/7] STM32F205: Connect the SPI device Alistair Francis
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list Alistair Francis
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:18 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Connect the ADC device to the STM32F205 SoC.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
include/hw/arm/stm32f205_soc.h | 3 +++
2 files changed, 25 insertions(+)
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 63893f3..641ecbb 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
0x40000800, 0x40000C00 };
static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
+static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
+ 0x40012200 };
static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
+#define ADC_IRQ 18
static void stm32f205_soc_initfn(Object *obj)
{
@@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
TYPE_STM32F2XX_TIMER);
qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
}
+
+ for (i = 0; i < STM_NUM_ADCS; i++) {
+ object_initialize(&s->adc[i], sizeof(s->adc[i]),
+ TYPE_STM32F2XX_ADC);
+ qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
+ }
}
static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
sysbus_mmio_map(busdev, 0, timer_addr[i]);
sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
}
+
+ /* ADC 1 to 3 */
+ for (i = 0; i < STM_NUM_ADCS; i++) {
+ dev = DEVICE(&(s->adc[i]));
+ object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, adc_addr[i]);
+ sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
+ }
}
static Property stm32f205_soc_properties[] = {
diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 0390eff..7d6603b 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -28,6 +28,7 @@
#include "hw/misc/stm32f2xx_syscfg.h"
#include "hw/timer/stm32f2xx_timer.h"
#include "hw/char/stm32f2xx_usart.h"
+#include "hw/misc/stm32f2xx_adc.h"
#define TYPE_STM32F205_SOC "stm32f205-soc"
#define STM32F205_SOC(obj) \
@@ -35,6 +36,7 @@
#define STM_NUM_USARTS 6
#define STM_NUM_TIMERS 4
+#define STM_NUM_ADCS 3
#define FLASH_BASE_ADDRESS 0x08000000
#define FLASH_SIZE (1024 * 1024)
@@ -52,6 +54,7 @@ typedef struct STM32F205State {
STM32F2XXSyscfgState syscfg;
STM32F2XXUsartState usart[STM_NUM_USARTS];
STM32F2XXTimerState timer[STM_NUM_TIMERS];
+ STM32F2XXAdcState adc[STM_NUM_ADCS];
} STM32F205State;
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device Alistair Francis
@ 2015-04-25 18:32 ` Peter Crosthwaite
2015-04-26 2:21 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:32 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
"devices"
On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Connect the ADC device to the STM32F205 SoC.
>
"devices"
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>
> hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
> include/hw/arm/stm32f205_soc.h | 3 +++
> 2 files changed, 25 insertions(+)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 63893f3..641ecbb 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
> 0x40000800, 0x40000C00 };
> static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
> 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
> +static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
> + 0x40012200 };
>
> static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
> static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
> +#define ADC_IRQ 18
>
> static void stm32f205_soc_initfn(Object *obj)
> {
> @@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
> TYPE_STM32F2XX_TIMER);
> qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
> }
> +
> + for (i = 0; i < STM_NUM_ADCS; i++) {
> + object_initialize(&s->adc[i], sizeof(s->adc[i]),
> + TYPE_STM32F2XX_ADC);
> + qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
> + }
> }
>
> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> sysbus_mmio_map(busdev, 0, timer_addr[i]);
> sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
> }
> +
> + /* ADC 1 to 3 */
> + for (i = 0; i < STM_NUM_ADCS; i++) {
> + dev = DEVICE(&(s->adc[i]));
> + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, adc_addr[i]);
> + sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
This looks inconsistent with other devs. Is it a shared IRQ?
Regards,
Peter
> + }
> }
>
> static Property stm32f205_soc_properties[] = {
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 0390eff..7d6603b 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -28,6 +28,7 @@
> #include "hw/misc/stm32f2xx_syscfg.h"
> #include "hw/timer/stm32f2xx_timer.h"
> #include "hw/char/stm32f2xx_usart.h"
> +#include "hw/misc/stm32f2xx_adc.h"
>
> #define TYPE_STM32F205_SOC "stm32f205-soc"
> #define STM32F205_SOC(obj) \
> @@ -35,6 +36,7 @@
>
> #define STM_NUM_USARTS 6
> #define STM_NUM_TIMERS 4
> +#define STM_NUM_ADCS 3
>
> #define FLASH_BASE_ADDRESS 0x08000000
> #define FLASH_SIZE (1024 * 1024)
> @@ -52,6 +54,7 @@ typedef struct STM32F205State {
> STM32F2XXSyscfgState syscfg;
> STM32F2XXUsartState usart[STM_NUM_USARTS];
> STM32F2XXTimerState timer[STM_NUM_TIMERS];
> + STM32F2XXAdcState adc[STM_NUM_ADCS];
> } STM32F205State;
>
> #endif
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device
2015-04-25 18:32 ` Peter Crosthwaite
@ 2015-04-26 2:21 ` Alistair Francis
2015-04-26 19:26 ` Peter Crosthwaite
0 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-26 2:21 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:32 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> "devices"
>
> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Connect the ADC device to the STM32F205 SoC.
>>
>
> "devices"
Will fix both.
>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>> ---
>>
>> hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
>> include/hw/arm/stm32f205_soc.h | 3 +++
>> 2 files changed, 25 insertions(+)
>>
>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> index 63893f3..641ecbb 100644
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
>> 0x40000800, 0x40000C00 };
>> static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
>> 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
>> +static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
>> + 0x40012200 };
>>
>> static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
>> static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
>> +#define ADC_IRQ 18
>>
>> static void stm32f205_soc_initfn(Object *obj)
>> {
>> @@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
>> TYPE_STM32F2XX_TIMER);
>> qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
>> }
>> +
>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>> + object_initialize(&s->adc[i], sizeof(s->adc[i]),
>> + TYPE_STM32F2XX_ADC);
>> + qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
>> + }
>> }
>>
>> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> @@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> sysbus_mmio_map(busdev, 0, timer_addr[i]);
>> sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
>> }
>> +
>> + /* ADC 1 to 3 */
>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>> + dev = DEVICE(&(s->adc[i]));
>> + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> + busdev = SYS_BUS_DEVICE(dev);
>> + sysbus_mmio_map(busdev, 0, adc_addr[i]);
>> + sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
>
> This looks inconsistent with other devs. Is it a shared IRQ?
Yeah, all the ADCs use the same IRQ. I guess I could have an array
like the others with the three values repeated, but it seemed
unnecessary. I'm ambivalent though, so happy to change it if you think
the consistency is better.
Thanks,
Alistair
>
> Regards,
> Peter
>
>> + }
>> }
>>
>> static Property stm32f205_soc_properties[] = {
>> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
>> index 0390eff..7d6603b 100644
>> --- a/include/hw/arm/stm32f205_soc.h
>> +++ b/include/hw/arm/stm32f205_soc.h
>> @@ -28,6 +28,7 @@
>> #include "hw/misc/stm32f2xx_syscfg.h"
>> #include "hw/timer/stm32f2xx_timer.h"
>> #include "hw/char/stm32f2xx_usart.h"
>> +#include "hw/misc/stm32f2xx_adc.h"
>>
>> #define TYPE_STM32F205_SOC "stm32f205-soc"
>> #define STM32F205_SOC(obj) \
>> @@ -35,6 +36,7 @@
>>
>> #define STM_NUM_USARTS 6
>> #define STM_NUM_TIMERS 4
>> +#define STM_NUM_ADCS 3
>>
>> #define FLASH_BASE_ADDRESS 0x08000000
>> #define FLASH_SIZE (1024 * 1024)
>> @@ -52,6 +54,7 @@ typedef struct STM32F205State {
>> STM32F2XXSyscfgState syscfg;
>> STM32F2XXUsartState usart[STM_NUM_USARTS];
>> STM32F2XXTimerState timer[STM_NUM_TIMERS];
>> + STM32F2XXAdcState adc[STM_NUM_ADCS];
>> } STM32F205State;
>>
>> #endif
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device
2015-04-26 2:21 ` Alistair Francis
@ 2015-04-26 19:26 ` Peter Crosthwaite
2015-05-14 9:54 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-26 19:26 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 7:21 PM, Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Apr 26, 2015 at 4:32 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> "devices"
>>
>> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> Connect the ADC device to the STM32F205 SoC.
>>>
>>
>> "devices"
>
> Will fix both.
>
>>
>>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>>> ---
>>>
>>> hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
>>> include/hw/arm/stm32f205_soc.h | 3 +++
>>> 2 files changed, 25 insertions(+)
>>>
>>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>>> index 63893f3..641ecbb 100644
>>> --- a/hw/arm/stm32f205_soc.c
>>> +++ b/hw/arm/stm32f205_soc.c
>>> @@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
>>> 0x40000800, 0x40000C00 };
>>> static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
>>> 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
>>> +static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
>>> + 0x40012200 };
>>>
>>> static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
>>> static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
>>> +#define ADC_IRQ 18
>>>
>>> static void stm32f205_soc_initfn(Object *obj)
>>> {
>>> @@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
>>> TYPE_STM32F2XX_TIMER);
>>> qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
>>> }
>>> +
>>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>>> + object_initialize(&s->adc[i], sizeof(s->adc[i]),
>>> + TYPE_STM32F2XX_ADC);
>>> + qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
>>> + }
>>> }
>>>
>>> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>> @@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>> sysbus_mmio_map(busdev, 0, timer_addr[i]);
>>> sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
>>> }
>>> +
>>> + /* ADC 1 to 3 */
>>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>>> + dev = DEVICE(&(s->adc[i]));
>>> + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
>>> + if (err != NULL) {
>>> + error_propagate(errp, err);
>>> + return;
>>> + }
>>> + busdev = SYS_BUS_DEVICE(dev);
>>> + sysbus_mmio_map(busdev, 0, adc_addr[i]);
>>> + sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
>>
>> This looks inconsistent with other devs. Is it a shared IRQ?
>
> Yeah, all the ADCs use the same IRQ. I guess I could have an array
> like the others with the three values repeated, but it seemed
> unnecessary. I'm ambivalent though, so happy to change it if you think
> the consistency is better.
>
So shared IRQs are shaky, as they only work if its an edge sensitive
semantic. Is that the case (I found the datasheet confusing on this)?
But as you aren't using the IRQ yet, you could just remove the
connection completely.
Regards,
Peter
> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> + }
>>> }
>>>
>>> static Property stm32f205_soc_properties[] = {
>>> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
>>> index 0390eff..7d6603b 100644
>>> --- a/include/hw/arm/stm32f205_soc.h
>>> +++ b/include/hw/arm/stm32f205_soc.h
>>> @@ -28,6 +28,7 @@
>>> #include "hw/misc/stm32f2xx_syscfg.h"
>>> #include "hw/timer/stm32f2xx_timer.h"
>>> #include "hw/char/stm32f2xx_usart.h"
>>> +#include "hw/misc/stm32f2xx_adc.h"
>>>
>>> #define TYPE_STM32F205_SOC "stm32f205-soc"
>>> #define STM32F205_SOC(obj) \
>>> @@ -35,6 +36,7 @@
>>>
>>> #define STM_NUM_USARTS 6
>>> #define STM_NUM_TIMERS 4
>>> +#define STM_NUM_ADCS 3
>>>
>>> #define FLASH_BASE_ADDRESS 0x08000000
>>> #define FLASH_SIZE (1024 * 1024)
>>> @@ -52,6 +54,7 @@ typedef struct STM32F205State {
>>> STM32F2XXSyscfgState syscfg;
>>> STM32F2XXUsartState usart[STM_NUM_USARTS];
>>> STM32F2XXTimerState timer[STM_NUM_TIMERS];
>>> + STM32F2XXAdcState adc[STM_NUM_ADCS];
>>> } STM32F205State;
>>>
>>> #endif
>>> --
>>> 2.1.4
>>>
>>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device
2015-04-26 19:26 ` Peter Crosthwaite
@ 2015-05-14 9:54 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-05-14 9:54 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Mon, Apr 27, 2015 at 5:26 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 7:21 PM, Alistair Francis <alistair23@gmail.com> wrote:
>> On Sun, Apr 26, 2015 at 4:32 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> "devices"
>>>
>>> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>>> Connect the ADC device to the STM32F205 SoC.
>>>>
>>>
>>> "devices"
>>
>> Will fix both.
>>
>>>
>>>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>>>> ---
>>>>
>>>> hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
>>>> include/hw/arm/stm32f205_soc.h | 3 +++
>>>> 2 files changed, 25 insertions(+)
>>>>
>>>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>>>> index 63893f3..641ecbb 100644
>>>> --- a/hw/arm/stm32f205_soc.c
>>>> +++ b/hw/arm/stm32f205_soc.c
>>>> @@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
>>>> 0x40000800, 0x40000C00 };
>>>> static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
>>>> 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
>>>> +static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
>>>> + 0x40012200 };
>>>>
>>>> static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
>>>> static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
>>>> +#define ADC_IRQ 18
>>>>
>>>> static void stm32f205_soc_initfn(Object *obj)
>>>> {
>>>> @@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
>>>> TYPE_STM32F2XX_TIMER);
>>>> qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
>>>> }
>>>> +
>>>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>>>> + object_initialize(&s->adc[i], sizeof(s->adc[i]),
>>>> + TYPE_STM32F2XX_ADC);
>>>> + qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
>>>> + }
>>>> }
>>>>
>>>> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>>> @@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>>> sysbus_mmio_map(busdev, 0, timer_addr[i]);
>>>> sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
>>>> }
>>>> +
>>>> + /* ADC 1 to 3 */
>>>> + for (i = 0; i < STM_NUM_ADCS; i++) {
>>>> + dev = DEVICE(&(s->adc[i]));
>>>> + object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
>>>> + if (err != NULL) {
>>>> + error_propagate(errp, err);
>>>> + return;
>>>> + }
>>>> + busdev = SYS_BUS_DEVICE(dev);
>>>> + sysbus_mmio_map(busdev, 0, adc_addr[i]);
>>>> + sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
>>>
>>> This looks inconsistent with other devs. Is it a shared IRQ?
>>
>> Yeah, all the ADCs use the same IRQ. I guess I could have an array
>> like the others with the three values repeated, but it seemed
>> unnecessary. I'm ambivalent though, so happy to change it if you think
>> the consistency is better.
>>
>
> So shared IRQs are shaky, as they only work if its an edge sensitive
> semantic. Is that the case (I found the datasheet confusing on this)?
Hey Peter,
Sorry about the long delay. What do you mean by edge sensitive?
The data sheet is pretty lacking on information on the NVIC, it just
says:
"For more information on exceptions and NVIC programming,
refer to PM0056 programming manual."
> But as you aren't using the IRQ yet, you could just remove the
> connection completely.
That's true, I would prefer it connected though. That way it's ready to
go when the feature is added to the ADC device (hopefully).
Thanks,
Alistair
>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>> + }
>>>> }
>>>>
>>>> static Property stm32f205_soc_properties[] = {
>>>> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
>>>> index 0390eff..7d6603b 100644
>>>> --- a/include/hw/arm/stm32f205_soc.h
>>>> +++ b/include/hw/arm/stm32f205_soc.h
>>>> @@ -28,6 +28,7 @@
>>>> #include "hw/misc/stm32f2xx_syscfg.h"
>>>> #include "hw/timer/stm32f2xx_timer.h"
>>>> #include "hw/char/stm32f2xx_usart.h"
>>>> +#include "hw/misc/stm32f2xx_adc.h"
>>>>
>>>> #define TYPE_STM32F205_SOC "stm32f205-soc"
>>>> #define STM32F205_SOC(obj) \
>>>> @@ -35,6 +36,7 @@
>>>>
>>>> #define STM_NUM_USARTS 6
>>>> #define STM_NUM_TIMERS 4
>>>> +#define STM_NUM_ADCS 3
>>>>
>>>> #define FLASH_BASE_ADDRESS 0x08000000
>>>> #define FLASH_SIZE (1024 * 1024)
>>>> @@ -52,6 +54,7 @@ typedef struct STM32F205State {
>>>> STM32F2XXSyscfgState syscfg;
>>>> STM32F2XXUsartState usart[STM_NUM_USARTS];
>>>> STM32F2XXTimerState timer[STM_NUM_TIMERS];
>>>> + STM32F2XXAdcState adc[STM_NUM_ADCS];
>>>> } STM32F205State;
>>>>
>>>> #endif
>>>> --
>>>> 2.1.4
>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 6/7] STM32F205: Connect the SPI device
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
` (4 preceding siblings ...)
2015-04-25 8:18 ` [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device Alistair Francis
@ 2015-04-25 8:19 ` Alistair Francis
2015-04-25 18:31 ` Peter Crosthwaite
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list Alistair Francis
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:19 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Connect the SPI device to the STM32F205 SoC.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
include/hw/arm/stm32f205_soc.h | 3 +++
2 files changed, 25 insertions(+)
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 641ecbb..cb8884f 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -33,10 +33,13 @@ static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
0x40012200 };
+static const uint32_t spi_addr[STM_NUM_SPIS] = { 0x40013000, 0x40003800,
+ 0x40003C00 };
static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
#define ADC_IRQ 18
+static const int spi_irq[STM_NUM_SPIS] = {35, 36, 51};
static void stm32f205_soc_initfn(Object *obj)
{
@@ -63,6 +66,12 @@ static void stm32f205_soc_initfn(Object *obj)
TYPE_STM32F2XX_ADC);
qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
}
+
+ for (i = 0; i < STM_NUM_SPIS; i++) {
+ object_initialize(&s->spi[i], sizeof(s->spi[i]),
+ TYPE_STM32F2XX_SPI);
+ qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+ }
}
static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -150,6 +159,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
sysbus_mmio_map(busdev, 0, adc_addr[i]);
sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
}
+
+ /* SPI 1 and 2 */
+ for (i = 0; i < STM_NUM_SPIS; i++) {
+ dev = DEVICE(&(s->spi[i]));
+ object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ return;
+ }
+ busdev = SYS_BUS_DEVICE(dev);
+ sysbus_mmio_map(busdev, 0, spi_addr[i]);
+ sysbus_connect_irq(busdev, 0, pic[spi_irq[i]]);
+ }
}
static Property stm32f205_soc_properties[] = {
diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 7d6603b..00c2f3d 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -29,6 +29,7 @@
#include "hw/timer/stm32f2xx_timer.h"
#include "hw/char/stm32f2xx_usart.h"
#include "hw/misc/stm32f2xx_adc.h"
+#include "hw/ssi/stm32f2xx_spi.h"
#define TYPE_STM32F205_SOC "stm32f205-soc"
#define STM32F205_SOC(obj) \
@@ -37,6 +38,7 @@
#define STM_NUM_USARTS 6
#define STM_NUM_TIMERS 4
#define STM_NUM_ADCS 3
+#define STM_NUM_SPIS 3
#define FLASH_BASE_ADDRESS 0x08000000
#define FLASH_SIZE (1024 * 1024)
@@ -55,6 +57,7 @@ typedef struct STM32F205State {
STM32F2XXUsartState usart[STM_NUM_USARTS];
STM32F2XXTimerState timer[STM_NUM_TIMERS];
STM32F2XXAdcState adc[STM_NUM_ADCS];
+ STM32F2XXSpiState spi[STM_NUM_SPIS];
} STM32F205State;
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 6/7] STM32F205: Connect the SPI device
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 6/7] STM32F205: Connect the SPI device Alistair Francis
@ 2015-04-25 18:31 ` Peter Crosthwaite
0 siblings, 0 replies; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:31 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
"devices"
On Sat, Apr 25, 2015 at 1:19 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Connect the SPI device to the STM32F205 SoC.
>
"devices".
Curious. Are you attaching the SPI slaves on the command line using -device?
Otherwise:
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Regards,
Peter
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>
> hw/arm/stm32f205_soc.c | 22 ++++++++++++++++++++++
> include/hw/arm/stm32f205_soc.h | 3 +++
> 2 files changed, 25 insertions(+)
>
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 641ecbb..cb8884f 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -33,10 +33,13 @@ static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
> 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
> static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
> 0x40012200 };
> +static const uint32_t spi_addr[STM_NUM_SPIS] = { 0x40013000, 0x40003800,
> + 0x40003C00 };
>
> static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
> static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
> #define ADC_IRQ 18
> +static const int spi_irq[STM_NUM_SPIS] = {35, 36, 51};
>
> static void stm32f205_soc_initfn(Object *obj)
> {
> @@ -63,6 +66,12 @@ static void stm32f205_soc_initfn(Object *obj)
> TYPE_STM32F2XX_ADC);
> qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
> }
> +
> + for (i = 0; i < STM_NUM_SPIS; i++) {
> + object_initialize(&s->spi[i], sizeof(s->spi[i]),
> + TYPE_STM32F2XX_SPI);
> + qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
> + }
> }
>
> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> @@ -150,6 +159,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> sysbus_mmio_map(busdev, 0, adc_addr[i]);
> sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
> }
> +
> + /* SPI 1 and 2 */
> + for (i = 0; i < STM_NUM_SPIS; i++) {
> + dev = DEVICE(&(s->spi[i]));
> + object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
> + if (err != NULL) {
> + error_propagate(errp, err);
> + return;
> + }
> + busdev = SYS_BUS_DEVICE(dev);
> + sysbus_mmio_map(busdev, 0, spi_addr[i]);
> + sysbus_connect_irq(busdev, 0, pic[spi_irq[i]]);
> + }
> }
>
> static Property stm32f205_soc_properties[] = {
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> index 7d6603b..00c2f3d 100644
> --- a/include/hw/arm/stm32f205_soc.h
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -29,6 +29,7 @@
> #include "hw/timer/stm32f2xx_timer.h"
> #include "hw/char/stm32f2xx_usart.h"
> #include "hw/misc/stm32f2xx_adc.h"
> +#include "hw/ssi/stm32f2xx_spi.h"
>
> #define TYPE_STM32F205_SOC "stm32f205-soc"
> #define STM32F205_SOC(obj) \
> @@ -37,6 +38,7 @@
> #define STM_NUM_USARTS 6
> #define STM_NUM_TIMERS 4
> #define STM_NUM_ADCS 3
> +#define STM_NUM_SPIS 3
>
> #define FLASH_BASE_ADDRESS 0x08000000
> #define FLASH_SIZE (1024 * 1024)
> @@ -55,6 +57,7 @@ typedef struct STM32F205State {
> STM32F2XXUsartState usart[STM_NUM_USARTS];
> STM32F2XXTimerState timer[STM_NUM_TIMERS];
> STM32F2XXAdcState adc[STM_NUM_ADCS];
> + STM32F2XXSpiState spi[STM_NUM_SPIS];
> } STM32F205State;
>
> #endif
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list
2015-04-25 8:18 [Qemu-devel] [PATCH v1 0/7] Update the Netduino 2 Machine Alistair Francis
` (5 preceding siblings ...)
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 6/7] STM32F205: Connect the SPI device Alistair Francis
@ 2015-04-25 8:19 ` Alistair Francis
2015-04-25 18:31 ` Peter Crosthwaite
6 siblings, 1 reply; 23+ messages in thread
From: Alistair Francis @ 2015-04-25 8:19 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: alistair23
Add Alistair Francis as the maintainer for the Netduino 2
and SMM32F205 SoC.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
MAINTAINERS | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d7e9ba2..ad9827a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -355,6 +355,21 @@ F: hw/misc/zynq_slcr.c
F: hw/*/cadence_*
F: hw/ssi/xilinx_spips.c
+STM32F205
+M: Alistair Francis <alistair@alistair23.me>
+S: Maintained
+F: hw/arm/stm32f205_soc.c
+F: hw/misc/stm32f2xx_syscfg.c
+F: hw/char/stm32f2xx_usart.c
+F: hw/timer/stm32f2xx_timer.c
+F: hw/misc/stm32f2xx_adc.c
+F: hw/ssi/stm32f2xx_spi.c
+
+Netduino 2
+M: Alistair Francis <alistair@alistair23.me>
+S: Maintained
+F: hw/arm/netduino2.c
+
CRIS Machines
-------------
Axis Dev88
--
2.1.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list
2015-04-25 8:19 ` [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list Alistair Francis
@ 2015-04-25 18:31 ` Peter Crosthwaite
2015-04-26 1:56 ` Alistair Francis
0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2015-04-25 18:31 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sat, Apr 25, 2015 at 1:19 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Add Alistair Francis as the maintainer for the Netduino 2
> and SMM32F205 SoC.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
> MAINTAINERS | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d7e9ba2..ad9827a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -355,6 +355,21 @@ F: hw/misc/zynq_slcr.c
> F: hw/*/cadence_*
> F: hw/ssi/xilinx_spips.c
>
> +STM32F205
> +M: Alistair Francis <alistair@alistair23.me>
> +S: Maintained
> +F: hw/arm/stm32f205_soc.c
> +F: hw/misc/stm32f2xx_syscfg.c
> +F: hw/char/stm32f2xx_usart.c
> +F: hw/timer/stm32f2xx_timer.c
> +F: hw/misc/stm32f2xx_adc.c
> +F: hw/ssi/stm32f2xx_spi.c
> +
> +Netduino 2
> +M: Alistair Francis <alistair@alistair23.me>
> +S: Maintained
> +F: hw/arm/netduino2.c
> +
> CRIS Machines
> -------------
> Axis Dev88
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v1 7/7] MAINTAINERS: Add Alistair to the maintainers list
2015-04-25 18:31 ` Peter Crosthwaite
@ 2015-04-26 1:56 ` Alistair Francis
0 siblings, 0 replies; 23+ messages in thread
From: Alistair Francis @ 2015-04-26 1:56 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Apr 26, 2015 at 4:31 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Apr 25, 2015 at 1:19 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Add Alistair Francis as the maintainer for the Netduino 2
>> and SMM32F205 SoC.
>>
>> Signed-off-by: Alistair Francis <alistair@alistair23.me>
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Thanks Peter
Alistair
>
>> ---
>>
>> MAINTAINERS | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d7e9ba2..ad9827a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -355,6 +355,21 @@ F: hw/misc/zynq_slcr.c
>> F: hw/*/cadence_*
>> F: hw/ssi/xilinx_spips.c
>>
>> +STM32F205
>> +M: Alistair Francis <alistair@alistair23.me>
>> +S: Maintained
>> +F: hw/arm/stm32f205_soc.c
>> +F: hw/misc/stm32f2xx_syscfg.c
>> +F: hw/char/stm32f2xx_usart.c
>> +F: hw/timer/stm32f2xx_timer.c
>> +F: hw/misc/stm32f2xx_adc.c
>> +F: hw/ssi/stm32f2xx_spi.c
>> +
>> +Netduino 2
>> +M: Alistair Francis <alistair@alistair23.me>
>> +S: Maintained
>> +F: hw/arm/netduino2.c
>> +
>> CRIS Machines
>> -------------
>> Axis Dev88
>> --
>> 2.1.4
>>
>>
^ permalink raw reply [flat|nested] 23+ messages in thread