Re: [PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
On 5/27/20 11:27 AM, Markus Armbruster wrote:
> Alistair Francis writes:
>
>> On Mon, May 18, 2020 at 10:08 PM Markus Armbruster wrote:
>>>
>>> Alistair Francis writes:
>>>
On Sun, May 17, 2020 at 10:06 PM Markus Armbruster
wrote:
>
> stm32f405_soc_initfn() creates six such devices, but
> stm32f405_soc_realize() realizes only one. Affects machine
> netduinoplus2.
>
> I wonder how this ever worked. If the "device becomes real only on
> realize" thing actually works, then we've always been missing five of
> six such devices, yet nobody noticed.
I must have just been testing the first ADC.
>
> Fix stm32f405_soc_realize() to realize all six. Visible in "info
> qtree":
>
> bus: main-system-bus
>type System
>dev: stm32f405-soc, id ""
> cpu-type = "cortex-m4-arm-cpu"
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: armv7m, id ""
>
> The mmio addresses look suspicious.
Good catch, thanks :)
>>>
>>> I'd love to squash in corrections, but I don't know the correct
>>> addresses. Can you help?
>>
>> Yep, thanks for squashing it in.
>>
>> The three addresses are:
>>
>> 0x40012000
>> 0x40012100
>> 0x40012200
>>
>> and they all share interrupt number 18.
>
> An the other three? There are six devices in total...
Alistair looked at the stm32f205, which has 3 ADCs (and seems correct).
>
>> Let me know if you want me to do it.
Alistair, the problem is the stm32f405. I guess the bug come from
copy/pasting then modifying for the shared IRQ.
/* Timer 2 to 5 */
for (i = 0; i < STM_NUM_TIMERS; i++) {
...
}
...
/* ADC device, the IRQs are ORed together */
...
dev = DEVICE(&(s->adc[i])); <=== i = STM_NUM_TIMERS = 4
object_property_set_bool(OBJECT(>adc[i]),
true, "realized", );
Only ADC#3 is realized, then mapped at 0x40012000.
You need to unparent the others anyway, so it is easier to realize
adc[0] and unparent the rest:
for (i = 1; i < STM_NUM_ADCS; i++) {
...
}
Re: [PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
Alistair Francis writes: > On Mon, May 18, 2020 at 10:08 PM Markus Armbruster wrote: >> >> Alistair Francis writes: >> >> > On Sun, May 17, 2020 at 10:06 PM Markus Armbruster >> > wrote: >> >> >> >> stm32f405_soc_initfn() creates six such devices, but >> >> stm32f405_soc_realize() realizes only one. Affects machine >> >> netduinoplus2. >> >> >> >> I wonder how this ever worked. If the "device becomes real only on >> >> realize" thing actually works, then we've always been missing five of >> >> six such devices, yet nobody noticed. >> > >> > I must have just been testing the first ADC. >> > >> >> >> >> Fix stm32f405_soc_realize() to realize all six. Visible in "info >> >> qtree": >> >> >> >> bus: main-system-bus >> >>type System >> >>dev: stm32f405-soc, id "" >> >> cpu-type = "cortex-m4-arm-cpu" >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> -mmio /00ff >> >> +mmio 40012000/00ff >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> -mmio /00ff >> >> +mmio 40012000/00ff >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> -mmio /00ff >> >> +mmio 40012000/00ff >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> -mmio /00ff >> >> +mmio 40012000/00ff >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> mmio 40012000/00ff >> >>dev: stm32f2xx-adc, id "" >> >> gpio-out "sysbus-irq" 1 >> >> -mmio /00ff >> >> +mmio 40012000/00ff >> >>dev: armv7m, id "" >> >> >> >> The mmio addresses look suspicious. >> > >> > Good catch, thanks :) >> >> I'd love to squash in corrections, but I don't know the correct >> addresses. Can you help? > > Yep, thanks for squashing it in. > > The three addresses are: > > 0x40012000 > 0x40012100 > 0x40012200 > > and they all share interrupt number 18. An the other three? There are six devices in total... > Let me know if you want me to do it.
Re: [PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
On Mon, May 18, 2020 at 10:08 PM Markus Armbruster wrote: > > Alistair Francis writes: > > > On Sun, May 17, 2020 at 10:06 PM Markus Armbruster > > wrote: > >> > >> stm32f405_soc_initfn() creates six such devices, but > >> stm32f405_soc_realize() realizes only one. Affects machine > >> netduinoplus2. > >> > >> I wonder how this ever worked. If the "device becomes real only on > >> realize" thing actually works, then we've always been missing five of > >> six such devices, yet nobody noticed. > > > > I must have just been testing the first ADC. > > > >> > >> Fix stm32f405_soc_realize() to realize all six. Visible in "info > >> qtree": > >> > >> bus: main-system-bus > >>type System > >>dev: stm32f405-soc, id "" > >> cpu-type = "cortex-m4-arm-cpu" > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> -mmio /00ff > >> +mmio 40012000/00ff > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> -mmio /00ff > >> +mmio 40012000/00ff > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> -mmio /00ff > >> +mmio 40012000/00ff > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> -mmio /00ff > >> +mmio 40012000/00ff > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> mmio 40012000/00ff > >>dev: stm32f2xx-adc, id "" > >> gpio-out "sysbus-irq" 1 > >> -mmio /00ff > >> +mmio 40012000/00ff > >>dev: armv7m, id "" > >> > >> The mmio addresses look suspicious. > > > > Good catch, thanks :) > > I'd love to squash in corrections, but I don't know the correct > addresses. Can you help? Yep, thanks for squashing it in. The three addresses are: 0x40012000 0x40012100 0x40012200 and they all share interrupt number 18. Let me know if you want me to do it. Alistair > > >> > >> Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442 > >> Cc: Alistair Francis > >> Cc: Peter Maydell > >> Cc: qemu-...@nongnu.org > >> Signed-off-by: Markus Armbruster > > > > Reviewed-by: Alistair Francis > > Thanks! >
Re: [PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
Alistair Francis writes: > On Sun, May 17, 2020 at 10:06 PM Markus Armbruster wrote: >> >> stm32f405_soc_initfn() creates six such devices, but >> stm32f405_soc_realize() realizes only one. Affects machine >> netduinoplus2. >> >> I wonder how this ever worked. If the "device becomes real only on >> realize" thing actually works, then we've always been missing five of >> six such devices, yet nobody noticed. > > I must have just been testing the first ADC. > >> >> Fix stm32f405_soc_realize() to realize all six. Visible in "info >> qtree": >> >> bus: main-system-bus >>type System >>dev: stm32f405-soc, id "" >> cpu-type = "cortex-m4-arm-cpu" >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> -mmio /00ff >> +mmio 40012000/00ff >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> -mmio /00ff >> +mmio 40012000/00ff >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> -mmio /00ff >> +mmio 40012000/00ff >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> -mmio /00ff >> +mmio 40012000/00ff >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> mmio 40012000/00ff >>dev: stm32f2xx-adc, id "" >> gpio-out "sysbus-irq" 1 >> -mmio /00ff >> +mmio 40012000/00ff >>dev: armv7m, id "" >> >> The mmio addresses look suspicious. > > Good catch, thanks :) I'd love to squash in corrections, but I don't know the correct addresses. Can you help? >> >> Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442 >> Cc: Alistair Francis >> Cc: Peter Maydell >> Cc: qemu-...@nongnu.org >> Signed-off-by: Markus Armbruster > > Reviewed-by: Alistair Francis Thanks!
Re: [PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
On Sun, May 17, 2020 at 10:06 PM Markus Armbruster wrote:
>
> stm32f405_soc_initfn() creates six such devices, but
> stm32f405_soc_realize() realizes only one. Affects machine
> netduinoplus2.
>
> I wonder how this ever worked. If the "device becomes real only on
> realize" thing actually works, then we've always been missing five of
> six such devices, yet nobody noticed.
I must have just been testing the first ADC.
>
> Fix stm32f405_soc_realize() to realize all six. Visible in "info
> qtree":
>
> bus: main-system-bus
>type System
>dev: stm32f405-soc, id ""
> cpu-type = "cortex-m4-arm-cpu"
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: armv7m, id ""
>
> The mmio addresses look suspicious.
Good catch, thanks :)
>
> Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442
> Cc: Alistair Francis
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/arm/stm32f405_soc.c | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index 4f10ce6176..4649502711 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -185,16 +185,18 @@ static void stm32f405_soc_realize(DeviceState *dev_soc,
> Error **errp)
> qdev_connect_gpio_out(DEVICE(>adc_irqs), 0,
>qdev_get_gpio_in(armv7m, ADC_IRQ));
>
> -dev = DEVICE(&(s->adc[i]));
> -object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> -if (err != NULL) {
> -error_propagate(errp, err);
> -return;
> +for (i = 0; i < STM_NUM_ADCS; i++) {
> +dev = DEVICE(&(s->adc[i]));
> +object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, ADC_ADDR);
> +sysbus_connect_irq(busdev, 0,
> + qdev_get_gpio_in(DEVICE(>adc_irqs), i));
> }
> -busdev = SYS_BUS_DEVICE(dev);
> -sysbus_mmio_map(busdev, 0, ADC_ADDR);
> -sysbus_connect_irq(busdev, 0,
> - qdev_get_gpio_in(DEVICE(>adc_irqs), i));
>
> /* SPI devices */
> for (i = 0; i < STM_NUM_SPIS; i++) {
> --
> 2.21.1
>
>
