From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:52794 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728059AbeISPLe (ORCPT ); Wed, 19 Sep 2018 11:11:34 -0400 Subject: Re: [PATCH v4 2/3] hw/arm/sysbus-fdt: Allow device matching with DT compatible value To: Geert Uytterhoeven , Peter Maydell , Alex Williamson Cc: Magnus Damm , Laurent Pinchart , Wolfram Sang , Kieran Bingham , qemu-arm@nongnu.org, qemu-devel@nongnu.org, linux-renesas-soc@vger.kernel.org References: <20180913154458.23939-1-geert+renesas@glider.be> <20180913154458.23939-3-geert+renesas@glider.be> From: Auger Eric Message-ID: <94e4b5cf-cf33-e239-da88-e513db19fbab@redhat.com> Date: Wed, 19 Sep 2018 11:34:24 +0200 MIME-Version: 1.0 In-Reply-To: <20180913154458.23939-3-geert+renesas@glider.be> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Geert, On 9/13/18 5:44 PM, Geert Uytterhoeven wrote: > From: Auger Eric > > Up to now we have relied on the device type to identify a device tree > node creation function. Since we would like the vfio-platform device to > be instantiatable with different compatible strings we introduce the s/instantiatable/instantiable > capability to specialize the node creation depending on actual > compatible value. > > NodeCreationPair is renamed into BindingEntry. The struct is enhanced > with compat and match_fn() fields. We introduce a new matching function > adapted to the vfio-platform generic device. > > Soon, the AMD XGBE can be instantiated with either manner, i.e.: > > -device vfio-amd-xgbe,host=e0900000.xgmac > > or using the new option line: > > -device vfio-platform,host=e0900000.xgmac > > Signed-off-by: Eric Auger > [geert: Match using compatible values in sysfs instead of user-supplied > manufacturer/model options, reword] > Signed-off-by: Geert Uytterhoeven > Tested-by: Eric Auger Reviewed-by: Eric Auger Feel free to add my T-b on the whole series (tested with amd_xgbe) Tested-by: Eric Auger Thanks Eric > --- > v4: > - Add Tested-by (with amd-xgbe), > - s/From now on/Soon/ in patch description, > > v3: > - Match using the compatible values from sysfs instead of using > user-supplied manufacturer and model options, > - Reword patch description, > - Drop RFC state, > > v2: > - No changes, > > v1: > - No changes, > > v0: > - Original version from Eric. > --- > hw/arm/sysbus-fdt.c | 61 ++++++++++++++++++++++++++++++++++----------- > 1 file changed, 47 insertions(+), 14 deletions(-) > > diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c > index 43d6a7bb48ddc351..0e24c803a1c2c734 100644 > --- a/hw/arm/sysbus-fdt.c > +++ b/hw/arm/sysbus-fdt.c > @@ -50,11 +50,13 @@ typedef struct PlatformBusFDTData { > PlatformBusDevice *pbus; > } PlatformBusFDTData; > > -/* struct that associates a device type name and a node creation function */ > -typedef struct NodeCreationPair { > +/* struct that allows to match a device and create its FDT node */ > +typedef struct BindingEntry { > const char *typename; > - int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque); > -} NodeCreationPair; > + const char *compat; > + int (*add_fn)(SysBusDevice *sbdev, void *opaque); > + bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo); > +} BindingEntry; > > /* helpers */ > > @@ -413,6 +415,27 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque) > return 0; > } > > +/* DT compatible matching */ > +static bool vfio_platform_match(SysBusDevice *sbdev, > + const BindingEntry *entry) > +{ > + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); > + const char *compat; > + unsigned int n; > + > + for (n = vdev->num_compat, compat = vdev->compat; n > 0; > + n--, compat += strlen(compat) + 1) { > + if (!strcmp(entry->compat, compat)) { > + return true; > + } > + } > + > + return false; > +} > + > +#define VFIO_PLATFORM_BINDING(compat, add_fn) \ > + {TYPE_VFIO_PLATFORM, (compat), (add_fn), vfio_platform_match} > + > #endif /* CONFIG_LINUX */ > > static int no_fdt_node(SysBusDevice *sbdev, void *opaque) > @@ -420,14 +443,23 @@ static int no_fdt_node(SysBusDevice *sbdev, void *opaque) > return 0; > } > > -/* list of supported dynamic sysbus devices */ > -static const NodeCreationPair add_fdt_node_functions[] = { > +/* Device type based matching */ > +static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry) > +{ > + return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename); > +} > + > +#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match} > + > +/* list of supported dynamic sysbus bindings */ > +static const BindingEntry bindings[] = { > #ifdef CONFIG_LINUX > - {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node}, > - {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node}, > + TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node), > + TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node), > + VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node), > #endif > - {TYPE_RAMFB_DEVICE, no_fdt_node}, > - {"", NULL}, /* last element */ > + TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node), > + TYPE_BINDING("", NULL), /* last element */ > }; > > /* Generic Code */ > @@ -446,10 +478,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void *opaque) > { > int i, ret; > > - for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) { > - if (!strcmp(object_get_typename(OBJECT(sbdev)), > - add_fdt_node_functions[i].typename)) { > - ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque); > + for (i = 0; i < ARRAY_SIZE(bindings); i++) { > + const BindingEntry *iter = &bindings[i]; > + > + if (iter->match_fn(sbdev, iter)) { > + ret = iter->add_fn(sbdev, opaque); > assert(!ret); > return; > } >