| Message ID | 20250325224310.8785-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Categorize few devices and add their descriptions | expand |
On 25/03/2025 23.42, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/audio/wm8750.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c > index 8d381dbc658..6c1bb20fb75 100644 > --- a/hw/audio/wm8750.c > +++ b/hw/audio/wm8750.c > @@ -721,6 +721,8 @@ static void wm8750_class_init(ObjectClass *klass, void *data) > sc->send = wm8750_tx; > dc->vmsd = &vmstate_wm8750; > device_class_set_props(dc, wm8750_properties); > + set_bit(DEVICE_CATEGORY_SOUND, dc->categories); > + dc->desc = "WM8750 Stereo CODEC"; > } > > static const TypeInfo wm8750_info = { Reviewed-by: Thomas Huth <thuth@redhat.com>
On 26/03/2025 07.47, Thomas Huth wrote: > On 25/03/2025 23.42, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/audio/wm8750.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c >> index 8d381dbc658..6c1bb20fb75 100644 >> --- a/hw/audio/wm8750.c >> +++ b/hw/audio/wm8750.c >> @@ -721,6 +721,8 @@ static void wm8750_class_init(ObjectClass *klass, void >> *data) >> sc->send = wm8750_tx; >> dc->vmsd = &vmstate_wm8750; >> device_class_set_props(dc, wm8750_properties); >> + set_bit(DEVICE_CATEGORY_SOUND, dc->categories); >> + dc->desc = "WM8750 Stereo CODEC"; >> } >> static const TypeInfo wm8750_info = { > > Reviewed-by: Thomas Huth <thuth@redhat.com> Looking at this twice, I think the patch is not OK in its current shape: The wm8750 device now shows up twice in the output of "-device help", once in the "Sound" category and once in the "Misc" category (inherited from I2C device). That's somewhat ugly. I guess you'd need to remove the MISC bit here to clean that up? Thomas
On Wed, 26 Mar 2025, Thomas Huth wrote: > On 26/03/2025 07.47, Thomas Huth wrote: >> On 25/03/2025 23.42, Philippe Mathieu-Daudé wrote: >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> hw/audio/wm8750.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c >>> index 8d381dbc658..6c1bb20fb75 100644 >>> --- a/hw/audio/wm8750.c >>> +++ b/hw/audio/wm8750.c >>> @@ -721,6 +721,8 @@ static void wm8750_class_init(ObjectClass *klass, void >>> *data) >>> sc->send = wm8750_tx; >>> dc->vmsd = &vmstate_wm8750; >>> device_class_set_props(dc, wm8750_properties); >>> + set_bit(DEVICE_CATEGORY_SOUND, dc->categories); >>> + dc->desc = "WM8750 Stereo CODEC"; >>> } >>> static const TypeInfo wm8750_info = { >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> > > Looking at this twice, I think the patch is not OK in its current shape: The > wm8750 device now shows up twice in the output of "-device help", once in the > "Sound" category and once in the "Misc" category (inherited from I2C device). > That's somewhat ugly. I guess you'd need to remove the MISC bit here to clean > that up? Maybe we could add an i2c category for those devices? But in this case it fits in multiple categories. Regards, BALATON Zoltan
On 26/03/2025 13.39, BALATON Zoltan wrote: > On Wed, 26 Mar 2025, Thomas Huth wrote: >> On 26/03/2025 07.47, Thomas Huth wrote: >>> On 25/03/2025 23.42, Philippe Mathieu-Daudé wrote: >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> --- >>>> hw/audio/wm8750.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c >>>> index 8d381dbc658..6c1bb20fb75 100644 >>>> --- a/hw/audio/wm8750.c >>>> +++ b/hw/audio/wm8750.c >>>> @@ -721,6 +721,8 @@ static void wm8750_class_init(ObjectClass *klass, >>>> void *data) >>>> sc->send = wm8750_tx; >>>> dc->vmsd = &vmstate_wm8750; >>>> device_class_set_props(dc, wm8750_properties); >>>> + set_bit(DEVICE_CATEGORY_SOUND, dc->categories); >>>> + dc->desc = "WM8750 Stereo CODEC"; >>>> } >>>> static const TypeInfo wm8750_info = { >>> >>> Reviewed-by: Thomas Huth <thuth@redhat.com> >> >> Looking at this twice, I think the patch is not OK in its current shape: >> The wm8750 device now shows up twice in the output of "-device help", once >> in the "Sound" category and once in the "Misc" category (inherited from >> I2C device). That's somewhat ugly. I guess you'd need to remove the MISC >> bit here to clean that up? > > Maybe we could add an i2c category for those devices? But in this case it > fits in multiple categories. I think we should aim for the most specific category only. For example, we also have things like "usb-mouse" or "usb-tablet", but these only show up in the "input" category, and not in the "USB device" category. By the way, it's somewhat weird that we have a USB category, but not a PCI or I2C category ... maybe we should rather get rid of that USB category and classify the HCDs as "controller/bridges" like we do it for the PCI host controllers? Thomas
diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c index 8d381dbc658..6c1bb20fb75 100644 --- a/hw/audio/wm8750.c +++ b/hw/audio/wm8750.c @@ -721,6 +721,8 @@ static void wm8750_class_init(ObjectClass *klass, void *data) sc->send = wm8750_tx; dc->vmsd = &vmstate_wm8750; device_class_set_props(dc, wm8750_properties); + set_bit(DEVICE_CATEGORY_SOUND, dc->categories); + dc->desc = "WM8750 Stereo CODEC"; } static const TypeInfo wm8750_info = {
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/audio/wm8750.c | 2 ++ 1 file changed, 2 insertions(+)