Hi Eric, On Tue, Aug 7, 2018 at 4:18 PM Auger Eric <eric.auger@xxxxxxxxxx> wrote: > On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote: > > From: Auger Eric <eric.auger@xxxxxxxxxx> > > > > Up to now the vfio-platform device has been abstract and could not be > > instantiated. The integration of a new vfio platform device required > > creating a dummy derived device which only set the compatible string. > > > > Following the few vfio-platform device integrations we have seen the > > actual requested adaptation happens on device tree node creation > > (sysbus-fdt). > > > > Hence remove the abstract setting, and read the list of compatible > > values from sysfs if not set by a derived device. > > > > Update the amd-xgbe and calxeda-xgmac drivers to fill in the number of > > compatible values, as there can now be more than one. > > > > Note that sysbus-fdt does not support the instantiation of the > > vfio-platform device yet. > > > > Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> > > [geert: Rebase, set user_creatable=true, use compatible values in sysfs > > instead of user-supplied manufacturer/model options, reword] > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > --- > > v3: > > - Read all compatible values from sysfs instead of using user-supplied > > manufacturer and model options, > > - Reword patch description, > > - Drop RFC state, > > > > v2: > > - No changes, > > > > v1: > > - Rebase, Set user_creatable=true, > > > > v0: > > - Original version from Eric. > > --- a/hw/vfio/platform.c > > +++ b/hw/vfio/platform.c > > @@ -655,6 +655,25 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp) > > goto out; > > } > > > > + if (!vdev->compat) { > > + gchar *contents; > > + gsize length; > > + char *tmp; > > + > > + tmp = g_strdup_printf("%s/of_node/compatible", vbasedev->sysfsdev); > > + if (!g_file_get_contents(tmp, &contents, &length, NULL)) { > > + error_report("failed to load \"%s\"", tmp); > > + exit(1); > You should set errp instead so that the error gets properly propagated. Thanks, will do. > > --- a/include/hw/vfio/vfio-platform.h > > +++ b/include/hw/vfio/vfio-platform.h > > @@ -54,7 +54,8 @@ typedef struct VFIOPlatformDevice { > > QLIST_HEAD(, VFIOINTp) intp_list; /* list of IRQs */ > > /* queue of pending IRQs */ > > QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue; > > - char *compat; /* compatibility string */ > > + char *compat; /* DT compatible values, separated by NUL */ > by NULL characters? "NUL" is the character ('\0'), "NULL" is the pointer. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds