From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57277) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fn3Ym-0006j1-0u for qemu-devel@nongnu.org; Tue, 07 Aug 2018 11:06:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fn3Yl-0002dE-1A for qemu-devel@nongnu.org; Tue, 07 Aug 2018 11:06:08 -0400 References: <20180725143413.9728-1-geert+renesas@glider.be> <20180725143413.9728-2-geert+renesas@glider.be> <276c2cab-659f-ad1a-d3a3-0e0df9a439c9@redhat.com> From: Auger Eric Message-ID: <303a110d-7cb6-9003-e225-7f9d87d78811@redhat.com> Date: Tue, 7 Aug 2018 17:05:54 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/4] vfio/platform: Make the vfio-platform device non-abstract List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Geert Uytterhoeven Cc: Peter Maydell , Laurent Pinchart , Geert Uytterhoeven , Magnus Damm , QEMU Developers , Linux-Renesas , Wolfram Sang , Alex Williamson , Kieran Bingham , qemu-arm Hi Geert, On 08/07/2018 05:00 PM, Geert Uytterhoeven wrote: > Hi Eric, > > On Tue, Aug 7, 2018 at 4:18 PM Auger Eric wrote: >> On 07/25/2018 04:34 PM, Geert Uytterhoeven wrote: >>> From: Auger Eric >>> >>> 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 >>> [geert: Rebase, set user_creatable=true, use compatible values in sysfs >>> instead of user-supplied manufacturer/model options, reword] >>> Signed-off-by: Geert Uytterhoeven >>> --- >>> 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. Ah OK ;-) Thanks Eric > > Gr{oetje,eeting}s, > > Geert >