Re: [Qemu-devel] [PATCH 0/2] add pci-serial device.

[Anthony Liguori <anthony@codemonkey.ws>]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 09/26/12 01:43, Anthony Liguori wrote:
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>>>   Hi,
>>>
>>> Two patches, first split up serial.c a bit,
>>> then actually add the pci-based serial device.
>> 
>> The series looks good to me.  A couple requests:
>> 
>> 1) Could you add a spec describing this new PCI device?  Doesn't need to
>>    be more than a couple paragraphs since the device is super simple.
>
> Well, it is pretty strait forward:  A single IO bar, 8 bytes in size,
> where the 16550 uart is mapped to:
>
> [kraxel@fedora ~]$ lspci -vse
> 00:0e.0 Serial controller: Red Hat, Inc. Device 0002 (rev 01) (prog-if
> 00 [8250])
> 	Subsystem: Red Hat, Inc Device 1100
> 	Physical Slot: 14
> 	Flags: fast devsel, IRQ 11
> 	I/O ports at c130 [size=8]
> 	Kernel driver in use: serial
>
> But I can surely add a comment about it.

Understood, but I'd really prefer a file in docs/.  We should be
rigorous about having formal specs for all of our paravirtual devices.
The code shouldn't be the spec.

>
>> 2) Could you make the inf file an separate patch and either include
>>    documentation in the commit message on how to use it with Windows or
>>    just add a comment to the inf file?
>
> I think a comment is better, easier to find than a commit message.
> Will do.

So do I.  Thanks.

>> This is a new PCI space for QEMU too.  
>
> It isn't new, I just followed what the pci bridge is doing (which has
> 1b36:0001).

Ah, Michael, could you add a quick spec to docs for the pci_bridge
device?

>
>> Is this a driver that is "owned"
>> by QEMU and Red Hat is donating the PCI id or is this a driver that RH
>> controls that we're implementing?
>
> I consider it being owned by qemu.

Great.

Regards,

Anthony Liguori

>
> cheers,
>   Gerd