[PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description

Philippe Mathieu-Daudé posted 12 patches 10 months, 3 weeks ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Alistair Francis <alistair@alistair23.me>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Samuel Tardieu <sam@rfc1149.net>, "Hervé Poussineau" <hpoussin@reactos.org>, Glenn Miles <milesg@linux.ibm.com>, Patrick Leis <venture@google.com>, Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, BALATON Zoltan <balaton@eik.bme.hu>, Bernhard Beschow <shentey@gmail.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>
[PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago
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 = {
-- 
2.47.1


Re: [PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description
Posted by Thomas Huth 10 months, 2 weeks ago
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>


Re: [PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description
Posted by Thomas Huth 10 months, 2 weeks ago
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


Re: [PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description
Posted by BALATON Zoltan 10 months, 2 weeks ago
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
Re: [PATCH-for-10.0 01/12] hw/audio/wm8750: Categorize and add description
Posted by Thomas Huth 10 months, 2 weeks ago
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