Re: [PATCH 3/3] hw/usb: Extract VT82C686 UHCI PCI function into a new unit

From: BALATON Zoltan <balaton@eik.bme.hu>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Huacai Chen <chenhuacai@kernel.org>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [PATCH 3/3] hw/usb: Extract VT82C686 UHCI PCI function into a new unit
Date: Wed, 10 Mar 2021 03:33:30 +0100 (CET)	[thread overview]
Message-ID: <a846dfb-ebf7-c84d-dbbd-119c357cdf64@eik.bme.hu> (raw)
In-Reply-To: <fd9c7195-1396-f622-4867-6dec8527dfd3@amsat.org>

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On Wed, 10 Mar 2021, Philippe Mathieu-Daudé wrote:
> On 3/9/21 9:13 PM, BALATON Zoltan wrote:
>> On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
>>> Extract the VT82C686 PCI UHCI function into a new unit so
>>> it is only build when the VT82C686 south bridge is selected.
>>
>> I'm not sure it's worth separating just this one device from the other
>> similar usb devices when the others that are also part of south bridge
>> chips are left there. Maybe you could just set user_creatable = false so
>> it can only be created as part of the chips that contain it or just
>> don't bother and leave it as it is which is the least likely to break
>> anything that may rely on it as removing it from the device list may
>> need to go through deprecation.
>>
>> But I don't really mind, so if others like this approach I don't want to
>> block the patch. I think it's unlikely anybody would use this device
>> other than part of fuloong2e or pegasos2 so probably it's unlikely to
>> break anything if it suddenly goes away from a new build.
>
> OK. As I sent this series to help you with your Pegasos2 board, do you
> mind sending a patch then?
>
> I suppose you mean using "#include CONFIG_DEVICES" and checking for the
> CONFIG_VT82C686 symbol to register the QOM type?

I don't see how this should help with the pegasos2 series as that's 
already working without this change. I really just meant I don't see this 
would improve much when you leave the piix and ich ones there that are the 
same for other south bridges so this patch makes one of these configurable 
but not the others, that makes them different when they are now similar. 
Additionally it may break something as it changes the availability of this 
device (although that's very unlikely to cause problems in practice). So 
I'd just leave this alone for now and not change it at all until all of 
these are cleaned up in a uniform way at some later point if that's deemed 
useful or desirable. Maybe I'm missing the point though.

But while pegasos2 does not need this change it should also not break it 
so I don't really mind.

Regards,
BALATON Zoltan