[PATCH SLOF] pci-properties: Remove redundant call to device-type

[PATCH SLOF] pci-properties: Remove redundant call to device-type

[Alexey Kardashevskiy]

At the moment SLOF adds a "device_type" property automatically for
every single PCI device based on its class even if there is no SLOF
driver for such a device. OF1275 says that "device_type" is for
implemented interfaces only. A side effect of this is virtio-balloon
getting device_type=="memory" while it should not have.

This removes automatic call to device-type from the common PCI code.
Since now, we rely on existing SLOF PCI drivers to call device-type if
needed. virtio-blk/net, e1000, ohci/ehci/xhci do this. virtio-scsi
does not create the property for itself but disks on its bus do.
virtio-ballon won't get the device_type property as there is no driver
for it.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

I'll wait for a couple of days and push it to github if nobody objects.
Ben suggested that the linux guest must not rely on device_type in the common
case.

---
 slof/fs/pci-properties.fs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
index a19c651..9efa87e 100644
--- a/slof/fs/pci-properties.fs
+++ b/slof/fs/pci-properties.fs
@@ -565,7 +565,7 @@
 \ ***************************************************************************************
 \ set up common properties for devices and bridges
 : pci-common-props ( addr -- )
-        dup pci-class-name 2dup device-name device-type
+        dup pci-class-name device-name
         dup pci-vendor@    encode-int s" vendor-id"      property
         dup pci-device@    encode-int s" device-id"      property
         dup pci-revision@  encode-int s" revision-id"    property
-- 
2.0.0

Re: [PATCH SLOF] pci-properties: Remove redundant call to device-type

[Thomas Huth]

On Wed, 11 Mar 2015 17:26:32 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> At the moment SLOF adds a "device_type" property automatically for
> every single PCI device based on its class even if there is no SLOF
> driver for such a device. OF1275 says that "device_type" is for
> implemented interfaces only. A side effect of this is virtio-balloon
> getting device_type=="memory" while it should not have.
> 
> This removes automatic call to device-type from the common PCI code.
> Since now, we rely on existing SLOF PCI drivers to call device-type if
> needed. virtio-blk/net, e1000, ohci/ehci/xhci do this. virtio-scsi
> does not create the property for itself but disks on its bus do.
> virtio-ballon won't get the device_type property as there is no driver
> for it.

Sounds very reasonable, but I think there are a couple of other things
you could/should check:

- Graphic cards should get the "display" device_type ... I guess that's
  missing now?

- pci bus nodes (like bridges) should get the "pci" device_type
  according to the Open Firmware PCI binding, is this still the case?

- Seems like there are some other devices with device_type property
  where SLOF does not provide an implemented interface, like in
  slof/fs/usb/dev-mouse.fs ... should these be revised, too?

 Thomas

Re: [PATCH SLOF] pci-properties: Remove redundant call to device-type

[Benjamin Herrenschmidt]

On Wed, 2015-03-11 at 08:50 +0100, Thomas Huth wrote:
> 
> - Graphic cards should get the "display" device_type ... I guess
> that's
>   missing now?

At least the code for qemu-vga does it

> - pci bus nodes (like bridges) should get the "pci" device_type
>   according to the Open Firmware PCI binding, is this still the case?

Ah yes we need to check that, specifically P2P bridges

> - Seems like there are some other devices with device_type property
>   where SLOF does not provide an implemented interface, like in
>   slof/fs/usb/dev-mouse.fs ... should these be revised, too?

not a *huge* issue but we can look into it when needed.

Cheers,
Ben.

Re: [PATCH SLOF] pci-properties: Remove redundant call to device-type

[Thomas Huth]

On Wed, 11 Mar 2015 19:21:02 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Wed, 2015-03-11 at 08:50 +0100, Thomas Huth wrote:
> > 
> > - Graphic cards should get the "display" device_type ... I guess
> > that's
> >   missing now?
> 
> At least the code for qemu-vga does it

Ah, right, I was only grep'ing for "device-type" (with "-" instead of
"_", the Forth word to set the property), that's why I didn't notice
it ... the code in board-qemu sets the property directly instead.
And so does the code in board-js2x, so this should be fine.

> > - pci bus nodes (like bridges) should get the "pci" device_type
> >   according to the Open Firmware PCI binding, is this still the case?
> 
> Ah yes we need to check that, specifically P2P bridges

There seems to be some code in pci-bridge-generic-setup at last ...
Might be still worth checking this on a real running system, but at a
first glance it looks ok.

> > - Seems like there are some other devices with device_type property
> >   where SLOF does not provide an implemented interface, like in
> >   slof/fs/usb/dev-mouse.fs ... should these be revised, too?
> 
> not a *huge* issue but we can look into it when needed.

Right.

So the patch sounds now fine to me as it is.

 Thomas

Re: [PATCH SLOF] pci-properties: Remove redundant call to device-type

[Alexey Kardashevskiy]

On 03/11/2015 07:21 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2015-03-11 at 08:50 +0100, Thomas Huth wrote:
>>
>> - Graphic cards should get the "display" device_type ... I guess
>> that's
>>    missing now?
>
> At least the code for qemu-vga does it
>
>> - pci bus nodes (like bridges) should get the "pci" device_type
>>    according to the Open Firmware PCI binding, is this still the case?
>
> Ah yes we need to check that, specifically P2P bridges
>
>> - Seems like there are some other devices with device_type property
>>    where SLOF does not provide an implemented interface, like in
>>    slof/fs/usb/dev-mouse.fs ... should these be revised, too?
>
> not a *huge* issue but we can look into it when needed.


Then I better remove it now as well, any reason to wait for "when needed"?



-- 
Alexey

Re: [PATCH SLOF] pci-properties: Remove redundant call to device-type

[Nikunj A Dadhania]

Thomas Huth <thuth@linux.vnet.ibm.com> writes:

> On Wed, 11 Mar 2015 17:26:32 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>> At the moment SLOF adds a "device_type" property automatically for
>> every single PCI device based on its class even if there is no SLOF
>> driver for such a device. OF1275 says that "device_type" is for
>> implemented interfaces only. A side effect of this is virtio-balloon
>> getting device_type=="memory" while it should not have.
>> 
>> This removes automatic call to device-type from the common PCI code.
>> Since now, we rely on existing SLOF PCI drivers to call device-type if
>> needed. virtio-blk/net, e1000, ohci/ehci/xhci do this. virtio-scsi
>> does not create the property for itself but disks on its bus do.
>> virtio-ballon won't get the device_type property as there is no driver
>> for it.
>

Looks good to me.

> Sounds very reasonable, but I think there are a couple of other things
> you could/should check:
>
> - Graphic cards should get the "display" device_type ... I guess that's
>   missing now?

No, its being set as "display"

> - pci bus nodes (like bridges) should get the "pci" device_type
>   according to the Open Firmware PCI binding, is this still the case?

Yes

> - Seems like there are some other devices with device_type property
>   where SLOF does not provide an implemented interface, like in
>   slof/fs/usb/dev-mouse.fs ... should these be revised, too?

./slof/fs/usb/dev-mouse.fs:s" mouse" device-type

Regards,
Nikunj