[PATCH] cxl: Make vPHB device node match adapter's

[PATCH] cxl: Make vPHB device node match adapter's

[Frederic Barrat]

Tested by cxlflash on bare-metal and powerVM.

Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
---
 drivers/misc/cxl/vphb.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c
index cdc7723..012b6aa 100644
--- a/drivers/misc/cxl/vphb.c
+++ b/drivers/misc/cxl/vphb.c
@@ -208,20 +208,19 @@ static struct pci_controller_ops cxl_pci_controller_ops =
 
 int cxl_pci_vphb_add(struct cxl_afu *afu)
 {
-	struct pci_dev *phys_dev;
-	struct pci_controller *phb, *phys_phb;
+	struct pci_controller *phb;
 	struct device_node *vphb_dn;
 	struct device *parent;
 
-	if (cpu_has_feature(CPU_FTR_HVMODE)) {
-		phys_dev = to_pci_dev(afu->adapter->dev.parent);
-		phys_phb = pci_bus_to_host(phys_dev->bus);
-		vphb_dn = phys_phb->dn;
-		parent = &phys_dev->dev;
-	} else {
-		vphb_dn = afu->adapter->dev.parent->of_node;
-		parent = afu->adapter->dev.parent;
-	}
+	/* The parent device is the adapter. Reuse the device node of
+	 * the adapter.
+	 * We don't seem to care what device node is used for the vPHB,
+	 * but tools such as lsvpd walk up the device parents looking
+	 * for a valid location code, so we might as well show devices
+	 * attached to the adapter as being located on that adapter.
+	 */
+	parent = afu->adapter->dev.parent;
+	vphb_dn = parent->of_node;
 
 	/* Alloc and setup PHB data structure */
 	phb = pcibios_alloc_controller(vphb_dn);
-- 
2.7.4

Re: [PATCH] cxl: Make vPHB device node match adapter's

[Matthew R. Ochs]

> On Jun 15, 2016, at 9:42 AM, Frederic Barrat =
<fbarrat@linux.vnet.ibm.com> wrote:
>=20
> Tested by cxlflash on bare-metal and powerVM.
>=20
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH] cxl: Make vPHB device node match adapter's

[Ian Munsie]

This could probably use a description in the commit message, perhaps
including output showing the before/after difference this makes to
lsvpd, but otherwise it looks fine to me.

@Mikey - this look OK to you?

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Excerpts from Frederic Barrat's message of 2016-06-15 16:42:16 +0200:
> Tested by cxlflash on bare-metal and powerVM.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
>  drivers/misc/cxl/vphb.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c
> index cdc7723..012b6aa 100644
> --- a/drivers/misc/cxl/vphb.c
> +++ b/drivers/misc/cxl/vphb.c
> @@ -208,20 +208,19 @@ static struct pci_controller_ops cxl_pci_controller_ops =
>  
>  int cxl_pci_vphb_add(struct cxl_afu *afu)
>  {
> -    struct pci_dev *phys_dev;
> -    struct pci_controller *phb, *phys_phb;
> +    struct pci_controller *phb;
>      struct device_node *vphb_dn;
>      struct device *parent;
>  
> -    if (cpu_has_feature(CPU_FTR_HVMODE)) {
> -        phys_dev = to_pci_dev(afu->adapter->dev.parent);
> -        phys_phb = pci_bus_to_host(phys_dev->bus);
> -        vphb_dn = phys_phb->dn;
> -        parent = &phys_dev->dev;
> -    } else {
> -        vphb_dn = afu->adapter->dev.parent->of_node;
> -        parent = afu->adapter->dev.parent;
> -    }
> +    /* The parent device is the adapter. Reuse the device node of
> +     * the adapter.
> +     * We don't seem to care what device node is used for the vPHB,
> +     * but tools such as lsvpd walk up the device parents looking
> +     * for a valid location code, so we might as well show devices
> +     * attached to the adapter as being located on that adapter.
> +     */
> +    parent = afu->adapter->dev.parent;
> +    vphb_dn = parent->of_node;
>  
>      /* Alloc and setup PHB data structure */
>      phb = pcibios_alloc_controller(vphb_dn);

Re: [PATCH] cxl: Make vPHB device node match adapter's

[Frederic Barrat]

Thanks, Ian. Here is a hopefully better commit message:

cxl: Make vPHB device node match adapter's

On bare-metal, when a device is attached to the cxl card, lsvpd shows
a location code such as (with cxlflash):
     # lsvpd -l sg22
     ...
     *YL U78CB.001.WZS0073-P1-C33-B0-T0-L0
which makes it hard to easily identify the cxl adapter owning the
flash device, since in this example C33 refers to a P8 processor.

lsvpd looks in the parent devices until it finds a location code, so the
device node for the vPHB ends up being used.

By reusing the device node of the adapter for the vPHB, lsvpd shows:
     # lsvpd -l sg16
     ...
     *YL U78C9.001.WZS09XA-P1-C7-B1-T0-L3
where C7 is the PCI slot of the cxl adapter.

On powerVM, the vPHB was already using the adapter device node, so
there's no change there.

Tested by cxlflash on bare-metal and powerVM.

Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>






Le 15/06/2016 23:53, Ian Munsie a écrit :
> This could probably use a description in the commit message, perhaps
> including output showing the before/after difference this makes to
> lsvpd, but otherwise it looks fine to me.
>
> @Mikey - this look OK to you?
>
> Acked-by: Ian Munsie <imunsie@au1.ibm.com>
>
> Excerpts from Frederic Barrat's message of 2016-06-15 16:42:16 +0200:
>> Tested by cxlflash on bare-metal and powerVM.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>> ---
>>   drivers/misc/cxl/vphb.c | 21 ++++++++++-----------
>>   1 file changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c
>> index cdc7723..012b6aa 100644
>> --- a/drivers/misc/cxl/vphb.c
>> +++ b/drivers/misc/cxl/vphb.c
>> @@ -208,20 +208,19 @@ static struct pci_controller_ops cxl_pci_controller_ops =
>>
>>   int cxl_pci_vphb_add(struct cxl_afu *afu)
>>   {
>> -    struct pci_dev *phys_dev;
>> -    struct pci_controller *phb, *phys_phb;
>> +    struct pci_controller *phb;
>>       struct device_node *vphb_dn;
>>       struct device *parent;
>>
>> -    if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> -        phys_dev = to_pci_dev(afu->adapter->dev.parent);
>> -        phys_phb = pci_bus_to_host(phys_dev->bus);
>> -        vphb_dn = phys_phb->dn;
>> -        parent = &phys_dev->dev;
>> -    } else {
>> -        vphb_dn = afu->adapter->dev.parent->of_node;
>> -        parent = afu->adapter->dev.parent;
>> -    }
>> +    /* The parent device is the adapter. Reuse the device node of
>> +     * the adapter.
>> +     * We don't seem to care what device node is used for the vPHB,
>> +     * but tools such as lsvpd walk up the device parents looking
>> +     * for a valid location code, so we might as well show devices
>> +     * attached to the adapter as being located on that adapter.
>> +     */
>> +    parent = afu->adapter->dev.parent;
>> +    vphb_dn = parent->of_node;
>>
>>       /* Alloc and setup PHB data structure */
>>       phb = pcibios_alloc_controller(vphb_dn);

Re: [PATCH] cxl: Make vPHB device node match adapter's

[Michael Neuling]

On Thu, 2016-06-16 at 07:53 +1000, Ian Munsie wrote:
> This could probably use a description in the commit message, perhaps
> including output showing the before/after difference this makes to
> lsvpd, but otherwise it looks fine to me.
>=20
> @Mikey - this look OK to you?

Yeah I think this is a good idea.

IIRC, we had something like this really early on but hit some random issue
and pulled it out before posting.

Mikey

> Acked-by: Ian Munsie <imunsie@au1.ibm.com>
>=20
> Excerpts from Frederic Barrat's message of 2016-06-15 16:42:16 +0200:
> >=20
> > Tested by cxlflash on bare-metal and powerVM.
> >=20
> > Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> > ---
> > =C2=A0drivers/misc/cxl/vphb.c | 21 ++++++++++-----------
> > =C2=A01 file changed, 10 insertions(+), 11 deletions(-)
> >=20
> > diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c
> > index cdc7723..012b6aa 100644
> > --- a/drivers/misc/cxl/vphb.c
> > +++ b/drivers/misc/cxl/vphb.c
> > @@ -208,20 +208,19 @@ static struct pci_controller_ops
> > cxl_pci_controller_ops =3D
> > =C2=A0
> > =C2=A0int cxl_pci_vphb_add(struct cxl_afu *afu)
> > =C2=A0{
> > -=C2=A0=C2=A0=C2=A0=C2=A0struct pci_dev *phys_dev;
> > -=C2=A0=C2=A0=C2=A0=C2=A0struct pci_controller *phb, *phys_phb;
> > +=C2=A0=C2=A0=C2=A0=C2=A0struct pci_controller *phb;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct device_node *vphb_dn;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct device *parent;
> > =C2=A0
> > -=C2=A0=C2=A0=C2=A0=C2=A0if (cpu_has_feature(CPU_FTR_HVMODE)) {
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0phys_dev =3D to_pci_de=
v(afu->adapter->dev.parent);
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0phys_phb =3D pci_bus_t=
o_host(phys_dev->bus);
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0vphb_dn =3D phys_phb->=
dn;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0parent =3D &phys_dev->=
dev;
> > -=C2=A0=C2=A0=C2=A0=C2=A0} else {
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0vphb_dn =3D afu->adapt=
er->dev.parent->of_node;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0parent =3D afu->adapte=
r->dev.parent;
> > -=C2=A0=C2=A0=C2=A0=C2=A0}
> > +=C2=A0=C2=A0=C2=A0=C2=A0/* The parent device is the adapter. Reuse the=
 device node of
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* the adapter.
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* We don't seem to care what device node=
 is used for the vPHB,
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* but tools such as lsvpd walk up the de=
vice parents looking
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* for a valid location code, so we might=
 as well show devices
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* attached to the adapter as being locat=
ed on that adapter.
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/
> > +=C2=A0=C2=A0=C2=A0=C2=A0parent =3D afu->adapter->dev.parent;
> > +=C2=A0=C2=A0=C2=A0=C2=A0vphb_dn =3D parent->of_node;
> > =C2=A0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* Alloc and setup PHB data structure */
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0phb =3D pcibios_alloc_controller(vphb_dn)=
;

Re: cxl: Make vPHB device node match adapter's

[Michael Ellerman]

On Wed, 2016-15-06 at 14:42:16 UTC, Frederic Barrat wrote:
> Tested by cxlflash on bare-metal and powerVM.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> Acked-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a430739009384ba2c4804f3a42

cheers