On Mon, Feb 08, 2010 at 06:43:51PM +0100, Gerd Hoffmann wrote:
> On 02/08/10 18:37, Michael S. Tsirkin wrote:
>> On Mon, Feb 08, 2010 at 06:37:41PM +0100, Gerd Hoffmann wrote:
>>> On 02/08/10 18:32, Michael S. Tsirkin wrote:
>>>> On Mon, Feb 08, 2010 at 06:24:57PM +0100, Gerd Hoffmann wrote:
>>>>> On 02/08/10 17:27, Michael S. Tsirkin wrote:
>>>>>> On Mon, Feb 08, 2010 at 12:14:11PM +0100, Gerd Hoffmann wrote:
>>>>>>> On 02/08/10 11:17, Michael S. Tsirkin wrote:
>>>>>>>> On Mon, Feb 08, 2010 at 03:41:47PM +0900, Isaku Yamahata wrote:
>>>>>>>>> initialize header type register in pci generic code.
>>>>>>>>>
>>>>>>>>> Cc: Blue Swirl<blauwir...@gmail.com>
>>>>>>>>> Cc: "Michael S. Tsirkin"<m...@redhat.com>
>>>>>>>>> Signed-off-by: Isaku Yamahata<yamah...@valinux.co.jp>
>>>>>>>>
>>>>>>>> No objections here, I am assuming this will be followed
>>>>>>>> by patches removing header type init from bridges?
>>>>>>>> From qdev perspective, it is probably cleaner to make
>>>>>>>> multifunction bit a separate qdev property though, right?
>>>>>>>
>>>>>>> From a qdev perspective it would make *alot* of sense to move a
>>>>>>> bunch of
>>>>>>> pci config stuff (including, but not limited to header type) into
>>>>>>> PCIDeviceInfo.
>>>>>>>
>>>>>>> cheers,
>>>>>>> Gerd
>>>>>>
>>>>>> Actually - won't this make it possible to create broken configurations
>>>>>> by tweaking properties from command-line?
>>>>>
>>>>> Not as property, as struct element in PCIDeviceInfo. i.e.
>>>>>
>>>>> static PCIDeviceInfo e1000_info = {
>>>>> [ stuff which is here right now ]
>>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>>> .device_id = E1000_DEVID,
>>>>> .class = PCI_CLASS_NETWORK_ETHERNET,
>>>>> [ probably more stuff which makes sense ]
>>>>> }
>>>>>
>>>>> Then setup this in generic pci code instead of having each driver doing
>>>>> a bunch of pci_config_set_*() calls.
>>>>>
>>>>> cheers,
>>>>> Gerd
>>>>
>>>> We still end up with class, vendor etc duplicated in 2 places.
>>>
>>> No. The info should be *only* in PCIDeviceInfo then.
>>
>> That would put a lot of code in pci config cycle path. A single array
>> mirroring the whole config space is much cleaner.
>>
>>>> Why do
>>>> we want stuff like vendor id in PCIDeviceInfo at all? Why can't
>>>> everyone just use pci_config_set/get calls?
>>>
>>> You can do nice stuff like printing vendor/device IDs in the '-device ?'
>>> list then.
>>
>> That should use pci functions as well.
>
> Hmm, do you mix up PCIDevice and PCIDeviceInfo structs?
>
> cheers,
> Gerd
OK. OTOH, I don't think we need to out print header type in -device ?
so what good is it there?
