| Message ID | 20230411103106.3992862-1-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [for-8.0] hw/i2c/allwinner-i2c: Fix subclassing of TYPE_AW_I2C_SUN6I | expand |
On 11/04/2023 12.31, Peter Maydell wrote: > In commit 8461bfdca9c we added the TYPE_AW_I2C_SUN6I, which is a > minor variant of the TYPE_AW_I2C device. However, we didn't quite > get the class hierarchy right. We made the new TYPE_AW_I2C_SUN6I a > subclass of TYPE_SYS_BUS_DEVICE, which means that you can't validly > use a pointer to this object via the AW_I2C() cast macro, which > insists on having something that is an instance of TYPE_AW_I2C or > some subclass of that type. > > This only causes a problem if QOM cast macro debugging is enabled; > that is supposed to be on by default, but a mistake in the meson > conversion in commit c55cf6ab03f4c meant that it ended up disabled by > default, and we didn't catch this bug. > > Fix the problem by arranging the classes in the same way we do for > TYPE_PL011 and TYPE_PL011_LUMINARY in hw/char/pl011.c -- make the > variant class be a subclass of the "normal" version of the device. > > This was reported in > https://gitlab.com/qemu-project/qemu/-/issues/1586 but this fix alone > isn't sufficient, as there is a separate cast-related issue in the > CXL code in pci_expander_bridge.c. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/i2c/allwinner-i2c.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c > index f24c3ac6f0c..9e8efa1d63f 100644 > --- a/hw/i2c/allwinner-i2c.c > +++ b/hw/i2c/allwinner-i2c.c > @@ -466,10 +466,8 @@ static void allwinner_i2c_sun6i_init(Object *obj) > > static const TypeInfo allwinner_i2c_sun6i_type_info = { > .name = TYPE_AW_I2C_SUN6I, > - .parent = TYPE_SYS_BUS_DEVICE, > - .instance_size = sizeof(AWI2CState), > + .parent = TYPE_AW_I2C, > .instance_init = allwinner_i2c_sun6i_init, > - .class_init = allwinner_i2c_class_init, > }; > > static void allwinner_i2c_register_types(void) Reviewed-by: Thomas Huth <thuth@redhat.com>
Peter Maydell <peter.maydell@linaro.org> writes: > In commit 8461bfdca9c we added the TYPE_AW_I2C_SUN6I, which is a > minor variant of the TYPE_AW_I2C device. However, we didn't quite > get the class hierarchy right. We made the new TYPE_AW_I2C_SUN6I a > subclass of TYPE_SYS_BUS_DEVICE, which means that you can't validly > use a pointer to this object via the AW_I2C() cast macro, which > insists on having something that is an instance of TYPE_AW_I2C or > some subclass of that type. > > This only causes a problem if QOM cast macro debugging is enabled; > that is supposed to be on by default, but a mistake in the meson > conversion in commit c55cf6ab03f4c meant that it ended up disabled by > default, and we didn't catch this bug. > > Fix the problem by arranging the classes in the same way we do for > TYPE_PL011 and TYPE_PL011_LUMINARY in hw/char/pl011.c -- make the > variant class be a subclass of the "normal" version of the device. > > This was reported in > https://gitlab.com/qemu-project/qemu/-/issues/1586 but this fix alone > isn't sufficient, as there is a separate cast-related issue in the > CXL code in pci_expander_bridge.c. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On Tue, Apr 11, 2023 at 11:31:06AM +0100, Peter Maydell wrote: > In commit 8461bfdca9c we added the TYPE_AW_I2C_SUN6I, which is a > minor variant of the TYPE_AW_I2C device. However, we didn't quite > get the class hierarchy right. We made the new TYPE_AW_I2C_SUN6I a > subclass of TYPE_SYS_BUS_DEVICE, which means that you can't validly > use a pointer to this object via the AW_I2C() cast macro, which > insists on having something that is an instance of TYPE_AW_I2C or > some subclass of that type. > > This only causes a problem if QOM cast macro debugging is enabled; > that is supposed to be on by default, but a mistake in the meson > conversion in commit c55cf6ab03f4c meant that it ended up disabled by > default, and we didn't catch this bug. > > Fix the problem by arranging the classes in the same way we do for > TYPE_PL011 and TYPE_PL011_LUMINARY in hw/char/pl011.c -- make the > variant class be a subclass of the "normal" version of the device. > > This was reported in > https://gitlab.com/qemu-project/qemu/-/issues/1586 but this fix alone > isn't sufficient, as there is a separate cast-related issue in the > CXL code in pci_expander_bridge.c. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Looks correct ot me. Reviewed-by: Corey Minyard <cminyard@mvista.com> > --- > hw/i2c/allwinner-i2c.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c > index f24c3ac6f0c..9e8efa1d63f 100644 > --- a/hw/i2c/allwinner-i2c.c > +++ b/hw/i2c/allwinner-i2c.c > @@ -466,10 +466,8 @@ static void allwinner_i2c_sun6i_init(Object *obj) > > static const TypeInfo allwinner_i2c_sun6i_type_info = { > .name = TYPE_AW_I2C_SUN6I, > - .parent = TYPE_SYS_BUS_DEVICE, > - .instance_size = sizeof(AWI2CState), > + .parent = TYPE_AW_I2C, > .instance_init = allwinner_i2c_sun6i_init, > - .class_init = allwinner_i2c_class_init, > }; > > static void allwinner_i2c_register_types(void) > -- > 2.34.1 > >
On 11/4/23 12:31, Peter Maydell wrote: > In commit 8461bfdca9c we added the TYPE_AW_I2C_SUN6I, which is a > minor variant of the TYPE_AW_I2C device. However, we didn't quite > get the class hierarchy right. We made the new TYPE_AW_I2C_SUN6I a > subclass of TYPE_SYS_BUS_DEVICE, which means that you can't validly > use a pointer to this object via the AW_I2C() cast macro, which > insists on having something that is an instance of TYPE_AW_I2C or > some subclass of that type. > > This only causes a problem if QOM cast macro debugging is enabled; > that is supposed to be on by default, but a mistake in the meson > conversion in commit c55cf6ab03f4c meant that it ended up disabled by > default, and we didn't catch this bug. > > Fix the problem by arranging the classes in the same way we do for > TYPE_PL011 and TYPE_PL011_LUMINARY in hw/char/pl011.c -- make the > variant class be a subclass of the "normal" version of the device. > > This was reported in > https://gitlab.com/qemu-project/qemu/-/issues/1586 but this fix alone > isn't sufficient, as there is a separate cast-related issue in the > CXL code in pci_expander_bridge.c. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/i2c/allwinner-i2c.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c index f24c3ac6f0c..9e8efa1d63f 100644 --- a/hw/i2c/allwinner-i2c.c +++ b/hw/i2c/allwinner-i2c.c @@ -466,10 +466,8 @@ static void allwinner_i2c_sun6i_init(Object *obj) static const TypeInfo allwinner_i2c_sun6i_type_info = { .name = TYPE_AW_I2C_SUN6I, - .parent = TYPE_SYS_BUS_DEVICE, - .instance_size = sizeof(AWI2CState), + .parent = TYPE_AW_I2C, .instance_init = allwinner_i2c_sun6i_init, - .class_init = allwinner_i2c_class_init, }; static void allwinner_i2c_register_types(void)
In commit 8461bfdca9c we added the TYPE_AW_I2C_SUN6I, which is a minor variant of the TYPE_AW_I2C device. However, we didn't quite get the class hierarchy right. We made the new TYPE_AW_I2C_SUN6I a subclass of TYPE_SYS_BUS_DEVICE, which means that you can't validly use a pointer to this object via the AW_I2C() cast macro, which insists on having something that is an instance of TYPE_AW_I2C or some subclass of that type. This only causes a problem if QOM cast macro debugging is enabled; that is supposed to be on by default, but a mistake in the meson conversion in commit c55cf6ab03f4c meant that it ended up disabled by default, and we didn't catch this bug. Fix the problem by arranging the classes in the same way we do for TYPE_PL011 and TYPE_PL011_LUMINARY in hw/char/pl011.c -- make the variant class be a subclass of the "normal" version of the device. This was reported in https://gitlab.com/qemu-project/qemu/-/issues/1586 but this fix alone isn't sufficient, as there is a separate cast-related issue in the CXL code in pci_expander_bridge.c. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/i2c/allwinner-i2c.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)