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
>
>
[PATCH 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
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.
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.
Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442
Cc: Alistair Francis
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
---
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
