diff mbox series

[1/8] spapr: Clean up device node name generation for PCI devices

Message ID 20190523052918.1129-1-david@gibson.dropbear.id.au
State New, archived
Headers show
Series [1/8] spapr: Clean up device node name generation for PCI devices | expand

Commit Message

David Gibson May 23, 2019, 5:29 a.m. UTC
spapr_populate_pci_child_dt() adds a 'name' property to the device tree
node for PCI devices.  This is never necessary for a flattened device tree,
it is implicit in the name added when the node is constructed.  In fact
anything we do add to a 'name' property will be overwritten with something
derived from the structural name in the guest firmware (but in fact it is
exactly the same bytes).

So, remove that.  In addition, pci_get_node_name() is very simple, so fold
it into its (also simple) sole caller spapr_create_pci_child_dt().

While we're there rename pci_find_device_name() to the shorter and more
accurate dt_name_from_class().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_pci.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

Comments

Greg Kurz May 24, 2019, 1:32 p.m. UTC | #1
On Thu, 23 May 2019 15:29:11 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> spapr_populate_pci_child_dt() adds a 'name' property to the device tree
> node for PCI devices.  This is never necessary for a flattened device tree,
> it is implicit in the name added when the node is constructed.  In fact
> anything we do add to a 'name' property will be overwritten with something
> derived from the structural name in the guest firmware (but in fact it is
> exactly the same bytes).
> 
> So, remove that.  In addition, pci_get_node_name() is very simple, so fold
> it into its (also simple) sole caller spapr_create_pci_child_dt().
> 
> While we're there rename pci_find_device_name() to the shorter and more
> accurate dt_name_from_class().
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_pci.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 97961b0128..b2db46ef1d 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1173,8 +1173,8 @@ static const PCIClass pci_classes[] = {
>      { "data-processing-controller", spc_subclass },
>  };
>  
> -static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> -                                        uint8_t iface)
> +static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
> +                                      uint8_t iface)
>  {
>      const PCIClass *pclass;
>      const PCISubClass *psubclass;
> @@ -1216,23 +1216,6 @@ static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
>      return name;
>  }
>  
> -static gchar *pci_get_node_name(PCIDevice *dev)
> -{
> -    int slot = PCI_SLOT(dev->devfn);
> -    int func = PCI_FUNC(dev->devfn);
> -    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> -    const char *name;
> -
> -    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> -                                ccode & 0xff);
> -
> -    if (func != 0) {
> -        return g_strdup_printf("%s@%x,%x", name, slot, func);
> -    } else {
> -        return g_strdup_printf("%s@%x", name, slot);
> -    }
> -}
> -
>  static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
>                                              PCIDevice *pdev);
>  
> @@ -1300,11 +1283,6 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>          _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
>      }
>  
> -    _FDT(fdt_setprop_string(fdt, offset, "name",
> -                            pci_find_device_name((ccode >> 16) & 0xff,
> -                                                 (ccode >> 8) & 0xff,
> -                                                 ccode & 0xff)));
> -
>      buf = spapr_phb_get_loc_code(sphb, dev);
>      _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
>      g_free(buf);
> @@ -1348,10 +1326,23 @@ static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
>                                       void *fdt, int node_offset)
>  {
>      int offset;
> -    gchar *nodename;
> +    const gchar *basename;
> +    char *nodename;

Not sure why you're changing nodename to be a char * instead of a gchar * ...

Apart from that, LGTM.

Reviewed-by: Greg Kurz <groug@kaod.org>

> +    int slot = PCI_SLOT(dev->devfn);
> +    int func = PCI_FUNC(dev->devfn);
> +    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> +
> +    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> +                                  ccode & 0xff);
> +
> +    if (func != 0) {
> +        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
> +    } else {
> +        nodename = g_strdup_printf("%s@%x", basename, slot);
> +    }
>  
> -    nodename = pci_get_node_name(dev);
>      _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
> +
>      g_free(nodename);
>  
>      spapr_populate_pci_child_dt(dev, fdt, offset, phb);
Michael S. Tsirkin May 29, 2019, 3:23 a.m. UTC | #2
On Thu, May 23, 2019 at 03:29:11PM +1000, David Gibson wrote:
> spapr_populate_pci_child_dt() adds a 'name' property to the device tree
> node for PCI devices.  This is never necessary for a flattened device tree,
> it is implicit in the name added when the node is constructed.  In fact
> anything we do add to a 'name' property will be overwritten with something
> derived from the structural name in the guest firmware (but in fact it is
> exactly the same bytes).
> 
> So, remove that.  In addition, pci_get_node_name() is very simple, so fold
> it into its (also simple) sole caller spapr_create_pci_child_dt().
> 
> While we're there rename pci_find_device_name() to the shorter and more
> accurate dt_name_from_class().
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

The threading is broken here btw.

I was CC'd but it's mostly PPC stuff.
I like how pci_XX functions that are not in pci.c are
going away :)

Acked-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/ppc/spapr_pci.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 97961b0128..b2db46ef1d 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1173,8 +1173,8 @@ static const PCIClass pci_classes[] = {
>      { "data-processing-controller", spc_subclass },
>  };
>  
> -static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> -                                        uint8_t iface)
> +static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
> +                                      uint8_t iface)
>  {
>      const PCIClass *pclass;
>      const PCISubClass *psubclass;
> @@ -1216,23 +1216,6 @@ static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
>      return name;
>  }
>  
> -static gchar *pci_get_node_name(PCIDevice *dev)
> -{
> -    int slot = PCI_SLOT(dev->devfn);
> -    int func = PCI_FUNC(dev->devfn);
> -    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> -    const char *name;
> -
> -    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> -                                ccode & 0xff);
> -
> -    if (func != 0) {
> -        return g_strdup_printf("%s@%x,%x", name, slot, func);
> -    } else {
> -        return g_strdup_printf("%s@%x", name, slot);
> -    }
> -}
> -
>  static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
>                                              PCIDevice *pdev);
>  
> @@ -1300,11 +1283,6 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
>          _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
>      }
>  
> -    _FDT(fdt_setprop_string(fdt, offset, "name",
> -                            pci_find_device_name((ccode >> 16) & 0xff,
> -                                                 (ccode >> 8) & 0xff,
> -                                                 ccode & 0xff)));
> -
>      buf = spapr_phb_get_loc_code(sphb, dev);
>      _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
>      g_free(buf);
> @@ -1348,10 +1326,23 @@ static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
>                                       void *fdt, int node_offset)
>  {
>      int offset;
> -    gchar *nodename;
> +    const gchar *basename;
> +    char *nodename;
> +    int slot = PCI_SLOT(dev->devfn);
> +    int func = PCI_FUNC(dev->devfn);
> +    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> +
> +    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> +                                  ccode & 0xff);
> +
> +    if (func != 0) {
> +        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
> +    } else {
> +        nodename = g_strdup_printf("%s@%x", basename, slot);
> +    }
>  
> -    nodename = pci_get_node_name(dev);
>      _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
> +
>      g_free(nodename);
>  
>      spapr_populate_pci_child_dt(dev, fdt, offset, phb);
> -- 
> 2.21.0
Michael S. Tsirkin May 29, 2019, 3:24 a.m. UTC | #3
On Tue, May 28, 2019 at 11:23:54PM -0400, Michael S. Tsirkin wrote:
> On Thu, May 23, 2019 at 03:29:11PM +1000, David Gibson wrote:
> > spapr_populate_pci_child_dt() adds a 'name' property to the device tree
> > node for PCI devices.  This is never necessary for a flattened device tree,
> > it is implicit in the name added when the node is constructed.  In fact
> > anything we do add to a 'name' property will be overwritten with something
> > derived from the structural name in the guest firmware (but in fact it is
> > exactly the same bytes).
> > 
> > So, remove that.  In addition, pci_get_node_name() is very simple, so fold
> > it into its (also simple) sole caller spapr_create_pci_child_dt().
> > 
> > While we're there rename pci_find_device_name() to the shorter and more
> > accurate dt_name_from_class().
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> The threading is broken here btw.
> 
> I was CC'd but it's mostly PPC stuff.
> I like how pci_XX functions that are not in pci.c are
> going away :)
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

and that's for the whole patchset.

> 
> > ---
> >  hw/ppc/spapr_pci.c | 43 +++++++++++++++++--------------------------
> >  1 file changed, 17 insertions(+), 26 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 97961b0128..b2db46ef1d 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1173,8 +1173,8 @@ static const PCIClass pci_classes[] = {
> >      { "data-processing-controller", spc_subclass },
> >  };
> >  
> > -static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> > -                                        uint8_t iface)
> > +static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
> > +                                      uint8_t iface)
> >  {
> >      const PCIClass *pclass;
> >      const PCISubClass *psubclass;
> > @@ -1216,23 +1216,6 @@ static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> >      return name;
> >  }
> >  
> > -static gchar *pci_get_node_name(PCIDevice *dev)
> > -{
> > -    int slot = PCI_SLOT(dev->devfn);
> > -    int func = PCI_FUNC(dev->devfn);
> > -    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> > -    const char *name;
> > -
> > -    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> > -                                ccode & 0xff);
> > -
> > -    if (func != 0) {
> > -        return g_strdup_printf("%s@%x,%x", name, slot, func);
> > -    } else {
> > -        return g_strdup_printf("%s@%x", name, slot);
> > -    }
> > -}
> > -
> >  static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
> >                                              PCIDevice *pdev);
> >  
> > @@ -1300,11 +1283,6 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
> >          _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
> >      }
> >  
> > -    _FDT(fdt_setprop_string(fdt, offset, "name",
> > -                            pci_find_device_name((ccode >> 16) & 0xff,
> > -                                                 (ccode >> 8) & 0xff,
> > -                                                 ccode & 0xff)));
> > -
> >      buf = spapr_phb_get_loc_code(sphb, dev);
> >      _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
> >      g_free(buf);
> > @@ -1348,10 +1326,23 @@ static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
> >                                       void *fdt, int node_offset)
> >  {
> >      int offset;
> > -    gchar *nodename;
> > +    const gchar *basename;
> > +    char *nodename;
> > +    int slot = PCI_SLOT(dev->devfn);
> > +    int func = PCI_FUNC(dev->devfn);
> > +    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> > +
> > +    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> > +                                  ccode & 0xff);
> > +
> > +    if (func != 0) {
> > +        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
> > +    } else {
> > +        nodename = g_strdup_printf("%s@%x", basename, slot);
> > +    }
> >  
> > -    nodename = pci_get_node_name(dev);
> >      _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
> > +
> >      g_free(nodename);
> >  
> >      spapr_populate_pci_child_dt(dev, fdt, offset, phb);
> > -- 
> > 2.21.0
David Gibson May 30, 2019, 1:50 a.m. UTC | #4
On Fri, May 24, 2019 at 03:32:19PM +0200, Greg Kurz wrote:
> On Thu, 23 May 2019 15:29:11 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > spapr_populate_pci_child_dt() adds a 'name' property to the device tree
> > node for PCI devices.  This is never necessary for a flattened device tree,
> > it is implicit in the name added when the node is constructed.  In fact
> > anything we do add to a 'name' property will be overwritten with something
> > derived from the structural name in the guest firmware (but in fact it is
> > exactly the same bytes).
> > 
> > So, remove that.  In addition, pci_get_node_name() is very simple, so fold
> > it into its (also simple) sole caller spapr_create_pci_child_dt().
> > 
> > While we're there rename pci_find_device_name() to the shorter and more
> > accurate dt_name_from_class().
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr_pci.c | 43 +++++++++++++++++--------------------------
> >  1 file changed, 17 insertions(+), 26 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 97961b0128..b2db46ef1d 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1173,8 +1173,8 @@ static const PCIClass pci_classes[] = {
> >      { "data-processing-controller", spc_subclass },
> >  };
> >  
> > -static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> > -                                        uint8_t iface)
> > +static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
> > +                                      uint8_t iface)
> >  {
> >      const PCIClass *pclass;
> >      const PCISubClass *psubclass;
> > @@ -1216,23 +1216,6 @@ static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
> >      return name;
> >  }
> >  
> > -static gchar *pci_get_node_name(PCIDevice *dev)
> > -{
> > -    int slot = PCI_SLOT(dev->devfn);
> > -    int func = PCI_FUNC(dev->devfn);
> > -    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> > -    const char *name;
> > -
> > -    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> > -                                ccode & 0xff);
> > -
> > -    if (func != 0) {
> > -        return g_strdup_printf("%s@%x,%x", name, slot, func);
> > -    } else {
> > -        return g_strdup_printf("%s@%x", name, slot);
> > -    }
> > -}
> > -
> >  static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
> >                                              PCIDevice *pdev);
> >  
> > @@ -1300,11 +1283,6 @@ static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
> >          _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
> >      }
> >  
> > -    _FDT(fdt_setprop_string(fdt, offset, "name",
> > -                            pci_find_device_name((ccode >> 16) & 0xff,
> > -                                                 (ccode >> 8) & 0xff,
> > -                                                 ccode & 0xff)));
> > -
> >      buf = spapr_phb_get_loc_code(sphb, dev);
> >      _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
> >      g_free(buf);
> > @@ -1348,10 +1326,23 @@ static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
> >                                       void *fdt, int node_offset)
> >  {
> >      int offset;
> > -    gchar *nodename;
> > +    const gchar *basename;
> > +    char *nodename;
> 
> Not sure why you're changing nodename to be a char * instead of a gchar * ...

Uh... by mistake.  I'll remove that.

> Apart from that, LGTM.
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> > +    int slot = PCI_SLOT(dev->devfn);
> > +    int func = PCI_FUNC(dev->devfn);
> > +    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
> > +
> > +    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
> > +                                  ccode & 0xff);
> > +
> > +    if (func != 0) {
> > +        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
> > +    } else {
> > +        nodename = g_strdup_printf("%s@%x", basename, slot);
> > +    }
> >  
> > -    nodename = pci_get_node_name(dev);
> >      _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
> > +
> >      g_free(nodename);
> >  
> >      spapr_populate_pci_child_dt(dev, fdt, offset, phb);
>
David Gibson May 30, 2019, 1:51 a.m. UTC | #5
On Tue, May 28, 2019 at 11:23:54PM -0400, Michael S. Tsirkin wrote:
> On Thu, May 23, 2019 at 03:29:11PM +1000, David Gibson wrote:
> > spapr_populate_pci_child_dt() adds a 'name' property to the device tree
> > node for PCI devices.  This is never necessary for a flattened device tree,
> > it is implicit in the name added when the node is constructed.  In fact
> > anything we do add to a 'name' property will be overwritten with something
> > derived from the structural name in the guest firmware (but in fact it is
> > exactly the same bytes).
> > 
> > So, remove that.  In addition, pci_get_node_name() is very simple, so fold
> > it into its (also simple) sole caller spapr_create_pci_child_dt().
> > 
> > While we're there rename pci_find_device_name() to the shorter and more
> > accurate dt_name_from_class().
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> The threading is broken here btw.

I'm not entirely sure what you mean by that.  I did forget to add a
cover letter.

> I was CC'd but it's mostly PPC stuff.

Yeah, this was just for reference, since it is PCI stuff for ppc.

> I like how pci_XX functions that are not in pci.c are
> going away :)

You're welcome.

> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks.
diff mbox series

Patch

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 97961b0128..b2db46ef1d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1173,8 +1173,8 @@  static const PCIClass pci_classes[] = {
     { "data-processing-controller", spc_subclass },
 };
 
-static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
-                                        uint8_t iface)
+static const char *dt_name_from_class(uint8_t class, uint8_t subclass,
+                                      uint8_t iface)
 {
     const PCIClass *pclass;
     const PCISubClass *psubclass;
@@ -1216,23 +1216,6 @@  static const char *pci_find_device_name(uint8_t class, uint8_t subclass,
     return name;
 }
 
-static gchar *pci_get_node_name(PCIDevice *dev)
-{
-    int slot = PCI_SLOT(dev->devfn);
-    int func = PCI_FUNC(dev->devfn);
-    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
-    const char *name;
-
-    name = pci_find_device_name((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
-                                ccode & 0xff);
-
-    if (func != 0) {
-        return g_strdup_printf("%s@%x,%x", name, slot, func);
-    } else {
-        return g_strdup_printf("%s@%x", name, slot);
-    }
-}
-
 static uint32_t spapr_phb_get_pci_drc_index(SpaprPhbState *phb,
                                             PCIDevice *pdev);
 
@@ -1300,11 +1283,6 @@  static void spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset,
         _FDT(fdt_setprop(fdt, offset, "udf-supported", NULL, 0));
     }
 
-    _FDT(fdt_setprop_string(fdt, offset, "name",
-                            pci_find_device_name((ccode >> 16) & 0xff,
-                                                 (ccode >> 8) & 0xff,
-                                                 ccode & 0xff)));
-
     buf = spapr_phb_get_loc_code(sphb, dev);
     _FDT(fdt_setprop_string(fdt, offset, "ibm,loc-code", buf));
     g_free(buf);
@@ -1348,10 +1326,23 @@  static int spapr_create_pci_child_dt(SpaprPhbState *phb, PCIDevice *dev,
                                      void *fdt, int node_offset)
 {
     int offset;
-    gchar *nodename;
+    const gchar *basename;
+    char *nodename;
+    int slot = PCI_SLOT(dev->devfn);
+    int func = PCI_FUNC(dev->devfn);
+    uint32_t ccode = pci_default_read_config(dev, PCI_CLASS_PROG, 3);
+
+    basename = dt_name_from_class((ccode >> 16) & 0xff, (ccode >> 8) & 0xff,
+                                  ccode & 0xff);
+
+    if (func != 0) {
+        nodename = g_strdup_printf("%s@%x,%x", basename, slot, func);
+    } else {
+        nodename = g_strdup_printf("%s@%x", basename, slot);
+    }
 
-    nodename = pci_get_node_name(dev);
     _FDT(offset = fdt_add_subnode(fdt, node_offset, nodename));
+
     g_free(nodename);
 
     spapr_populate_pci_child_dt(dev, fdt, offset, phb);