Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device

[Alistair Francis]

On Mon, Apr 27, 2015 at 5:26 AM, Peter Crosthwaite
<address@hidden> wrote:
> On Sat, Apr 25, 2015 at 7:21 PM, Alistair Francis <address@hidden> wrote:
>> On Sun, Apr 26, 2015 at 4:32 AM, Peter Crosthwaite
>> <address@hidden> wrote:
>>> "devices"
>>>
>>> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis <address@hidden> wrote:
>>>> Connect the ADC device to the STM32F205 SoC.
>>>>
>>>
>>> "devices"
>>
>> Will fix both.
>>
>>>
>>>> Signed-off-by: Alistair Francis <address@hidden>
>>>> ---
>>>>
>>>>  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
>>>>
>>>>
>>