LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] i2c: Driver to expose PowerNV platform i2c busses
From: Wolfram Sang @ 2014-11-25 17:53 UTC (permalink / raw)
  To: Neelesh Gupta; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <20141116171605.4750.17472.stgit@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 3378 bytes --]

On Sun, Nov 16, 2014 at 10:47:46PM +0530, Neelesh Gupta wrote:
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
> The driver uses the platform device infrastructure to probe the busses
> on the platform and registers them with the i2c driver framework.
> 
> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

...

> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 917c358..71ad6e1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1044,4 +1044,15 @@ config SCx200_ACB
>  	  This support is also available as a module.  If so, the module
>  	  will be called scx200_acb.
>  
> +config I2C_OPAL
> +	tristate "IBM OPAL I2C driver"
> +	depends on PPC_POWERNV
> +	default y
> +	help
> +	  This exposes the PowerNV platform i2c busses to the linux i2c layer,
> +	  the driver is based on the OPAL interfaces.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called as i2c-opal.
> +
>  endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 78d56c5..350aa86 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -102,5 +102,6 @@ obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
>  obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
>  obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
>  obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
> +obj-$(CONFIG_I2C_OPAL)		+= i2c-opal.o

Please keep it proprly sorted.

> +	rc = of_property_read_u32(pdev->dev.of_node, "ibm,opal-id", &opal_id);
> +	if (rc) {
> +		dev_err(&pdev->dev, "Missing ibm,opal-id property !\n");
> +		return -EIO;
> +	}

You introduce new bindings which need to be documented in
Docuemntation/devicetree/bindings/i2c.

They should be posted as a seperate patch with
devicetree@vger.kernel.org CCed, so they can comment on it. This is
required these days and especially important..

> +	adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL);
> +	if (!adapter)
> +		return -ENOMEM;
> +
> +	adapter->algo = &i2c_opal_algo;
> +	adapter->algo_data = (void *)(unsigned long)opal_id;
> +	adapter->dev.parent = &pdev->dev;
> +	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> +	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
> +	if (pname)
> +		strlcpy(adapter->name, pname, sizeof(adapter->name));
> +	else
> +		strlcpy(adapter->name, "opal", sizeof(adapter->name));

... because I'd like to get an ack from them because of this binding. I
don't know if we can just say "this comes from firmware, so we must
support it" (although you wrote the firmware IIUC) or if we have to
judge if this is a HW description which should go into DT? I am open
meanwhile that the adapter name does not need to be static anymore.
However, I don't know much about server world and FW, so maybe they can
assist.

An example binding in that document would also be very helpful.


> +static struct platform_driver i2c_opal_driver = {
> +	.probe	= i2c_opal_probe,
> +	.remove	= i2c_opal_remove,
> +	.driver	= {
> +		.name		= "i2c-opal",
> +		.owner		= THIS_MODULE,

owner not needed.

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] i2c-qoriq: modified compatibility for correct prescaler
From: Wolfram Sang @ 2014-11-25 18:13 UTC (permalink / raw)
  To: Scott Wood
  Cc: Valentin Longchamp, Linux device trees, Boschung, Rainer,
	Brunck, Holger, Linux I2C, Linux PowerPC Kernel
In-Reply-To: <1416274083.15957.96.camel@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote:
> On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote:
> > > > 
> > > > If we're going to change the device tree I'd rather just add a property
> > > > to say what the prescaler is.
> > > 
> > >  We would however, leave the boards' device trees that use things like
> > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it.
> > > 
> > > 
> > > Now the drawback is that the driver would require a change, to parse this
> > > prescaler new prescaler property. Would this be OK from your point of view
> > > Wolfram ? If yes, I will send the patches for it.
> > 
> > I don't think it is OK.
> 
> Why?

Because I thought it could be deduced. Then, a seperate property would
not be OK.

> >  I'd think it can be deduced from the compatible property.
> 
> For almost all existing device trees it cannot be.

Pity :( If we do introduce a new property, it should probably be
"clock-div". Grepping through binding documentation, that seems
accepted. We should ask DT maintainers, too, to be safe.

> If you want something that will work without changing device trees,
> you'll need to use SVR to identify the SoC.

The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno
if it makes sense to add to it for consistency reasons?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2] i2c: Driver to expose PowerNV platform i2c busses
From: Benjamin Herrenschmidt @ 2014-11-25 20:36 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Neelesh Gupta, linuxppc-dev, linux-i2c
In-Reply-To: <20141125175316.GB9716@katana>

On Tue, 2014-11-25 at 18:53 +0100, Wolfram Sang wrote:
> On Sun, Nov 16, 2014 at 10:47:46PM +0530, Neelesh Gupta wrote:
> > The patch exposes the available i2c busses on the PowerNV platform
> > to the kernel and implements the bus driver to support i2c and
> > smbus commands.
> > The driver uses the platform device infrastructure to probe the busses
> > on the platform and registers them with the i2c driver framework.
> > 
> > Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> ...
> 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 917c358..71ad6e1 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -1044,4 +1044,15 @@ config SCx200_ACB
> >  	  This support is also available as a module.  If so, the module
> >  	  will be called scx200_acb.
> >  
> > +config I2C_OPAL
> > +	tristate "IBM OPAL I2C driver"
> > +	depends on PPC_POWERNV
> > +	default y
> > +	help
> > +	  This exposes the PowerNV platform i2c busses to the linux i2c layer,
> > +	  the driver is based on the OPAL interfaces.
> > +
> > +	  This driver can also be built as a module. If so, the module will be
> > +	  called as i2c-opal.
> > +
> >  endmenu
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index 78d56c5..350aa86 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -102,5 +102,6 @@ obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
> >  obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
> >  obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
> >  obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
> > +obj-$(CONFIG_I2C_OPAL)		+= i2c-opal.o
> 
> Please keep it proprly sorted.
> 
> > +	rc = of_property_read_u32(pdev->dev.of_node, "ibm,opal-id", &opal_id);
> > +	if (rc) {
> > +		dev_err(&pdev->dev, "Missing ibm,opal-id property !\n");
> > +		return -EIO;
> > +	}
> 
> You introduce new bindings which need to be documented in
> Docuemntation/devicetree/bindings/i2c.

Ugh ... thanks god the powerpc maintainer (me) didn't request binding
document for everything that went into the device-tree on those
platforms or we would still be writing them....

> They should be posted as a seperate patch with
> devicetree@vger.kernel.org CCed, so they can comment on it. This is
> required these days and especially important..

Suuure, let's create more process and committees, and make sure nothing
gets done in any reasonable amount of time. Have we gone completely
insane ?

This is completely useless as an exercise. It's not like I'm going to
change the firmware interfaces anymore anyway so the "comments" are
going to be at best bike shed painting.

I'm getting close to just put that driver in powerpc.git ...
 
The point of publicly discussing bindings in the ARM world was in part
because we attacked a community with no understanding of the DT, but in
LARGE part because we had to define them for components and SoC that
would potentially be re-used in different platforms etc..

Here we are talking about a representation specific to a given FW
interface on PowerPC only which isn't going to be used by any other
platform that PowerNV (since the OPAL fw is what makes PowerNV) by the
people who invented the frigging flat device tree in the first place, so
give me a break.

It's getting quite tempting to just throw that driver into powerpc.git

> > +	adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL);
> > +	if (!adapter)
> > +		return -ENOMEM;
> > +
> > +	adapter->algo = &i2c_opal_algo;
> > +	adapter->algo_data = (void *)(unsigned long)opal_id;
> > +	adapter->dev.parent = &pdev->dev;
> > +	adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> > +	pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
> > +	if (pname)
> > +		strlcpy(adapter->name, pname, sizeof(adapter->name));
> > +	else
> > +		strlcpy(adapter->name, "opal", sizeof(adapter->name));
> 
> ... because I'd like to get an ack from them because of this binding.

And I don't give a flying crap about what random ARM SOC vendor thinks
of my powerpc FW interface for a powerpc unique FW interface.

>  I
> don't know if we can just say "this comes from firmware, so we must
> support it" (although you wrote the firmware IIUC) or if we have to
> judge if this is a HW description which should go into DT? I am open
> meanwhile that the adapter name does not need to be static anymore.
> However, I don't know much about server world and FW, so maybe they can
> assist.

I doubt it, all we're going to do is waste a few more monthes and make
it more painful for us to get our support into distros in time with 0
benefit whatsoever.

> An example binding in that document would also be very helpful.
> 
> 
> > +static struct platform_driver i2c_opal_driver = {
> > +	.probe	= i2c_opal_probe,
> > +	.remove	= i2c_opal_remove,
> > +	.driver	= {
> > +		.name		= "i2c-opal",
> > +		.owner		= THIS_MODULE,
> 
> owner not needed.
> 
> Thanks,
> 
>    Wolfram

^ permalink raw reply

* Re: [PATCH] powerpc: drop useless warning in eeh_init()
From: Gavin Shan @ 2014-11-25 22:28 UTC (permalink / raw)
  To: Greg Kurz; +Cc: linuxppc-dev
In-Reply-To: <20141125161006.1995.31789.stgit@bahia.lab.toulouse-stg.fr.ibm.com>

On Tue, Nov 25, 2014 at 05:10:06PM +0100, Greg Kurz wrote:
>This is what we get in dmesg when booting a pseries guest and
>the hypervisor doesn't provide EEH support.
>
>[    0.166655] EEH functionality not supported
>[    0.166778] eeh_init: Failed to call platform init function (-22)
>
>Since both powernv_eeh_init() and pseries_eeh_init() already complain when
>hitting an error, it is not needed to print more (especially such an
>uninformative message).
>
>Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/kernel/eeh.c |    5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>index 2248a19..7d62018 100644
>--- a/arch/powerpc/kernel/eeh.c
>+++ b/arch/powerpc/kernel/eeh.c
>@@ -920,11 +920,8 @@ int eeh_init(void)
> 		pr_warn("%s: Platform EEH operation not found\n",
> 			__func__);
> 		return -EEXIST;
>-	} else if ((ret = eeh_ops->init())) {
>-		pr_warn("%s: Failed to call platform init function (%d)\n",
>-			__func__, ret);
>+	} else if ((ret = eeh_ops->init()))
> 		return ret;
>-	}
> 
> 	/* Initialize EEH event */
> 	ret = eeh_event_init();
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v1 0/5] powerpc/pci: PCI slot unified reset
From: Benjamin Herrenschmidt @ 2014-11-25 22:54 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1416868727-5814-1-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:38 +1100, Gavin Shan wrote:
> The patchset corresponds to skiboot changes, which manages PCI slots
> in a unified way: OPAL APIs used to do slot reset, power management,
> presence status retrival. The patchset shouldn't be merged before
> the OPAL firmware counterpart is merged.

This doesn't work. We need to have both backward and forward
compatibility.

IE. New kernels must work with old OPAL and vice-versa.

> The kernel changes have been split into 2 parts: (A) Use the unified
> PCI slot reset OPAL API - this patchset; (B) Changes to rpaphp.ko to
> support PCI slot hotplug via sysfs entries for PowerPC PowerNV platform.

I don't think we should be using rpaphp... we should be creating our new
backend, there is almost no commonalities between RPA/PAPR and OPAL.

> This patchset affects EEH, PCI reset logics, which is used mainly by
> VFIO. A couple of cases have been tested out on P7/P8 boxes and looks
> good: EEH on PowerNV, EEH support for guest, PCI passthrou.
> 
> Gavin Shan (5):
>   powerpc/powernv: Use PCI slot reset infrastructure
>   powerpc/powernv: Refactor ioda_eeh_reset()
>   powerpc/powernv: Avoid do complete reset for twice
>   powerpc/powernv: Issue fundamental reset if required
>   powerpc/powernv: Drop opal_pci_reinit()
> 
>  arch/powerpc/include/asm/eeh.h                 |   2 +-
>  arch/powerpc/include/asm/opal.h                |  31 +---
>  arch/powerpc/kernel/eeh_pe.c                   |   3 -
>  arch/powerpc/platforms/powernv/eeh-ioda.c      | 230 ++++++++++++++-----------
>  arch/powerpc/platforms/powernv/eeh-powernv.c   |  24 +--
>  arch/powerpc/platforms/powernv/opal-wrappers.S |   1 -
>  arch/powerpc/platforms/powernv/pci-ioda.c      |   5 +-
>  arch/powerpc/platforms/pseries/eeh_pseries.c   |   3 +-
>  8 files changed, 139 insertions(+), 160 deletions(-)
> 
> Thanks,
> Gavin

^ permalink raw reply

* Re: [PATCH 2/8] powerpc/pci: Don't scan empty slot
From: Benjamin Herrenschmidt @ 2014-11-25 22:58 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1416869365-7671-3-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> In hotplug case, function pcibios_add_pci_devices() is called to
> rescan the specified PCI bus, which possibly doesn't have any child
> devices. Access to the PCI bus's child device node will cause kernel
> crash without exception. The patch adds two more conditions to avoid
> the kernel crash.
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/pci-hotplug.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index 6e2b4e3..ea5513b 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -105,7 +105,7 @@ EXPORT_SYMBOL_GPL(pcibios_remove_pci_devices);
>   * is how this routine differs from other, similar pcibios
>   * routines.)
>   */
> -void pcibios_add_pci_devices(struct pci_bus * bus)
> +void pcibios_add_pci_devices(struct pci_bus *bus)

Unrelated whitespace change

>  {
>  	int slotno, mode, pass, max;
>  	struct pci_dev *dev;
> @@ -120,7 +120,8 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
>  	if (mode == PCI_PROBE_DEVTREE) {
>  		/* use ofdt-based probe */
>  		of_rescan_bus(dn, bus);
> -	} else if (mode == PCI_PROBE_NORMAL) {
> +	} else if (mode == PCI_PROBE_NORMAL &&
> +		   dn->child && PCI_DN(dn->child)) {
>  		/*
>  		 * Use legacy probe. In the partial hotplug case, we
>  		 * probably have grandchildren devices unplugged. So

^ permalink raw reply

* Re: [PATCH 1/8] powerpc/pci: Move pcibios_find_pci_bus() around
From: Benjamin Herrenschmidt @ 2014-11-25 22:58 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1416869365-7671-2-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> The patch moves pcibios_find_pci_bus() to PPC kerenl directory so
> that it can be reused by hotplug code for pSeries and PowerNV
> platform at the same time.
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  arch/powerpc/kernel/pci-hotplug.c          | 36 ++++++++++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/pci_dlpar.c | 32 --------------------------
>  2 files changed, 36 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index 5b78917..6e2b4e3 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -21,6 +21,42 @@
>  #include <asm/firmware.h>
>  #include <asm/eeh.h>
>  
> +static struct pci_bus *find_pci_bus(struct pci_bus *bus,
> +				    struct device_node *dn)
> +{
> +	struct pci_bus *tmp, *child = NULL;
> +	struct device_node *busdn;
> +
> +	busdn = pci_bus_to_OF_node(bus);
> +	if (busdn == dn)
> +		return bus;
> +
> +	list_for_each_entry(tmp, &bus->children, node) {
> +		child = find_pci_bus(tmp, dn);
> +		if (child)
> +			break;
> +	}
> +
> +	return child;
> +}
> +
> +/**
> + * pcibios_find_pci_bus - find PCI bus according to the given device node
> + * @dn: Device node
> + *
> + * Find the corresponding PCI bus according to the given device node.
> + */
> +struct pci_bus *pcibios_find_pci_bus(struct device_node *dn)
> +{
> +	struct pci_dn *pdn = PCI_DN(dn);
> +
> +	if (!pdn  || !pdn->phb || !pdn->phb->bus)
> +		return NULL;
> +
> +	return find_pci_bus(pdn->phb->bus, dn);
> +}
> +EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
> +
>  /**
>   * pcibios_release_device - release PCI device
>   * @dev: PCI device
> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> index 89e2381..98c50bc 100644
> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> @@ -32,38 +32,6 @@
>  #include <asm/firmware.h>
>  #include <asm/eeh.h>
>  
> -static struct pci_bus *
> -find_bus_among_children(struct pci_bus *bus,
> -                        struct device_node *dn)
> -{
> -	struct pci_bus *child = NULL;
> -	struct pci_bus *tmp;
> -	struct device_node *busdn;
> -
> -	busdn = pci_bus_to_OF_node(bus);
> -	if (busdn == dn)
> -		return bus;
> -
> -	list_for_each_entry(tmp, &bus->children, node) {
> -		child = find_bus_among_children(tmp, dn);
> -		if (child)
> -			break;
> -	};
> -	return child;
> -}
> -
> -struct pci_bus *
> -pcibios_find_pci_bus(struct device_node *dn)
> -{
> -	struct pci_dn *pdn = dn->data;
> -
> -	if (!pdn  || !pdn->phb || !pdn->phb->bus)
> -		return NULL;
> -
> -	return find_bus_among_children(pdn->phb->bus, dn);
> -}
> -EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
> -
>  struct pci_controller *init_phb_dynamic(struct device_node *dn)
>  {
>  	struct pci_controller *phb;

^ permalink raw reply

* Re: [PATCH 3/8] powerpc/powernv: Export functions to retrieve slot status
From: Benjamin Herrenschmidt @ 2014-11-25 23:00 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1416869365-7671-4-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> The patch exports two functions, which base on corresponding OPAL
> APIs to retrieve PCI slot status:
> 
>    pnv_pci_get_power_status()     opal_pci_get_power_status()
>    pnv_pci_get_presence_status()  opal_pci_get_presence_status()
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/opal.h                |  6 +++-
>  arch/powerpc/include/asm/pnv-pci.h             |  3 ++
>  arch/powerpc/platforms/powernv/eeh-ioda.c      | 34 ++---------------------
>  arch/powerpc/platforms/powernv/opal-wrappers.S |  2 ++
>  arch/powerpc/platforms/powernv/pci.c           | 38 ++++++++++++++++++++++++++
>  arch/powerpc/platforms/powernv/pci.h           |  1 +
>  6 files changed, 51 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index b59811f..b75be1c 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -153,6 +153,8 @@ struct opal_sg_list {
>  #define OPAL_HANDLE_HMI				98
>  #define OPAL_REGISTER_DUMP_REGION		101
>  #define OPAL_UNREGISTER_DUMP_REGION		102
> +#define OPAL_PCI_GET_POWER_STATUS		110
> +#define OPAL_PCI_GET_PRESENCE_STATUS		111
>  
>  #ifndef __ASSEMBLY__
>  
> @@ -905,7 +907,7 @@ int64_t opal_get_epow_status(__be64 *status);
>  int64_t opal_set_system_attention_led(uint8_t led_action);
>  int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
>  			    __be16 *pci_error_type, __be16 *severity);
> -int64_t opal_pci_poll(uint64_t id);
> +int64_t opal_pci_poll(uint64_t id, uint8_t *val);
>  int64_t opal_return_cpu(void);
>  int64_t opal_check_token(uint64_t token);
>  int64_t opal_reinit_cpus(uint64_t flags);
> @@ -946,6 +948,8 @@ int64_t opal_handle_hmi(void);
>  int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
>  int64_t opal_unregister_dump_region(uint32_t id);
>  int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
> +int64_t opal_pci_get_power_status(uint64_t id, uint8_t *status);
> +int64_t opal_pci_get_presence_status(uint64_t id, uint8_t *status);
>  
>  /* Internal functions */
>  extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
> diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
> index f09a22f..527d97a 100644
> --- a/arch/powerpc/include/asm/pnv-pci.h
> +++ b/arch/powerpc/include/asm/pnv-pci.h
> @@ -13,6 +13,9 @@
>  #include <linux/pci.h>
>  #include <misc/cxl.h>
>  
> +extern int pnv_pci_get_power_status(unsigned int index, unsigned char *status);
> +extern int pnv_pci_get_presence_status(unsigned int index, unsigned char *status);
> +
>  int pnv_phb_to_cxl(struct pci_dev *dev);
>  int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
>  			   unsigned int virq);
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index 4bf2fb5..fb66c74 100644
> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
> @@ -490,24 +490,6 @@ static int ioda_eeh_get_state(struct eeh_pe *pe)
>  	return ioda_eeh_get_pe_state(pe);
>  }
>  
> -static s64 ioda_eeh_phb_poll(uint64_t id)
> -{
> -	s64 rc = OPAL_HARDWARE;
> -
> -	while (1) {
> -		rc = opal_pci_poll(id);
> -		if (rc <= 0)
> -			break;
> -
> -		if (system_state < SYSTEM_RUNNING)
> -			udelay(1000 * rc);
> -		else
> -			msleep(rc);
> -	}
> -
> -	return rc;
> -}
> -
>  int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
>  {
>  	struct pnv_phb *phb = hose->private_data;
> @@ -542,13 +524,7 @@ int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
>  	 * need the PCI bus settlement delay.
>  	 */
>  	rc = opal_pci_reset(phb->opal_id, reset_scope);
> -	if (rc > 0)
> -		rc = ioda_eeh_phb_poll(phb->opal_id);
> -
> -	if (rc != OPAL_SUCCESS)
> -		return -EIO;
> -
> -	return 0;
> +	return pnv_pci_poll(phb->opal_id, rc, NULL);
>  }

Patch does too mutch, the factoring out of pnv_pci_poll should be a
separate patch.

>  static int __ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
> @@ -641,13 +617,7 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
>  	phb = hose->private_data;
>  	id |= ((phb->opal_id << 16) | (dev->bus->number << 8) | (dev->devfn));
>  	rc = opal_pci_reset(id, reset_scope);
> -	if (rc > 0)
> -		rc = ioda_eeh_phb_poll(id);
> -
> -	if (rc != OPAL_SUCCESS)
> -		return -EIO;
> -
> -	return 0;
> +	return pnv_pci_poll(id, rc, NULL);
>  }
>  
>  static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data)
> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
> index b6474a1..b2c15ff 100644
> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
> @@ -249,3 +249,5 @@ OPAL_CALL(opal_handle_hmi,			OPAL_HANDLE_HMI);
>  OPAL_CALL(opal_register_dump_region,		OPAL_REGISTER_DUMP_REGION);
>  OPAL_CALL(opal_unregister_dump_region,		OPAL_UNREGISTER_DUMP_REGION);
>  OPAL_CALL(opal_pci_set_phb_cxl_mode,		OPAL_PCI_SET_PHB_CXL_MODE);
> +OPAL_CALL(opal_pci_get_power_status,		OPAL_PCI_GET_POWER_STATUS);
> +OPAL_CALL(opal_pci_get_presence_status,		OPAL_PCI_GET_PRESENCE_STATUS);
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b2187d0..f5bae4f 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -45,6 +45,44 @@
>  #define cfg_dbg(fmt...)	do { } while(0)
>  //#define cfg_dbg(fmt...)	printk(fmt)
>  
> +int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *pval)
> +{
> +	while (rval > 0) {
> +		rval = opal_pci_poll(id, pval);
> +		if (rval <= 0)
> +			break;
> +
> +		if (system_state < SYSTEM_RUNNING)
> +			udelay(1000 * rval);
> +		else
> +			msleep(rval);
> +	}
> +
> +	return rval ? -EIO : 0;
> +}

Do we need the udelay case still nowadays ? When do we call this early
enough that msleep won't work ?

> +int pnv_pci_get_power_status(unsigned int index, unsigned char *status)
> +{
> +	uint64_t id = (0x1ul << 63);
> +	long rc;
> +
> +	id |= index;
> +	rc = opal_pci_get_power_status(id, status);
> +	return pnv_pci_poll(id, rc, status);
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_get_power_status);
> +
> +int pnv_pci_get_presence_status(unsigned int index, unsigned char *status)
> +{
> +	uint64_t id = (0x1ul << 63);
> +	long rc;
> +
> +	id |= index;
> +	rc = opal_pci_get_presence_status(id, status);
> +	return pnv_pci_poll(id, rc, status);
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_get_presence_status);
> +
>  #ifdef CONFIG_PCI_MSI
>  static int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  {
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 6c02ff8..396fe02 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -217,6 +217,7 @@ extern struct pci_ops pnv_pci_ops;
>  extern struct pnv_eeh_ops ioda_eeh_ops;
>  #endif
>  
> +int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *pval);
>  void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
>  				unsigned char *log_buff);
>  int pnv_pci_cfg_read(struct device_node *dn,

^ permalink raw reply

* Re: [PATCH 4/8] PCI/hotplug/rpa: Code cleanup
From: Benjamin Herrenschmidt @ 2014-11-25 23:02 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1416869365-7671-5-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> The patch applies code cleanup to RPA modules to address following
> issues and it shouldn't affect the logic:
> 
>    * Coding style issue: removed unnecessary "break" for default case
>      in switch statement and added default case; removed unnecessary
>      braces or parenthese for if or return statements; removed
>      unecessary return statements
>    * Refactor rpaphp_get_sensor_state() and find_php_slot_pci_node()
>      to avoid nested if statements
>    * Drop is_registered(), is_php_type(), is_dlpar_capable()

Why dropping those helpers ? Make them inline if you want to avoid
polluting the namespace but I don't see what you gain in code
readability by making the functions bigger...

Ben.

> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  drivers/pci/hotplug/rpadlpar_core.c | 90 +++++++++++++++++++------------------
>  drivers/pci/hotplug/rpaphp_core.c   | 57 ++++++++++-------------
>  drivers/pci/hotplug/rpaphp_pci.c    | 43 +++++++++---------
>  drivers/pci/hotplug/rpaphp_slot.c   | 25 ++++-------
>  4 files changed, 101 insertions(+), 114 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index 7660232..35da3b3 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -52,30 +52,35 @@ static struct device_node *find_vio_slot_node(char *drc_name)
>  
>  	while ((dn = of_get_next_child(parent, dn))) {
>  		rc = rpaphp_get_drc_props(dn, NULL, &name, NULL, NULL);
> -		if ((rc == 0) && (!strcmp(drc_name, name)))
> -			break;
> +		if (rc)
> +			continue;
> +
> +		if (!strcmp(drc_name, name))
> +			return dn;
>  	}
>  
> -	return dn;
> +	return NULL;
>  }
>  
>  /* Find dlpar-capable pci node that contains the specified name and type */
>  static struct device_node *find_php_slot_pci_node(char *drc_name,
>  						  char *drc_type)
>  {
> -	struct device_node *np = NULL;
> +	struct device_node *dn = NULL;
>  	char *name;
>  	char *type;
>  	int rc;
>  
> -	while ((np = of_find_node_by_name(np, "pci"))) {
> -		rc = rpaphp_get_drc_props(np, NULL, &name, &type, NULL);
> -		if (rc == 0)
> -			if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
> -				break;
> +	while ((dn = of_find_node_by_name(dn, "pci"))) {
> +		rc = rpaphp_get_drc_props(dn, NULL, &name, &type, NULL);
> +		if (rc)
> +			continue;
> +
> +		if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
> +			return dn;
>  	}
>  
> -	return np;
> +	return NULL;
>  }
>  
>  static struct device_node *find_dlpar_node(char *drc_name, int *node_type)
> @@ -239,10 +244,9 @@ static int dlpar_add_phb(char *drc_name, struct device_node *dn)
>  {
>  	struct pci_controller *phb;
>  
> -	if (PCI_DN(dn) && PCI_DN(dn)->phb) {
> -		/* PHB already exists */
> +	/* PHB already exists */
> +	if (PCI_DN(dn) && PCI_DN(dn)->phb)
>  		return -EINVAL;
> -	}
>  
>  	phb = init_phb_dynamic(dn);
>  	if (!phb)
> @@ -299,15 +303,18 @@ int dlpar_add_slot(char *drc_name)
>  	}
>  
>  	switch (node_type) {
> -		case NODE_TYPE_VIO:
> -			rc = dlpar_add_vio_slot(drc_name, dn);
> -			break;
> -		case NODE_TYPE_SLOT:
> -			rc = dlpar_add_pci_slot(drc_name, dn);
> -			break;
> -		case NODE_TYPE_PHB:
> -			rc = dlpar_add_phb(drc_name, dn);
> -			break;
> +	case NODE_TYPE_VIO:
> +		rc = dlpar_add_vio_slot(drc_name, dn);
> +		break;
> +	case NODE_TYPE_SLOT:
> +		rc = dlpar_add_pci_slot(drc_name, dn);
> +		break;
> +	case NODE_TYPE_PHB:
> +		rc = dlpar_add_phb(drc_name, dn);
> +		break;
> +	default:
> +		rc = -EINVAL;
> +		goto exit;
>  	}
>  
>  	printk(KERN_INFO "%s: slot %s added\n", DLPAR_MODULE_NAME, drc_name);
> @@ -429,15 +436,18 @@ int dlpar_remove_slot(char *drc_name)
>  	}
>  
>  	switch (node_type) {
> -		case NODE_TYPE_VIO:
> -			rc = dlpar_remove_vio_slot(drc_name, dn);
> -			break;
> -		case NODE_TYPE_PHB:
> -			rc = dlpar_remove_phb(drc_name, dn);
> -			break;
> -		case NODE_TYPE_SLOT:
> -			rc = dlpar_remove_pci_slot(drc_name, dn);
> -			break;
> +	case NODE_TYPE_VIO:
> +		rc = dlpar_remove_vio_slot(drc_name, dn);
> +		break;
> +	case NODE_TYPE_PHB:
> +		rc = dlpar_remove_phb(drc_name, dn);
> +		break;
> +	case NODE_TYPE_SLOT:
> +		rc = dlpar_remove_pci_slot(drc_name, dn);
> +		break;
> +	default:
> +		rc = -EINVAL;
> +		goto exit;
>  	}
>  	vm_unmap_aliases();
>  
> @@ -447,20 +457,15 @@ exit:
>  	return rc;
>  }
>  
> -static inline int is_dlpar_capable(void)
> -{
> -	int rc = rtas_token("ibm,configure-connector");
> -
> -	return (int) (rc != RTAS_UNKNOWN_SERVICE);
> -}
> -
> -int __init rpadlpar_io_init(void)
> +static int __init rpadlpar_io_init(void)
>  {
>  	int rc = 0;
>  
> -	if (!is_dlpar_capable()) {
> +	/* Check if we have DLPAR capability */
> +	rc = rtas_token("ibm,configure-connector");
> +	if (rc == RTAS_UNKNOWN_SERVICE) {
>  		printk(KERN_WARNING "%s: partition not DLPAR capable\n",
> -			__func__);
> +		       __func__);
>  		return -EPERM;
>  	}
>  
> @@ -468,10 +473,9 @@ int __init rpadlpar_io_init(void)
>  	return rc;
>  }
>  
> -void rpadlpar_io_exit(void)
> +static void __exit rpadlpar_io_exit(void)
>  {
>  	dlpar_sysfs_exit();
> -	return;
>  }
>  
>  module_init(rpadlpar_io_init);
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index f2945fa..ff800df 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -74,7 +74,6 @@ static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
>  		break;
>  	default:
>  		value = 1;
> -		break;
>  	}
>  
>  	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
> @@ -94,7 +93,7 @@ static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
>  	int retval, level;
>  	struct slot *slot = (struct slot *)hotplug_slot->private;
>  
> -	retval = rtas_get_power_level (slot->power_domain, &level);
> +	retval = rtas_get_power_level(slot->power_domain, &level);
>  	if (!retval)
>  		*value = level;
>  	return retval;
> @@ -161,7 +160,6 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>  		break;
>  	default:
>  		speed = PCI_SPEED_UNKNOWN;
> -		break;
>  	}
>  
>  	return speed;
> @@ -178,17 +176,17 @@ static int get_children_props(struct device_node *dn, const int **drc_indexes,
>  	types = of_get_property(dn, "ibm,drc-types", NULL);
>  	domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>  
> -	if (!indexes || !names || !types || !domains) {
> -		/* Slot does not have dynamically-removable children */
> +	/* Slot does not have dynamically-removable children */
> +	if (!indexes || !names || !types || !domains)
>  		return -EINVAL;
> -	}
> +
>  	if (drc_indexes)
>  		*drc_indexes = indexes;
> +	/* &drc_names[1] contains NULL terminated slot names */
>  	if (drc_names)
> -		/* &drc_names[1] contains NULL terminated slot names */
>  		*drc_names = names;
> +	/* &drc_types[1] contains NULL terminated slot types */
>  	if (drc_types)
> -		/* &drc_types[1] contains NULL terminated slot types */
>  		*drc_types = types;
>  	if (drc_power_domains)
>  		*drc_power_domains = domains;
> @@ -210,15 +208,13 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>  	int i, rc;
>  
>  	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> -	if (!my_index) {
> -		/* Node isn't DLPAR/hotplug capable */
> +	/* Node isn't DLPAR/hotplug capable */
> +	if (!my_index)
>  		return -EINVAL;
> -	}
>  
>  	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
> -	if (rc < 0) {
> +	if (rc < 0)
>  		return -EINVAL;
> -	}
>  
>  	name_tmp = (char *) &names[1];
>  	type_tmp = (char *) &types[1];
> @@ -244,21 +240,8 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>  }
>  EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>  
> -static int is_php_type(char *drc_type)
> -{
> -	unsigned long value;
> -	char *endptr;
> -
> -	/* PCI Hotplug nodes have an integer for drc_type */
> -	value = simple_strtoul(drc_type, &endptr, 10);
> -	if (endptr == drc_type)
> -		return 0;
> -
> -	return 1;
> -}
> -
>  /**
> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
> + * is_php_dn() - return true if this is a hotpluggable pci slot, else false
>   * @dn: target &device_node
>   * @indexes: passed to get_children_props()
>   * @names: passed to get_children_props()
> @@ -270,21 +253,28 @@ static int is_php_type(char *drc_type)
>   * for built-in pci slots (even when the built-in slots are
>   * dlparable.)
>   */
> -static int is_php_dn(struct device_node *dn, const int **indexes,
> -		const int **names, const int **types, const int **power_domains)
> +static bool is_php_dn(struct device_node *dn,
> +		      const int **indexes, const int **names,
> +		      const int **types, const int **power_domains)
>  {
>  	const int *drc_types;
> +	const char *drc_type_str;
> +	char *endptr;
> +	unsigned long val;
>  	int rc;
>  
>  	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>  	if (rc < 0)
> -		return 0;
> +		return false;
>  
> -	if (!is_php_type((char *) &drc_types[1]))
> -		return 0;
> +	/* PCI Hotplug nodes have an integer for drc_type */
> +	drc_type_str = (char *)&drc_types[1];
> +	val = simple_strtoul(drc_type_str, &endptr, 10);
> +	if (endptr == drc_type_str)
> +		return false;
>  
>  	*types = drc_types;
> -	return 1;
> +	return true;
>  }
>  
>  /**
> @@ -370,7 +360,6 @@ static void __exit cleanup_slots(void)
>  		list_del(&slot->rpaphp_slot_list);
>  		pci_hp_deregister(slot->hotplug_slot);
>  	}
> -	return;
>  }
>  
>  static int __init rpaphp_init(void)
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
> index 9243f3e7..a4aa65c 100644
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ b/drivers/pci/hotplug/rpaphp_pci.c
> @@ -38,30 +38,31 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state)
>  	int setlevel;
>  
>  	rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
> +	if (rc >= 0)
> +		return rc;
> +	if (rc != -EFAULT && rc != -EEXIST) {
> +		err("%s: Failure %d getting sensor state on slot[%s]\n",
> +		    __func__, rc, slot->name);
> +		return rc;
> +	}
>  
> +
> +	/*
> +	 * Some slots have to be powered up before
> +	 * get-sensor will succeed
> +	 */
> +	dbg("%s: Slot[%s] must be power up to get sensor-state\n",
> +	    __func__, slot->name);
> +	rc = rtas_set_power_level(slot->power_domain, POWER_ON,
> +				  &setlevel);
>  	if (rc < 0) {
> -		if (rc == -EFAULT || rc == -EEXIST) {
> -			dbg("%s: slot must be power up to get sensor-state\n",
> -			    __func__);
> -
> -			/* some slots have to be powered up
> -			 * before get-sensor will succeed.
> -			 */
> -			rc = rtas_set_power_level(slot->power_domain, POWER_ON,
> -						  &setlevel);
> -			if (rc < 0) {
> -				dbg("%s: power on slot[%s] failed rc=%d.\n",
> -				    __func__, slot->name, rc);
> -			} else {
> -				rc = rtas_get_sensor(DR_ENTITY_SENSE,
> -						     slot->index, state);
> -			}
> -		} else if (rc == -ENODEV)
> -			info("%s: slot is unusable\n", __func__);
> -		else
> -			err("%s failed to get sensor state\n", __func__);
> +		dbg("%s: Failure %d powerng on slot[%s]\n",
> +		    __func__, rc, slot->name);
> +		return rc;
>  	}
> -	return rc;
> +
> +	return rtas_get_sensor(DR_ENTITY_SENSE,
> +			       slot->index, state);
>  }
>  
>  /**
> diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
> index a6082cc..be48e69 100644
> --- a/drivers/pci/hotplug/rpaphp_slot.c
> +++ b/drivers/pci/hotplug/rpaphp_slot.c
> @@ -72,7 +72,7 @@ struct slot *alloc_slot_struct(struct device_node *dn,
>  	slot->hotplug_slot->ops = &rpaphp_hotplug_slot_ops;
>  	slot->hotplug_slot->release = &rpaphp_release_slot;
>  
> -	return (slot);
> +	return slot;
>  
>  error_info:
>  	kfree(slot->hotplug_slot->info);
> @@ -84,17 +84,6 @@ error_nomem:
>  	return NULL;
>  }
>  
> -static int is_registered(struct slot *slot)
> -{
> -	struct slot *tmp_slot;
> -
> -	list_for_each_entry(tmp_slot, &rpaphp_slot_head, rpaphp_slot_list) {
> -		if (!strcmp(tmp_slot->name, slot->name))
> -			return 1;
> -	}
> -	return 0;
> -}
> -
>  int rpaphp_deregister_slot(struct slot *slot)
>  {
>  	int retval = 0;
> @@ -117,6 +106,7 @@ EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>  int rpaphp_register_slot(struct slot *slot)
>  {
>  	struct hotplug_slot *php_slot = slot->hotplug_slot;
> +	struct slot *tmp;
>  	int retval;
>  	int slotno;
>  
> @@ -124,10 +114,13 @@ int rpaphp_register_slot(struct slot *slot)
>  		__func__, slot->dn->full_name, slot->index, slot->name,
>  		slot->power_domain, slot->type);
>  
> -	/* should not try to register the same slot twice */
> -	if (is_registered(slot)) {
> -		err("rpaphp_register_slot: slot[%s] is already registered\n", slot->name);
> -		return -EAGAIN;
> +	/* Should not try to register the same slot twice */
> +	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
> +		if (!strcmp(tmp->name, slot->name)) {
> +			err("%s: Slot[%s] is already registered\n",
> +			    __func__, slot->name);
> +			return -EAGAIN;
> +		}
>  	}
>  
>  	if (slot->dn->child)

^ permalink raw reply

* Re: [PATCH 6/8] PCI/hotplug/rpa: Abstract slot operations
From: Benjamin Herrenschmidt @ 2014-11-25 23:04 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1416869365-7671-7-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> The patch splits the code into 2 parts: RPA PCI hotplug slot
> management and RTAS backend. It enables us to support PowerNV,
> which is built on top of OPAL firmware in future.
> 
> The patch also refactors the code for a bit:
> 
>     * Rename "struct slot" to "struct rpa_php_slot"
>     * All macros have prefix "RPA_PHP_SLOT"
>     * rpaphp_slot.c is removed and all logics moved to rpaphp_core.c


I don't see the point of this. rpaphp is already itself a "backend", so
we end up with yet another layer.

Just create a powernv-php or opal-php and if there is common code,
factor it into a common helper but I wouldn't bother too much initially
unless there is a lot of it.

> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  drivers/pci/hotplug/Makefile        |   3 +-
>  drivers/pci/hotplug/rpadlpar_core.c |  10 +-
>  drivers/pci/hotplug/rpaphp.h        |  64 +++----
>  drivers/pci/hotplug/rpaphp_core.c   | 347 +++++++++++-------------------------
>  drivers/pci/hotplug/rpaphp_pci.c    | 136 --------------
>  drivers/pci/hotplug/rpaphp_rtas.c   | 320 +++++++++++++++++++++++++++++++++
>  drivers/pci/hotplug/rpaphp_slot.c   | 140 ---------------
>  7 files changed, 459 insertions(+), 561 deletions(-)
>  delete mode 100644 drivers/pci/hotplug/rpaphp_pci.c
>  create mode 100644 drivers/pci/hotplug/rpaphp_rtas.c
>  delete mode 100644 drivers/pci/hotplug/rpaphp_slot.c
> 
> diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
> index 4a9aa08..630313da 100644
> --- a/drivers/pci/hotplug/Makefile
> +++ b/drivers/pci/hotplug/Makefile
> @@ -51,8 +51,7 @@ acpiphp-objs		:=	acpiphp_core.o	\
>  				acpiphp_glue.o
>  
>  rpaphp-objs		:=	rpaphp_core.o	\
> -				rpaphp_pci.o	\
> -				rpaphp_slot.o
> +				rpaphp_rtas.o
>  
>  rpadlpar_io-objs	:=	rpadlpar_core.o \
>  				rpadlpar_sysfs.o
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index 35da3b3..a36d2c9 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -117,13 +117,13 @@ static struct device_node *find_dlpar_node(char *drc_name, int *node_type)
>   * may be dlpar-able, but not hot-pluggable, so this routine
>   * will return NULL for built-in PCI slots.
>   */
> -static struct slot *find_php_slot(struct device_node *dn)
> +static struct rpa_php_slot *find_php_slot(struct device_node *dn)
>  {
>  	struct list_head *tmp, *n;
> -	struct slot *slot;
> +	struct rpa_php_slot *slot;
>  
>  	list_for_each_safe(tmp, n, &rpaphp_slot_head) {
> -		slot = list_entry(tmp, struct slot, rpaphp_slot_list);
> +		slot = list_entry(tmp, struct rpa_php_slot, rpaphp_slot_list);
>  		if (slot->dn == dn)
>  			return slot;
>  	}
> @@ -214,7 +214,7 @@ static int dlpar_add_pci_slot(char *drc_name, struct device_node *dn)
>  
>  static int dlpar_remove_phb(char *drc_name, struct device_node *dn)
>  {
> -	struct slot *slot;
> +	struct rpa_php_slot *slot;
>  	struct pci_dn *pdn;
>  	int rc = 0;
>  
> @@ -359,7 +359,7 @@ static int dlpar_remove_vio_slot(char *drc_name, struct device_node *dn)
>  int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn)
>  {
>  	struct pci_bus *bus;
> -	struct slot *slot;
> +	struct rpa_php_slot *slot;
>  	int ret = 0;
>  
>  	pci_lock_rescan_remove();
> diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
> index 39ddbdf..09dd516 100644
> --- a/drivers/pci/hotplug/rpaphp.h
> +++ b/drivers/pci/hotplug/rpaphp.h
> @@ -30,21 +30,6 @@
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
>  
> -#define DR_INDICATOR 9002
> -#define DR_ENTITY_SENSE 9003
> -
> -#define POWER_ON	100
> -#define POWER_OFF	0
> -
> -#define LED_OFF		0
> -#define LED_ON		1	/* continuous on */
> -#define LED_ID		2	/* slow blinking */
> -#define LED_ACTION	3	/* fast blinking */
> -
> -/* Sensor values from rtas_get-sensor */
> -#define EMPTY           0	/* No card in slot */
> -#define PRESENT         1	/* Card in slot */
> -
>  #define MY_NAME "rpaphp"
>  extern bool rpaphp_debug;
>  #define dbg(format, arg...)					\
> @@ -57,19 +42,26 @@ extern bool rpaphp_debug;
>  #define info(format, arg...) printk(KERN_INFO "%s: " format, MY_NAME , ## arg)
>  #define warn(format, arg...) printk(KERN_WARNING "%s: " format, MY_NAME , ## arg)
>  
> -/* slot states */
> +/* Power */
> +#define RPA_PHP_SLOT_POWER_ON	1	/* On            */
> +#define RPA_PHP_SLOT_POWER_OFF	0	/* Off           */
>  
> -#define	NOT_VALID	3
> -#define	NOT_CONFIGURED	2
> -#define	CONFIGURED	1
> -#define	EMPTY		0
> +/* Attention */
> +#define RPA_PHP_SLOT_ATTEN_OFF	0	/* Off           */
> +#define RPA_PHP_SLOT_ATTEN_ON	1	/* On            */
> +#define RPA_PHP_SLOT_ATTEN_IND	2	/* Slow blinking */
> +#define RPA_PHP_SLOT_ATTEN_ACT	3	/* Fast blinking */
>  
> -/*
> - * struct slot - slot information for each *physical* slot
> - */
> -struct slot {
> +/* Presence */
> +#define RPA_PHP_SLOT_EMPTY	0	/* No card       */
> +#define RPA_PHP_SLOT_PRESENT	1	/* Presented     */
> +
> +struct rpa_php_slot {
>  	struct list_head rpaphp_slot_list;
>  	int state;
> +#define RPA_PHP_SLOT_NOT_CONFIGURED	0
> +#define RPA_PHP_SLOT_CONFIGURED		1
> +#define RPA_PHP_SLOT_NOT_VALID		2
>  	u32 index;
>  	u32 type;
>  	u32 power_domain;
> @@ -80,24 +72,20 @@ struct slot {
>  	struct hotplug_slot *hotplug_slot;
>  };
>  
> -extern struct hotplug_slot_ops rpaphp_hotplug_slot_ops;
>  extern struct list_head rpaphp_slot_head;
>  
> -/* function prototypes */
> -
> -/* rpaphp_pci.c */
> -int rpaphp_enable_slot(struct slot *slot);
> -int rpaphp_get_sensor_state(struct slot *slot, int *state);
> -
>  /* rpaphp_core.c */
>  int rpaphp_add_slot(struct device_node *dn);
> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -		char **drc_name, char **drc_type, int *drc_power);
> +void dealloc_slot_struct(struct rpa_php_slot *slot);
> +struct rpa_php_slot *alloc_slot_struct(struct device_node *dn, int drc_index,
> +				       char *drc_name, int power_domain);
> +int rpaphp_register_slot(struct rpa_php_slot *slot);
> +int rpaphp_deregister_slot(struct rpa_php_slot *slot);
> +int rpaphp_add_slot(struct device_node *dn);
>  
> -/* rpaphp_slot.c */
> -void dealloc_slot_struct(struct slot *slot);
> -struct slot *alloc_slot_struct(struct device_node *dn, int drc_index, char *drc_name, int power_domain);
> -int rpaphp_register_slot(struct slot *slot);
> -int rpaphp_deregister_slot(struct slot *slot);
> +/* rpaphp_rtas.c */
> +int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> +			 char **drc_name, char **drc_type, int *drc_power);
> +struct rpa_php_slot *rpaphp_rtas_add_slot(struct device_node *dn);
>  
>  #endif				/* _PPC64PHP_H */
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index a639c5c..91eff8f 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -45,194 +45,118 @@ EXPORT_SYMBOL_GPL(rpaphp_slot_head);
>  #define DRIVER_AUTHOR	"Linda Xie <lxie@us.ibm.com>"
>  #define DRIVER_DESC	"RPA HOT Plug PCI Controller Driver"
>  
> -#define MAX_LOC_CODE 128
> -
> -MODULE_AUTHOR(DRIVER_AUTHOR);
> -MODULE_DESCRIPTION(DRIVER_DESC);
> -MODULE_LICENSE("GPL");
> -
>  module_param_named(debug, rpaphp_debug, bool, 0644);
>  
> -/**
> - * set_attention_status - set attention LED
> - * @hotplug_slot: target &hotplug_slot
> - * @value: LED control value
> - *
> - * echo 0 > attention -- set LED OFF
> - * echo 1 > attention -- set LED ON
> - * echo 2 > attention -- set LED ID(identify, light is blinking)
> - */
> -static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
> +/* free up the memory used by a slot */
> +static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
>  {
> -	int rc;
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -
> -	switch (value) {
> -	case 0:
> -	case 1:
> -	case 2:
> -		break;
> -	default:
> -		value = 1;
> -	}
> +	struct rpa_php_slot *slot = hotplug_slot->private;
>  
> -	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
> -	if (!rc)
> -		hotplug_slot->info->attention_status = value;
> -
> -	return rc;
> +	dealloc_slot_struct(slot);
>  }
>  
> -/**
> - * get_power_status - get power status of a slot
> - * @hotplug_slot: slot to get status
> - * @value: pointer to store status
> - */
> -static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
> +void dealloc_slot_struct(struct rpa_php_slot *slot)
>  {
> -	int retval, level;
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -
> -	retval = rtas_get_power_level(slot->power_domain, &level);
> -	if (!retval)
> -		*value = level;
> -	return retval;
> +	kfree(slot->hotplug_slot->info);
> +	kfree(slot->name);
> +	kfree(slot->hotplug_slot);
> +	kfree(slot);
>  }
>  
> -/**
> - * get_attention_status - get attention LED status
> - * @hotplug_slot: slot to get status
> - * @value: pointer to store status
> - */
> -static int get_attention_status(struct hotplug_slot *hotplug_slot, u8 *value)
> +struct rpa_php_slot *alloc_slot_struct(struct device_node *dn,
> +				       int drc_index, char *drc_name,
> +				       int power_domain)
>  {
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -	*value = slot->hotplug_slot->info->attention_status;
> -	return 0;
> -}
> -
> -static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
> -{
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -	int rc, state;
> -
> -	rc = rpaphp_get_sensor_state(slot, &state);
> -
> -	*value = NOT_VALID;
> -	if (rc)
> -		return rc;
> +	struct rpa_php_slot *slot;
>  
> -	if (state == EMPTY)
> -		*value = EMPTY;
> -	else if (state == PRESENT)
> -		*value = slot->state;
> -
> -	return 0;
> +	slot = kzalloc(sizeof(*slot), GFP_KERNEL);
> +	if (!slot)
> +		goto error_nomem;
> +	slot->hotplug_slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
> +	if (!slot->hotplug_slot)
> +		goto error_slot;
> +	slot->hotplug_slot->info = kzalloc(sizeof(struct hotplug_slot_info),
> +					   GFP_KERNEL);
> +	if (!slot->hotplug_slot->info)
> +		goto error_hpslot;
> +	slot->name = kstrdup(drc_name, GFP_KERNEL);
> +	if (!slot->name)
> +		goto error_info;
> +	slot->dn = dn;
> +	slot->index = drc_index;
> +	slot->power_domain = power_domain;
> +	slot->hotplug_slot->private = slot;
> +	slot->hotplug_slot->release = &rpaphp_release_slot;
> +
> +	slot->hotplug_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
> +	slot->hotplug_slot->info->attention_status = RPA_PHP_SLOT_ATTEN_OFF;
> +	slot->hotplug_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
> +	slot->state = RPA_PHP_SLOT_NOT_VALID;
> +
> +	return slot;
> +
> +error_info:
> +	kfree(slot->hotplug_slot->info);
> +error_hpslot:
> +	kfree(slot->hotplug_slot);
> +error_slot:
> +	kfree(slot);
> +error_nomem:
> +	return NULL;
>  }
>  
> -static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
> +int rpaphp_register_slot(struct rpa_php_slot *slot)
>  {
> -	enum pci_bus_speed speed;
> -	switch (slot->type) {
> -	case 1:
> -	case 2:
> -	case 3:
> -	case 4:
> -	case 5:
> -	case 6:
> -		speed = PCI_SPEED_33MHz;	/* speed for case 1-6 */
> -		break;
> -	case 7:
> -	case 8:
> -		speed = PCI_SPEED_66MHz;
> -		break;
> -	case 11:
> -	case 14:
> -		speed = PCI_SPEED_66MHz_PCIX;
> -		break;
> -	case 12:
> -	case 15:
> -		speed = PCI_SPEED_100MHz_PCIX;
> -		break;
> -	case 13:
> -	case 16:
> -		speed = PCI_SPEED_133MHz_PCIX;
> -		break;
> -	default:
> -		speed = PCI_SPEED_UNKNOWN;
> +	struct hotplug_slot *php_slot = slot->hotplug_slot;
> +	struct rpa_php_slot *tmp;
> +	int slotno, retval;
> +
> +	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
> +	    __func__, slot->dn->full_name, slot->index, slot->name,
> +	    slot->power_domain, slot->type);
> +
> +	/* Should not try to register the same slot twice */
> +	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
> +		if (!strcmp(tmp->name, slot->name)) {
> +			err("%s: Slot[%s] is already registered\n",
> +			    __func__, slot->name);
> +			return -EAGAIN;
> +		}
> +	}
> +	if (slot->dn->child)
> +		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
> +	else
> +		slotno = -1;
> +	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
> +	if (retval) {
> +		err("pci_hp_register failed with error %d\n", retval);
> +		return retval;
>  	}
>  
> -	return speed;
> +	/* add slot to our internal list */
> +	list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
> +	info("Slot [%s] registered\n", slot->name);
> +	return 0;
>  }
>  
> -static int parse_drc_props(struct device_node *dn, u32 drc_index,
> -			   char **drc_name, char **drc_type, u32 *drc_power)
> +int rpaphp_deregister_slot(struct rpa_php_slot *slot)
>  {
> -	const u32 *indexes, *names, *types, *domains;
> -	char *name, *type;
> -	struct device_node *parent = dn;
> -	u32 i;
> -
> -	while ((parent = of_get_parent(parent))) {
> -		indexes = of_get_property(parent, "ibm,drc-indexes", NULL);
> -		names   = of_get_property(parent, "ibm,drc-names", NULL);
> -		types   = of_get_property(parent, "ibm,drc-types", NULL);
> -		domains = of_get_property(parent, "ibm,drc-power-domains", NULL);
> -
> -		if (!indexes || !names || !types || !domains) {
> -			of_node_put(parent);
> -			continue;
> -		}
> +	struct hotplug_slot *php_slot = slot->hotplug_slot;
> +	int retval = 0;
>  
> -		name = (char *)&names[1];
> -		type = (char *)&types[1];
> -		for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> -			if (be32_to_cpu(indexes[i + 1]) != drc_index) {
> -				name += (strlen(name) + 1);
> -				type += (strlen(type) + 1);
> -				continue;
> -			}
> -
> -			/* Matched index */
> -			if (drc_name)
> -				*drc_name = name;
> -			if (drc_type)
> -				*drc_type = type;
> -			if (drc_power)
> -				*drc_power = be32_to_cpu(domains[i + 1]);
> -
> -			of_node_put(parent);
> -			return 0;
> -		}
> -
> -		/* Next level parent */
> -		of_node_put(parent);
> -	}
> +	dbg("%s - Entry: deregistering slot=%s\n",
> +	    __func__, slot->name);
>  
> -	return -ENODEV;
> -}
> +	list_del(&slot->rpaphp_slot_list);
>  
> -/*
> - * To get the DRC props describing the current node, first obtain it's
> - * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
> - * the my-drc-index for correlation, and obtain the requested properties.
> - */
> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -			 char **drc_name, char **drc_type, int *drc_power)
> -{
> -	const u32 *my_index;
> -
> -	/* Check if node is capable of hotplug */
> -	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> -	if (!my_index)
> -		return -EINVAL;
> -	if (drc_index)
> -		*drc_index = be32_to_cpu(*my_index);
> +	retval = pci_hp_deregister(php_slot);
> +	if (retval)
> +		err("Problem unregistering a slot %s\n", slot->name);
>  
> -	return parse_drc_props(dn, be32_to_cpu(*my_index),
> -			       drc_name, drc_type, drc_power);
> +	dbg("%s - Exit: rc[%d]\n", __func__, retval);
> +	return retval;
>  }
> -EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
> +EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>  
>  /**
>   * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
> @@ -252,29 +176,22 @@ EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>   */
>  int rpaphp_add_slot(struct device_node *dn)
>  {
> -	char *name, *type, *endptr;
> -	int index, power_domain;
> -	struct slot *slot;
> -	int val, ret;
> -
> -	/* Get and parse the hotplug properties */
> -	ret = rpaphp_get_drc_props(dn, &index, &name, &type, &power_domain);
> -	if (ret)
> -		return ret;
> -
> -	/* PCI Hotplug nodes have an integer for drc_type */
> -	val = simple_strtoul(type, &endptr, 10);
> -	if (endptr == type)
> -		return -EINVAL;
> +	struct rpa_php_slot *slot = NULL;
> +	int ret;
>  
> -	slot = alloc_slot_struct(dn, index, name, power_domain);
> +	/* Create slot */
> +	if (machine_is(pseries))
> +		slot = rpaphp_rtas_add_slot(dn);
>  	if (!slot)
> -		return -ENOMEM;
> +		return -EIO;
>  
> -	slot->type = val;
> -	ret = rpaphp_enable_slot(slot);
> -	if (!ret)
> -		ret = rpaphp_register_slot(slot);
> +	/* Enable slot */
> +	ret = slot->hotplug_slot->ops->enable_slot(slot->hotplug_slot);
> +	if (ret)
> +		goto fail;
> +
> +	/* Register slot */
> +	ret = rpaphp_register_slot(slot);
>  	if (ret)
>  		goto fail;
>  
> @@ -288,7 +205,7 @@ EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>  static void __exit cleanup_slots(void)
>  {
>  	struct list_head *tmp, *n;
> -	struct slot *slot;
> +	struct rpa_php_slot *slot;
>  
>  	/*
>  	 * Unregister all of our slots with the pci_hotplug subsystem,
> @@ -297,7 +214,7 @@ static void __exit cleanup_slots(void)
>  	 */
>  
>  	list_for_each_safe(tmp, n, &rpaphp_slot_head) {
> -		slot = list_entry(tmp, struct slot, rpaphp_slot_list);
> +		slot = list_entry(tmp, struct rpa_php_slot, rpaphp_slot_list);
>  		list_del(&slot->rpaphp_slot_list);
>  		pci_hp_deregister(slot->hotplug_slot);
>  	}
> @@ -320,59 +237,9 @@ static void __exit rpaphp_exit(void)
>  	cleanup_slots();
>  }
>  
> -static int enable_slot(struct hotplug_slot *hotplug_slot)
> -{
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -	int state;
> -	int retval;
> -
> -	if (slot->state == CONFIGURED)
> -		return 0;
> -
> -	retval = rpaphp_get_sensor_state(slot, &state);
> -	if (retval)
> -		return retval;
> -
> -	if (state == PRESENT) {
> -		pci_lock_rescan_remove();
> -		pcibios_add_pci_devices(slot->bus);
> -		pci_unlock_rescan_remove();
> -		slot->state = CONFIGURED;
> -	} else if (state == EMPTY) {
> -		slot->state = EMPTY;
> -	} else {
> -		err("%s: slot[%s] is in invalid state\n", __func__, slot->name);
> -		slot->state = NOT_VALID;
> -		return -EINVAL;
> -	}
> -
> -	slot->bus->max_bus_speed = get_max_bus_speed(slot);
> -	return 0;
> -}
> -
> -static int disable_slot(struct hotplug_slot *hotplug_slot)
> -{
> -	struct slot *slot = (struct slot *)hotplug_slot->private;
> -	if (slot->state == NOT_CONFIGURED)
> -		return -EINVAL;
> -
> -	pci_lock_rescan_remove();
> -	pcibios_remove_pci_devices(slot->bus);
> -	pci_unlock_rescan_remove();
> -	vm_unmap_aliases();
> -
> -	slot->state = NOT_CONFIGURED;
> -	return 0;
> -}
> -
> -struct hotplug_slot_ops rpaphp_hotplug_slot_ops = {
> -	.enable_slot = enable_slot,
> -	.disable_slot = disable_slot,
> -	.set_attention_status = set_attention_status,
> -	.get_power_status = get_power_status,
> -	.get_attention_status = get_attention_status,
> -	.get_adapter_status = get_adapter_status,
> -};
> -
>  module_init(rpaphp_init);
>  module_exit(rpaphp_exit);
> +
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
> deleted file mode 100644
> index a4aa65c..0000000
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ /dev/null
> @@ -1,136 +0,0 @@
> -/*
> - * PCI Hot Plug Controller Driver for RPA-compliant PPC64 platform.
> - * Copyright (C) 2003 Linda Xie <lxie@us.ibm.com>
> - *
> - * All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or (at
> - * your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> - * NON INFRINGEMENT.  See the GNU General Public License for more
> - * details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> - *
> - * Send feedback to <lxie@us.ibm.com>
> - *
> - */
> -#include <linux/pci.h>
> -#include <linux/string.h>
> -
> -#include <asm/pci-bridge.h>
> -#include <asm/rtas.h>
> -#include <asm/machdep.h>
> -
> -#include "../pci.h"		/* for pci_add_new_bus */
> -#include "rpaphp.h"
> -
> -int rpaphp_get_sensor_state(struct slot *slot, int *state)
> -{
> -	int rc;
> -	int setlevel;
> -
> -	rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
> -	if (rc >= 0)
> -		return rc;
> -	if (rc != -EFAULT && rc != -EEXIST) {
> -		err("%s: Failure %d getting sensor state on slot[%s]\n",
> -		    __func__, rc, slot->name);
> -		return rc;
> -	}
> -
> -
> -	/*
> -	 * Some slots have to be powered up before
> -	 * get-sensor will succeed
> -	 */
> -	dbg("%s: Slot[%s] must be power up to get sensor-state\n",
> -	    __func__, slot->name);
> -	rc = rtas_set_power_level(slot->power_domain, POWER_ON,
> -				  &setlevel);
> -	if (rc < 0) {
> -		dbg("%s: Failure %d powerng on slot[%s]\n",
> -		    __func__, rc, slot->name);
> -		return rc;
> -	}
> -
> -	return rtas_get_sensor(DR_ENTITY_SENSE,
> -			       slot->index, state);
> -}
> -
> -/**
> - * rpaphp_enable_slot - record slot state, config pci device
> - * @slot: target &slot
> - *
> - * Initialize values in the slot, and the hotplug_slot info
> - * structures to indicate if there is a pci card plugged into
> - * the slot. If the slot is not empty, run the pcibios routine
> - * to get pcibios stuff correctly set up.
> - */
> -int rpaphp_enable_slot(struct slot *slot)
> -{
> -	int rc, level, state;
> -	struct pci_bus *bus;
> -	struct hotplug_slot_info *info = slot->hotplug_slot->info;
> -
> -	info->adapter_status = NOT_VALID;
> -	slot->state = EMPTY;
> -
> -	/* Find out if the power is turned on for the slot */
> -	rc = rtas_get_power_level(slot->power_domain, &level);
> -	if (rc)
> -		return rc;
> -	info->power_status = level;
> -
> -	/* Figure out if there is an adapter in the slot */
> -	rc = rpaphp_get_sensor_state(slot, &state);
> -	if (rc)
> -		return rc;
> -
> -	bus = pcibios_find_pci_bus(slot->dn);
> -	if (!bus) {
> -		err("%s: no pci_bus for dn %s\n", __func__, slot->dn->full_name);
> -		return -EINVAL;
> -	}
> -
> -	info->adapter_status = EMPTY;
> -	slot->bus = bus;
> -	slot->pci_devs = &bus->devices;
> -
> -	/* if there's an adapter in the slot, go add the pci devices */
> -	if (state == PRESENT) {
> -		info->adapter_status = NOT_CONFIGURED;
> -		slot->state = NOT_CONFIGURED;
> -
> -		/* non-empty slot has to have child */
> -		if (!slot->dn->child) {
> -			err("%s: slot[%s]'s device_node doesn't have child for adapter\n",
> -			    __func__, slot->name);
> -			return -EINVAL;
> -		}
> -
> -		if (list_empty(&bus->devices))
> -			pcibios_add_pci_devices(bus);
> -
> -		if (!list_empty(&bus->devices)) {
> -			info->adapter_status = CONFIGURED;
> -			slot->state = CONFIGURED;
> -		}
> -
> -		if (rpaphp_debug) {
> -			struct pci_dev *dev;
> -			dbg("%s: pci_devs of slot[%s]\n", __func__, slot->dn->full_name);
> -			list_for_each_entry (dev, &bus->devices, bus_list)
> -				dbg("\t%s\n", pci_name(dev));
> -		}
> -	}
> -
> -	return 0;
> -}
> diff --git a/drivers/pci/hotplug/rpaphp_rtas.c b/drivers/pci/hotplug/rpaphp_rtas.c
> new file mode 100644
> index 0000000..74f024a
> --- /dev/null
> +++ b/drivers/pci/hotplug/rpaphp_rtas.c
> @@ -0,0 +1,320 @@
> +/*
> + * RTAS backend for RPA-compliant PP64 platform
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/pci.h>
> +#include <linux/pci_hotplug.h>
> +#include <linux/smp.h>
> +#include <linux/init.h>
> +#include <linux/vmalloc.h>
> +#include <asm/eeh.h>
> +#include <asm/rtas.h>
> +#include <asm/pci-bridge.h>
> +
> +#include "../pci.h"
> +#include "rpaphp.h"
> +
> +#define	RPA_PHP_DR_INDICATOR	9002
> +#define RPA_PHP_DR_ENTITY_SENSE	9003
> +
> +static int get_power_status(struct hotplug_slot *hp_slot, u8 *val)
> +{
> +	struct rpa_php_slot *slot = hp_slot->private;
> +	int state, ret;
> +
> +	/* By default, the power is on */
> +	*val = RPA_PHP_SLOT_POWER_ON;
> +
> +	/* Retrieve power state from firmware, which might fail */
> +	ret = rtas_get_power_level(slot->power_domain, &state);
> +	if (!ret) {
> +		if (state > 0)
> +			hp_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
> +		else
> +			hp_slot->info->power_status = RPA_PHP_SLOT_POWER_OFF;
> +		*val = hp_slot->info->power_status;
> +	}
> +
> +	return 0;
> +}
> +
> +static int get_adapter_status(struct hotplug_slot *hp_slot, u8 *val)
> +{
> +	struct rpa_php_slot *slot = hp_slot->private;
> +        int state, ret;
> +
> +	/* By default, the slot is empty */
> +	*val = RPA_PHP_SLOT_EMPTY;
> +
> +	/* Retrieve presence from firmware */
> +	ret = rtas_get_sensor(RPA_PHP_DR_ENTITY_SENSE, slot->index, &state);
> +	if (ret >= 0) {
> +		if (state > 0)
> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_PRESENT;
> +		else
> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
> +		*val = hp_slot->info->adapter_status;
> +		return 0;
> +	}
> +
> +	/* Check if we need power slot on and retry */
> +	if (ret != -EFAULT && ret != -EEXIST) {
> +		err("%s: Error %d getting slot[%s] presence\n",
> +		    __func__, ret, slot->name);
> +		return ret;
> +	}
> +
> +	/* Power slot on, which might fail */
> +	ret = rtas_set_power_level(slot->power_domain,
> +				   RPA_PHP_SLOT_POWER_ON, &state);
> +	if (!ret)
> +		hp_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
> +
> +	/* Recheck the presence */
> +	ret = rtas_get_sensor(RPA_PHP_DR_ENTITY_SENSE, slot->index, &state);
> +	if (ret >= 0) {
> +		if (state > 0)
> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_PRESENT;
> +		else
> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
> +		*val = hp_slot->info->adapter_status;
> +	}
> +
> +	return 0;
> +}
> +
> +static int set_attention_status(struct hotplug_slot *hp_slot, u8 val)
> +{
> +	struct rpa_php_slot *slot = hp_slot->private;
> +	int ret;
> +
> +	/*
> +	 * The default operation would to turn on
> +	 * the attention
> +	 */
> +	switch (val) {
> +	case RPA_PHP_SLOT_ATTEN_OFF:
> +	case RPA_PHP_SLOT_ATTEN_ON:
> +	case RPA_PHP_SLOT_ATTEN_IND:
> +	case RPA_PHP_SLOT_ATTEN_ACT:
> +		break;
> +	default:
> +		val = RPA_PHP_SLOT_ATTEN_ON;
> +	}
> +
> +	/* Set the attention */
> +	ret = rtas_set_indicator(RPA_PHP_DR_INDICATOR, slot->index, val);
> +	if (!ret)
> +		hp_slot->info->attention_status = val;
> +
> +	return ret;
> +}
> +
> +static enum pci_bus_speed get_max_bus_speed(struct rpa_php_slot *slot)
> +{
> +	enum pci_bus_speed speed;
> +
> +	switch (slot->type) {
> +	case 1 ... 6:
> +		speed = PCI_SPEED_33MHz;
> +		break;
> +	case 7 ... 8:
> +		speed = PCI_SPEED_66MHz;
> +		break;
> +	case 11:
> +	case 14:
> +		speed = PCI_SPEED_66MHz_PCIX;
> +		break;
> +	case 12:
> +	case 15:
> +		speed = PCI_SPEED_100MHz_PCIX;
> +		break;
> +	case 13:
> +	case 16:
> +		speed = PCI_SPEED_133MHz_PCIX;
> +		break;
> +	default:
> +		speed = PCI_SPEED_UNKNOWN;
> +	}
> +
> +	return speed;
> +}
> +
> +static int enable_slot(struct hotplug_slot *hp_slot)
> +{
> +	struct rpa_php_slot *slot = hp_slot->private;
> +	uint8_t presence;
> +	int ret;
> +
> +	/* Check if the slot has been configured */
> +	if (slot->state == RPA_PHP_SLOT_CONFIGURED)
> +		return 0;
> +
> +	/* Retrieve slot presence status */
> +	ret = hp_slot->ops->get_adapter_status(hp_slot, &presence);
> +	if (ret)
> +		return ret;
> +
> +	switch (presence) {
> +	case RPA_PHP_SLOT_PRESENT:
> +		pci_lock_rescan_remove();
> +		pcibios_add_pci_devices(slot->bus);
> +		pci_unlock_rescan_remove();
> +		slot->state = RPA_PHP_SLOT_CONFIGURED;
> +		break;
> +	case RPA_PHP_SLOT_EMPTY:
> +		slot->state = RPA_PHP_SLOT_NOT_CONFIGURED;
> +		break;
> +	default:
> +		slot->state = RPA_PHP_SLOT_NOT_VALID;
> +		return -EINVAL;
> +	}
> +
> +	/* Fix the bus maximal speed */
> +	slot->bus->max_bus_speed = get_max_bus_speed(slot);
> +	return 0;
> +}
> +
> +static int disable_slot(struct hotplug_slot *hp_slot)
> +{
> +	struct rpa_php_slot *slot = hp_slot->private;
> +
> +	if (slot->state != RPA_PHP_SLOT_CONFIGURED)
> +		return 0;
> +
> +	pci_lock_rescan_remove();
> +	pcibios_remove_pci_devices(slot->bus);
> +	pci_unlock_rescan_remove();
> +	vm_unmap_aliases();
> +
> +	slot->state = RPA_PHP_SLOT_NOT_CONFIGURED;
> +	return 0;
> +}
> +
> +static struct hotplug_slot_ops rpaphp_rtas_ops = {
> +	.enable_slot		= enable_slot,
> +	.disable_slot		= disable_slot,
> +	.set_attention_status	= set_attention_status,
> +	.get_power_status	= get_power_status,
> +	.get_adapter_status	= get_adapter_status,
> +};
> +
> +static int parse_drc_props(struct device_node *dn, u32 drc_index,
> +                           char **drc_name, char **drc_type, u32 *drc_power)
> +{
> +	const u32 *indexes, *names, *types, *domains;
> +	char *name, *type;
> +	struct device_node *parent = dn;
> +	u32 i;
> +
> +	while ((parent = of_get_parent(parent))) {
> +		indexes = of_get_property(parent, "ibm,drc-indexes", NULL);
> +		names   = of_get_property(parent, "ibm,drc-names", NULL);
> +		types   = of_get_property(parent, "ibm,drc-types", NULL);
> +		domains = of_get_property(parent, "ibm,drc-power-domains", NULL);
> +
> +		if (!indexes || !names || !types || !domains) {
> +			of_node_put(parent);
> +			continue;
> +		}
> +
> +		name = (char *)&names[1];
> +		type = (char *)&types[1];
> +		for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> +			if (be32_to_cpu(indexes[i + 1]) != drc_index) {
> +				name += (strlen(name) + 1);
> +				type += (strlen(type) + 1);
> +				continue;
> +			}
> +
> +			/* Matched index */
> +			if (drc_name)
> +				*drc_name = name;
> +			if (drc_type)
> +				*drc_type = type;
> +			if (drc_power)
> +				*drc_power = be32_to_cpu(domains[i + 1]);
> +
> +			of_node_put(parent);
> +			return 0;
> +		}
> +
> +		/* Next level parent */
> +		of_node_put(parent);
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +/*
> + * To get the DRC props describing the current node, first obtain it's
> + * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
> + * the my-drc-index for correlation, and obtain the requested properties.
> + */
> +int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> +                         char **drc_name, char **drc_type, int *drc_power)
> +{
> +	const u32 *my_index;
> +
> +	/* Check if node is capable of hotplug */
> +	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> +	if (!my_index)
> +		return -EINVAL;
> +	if (drc_index)
> +		*drc_index = be32_to_cpu(*my_index);
> +
> +	return parse_drc_props(dn, be32_to_cpu(*my_index),
> +			       drc_name, drc_type, drc_power);
> +}
> +EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
> +
> +struct rpa_php_slot *rpaphp_rtas_add_slot(struct device_node *dn)
> +{
> +	char *name, *type, *endptr;
> +	int index, power_domain;
> +	struct rpa_php_slot *slot;
> +	struct pci_bus *bus;
> +	int val, ret;
> +
> +	/* Get and parse the hotplug properties */
> +	ret = rpaphp_get_drc_props(dn, &index, &name, &type, &power_domain);
> +	if (ret)
> +		return NULL;
> +
> +	/*
> +	 * PCI hotplug slots have integer DRC type. That of
> +	 * PHB slot is fixed to "PHB"
> +	 */
> +        val = simple_strtoul(type, &endptr, 10);
> +	if (strcmp(type, "PHB") && (endptr == type))
> +		return NULL;
> +
> +	slot = alloc_slot_struct(dn, index, name, power_domain);
> +	if (!slot)
> +		return NULL;
> +
> +        /* The slot should have an associated bus */
> +	bus = pcibios_find_pci_bus(dn);
> +	if (!bus) {
> +		err("%s: No PCI bus for device node %s\n",
> +			__func__, dn->full_name);
> +		goto fail;
> +	}
> +
> +	slot->hotplug_slot->ops = &rpaphp_rtas_ops;
> +	slot->type     = val;
> +	slot->bus      = bus;
> +	slot->pci_devs = &bus->devices;
> +	return slot;
> +fail:
> +	dealloc_slot_struct(slot);
> +	return NULL;
> +}
> diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
> deleted file mode 100644
> index be48e69..0000000
> --- a/drivers/pci/hotplug/rpaphp_slot.c
> +++ /dev/null
> @@ -1,140 +0,0 @@
> -/*
> - * RPA Virtual I/O device functions
> - * Copyright (C) 2004 Linda Xie <lxie@us.ibm.com>
> - *
> - * All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or (at
> - * your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> - * NON INFRINGEMENT.  See the GNU General Public License for more
> - * details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> - *
> - * Send feedback to <lxie@us.ibm.com>
> - *
> - */
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/sysfs.h>
> -#include <linux/pci.h>
> -#include <linux/string.h>
> -#include <linux/slab.h>
> -
> -#include <asm/rtas.h>
> -#include "rpaphp.h"
> -
> -/* free up the memory used by a slot */
> -static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
> -{
> -	struct slot *slot = (struct slot *) hotplug_slot->private;
> -	dealloc_slot_struct(slot);
> -}
> -
> -void dealloc_slot_struct(struct slot *slot)
> -{
> -	kfree(slot->hotplug_slot->info);
> -	kfree(slot->name);
> -	kfree(slot->hotplug_slot);
> -	kfree(slot);
> -}
> -
> -struct slot *alloc_slot_struct(struct device_node *dn,
> -                       int drc_index, char *drc_name, int power_domain)
> -{
> -	struct slot *slot;
> -
> -	slot = kzalloc(sizeof(struct slot), GFP_KERNEL);
> -	if (!slot)
> -		goto error_nomem;
> -	slot->hotplug_slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
> -	if (!slot->hotplug_slot)
> -		goto error_slot;
> -	slot->hotplug_slot->info = kzalloc(sizeof(struct hotplug_slot_info),
> -					   GFP_KERNEL);
> -	if (!slot->hotplug_slot->info)
> -		goto error_hpslot;
> -	slot->name = kstrdup(drc_name, GFP_KERNEL);
> -	if (!slot->name)
> -		goto error_info;
> -	slot->dn = dn;
> -	slot->index = drc_index;
> -	slot->power_domain = power_domain;
> -	slot->hotplug_slot->private = slot;
> -	slot->hotplug_slot->ops = &rpaphp_hotplug_slot_ops;
> -	slot->hotplug_slot->release = &rpaphp_release_slot;
> -
> -	return slot;
> -
> -error_info:
> -	kfree(slot->hotplug_slot->info);
> -error_hpslot:
> -	kfree(slot->hotplug_slot);
> -error_slot:
> -	kfree(slot);
> -error_nomem:
> -	return NULL;
> -}
> -
> -int rpaphp_deregister_slot(struct slot *slot)
> -{
> -	int retval = 0;
> -	struct hotplug_slot *php_slot = slot->hotplug_slot;
> -
> -	 dbg("%s - Entry: deregistering slot=%s\n",
> -		__func__, slot->name);
> -
> -	list_del(&slot->rpaphp_slot_list);
> -
> -	retval = pci_hp_deregister(php_slot);
> -	if (retval)
> -		err("Problem unregistering a slot %s\n", slot->name);
> -
> -	dbg("%s - Exit: rc[%d]\n", __func__, retval);
> -	return retval;
> -}
> -EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
> -
> -int rpaphp_register_slot(struct slot *slot)
> -{
> -	struct hotplug_slot *php_slot = slot->hotplug_slot;
> -	struct slot *tmp;
> -	int retval;
> -	int slotno;
> -
> -	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
> -		__func__, slot->dn->full_name, slot->index, slot->name,
> -		slot->power_domain, slot->type);
> -
> -	/* Should not try to register the same slot twice */
> -	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
> -		if (!strcmp(tmp->name, slot->name)) {
> -			err("%s: Slot[%s] is already registered\n",
> -			    __func__, slot->name);
> -			return -EAGAIN;
> -		}
> -	}
> -
> -	if (slot->dn->child)
> -		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
> -	else
> -		slotno = -1;
> -	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
> -	if (retval) {
> -		err("pci_hp_register failed with error %d\n", retval);
> -		return retval;
> -	}
> -
> -	/* add slot to our internal list */
> -	list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
> -	info("Slot [%s] registered\n", slot->name);
> -	return 0;
> -}

^ permalink raw reply

* Re: [PATCH 5/8] PCI/hotplug/rpa: Create PCI slot properly
From: Benjamin Herrenschmidt @ 2014-11-25 23:04 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev, Nathan Fontenot
In-Reply-To: <1416869365-7671-6-git-send-email-gwshan@linux.vnet.ibm.com>

On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> When loading rpaphp.ko on a P7 box, I didn't see any PCI slots
> created under /sys/bus/pci/slots as expected. It seems that the
> RPA PCI slot stuff has been broken for long time. The driver
> doesn't use the properties of PCI device-tree nodes properly to
> populate PCI slots: device-tree node property "ibm,my-drc-index"
> is the identifier of hotpluggable PCI slot. The (direct or indirect)
> parent device-tree node should have properties associated with the
> "ibm,my-drc-index", which are "ibm,drc-indexes","ibm,drc-names",
> "ibm,drc-types", "ibm,drc-power-domains".
> 
> The patch parses above device-tree node properties to create PCI
> slots properly. One PCI slot is created for PCI device-tree node,
> which has meaningful "ibm,my-drc-index".

Nathan, can you review this ?

Cheers,
Ben.

> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  drivers/pci/hotplug/rpaphp.h      |   2 +-
>  drivers/pci/hotplug/rpaphp_core.c | 205 ++++++++++++++------------------------
>  2 files changed, 74 insertions(+), 133 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
> index b2593e8..39ddbdf 100644
> --- a/drivers/pci/hotplug/rpaphp.h
> +++ b/drivers/pci/hotplug/rpaphp.h
> @@ -92,7 +92,7 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state);
>  /* rpaphp_core.c */
>  int rpaphp_add_slot(struct device_node *dn);
>  int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -		char **drc_name, char **drc_type, int *drc_power_domain);
> +		char **drc_name, char **drc_type, int *drc_power);
>  
>  /* rpaphp_slot.c */
>  void dealloc_slot_struct(struct slot *slot);
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index ff800df..a639c5c 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -165,119 +165,76 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>  	return speed;
>  }
>  
> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
> -		const int **drc_names, const int **drc_types,
> -		const int **drc_power_domains)
> +static int parse_drc_props(struct device_node *dn, u32 drc_index,
> +			   char **drc_name, char **drc_type, u32 *drc_power)
>  {
> -	const int *indexes, *names, *types, *domains;
> +	const u32 *indexes, *names, *types, *domains;
> +	char *name, *type;
> +	struct device_node *parent = dn;
> +	u32 i;
> +
> +	while ((parent = of_get_parent(parent))) {
> +		indexes = of_get_property(parent, "ibm,drc-indexes", NULL);
> +		names   = of_get_property(parent, "ibm,drc-names", NULL);
> +		types   = of_get_property(parent, "ibm,drc-types", NULL);
> +		domains = of_get_property(parent, "ibm,drc-power-domains", NULL);
> +
> +		if (!indexes || !names || !types || !domains) {
> +			of_node_put(parent);
> +			continue;
> +		}
>  
> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> -	names = of_get_property(dn, "ibm,drc-names", NULL);
> -	types = of_get_property(dn, "ibm,drc-types", NULL);
> -	domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
> +		name = (char *)&names[1];
> +		type = (char *)&types[1];
> +		for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> +			if (be32_to_cpu(indexes[i + 1]) != drc_index) {
> +				name += (strlen(name) + 1);
> +				type += (strlen(type) + 1);
> +				continue;
> +			}
>  
> -	/* Slot does not have dynamically-removable children */
> -	if (!indexes || !names || !types || !domains)
> -		return -EINVAL;
> +			/* Matched index */
> +			if (drc_name)
> +				*drc_name = name;
> +			if (drc_type)
> +				*drc_type = type;
> +			if (drc_power)
> +				*drc_power = be32_to_cpu(domains[i + 1]);
> +
> +			of_node_put(parent);
> +			return 0;
> +		}
>  
> -	if (drc_indexes)
> -		*drc_indexes = indexes;
> -	/* &drc_names[1] contains NULL terminated slot names */
> -	if (drc_names)
> -		*drc_names = names;
> -	/* &drc_types[1] contains NULL terminated slot types */
> -	if (drc_types)
> -		*drc_types = types;
> -	if (drc_power_domains)
> -		*drc_power_domains = domains;
> +		/* Next level parent */
> +		of_node_put(parent);
> +	}
>  
> -	return 0;
> +	return -ENODEV;
>  }
>  
> -/* To get the DRC props describing the current node, first obtain it's
> +/*
> + * To get the DRC props describing the current node, first obtain it's
>   * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
>   * the my-drc-index for correlation, and obtain the requested properties.
>   */
>  int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -		char **drc_name, char **drc_type, int *drc_power_domain)
> +			 char **drc_name, char **drc_type, int *drc_power)
>  {
> -	const int *indexes, *names;
> -	const int *types, *domains;
> -	const unsigned int *my_index;
> -	char *name_tmp, *type_tmp;
> -	int i, rc;
> +	const u32 *my_index;
>  
> +	/* Check if node is capable of hotplug */
>  	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> -	/* Node isn't DLPAR/hotplug capable */
>  	if (!my_index)
>  		return -EINVAL;
> +	if (drc_index)
> +		*drc_index = be32_to_cpu(*my_index);
>  
> -	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
> -	if (rc < 0)
> -		return -EINVAL;
> -
> -	name_tmp = (char *) &names[1];
> -	type_tmp = (char *) &types[1];
> -
> -	/* Iterate through parent properties, looking for my-drc-index */
> -	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> -		if ((unsigned int) indexes[i + 1] == *my_index) {
> -			if (drc_name)
> -				*drc_name = name_tmp;
> -			if (drc_type)
> -				*drc_type = type_tmp;
> -			if (drc_index)
> -				*drc_index = be32_to_cpu(*my_index);
> -			if (drc_power_domain)
> -				*drc_power_domain = be32_to_cpu(domains[i+1]);
> -			return 0;
> -		}
> -		name_tmp += (strlen(name_tmp) + 1);
> -		type_tmp += (strlen(type_tmp) + 1);
> -	}
> -
> -	return -EINVAL;
> +	return parse_drc_props(dn, be32_to_cpu(*my_index),
> +			       drc_name, drc_type, drc_power);
>  }
>  EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>  
>  /**
> - * is_php_dn() - return true if this is a hotpluggable pci slot, else false
> - * @dn: target &device_node
> - * @indexes: passed to get_children_props()
> - * @names: passed to get_children_props()
> - * @types: returned from get_children_props()
> - * @power_domains:
> - *
> - * This routine will return true only if the device node is
> - * a hotpluggable slot. This routine will return false
> - * for built-in pci slots (even when the built-in slots are
> - * dlparable.)
> - */
> -static bool is_php_dn(struct device_node *dn,
> -		      const int **indexes, const int **names,
> -		      const int **types, const int **power_domains)
> -{
> -	const int *drc_types;
> -	const char *drc_type_str;
> -	char *endptr;
> -	unsigned long val;
> -	int rc;
> -
> -	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
> -	if (rc < 0)
> -		return false;
> -
> -	/* PCI Hotplug nodes have an integer for drc_type */
> -	drc_type_str = (char *)&drc_types[1];
> -	val = simple_strtoul(drc_type_str, &endptr, 10);
> -	if (endptr == drc_type_str)
> -		return false;
> -
> -	*types = drc_types;
> -	return true;
> -}
> -
> -/**
>   * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>   * @dn: device node of slot
>   *
> @@ -295,52 +252,36 @@ static bool is_php_dn(struct device_node *dn,
>   */
>  int rpaphp_add_slot(struct device_node *dn)
>  {
> +	char *name, *type, *endptr;
> +	int index, power_domain;
>  	struct slot *slot;
> -	int retval = 0;
> -	int i;
> -	const int *indexes, *names, *types, *power_domains;
> -	char *name, *type;
> -
> -	if (!dn->name || strcmp(dn->name, "pci"))
> -		return 0;
> +	int val, ret;
>  
> -	/* If this is not a hotplug slot, return without doing anything. */
> -	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
> -		return 0;
> -
> -	dbg("Entry %s: dn->full_name=%s\n", __func__, dn->full_name);
> -
> -	/* register PCI devices */
> -	name = (char *) &names[1];
> -	type = (char *) &types[1];
> -	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> -		int index;
> -
> -		index = be32_to_cpu(indexes[i + 1]);
> -		slot = alloc_slot_struct(dn, index, name,
> -					 be32_to_cpu(power_domains[i + 1]));
> -		if (!slot)
> -			return -ENOMEM;
> -
> -		slot->type = simple_strtoul(type, NULL, 10);
> -
> -		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> -				index, name, type);
> +	/* Get and parse the hotplug properties */
> +	ret = rpaphp_get_drc_props(dn, &index, &name, &type, &power_domain);
> +	if (ret)
> +		return ret;
>  
> -		retval = rpaphp_enable_slot(slot);
> -		if (!retval)
> -			retval = rpaphp_register_slot(slot);
> +	/* PCI Hotplug nodes have an integer for drc_type */
> +	val = simple_strtoul(type, &endptr, 10);
> +	if (endptr == type)
> +		return -EINVAL;
>  
> -		if (retval)
> -			dealloc_slot_struct(slot);
> +	slot = alloc_slot_struct(dn, index, name, power_domain);
> +	if (!slot)
> +		return -ENOMEM;
>  
> -		name += strlen(name) + 1;
> -		type += strlen(type) + 1;
> -	}
> -	dbg("%s - Exit: rc[%d]\n", __func__, retval);
> +	slot->type = val;
> +	ret = rpaphp_enable_slot(slot);
> +	if (!ret)
> +		ret = rpaphp_register_slot(slot);
> +	if (ret)
> +		goto fail;
>  
> -	/* XXX FIXME: reports a failure only if last entry in loop failed */
> -	return retval;
> +	return 0;
> +fail:
> +	dealloc_slot_struct(slot);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>  

^ permalink raw reply

* Re: [PATCH v2 07/14] of/reconfig: Always use the same structure for notifiers
From: Benjamin Herrenschmidt @ 2014-11-25 23:07 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree, Wolfram Sang, Pantelis Antoniou, linux-kernel,
	Rob Herring, Mark Brown, linuxppc-dev
In-Reply-To: <1416868422-22103-8-git-send-email-grant.likely@linaro.org>

On Mon, 2014-11-24 at 22:33 +0000, Grant Likely wrote:
> The OF_RECONFIG notifier callback uses a different structure depending
> on whether it is a node change or a property change. This is silly, and
> not very safe. Rework the code to use the same data structure regardless
> of the type of notifier.

I fell pretty good about this one except...

> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index b9d1dfdbe5bb..9fe6002c1d5a 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1711,12 +1711,11 @@ static void stage_topology_update(int core_id)
>  static int dt_update_callback(struct notifier_block *nb,
>  				unsigned long action, void *data)
>  {
> -	struct of_prop_reconfig *update;
> +	struct of_reconfig_data *update = data;
>  	int rc = NOTIFY_DONE;
>  
>  	switch (action) {
>  	case OF_RECONFIG_UPDATE_PROPERTY:
> -		update = (struct of_prop_reconfig *)data;

Should we assert/bug on !update->dn / update->prop ?

(Same for the rest of the patch)

Or do you reckon it's pointless ?

Ben.

^ permalink raw reply

* Re: [PATCH 2/8] powerpc/pci: Don't scan empty slot
From: Gavin Shan @ 2014-11-25 23:57 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <1416956282.5089.4.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 09:58:02AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
>> In hotplug case, function pcibios_add_pci_devices() is called to
>> rescan the specified PCI bus, which possibly doesn't have any child
>> devices. Access to the PCI bus's child device node will cause kernel
>> crash without exception. The patch adds two more conditions to avoid
>> the kernel crash.
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/pci-hotplug.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
>> index 6e2b4e3..ea5513b 100644
>> --- a/arch/powerpc/kernel/pci-hotplug.c
>> +++ b/arch/powerpc/kernel/pci-hotplug.c
>> @@ -105,7 +105,7 @@ EXPORT_SYMBOL_GPL(pcibios_remove_pci_devices);
>>   * is how this routine differs from other, similar pcibios
>>   * routines.)
>>   */
>> -void pcibios_add_pci_devices(struct pci_bus * bus)
>> +void pcibios_add_pci_devices(struct pci_bus *bus)
>
>Unrelated whitespace change
>

I'll remove this change out of the patch.

Thanks,
Gavin

>>  {
>>  	int slotno, mode, pass, max;
>>  	struct pci_dev *dev;
>> @@ -120,7 +120,8 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
>>  	if (mode == PCI_PROBE_DEVTREE) {
>>  		/* use ofdt-based probe */
>>  		of_rescan_bus(dn, bus);
>> -	} else if (mode == PCI_PROBE_NORMAL) {
>> +	} else if (mode == PCI_PROBE_NORMAL &&
>> +		   dn->child && PCI_DN(dn->child)) {
>>  		/*
>>  		 * Use legacy probe. In the partial hotplug case, we
>>  		 * probably have grandchildren devices unplugged. So
>
>

^ permalink raw reply

* Re: [PATCH 3/8] powerpc/powernv: Export functions to retrieve slot status
From: Gavin Shan @ 2014-11-25 23:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <1416956433.5089.7.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 10:00:33AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
>> The patch exports two functions, which base on corresponding OPAL
>> APIs to retrieve PCI slot status:
>> 
>>    pnv_pci_get_power_status()     opal_pci_get_power_status()
>>    pnv_pci_get_presence_status()  opal_pci_get_presence_status()
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/opal.h                |  6 +++-
>>  arch/powerpc/include/asm/pnv-pci.h             |  3 ++
>>  arch/powerpc/platforms/powernv/eeh-ioda.c      | 34 ++---------------------
>>  arch/powerpc/platforms/powernv/opal-wrappers.S |  2 ++
>>  arch/powerpc/platforms/powernv/pci.c           | 38 ++++++++++++++++++++++++++
>>  arch/powerpc/platforms/powernv/pci.h           |  1 +
>>  6 files changed, 51 insertions(+), 33 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index b59811f..b75be1c 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -153,6 +153,8 @@ struct opal_sg_list {
>>  #define OPAL_HANDLE_HMI				98
>>  #define OPAL_REGISTER_DUMP_REGION		101
>>  #define OPAL_UNREGISTER_DUMP_REGION		102
>> +#define OPAL_PCI_GET_POWER_STATUS		110
>> +#define OPAL_PCI_GET_PRESENCE_STATUS		111
>>  
>>  #ifndef __ASSEMBLY__
>>  
>> @@ -905,7 +907,7 @@ int64_t opal_get_epow_status(__be64 *status);
>>  int64_t opal_set_system_attention_led(uint8_t led_action);
>>  int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
>>  			    __be16 *pci_error_type, __be16 *severity);
>> -int64_t opal_pci_poll(uint64_t id);
>> +int64_t opal_pci_poll(uint64_t id, uint8_t *val);
>>  int64_t opal_return_cpu(void);
>>  int64_t opal_check_token(uint64_t token);
>>  int64_t opal_reinit_cpus(uint64_t flags);
>> @@ -946,6 +948,8 @@ int64_t opal_handle_hmi(void);
>>  int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
>>  int64_t opal_unregister_dump_region(uint32_t id);
>>  int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
>> +int64_t opal_pci_get_power_status(uint64_t id, uint8_t *status);
>> +int64_t opal_pci_get_presence_status(uint64_t id, uint8_t *status);
>>  
>>  /* Internal functions */
>>  extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
>> diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
>> index f09a22f..527d97a 100644
>> --- a/arch/powerpc/include/asm/pnv-pci.h
>> +++ b/arch/powerpc/include/asm/pnv-pci.h
>> @@ -13,6 +13,9 @@
>>  #include <linux/pci.h>
>>  #include <misc/cxl.h>
>>  
>> +extern int pnv_pci_get_power_status(unsigned int index, unsigned char *status);
>> +extern int pnv_pci_get_presence_status(unsigned int index, unsigned char *status);
>> +
>>  int pnv_phb_to_cxl(struct pci_dev *dev);
>>  int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
>>  			   unsigned int virq);
>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> index 4bf2fb5..fb66c74 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> @@ -490,24 +490,6 @@ static int ioda_eeh_get_state(struct eeh_pe *pe)
>>  	return ioda_eeh_get_pe_state(pe);
>>  }
>>  
>> -static s64 ioda_eeh_phb_poll(uint64_t id)
>> -{
>> -	s64 rc = OPAL_HARDWARE;
>> -
>> -	while (1) {
>> -		rc = opal_pci_poll(id);
>> -		if (rc <= 0)
>> -			break;
>> -
>> -		if (system_state < SYSTEM_RUNNING)
>> -			udelay(1000 * rc);
>> -		else
>> -			msleep(rc);
>> -	}
>> -
>> -	return rc;
>> -}
>> -
>>  int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
>>  {
>>  	struct pnv_phb *phb = hose->private_data;
>> @@ -542,13 +524,7 @@ int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
>>  	 * need the PCI bus settlement delay.
>>  	 */
>>  	rc = opal_pci_reset(phb->opal_id, reset_scope);
>> -	if (rc > 0)
>> -		rc = ioda_eeh_phb_poll(phb->opal_id);
>> -
>> -	if (rc != OPAL_SUCCESS)
>> -		return -EIO;
>> -
>> -	return 0;
>> +	return pnv_pci_poll(phb->opal_id, rc, NULL);
>>  }
>
>Patch does too mutch, the factoring out of pnv_pci_poll should be a
>separate patch.
>

Agree, I'll have separate patch for factoring pnv_pci_poll().

>>  static int __ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
>> @@ -641,13 +617,7 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
>>  	phb = hose->private_data;
>>  	id |= ((phb->opal_id << 16) | (dev->bus->number << 8) | (dev->devfn));
>>  	rc = opal_pci_reset(id, reset_scope);
>> -	if (rc > 0)
>> -		rc = ioda_eeh_phb_poll(id);
>> -
>> -	if (rc != OPAL_SUCCESS)
>> -		return -EIO;
>> -
>> -	return 0;
>> +	return pnv_pci_poll(id, rc, NULL);
>>  }
>>  
>>  static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data)
>> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
>> index b6474a1..b2c15ff 100644
>> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
>> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
>> @@ -249,3 +249,5 @@ OPAL_CALL(opal_handle_hmi,			OPAL_HANDLE_HMI);
>>  OPAL_CALL(opal_register_dump_region,		OPAL_REGISTER_DUMP_REGION);
>>  OPAL_CALL(opal_unregister_dump_region,		OPAL_UNREGISTER_DUMP_REGION);
>>  OPAL_CALL(opal_pci_set_phb_cxl_mode,		OPAL_PCI_SET_PHB_CXL_MODE);
>> +OPAL_CALL(opal_pci_get_power_status,		OPAL_PCI_GET_POWER_STATUS);
>> +OPAL_CALL(opal_pci_get_presence_status,		OPAL_PCI_GET_PRESENCE_STATUS);
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index b2187d0..f5bae4f 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -45,6 +45,44 @@
>>  #define cfg_dbg(fmt...)	do { } while(0)
>>  //#define cfg_dbg(fmt...)	printk(fmt)
>>  
>> +int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *pval)
>> +{
>> +	while (rval > 0) {
>> +		rval = opal_pci_poll(id, pval);
>> +		if (rval <= 0)
>> +			break;
>> +
>> +		if (system_state < SYSTEM_RUNNING)
>> +			udelay(1000 * rval);
>> +		else
>> +			msleep(rval);
>> +	}
>> +
>> +	return rval ? -EIO : 0;
>> +}
>
>Do we need the udelay case still nowadays ? When do we call this early
>enough that msleep won't work ?
>

The second kernel in kexec scenario issue reset on PHBs to eliminate
bogus PCI traffic. That time, msleep() can't work yet.

Thanks,
Gavin

>> +int pnv_pci_get_power_status(unsigned int index, unsigned char *status)
>> +{
>> +	uint64_t id = (0x1ul << 63);
>> +	long rc;
>> +
>> +	id |= index;
>> +	rc = opal_pci_get_power_status(id, status);
>> +	return pnv_pci_poll(id, rc, status);
>> +}
>> +EXPORT_SYMBOL_GPL(pnv_pci_get_power_status);
>> +
>> +int pnv_pci_get_presence_status(unsigned int index, unsigned char *status)
>> +{
>> +	uint64_t id = (0x1ul << 63);
>> +	long rc;
>> +
>> +	id |= index;
>> +	rc = opal_pci_get_presence_status(id, status);
>> +	return pnv_pci_poll(id, rc, status);
>> +}
>> +EXPORT_SYMBOL_GPL(pnv_pci_get_presence_status);
>> +
>>  #ifdef CONFIG_PCI_MSI
>>  static int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>>  {
>> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>> index 6c02ff8..396fe02 100644
>> --- a/arch/powerpc/platforms/powernv/pci.h
>> +++ b/arch/powerpc/platforms/powernv/pci.h
>> @@ -217,6 +217,7 @@ extern struct pci_ops pnv_pci_ops;
>>  extern struct pnv_eeh_ops ioda_eeh_ops;
>>  #endif
>>  
>> +int pnv_pci_poll(uint64_t id, int64_t rval, uint8_t *pval);
>>  void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
>>  				unsigned char *log_buff);
>>  int pnv_pci_cfg_read(struct device_node *dn,
>
>

^ permalink raw reply

* Re: [PATCH 4/8] PCI/hotplug/rpa: Code cleanup
From: Gavin Shan @ 2014-11-26  0:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <1416956551.5089.8.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 10:02:31AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
>> The patch applies code cleanup to RPA modules to address following
>> issues and it shouldn't affect the logic:
>> 
>>    * Coding style issue: removed unnecessary "break" for default case
>>      in switch statement and added default case; removed unnecessary
>>      braces or parenthese for if or return statements; removed
>>      unecessary return statements
>>    * Refactor rpaphp_get_sensor_state() and find_php_slot_pci_node()
>>      to avoid nested if statements
>>    * Drop is_registered(), is_php_type(), is_dlpar_capable()
>
>Why dropping those helpers ? Make them inline if you want to avoid
>polluting the namespace but I don't see what you gain in code
>readability by making the functions bigger...
>

Yeah, it's pointless to drop those functions and I'll keep them in
next revision.

Thanks,
Gavin

>Ben.
>
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/hotplug/rpadlpar_core.c | 90 +++++++++++++++++++------------------
>>  drivers/pci/hotplug/rpaphp_core.c   | 57 ++++++++++-------------
>>  drivers/pci/hotplug/rpaphp_pci.c    | 43 +++++++++---------
>>  drivers/pci/hotplug/rpaphp_slot.c   | 25 ++++-------
>>  4 files changed, 101 insertions(+), 114 deletions(-)
>> 
>> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
>> index 7660232..35da3b3 100644
>> --- a/drivers/pci/hotplug/rpadlpar_core.c
>> +++ b/drivers/pci/hotplug/rpadlpar_core.c
>> @@ -52,30 +52,35 @@ static struct device_node *find_vio_slot_node(char *drc_name)
>>  
>>  	while ((dn = of_get_next_child(parent, dn))) {
>>  		rc = rpaphp_get_drc_props(dn, NULL, &name, NULL, NULL);
>> -		if ((rc == 0) && (!strcmp(drc_name, name)))
>> -			break;
>> +		if (rc)
>> +			continue;
>> +
>> +		if (!strcmp(drc_name, name))
>> +			return dn;
>>  	}
>>  
>> -	return dn;
>> +	return NULL;
>>  }
>>  
>>  /* Find dlpar-capable pci node that contains the specified name and type */
>>  static struct device_node *find_php_slot_pci_node(char *drc_name,
>>  						  char *drc_type)
>>  {
>> -	struct device_node *np = NULL;
>> +	struct device_node *dn = NULL;
>>  	char *name;
>>  	char *type;
>>  	int rc;
>>  
>> -	while ((np = of_find_node_by_name(np, "pci"))) {
>> -		rc = rpaphp_get_drc_props(np, NULL, &name, &type, NULL);
>> -		if (rc == 0)
>> -			if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
>> -				break;
>> +	while ((dn = of_find_node_by_name(dn, "pci"))) {
>> +		rc = rpaphp_get_drc_props(dn, NULL, &name, &type, NULL);
>> +		if (rc)
>> +			continue;
>> +
>> +		if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
>> +			return dn;
>>  	}
>>  
>> -	return np;
>> +	return NULL;
>>  }
>>  
>>  static struct device_node *find_dlpar_node(char *drc_name, int *node_type)
>> @@ -239,10 +244,9 @@ static int dlpar_add_phb(char *drc_name, struct device_node *dn)
>>  {
>>  	struct pci_controller *phb;
>>  
>> -	if (PCI_DN(dn) && PCI_DN(dn)->phb) {
>> -		/* PHB already exists */
>> +	/* PHB already exists */
>> +	if (PCI_DN(dn) && PCI_DN(dn)->phb)
>>  		return -EINVAL;
>> -	}
>>  
>>  	phb = init_phb_dynamic(dn);
>>  	if (!phb)
>> @@ -299,15 +303,18 @@ int dlpar_add_slot(char *drc_name)
>>  	}
>>  
>>  	switch (node_type) {
>> -		case NODE_TYPE_VIO:
>> -			rc = dlpar_add_vio_slot(drc_name, dn);
>> -			break;
>> -		case NODE_TYPE_SLOT:
>> -			rc = dlpar_add_pci_slot(drc_name, dn);
>> -			break;
>> -		case NODE_TYPE_PHB:
>> -			rc = dlpar_add_phb(drc_name, dn);
>> -			break;
>> +	case NODE_TYPE_VIO:
>> +		rc = dlpar_add_vio_slot(drc_name, dn);
>> +		break;
>> +	case NODE_TYPE_SLOT:
>> +		rc = dlpar_add_pci_slot(drc_name, dn);
>> +		break;
>> +	case NODE_TYPE_PHB:
>> +		rc = dlpar_add_phb(drc_name, dn);
>> +		break;
>> +	default:
>> +		rc = -EINVAL;
>> +		goto exit;
>>  	}
>>  
>>  	printk(KERN_INFO "%s: slot %s added\n", DLPAR_MODULE_NAME, drc_name);
>> @@ -429,15 +436,18 @@ int dlpar_remove_slot(char *drc_name)
>>  	}
>>  
>>  	switch (node_type) {
>> -		case NODE_TYPE_VIO:
>> -			rc = dlpar_remove_vio_slot(drc_name, dn);
>> -			break;
>> -		case NODE_TYPE_PHB:
>> -			rc = dlpar_remove_phb(drc_name, dn);
>> -			break;
>> -		case NODE_TYPE_SLOT:
>> -			rc = dlpar_remove_pci_slot(drc_name, dn);
>> -			break;
>> +	case NODE_TYPE_VIO:
>> +		rc = dlpar_remove_vio_slot(drc_name, dn);
>> +		break;
>> +	case NODE_TYPE_PHB:
>> +		rc = dlpar_remove_phb(drc_name, dn);
>> +		break;
>> +	case NODE_TYPE_SLOT:
>> +		rc = dlpar_remove_pci_slot(drc_name, dn);
>> +		break;
>> +	default:
>> +		rc = -EINVAL;
>> +		goto exit;
>>  	}
>>  	vm_unmap_aliases();
>>  
>> @@ -447,20 +457,15 @@ exit:
>>  	return rc;
>>  }
>>  
>> -static inline int is_dlpar_capable(void)
>> -{
>> -	int rc = rtas_token("ibm,configure-connector");
>> -
>> -	return (int) (rc != RTAS_UNKNOWN_SERVICE);
>> -}
>> -
>> -int __init rpadlpar_io_init(void)
>> +static int __init rpadlpar_io_init(void)
>>  {
>>  	int rc = 0;
>>  
>> -	if (!is_dlpar_capable()) {
>> +	/* Check if we have DLPAR capability */
>> +	rc = rtas_token("ibm,configure-connector");
>> +	if (rc == RTAS_UNKNOWN_SERVICE) {
>>  		printk(KERN_WARNING "%s: partition not DLPAR capable\n",
>> -			__func__);
>> +		       __func__);
>>  		return -EPERM;
>>  	}
>>  
>> @@ -468,10 +473,9 @@ int __init rpadlpar_io_init(void)
>>  	return rc;
>>  }
>>  
>> -void rpadlpar_io_exit(void)
>> +static void __exit rpadlpar_io_exit(void)
>>  {
>>  	dlpar_sysfs_exit();
>> -	return;
>>  }
>>  
>>  module_init(rpadlpar_io_init);
>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>> index f2945fa..ff800df 100644
>> --- a/drivers/pci/hotplug/rpaphp_core.c
>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>> @@ -74,7 +74,6 @@ static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
>>  		break;
>>  	default:
>>  		value = 1;
>> -		break;
>>  	}
>>  
>>  	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
>> @@ -94,7 +93,7 @@ static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
>>  	int retval, level;
>>  	struct slot *slot = (struct slot *)hotplug_slot->private;
>>  
>> -	retval = rtas_get_power_level (slot->power_domain, &level);
>> +	retval = rtas_get_power_level(slot->power_domain, &level);
>>  	if (!retval)
>>  		*value = level;
>>  	return retval;
>> @@ -161,7 +160,6 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>>  		break;
>>  	default:
>>  		speed = PCI_SPEED_UNKNOWN;
>> -		break;
>>  	}
>>  
>>  	return speed;
>> @@ -178,17 +176,17 @@ static int get_children_props(struct device_node *dn, const int **drc_indexes,
>>  	types = of_get_property(dn, "ibm,drc-types", NULL);
>>  	domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>>  
>> -	if (!indexes || !names || !types || !domains) {
>> -		/* Slot does not have dynamically-removable children */
>> +	/* Slot does not have dynamically-removable children */
>> +	if (!indexes || !names || !types || !domains)
>>  		return -EINVAL;
>> -	}
>> +
>>  	if (drc_indexes)
>>  		*drc_indexes = indexes;
>> +	/* &drc_names[1] contains NULL terminated slot names */
>>  	if (drc_names)
>> -		/* &drc_names[1] contains NULL terminated slot names */
>>  		*drc_names = names;
>> +	/* &drc_types[1] contains NULL terminated slot types */
>>  	if (drc_types)
>> -		/* &drc_types[1] contains NULL terminated slot types */
>>  		*drc_types = types;
>>  	if (drc_power_domains)
>>  		*drc_power_domains = domains;
>> @@ -210,15 +208,13 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>>  	int i, rc;
>>  
>>  	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> -	if (!my_index) {
>> -		/* Node isn't DLPAR/hotplug capable */
>> +	/* Node isn't DLPAR/hotplug capable */
>> +	if (!my_index)
>>  		return -EINVAL;
>> -	}
>>  
>>  	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>> -	if (rc < 0) {
>> +	if (rc < 0)
>>  		return -EINVAL;
>> -	}
>>  
>>  	name_tmp = (char *) &names[1];
>>  	type_tmp = (char *) &types[1];
>> @@ -244,21 +240,8 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>>  }
>>  EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>>  
>> -static int is_php_type(char *drc_type)
>> -{
>> -	unsigned long value;
>> -	char *endptr;
>> -
>> -	/* PCI Hotplug nodes have an integer for drc_type */
>> -	value = simple_strtoul(drc_type, &endptr, 10);
>> -	if (endptr == drc_type)
>> -		return 0;
>> -
>> -	return 1;
>> -}
>> -
>>  /**
>> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>> + * is_php_dn() - return true if this is a hotpluggable pci slot, else false
>>   * @dn: target &device_node
>>   * @indexes: passed to get_children_props()
>>   * @names: passed to get_children_props()
>> @@ -270,21 +253,28 @@ static int is_php_type(char *drc_type)
>>   * for built-in pci slots (even when the built-in slots are
>>   * dlparable.)
>>   */
>> -static int is_php_dn(struct device_node *dn, const int **indexes,
>> -		const int **names, const int **types, const int **power_domains)
>> +static bool is_php_dn(struct device_node *dn,
>> +		      const int **indexes, const int **names,
>> +		      const int **types, const int **power_domains)
>>  {
>>  	const int *drc_types;
>> +	const char *drc_type_str;
>> +	char *endptr;
>> +	unsigned long val;
>>  	int rc;
>>  
>>  	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>>  	if (rc < 0)
>> -		return 0;
>> +		return false;
>>  
>> -	if (!is_php_type((char *) &drc_types[1]))
>> -		return 0;
>> +	/* PCI Hotplug nodes have an integer for drc_type */
>> +	drc_type_str = (char *)&drc_types[1];
>> +	val = simple_strtoul(drc_type_str, &endptr, 10);
>> +	if (endptr == drc_type_str)
>> +		return false;
>>  
>>  	*types = drc_types;
>> -	return 1;
>> +	return true;
>>  }
>>  
>>  /**
>> @@ -370,7 +360,6 @@ static void __exit cleanup_slots(void)
>>  		list_del(&slot->rpaphp_slot_list);
>>  		pci_hp_deregister(slot->hotplug_slot);
>>  	}
>> -	return;
>>  }
>>  
>>  static int __init rpaphp_init(void)
>> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
>> index 9243f3e7..a4aa65c 100644
>> --- a/drivers/pci/hotplug/rpaphp_pci.c
>> +++ b/drivers/pci/hotplug/rpaphp_pci.c
>> @@ -38,30 +38,31 @@ int rpaphp_get_sensor_state(struct slot *slot, int *state)
>>  	int setlevel;
>>  
>>  	rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
>> +	if (rc >= 0)
>> +		return rc;
>> +	if (rc != -EFAULT && rc != -EEXIST) {
>> +		err("%s: Failure %d getting sensor state on slot[%s]\n",
>> +		    __func__, rc, slot->name);
>> +		return rc;
>> +	}
>>  
>> +
>> +	/*
>> +	 * Some slots have to be powered up before
>> +	 * get-sensor will succeed
>> +	 */
>> +	dbg("%s: Slot[%s] must be power up to get sensor-state\n",
>> +	    __func__, slot->name);
>> +	rc = rtas_set_power_level(slot->power_domain, POWER_ON,
>> +				  &setlevel);
>>  	if (rc < 0) {
>> -		if (rc == -EFAULT || rc == -EEXIST) {
>> -			dbg("%s: slot must be power up to get sensor-state\n",
>> -			    __func__);
>> -
>> -			/* some slots have to be powered up
>> -			 * before get-sensor will succeed.
>> -			 */
>> -			rc = rtas_set_power_level(slot->power_domain, POWER_ON,
>> -						  &setlevel);
>> -			if (rc < 0) {
>> -				dbg("%s: power on slot[%s] failed rc=%d.\n",
>> -				    __func__, slot->name, rc);
>> -			} else {
>> -				rc = rtas_get_sensor(DR_ENTITY_SENSE,
>> -						     slot->index, state);
>> -			}
>> -		} else if (rc == -ENODEV)
>> -			info("%s: slot is unusable\n", __func__);
>> -		else
>> -			err("%s failed to get sensor state\n", __func__);
>> +		dbg("%s: Failure %d powerng on slot[%s]\n",
>> +		    __func__, rc, slot->name);
>> +		return rc;
>>  	}
>> -	return rc;
>> +
>> +	return rtas_get_sensor(DR_ENTITY_SENSE,
>> +			       slot->index, state);
>>  }
>>  
>>  /**
>> diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
>> index a6082cc..be48e69 100644
>> --- a/drivers/pci/hotplug/rpaphp_slot.c
>> +++ b/drivers/pci/hotplug/rpaphp_slot.c
>> @@ -72,7 +72,7 @@ struct slot *alloc_slot_struct(struct device_node *dn,
>>  	slot->hotplug_slot->ops = &rpaphp_hotplug_slot_ops;
>>  	slot->hotplug_slot->release = &rpaphp_release_slot;
>>  
>> -	return (slot);
>> +	return slot;
>>  
>>  error_info:
>>  	kfree(slot->hotplug_slot->info);
>> @@ -84,17 +84,6 @@ error_nomem:
>>  	return NULL;
>>  }
>>  
>> -static int is_registered(struct slot *slot)
>> -{
>> -	struct slot *tmp_slot;
>> -
>> -	list_for_each_entry(tmp_slot, &rpaphp_slot_head, rpaphp_slot_list) {
>> -		if (!strcmp(tmp_slot->name, slot->name))
>> -			return 1;
>> -	}
>> -	return 0;
>> -}
>> -
>>  int rpaphp_deregister_slot(struct slot *slot)
>>  {
>>  	int retval = 0;
>> @@ -117,6 +106,7 @@ EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>>  int rpaphp_register_slot(struct slot *slot)
>>  {
>>  	struct hotplug_slot *php_slot = slot->hotplug_slot;
>> +	struct slot *tmp;
>>  	int retval;
>>  	int slotno;
>>  
>> @@ -124,10 +114,13 @@ int rpaphp_register_slot(struct slot *slot)
>>  		__func__, slot->dn->full_name, slot->index, slot->name,
>>  		slot->power_domain, slot->type);
>>  
>> -	/* should not try to register the same slot twice */
>> -	if (is_registered(slot)) {
>> -		err("rpaphp_register_slot: slot[%s] is already registered\n", slot->name);
>> -		return -EAGAIN;
>> +	/* Should not try to register the same slot twice */
>> +	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
>> +		if (!strcmp(tmp->name, slot->name)) {
>> +			err("%s: Slot[%s] is already registered\n",
>> +			    __func__, slot->name);
>> +			return -EAGAIN;
>> +		}
>>  	}
>>  
>>  	if (slot->dn->child)
>
>

^ permalink raw reply

* Re: [PATCH 6/8] PCI/hotplug/rpa: Abstract slot operations
From: Gavin Shan @ 2014-11-26  0:01 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <1416956676.5089.10.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 10:04:36AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
>> The patch splits the code into 2 parts: RPA PCI hotplug slot
>> management and RTAS backend. It enables us to support PowerNV,
>> which is built on top of OPAL firmware in future.
>> 
>> The patch also refactors the code for a bit:
>> 
>>     * Rename "struct slot" to "struct rpa_php_slot"
>>     * All macros have prefix "RPA_PHP_SLOT"
>>     * rpaphp_slot.c is removed and all logics moved to rpaphp_core.c
>
>
>I don't see the point of this. rpaphp is already itself a "backend", so
>we end up with yet another layer.
>

Yes, that's what I did.

>Just create a powernv-php or opal-php and if there is common code,
>factor it into a common helper but I wouldn't bother too much initially
>unless there is a lot of it.
>

Ok. I'll create powernv-php.

Thanks,
Gavin

>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/hotplug/Makefile        |   3 +-
>>  drivers/pci/hotplug/rpadlpar_core.c |  10 +-
>>  drivers/pci/hotplug/rpaphp.h        |  64 +++----
>>  drivers/pci/hotplug/rpaphp_core.c   | 347 +++++++++++-------------------------
>>  drivers/pci/hotplug/rpaphp_pci.c    | 136 --------------
>>  drivers/pci/hotplug/rpaphp_rtas.c   | 320 +++++++++++++++++++++++++++++++++
>>  drivers/pci/hotplug/rpaphp_slot.c   | 140 ---------------
>>  7 files changed, 459 insertions(+), 561 deletions(-)
>>  delete mode 100644 drivers/pci/hotplug/rpaphp_pci.c
>>  create mode 100644 drivers/pci/hotplug/rpaphp_rtas.c
>>  delete mode 100644 drivers/pci/hotplug/rpaphp_slot.c
>> 
>> diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
>> index 4a9aa08..630313da 100644
>> --- a/drivers/pci/hotplug/Makefile
>> +++ b/drivers/pci/hotplug/Makefile
>> @@ -51,8 +51,7 @@ acpiphp-objs		:=	acpiphp_core.o	\
>>  				acpiphp_glue.o
>>  
>>  rpaphp-objs		:=	rpaphp_core.o	\
>> -				rpaphp_pci.o	\
>> -				rpaphp_slot.o
>> +				rpaphp_rtas.o
>>  
>>  rpadlpar_io-objs	:=	rpadlpar_core.o \
>>  				rpadlpar_sysfs.o
>> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
>> index 35da3b3..a36d2c9 100644
>> --- a/drivers/pci/hotplug/rpadlpar_core.c
>> +++ b/drivers/pci/hotplug/rpadlpar_core.c
>> @@ -117,13 +117,13 @@ static struct device_node *find_dlpar_node(char *drc_name, int *node_type)
>>   * may be dlpar-able, but not hot-pluggable, so this routine
>>   * will return NULL for built-in PCI slots.
>>   */
>> -static struct slot *find_php_slot(struct device_node *dn)
>> +static struct rpa_php_slot *find_php_slot(struct device_node *dn)
>>  {
>>  	struct list_head *tmp, *n;
>> -	struct slot *slot;
>> +	struct rpa_php_slot *slot;
>>  
>>  	list_for_each_safe(tmp, n, &rpaphp_slot_head) {
>> -		slot = list_entry(tmp, struct slot, rpaphp_slot_list);
>> +		slot = list_entry(tmp, struct rpa_php_slot, rpaphp_slot_list);
>>  		if (slot->dn == dn)
>>  			return slot;
>>  	}
>> @@ -214,7 +214,7 @@ static int dlpar_add_pci_slot(char *drc_name, struct device_node *dn)
>>  
>>  static int dlpar_remove_phb(char *drc_name, struct device_node *dn)
>>  {
>> -	struct slot *slot;
>> +	struct rpa_php_slot *slot;
>>  	struct pci_dn *pdn;
>>  	int rc = 0;
>>  
>> @@ -359,7 +359,7 @@ static int dlpar_remove_vio_slot(char *drc_name, struct device_node *dn)
>>  int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn)
>>  {
>>  	struct pci_bus *bus;
>> -	struct slot *slot;
>> +	struct rpa_php_slot *slot;
>>  	int ret = 0;
>>  
>>  	pci_lock_rescan_remove();
>> diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
>> index 39ddbdf..09dd516 100644
>> --- a/drivers/pci/hotplug/rpaphp.h
>> +++ b/drivers/pci/hotplug/rpaphp.h
>> @@ -30,21 +30,6 @@
>>  #include <linux/pci.h>
>>  #include <linux/pci_hotplug.h>
>>  
>> -#define DR_INDICATOR 9002
>> -#define DR_ENTITY_SENSE 9003
>> -
>> -#define POWER_ON	100
>> -#define POWER_OFF	0
>> -
>> -#define LED_OFF		0
>> -#define LED_ON		1	/* continuous on */
>> -#define LED_ID		2	/* slow blinking */
>> -#define LED_ACTION	3	/* fast blinking */
>> -
>> -/* Sensor values from rtas_get-sensor */
>> -#define EMPTY           0	/* No card in slot */
>> -#define PRESENT         1	/* Card in slot */
>> -
>>  #define MY_NAME "rpaphp"
>>  extern bool rpaphp_debug;
>>  #define dbg(format, arg...)					\
>> @@ -57,19 +42,26 @@ extern bool rpaphp_debug;
>>  #define info(format, arg...) printk(KERN_INFO "%s: " format, MY_NAME , ## arg)
>>  #define warn(format, arg...) printk(KERN_WARNING "%s: " format, MY_NAME , ## arg)
>>  
>> -/* slot states */
>> +/* Power */
>> +#define RPA_PHP_SLOT_POWER_ON	1	/* On            */
>> +#define RPA_PHP_SLOT_POWER_OFF	0	/* Off           */
>>  
>> -#define	NOT_VALID	3
>> -#define	NOT_CONFIGURED	2
>> -#define	CONFIGURED	1
>> -#define	EMPTY		0
>> +/* Attention */
>> +#define RPA_PHP_SLOT_ATTEN_OFF	0	/* Off           */
>> +#define RPA_PHP_SLOT_ATTEN_ON	1	/* On            */
>> +#define RPA_PHP_SLOT_ATTEN_IND	2	/* Slow blinking */
>> +#define RPA_PHP_SLOT_ATTEN_ACT	3	/* Fast blinking */
>>  
>> -/*
>> - * struct slot - slot information for each *physical* slot
>> - */
>> -struct slot {
>> +/* Presence */
>> +#define RPA_PHP_SLOT_EMPTY	0	/* No card       */
>> +#define RPA_PHP_SLOT_PRESENT	1	/* Presented     */
>> +
>> +struct rpa_php_slot {
>>  	struct list_head rpaphp_slot_list;
>>  	int state;
>> +#define RPA_PHP_SLOT_NOT_CONFIGURED	0
>> +#define RPA_PHP_SLOT_CONFIGURED		1
>> +#define RPA_PHP_SLOT_NOT_VALID		2
>>  	u32 index;
>>  	u32 type;
>>  	u32 power_domain;
>> @@ -80,24 +72,20 @@ struct slot {
>>  	struct hotplug_slot *hotplug_slot;
>>  };
>>  
>> -extern struct hotplug_slot_ops rpaphp_hotplug_slot_ops;
>>  extern struct list_head rpaphp_slot_head;
>>  
>> -/* function prototypes */
>> -
>> -/* rpaphp_pci.c */
>> -int rpaphp_enable_slot(struct slot *slot);
>> -int rpaphp_get_sensor_state(struct slot *slot, int *state);
>> -
>>  /* rpaphp_core.c */
>>  int rpaphp_add_slot(struct device_node *dn);
>> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> -		char **drc_name, char **drc_type, int *drc_power);
>> +void dealloc_slot_struct(struct rpa_php_slot *slot);
>> +struct rpa_php_slot *alloc_slot_struct(struct device_node *dn, int drc_index,
>> +				       char *drc_name, int power_domain);
>> +int rpaphp_register_slot(struct rpa_php_slot *slot);
>> +int rpaphp_deregister_slot(struct rpa_php_slot *slot);
>> +int rpaphp_add_slot(struct device_node *dn);
>>  
>> -/* rpaphp_slot.c */
>> -void dealloc_slot_struct(struct slot *slot);
>> -struct slot *alloc_slot_struct(struct device_node *dn, int drc_index, char *drc_name, int power_domain);
>> -int rpaphp_register_slot(struct slot *slot);
>> -int rpaphp_deregister_slot(struct slot *slot);
>> +/* rpaphp_rtas.c */
>> +int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> +			 char **drc_name, char **drc_type, int *drc_power);
>> +struct rpa_php_slot *rpaphp_rtas_add_slot(struct device_node *dn);
>>  
>>  #endif				/* _PPC64PHP_H */
>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>> index a639c5c..91eff8f 100644
>> --- a/drivers/pci/hotplug/rpaphp_core.c
>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>> @@ -45,194 +45,118 @@ EXPORT_SYMBOL_GPL(rpaphp_slot_head);
>>  #define DRIVER_AUTHOR	"Linda Xie <lxie@us.ibm.com>"
>>  #define DRIVER_DESC	"RPA HOT Plug PCI Controller Driver"
>>  
>> -#define MAX_LOC_CODE 128
>> -
>> -MODULE_AUTHOR(DRIVER_AUTHOR);
>> -MODULE_DESCRIPTION(DRIVER_DESC);
>> -MODULE_LICENSE("GPL");
>> -
>>  module_param_named(debug, rpaphp_debug, bool, 0644);
>>  
>> -/**
>> - * set_attention_status - set attention LED
>> - * @hotplug_slot: target &hotplug_slot
>> - * @value: LED control value
>> - *
>> - * echo 0 > attention -- set LED OFF
>> - * echo 1 > attention -- set LED ON
>> - * echo 2 > attention -- set LED ID(identify, light is blinking)
>> - */
>> -static int set_attention_status(struct hotplug_slot *hotplug_slot, u8 value)
>> +/* free up the memory used by a slot */
>> +static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
>>  {
>> -	int rc;
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -
>> -	switch (value) {
>> -	case 0:
>> -	case 1:
>> -	case 2:
>> -		break;
>> -	default:
>> -		value = 1;
>> -	}
>> +	struct rpa_php_slot *slot = hotplug_slot->private;
>>  
>> -	rc = rtas_set_indicator(DR_INDICATOR, slot->index, value);
>> -	if (!rc)
>> -		hotplug_slot->info->attention_status = value;
>> -
>> -	return rc;
>> +	dealloc_slot_struct(slot);
>>  }
>>  
>> -/**
>> - * get_power_status - get power status of a slot
>> - * @hotplug_slot: slot to get status
>> - * @value: pointer to store status
>> - */
>> -static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)
>> +void dealloc_slot_struct(struct rpa_php_slot *slot)
>>  {
>> -	int retval, level;
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -
>> -	retval = rtas_get_power_level(slot->power_domain, &level);
>> -	if (!retval)
>> -		*value = level;
>> -	return retval;
>> +	kfree(slot->hotplug_slot->info);
>> +	kfree(slot->name);
>> +	kfree(slot->hotplug_slot);
>> +	kfree(slot);
>>  }
>>  
>> -/**
>> - * get_attention_status - get attention LED status
>> - * @hotplug_slot: slot to get status
>> - * @value: pointer to store status
>> - */
>> -static int get_attention_status(struct hotplug_slot *hotplug_slot, u8 *value)
>> +struct rpa_php_slot *alloc_slot_struct(struct device_node *dn,
>> +				       int drc_index, char *drc_name,
>> +				       int power_domain)
>>  {
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -	*value = slot->hotplug_slot->info->attention_status;
>> -	return 0;
>> -}
>> -
>> -static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
>> -{
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -	int rc, state;
>> -
>> -	rc = rpaphp_get_sensor_state(slot, &state);
>> -
>> -	*value = NOT_VALID;
>> -	if (rc)
>> -		return rc;
>> +	struct rpa_php_slot *slot;
>>  
>> -	if (state == EMPTY)
>> -		*value = EMPTY;
>> -	else if (state == PRESENT)
>> -		*value = slot->state;
>> -
>> -	return 0;
>> +	slot = kzalloc(sizeof(*slot), GFP_KERNEL);
>> +	if (!slot)
>> +		goto error_nomem;
>> +	slot->hotplug_slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
>> +	if (!slot->hotplug_slot)
>> +		goto error_slot;
>> +	slot->hotplug_slot->info = kzalloc(sizeof(struct hotplug_slot_info),
>> +					   GFP_KERNEL);
>> +	if (!slot->hotplug_slot->info)
>> +		goto error_hpslot;
>> +	slot->name = kstrdup(drc_name, GFP_KERNEL);
>> +	if (!slot->name)
>> +		goto error_info;
>> +	slot->dn = dn;
>> +	slot->index = drc_index;
>> +	slot->power_domain = power_domain;
>> +	slot->hotplug_slot->private = slot;
>> +	slot->hotplug_slot->release = &rpaphp_release_slot;
>> +
>> +	slot->hotplug_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
>> +	slot->hotplug_slot->info->attention_status = RPA_PHP_SLOT_ATTEN_OFF;
>> +	slot->hotplug_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
>> +	slot->state = RPA_PHP_SLOT_NOT_VALID;
>> +
>> +	return slot;
>> +
>> +error_info:
>> +	kfree(slot->hotplug_slot->info);
>> +error_hpslot:
>> +	kfree(slot->hotplug_slot);
>> +error_slot:
>> +	kfree(slot);
>> +error_nomem:
>> +	return NULL;
>>  }
>>  
>> -static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>> +int rpaphp_register_slot(struct rpa_php_slot *slot)
>>  {
>> -	enum pci_bus_speed speed;
>> -	switch (slot->type) {
>> -	case 1:
>> -	case 2:
>> -	case 3:
>> -	case 4:
>> -	case 5:
>> -	case 6:
>> -		speed = PCI_SPEED_33MHz;	/* speed for case 1-6 */
>> -		break;
>> -	case 7:
>> -	case 8:
>> -		speed = PCI_SPEED_66MHz;
>> -		break;
>> -	case 11:
>> -	case 14:
>> -		speed = PCI_SPEED_66MHz_PCIX;
>> -		break;
>> -	case 12:
>> -	case 15:
>> -		speed = PCI_SPEED_100MHz_PCIX;
>> -		break;
>> -	case 13:
>> -	case 16:
>> -		speed = PCI_SPEED_133MHz_PCIX;
>> -		break;
>> -	default:
>> -		speed = PCI_SPEED_UNKNOWN;
>> +	struct hotplug_slot *php_slot = slot->hotplug_slot;
>> +	struct rpa_php_slot *tmp;
>> +	int slotno, retval;
>> +
>> +	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
>> +	    __func__, slot->dn->full_name, slot->index, slot->name,
>> +	    slot->power_domain, slot->type);
>> +
>> +	/* Should not try to register the same slot twice */
>> +	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
>> +		if (!strcmp(tmp->name, slot->name)) {
>> +			err("%s: Slot[%s] is already registered\n",
>> +			    __func__, slot->name);
>> +			return -EAGAIN;
>> +		}
>> +	}
>> +	if (slot->dn->child)
>> +		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
>> +	else
>> +		slotno = -1;
>> +	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
>> +	if (retval) {
>> +		err("pci_hp_register failed with error %d\n", retval);
>> +		return retval;
>>  	}
>>  
>> -	return speed;
>> +	/* add slot to our internal list */
>> +	list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
>> +	info("Slot [%s] registered\n", slot->name);
>> +	return 0;
>>  }
>>  
>> -static int parse_drc_props(struct device_node *dn, u32 drc_index,
>> -			   char **drc_name, char **drc_type, u32 *drc_power)
>> +int rpaphp_deregister_slot(struct rpa_php_slot *slot)
>>  {
>> -	const u32 *indexes, *names, *types, *domains;
>> -	char *name, *type;
>> -	struct device_node *parent = dn;
>> -	u32 i;
>> -
>> -	while ((parent = of_get_parent(parent))) {
>> -		indexes = of_get_property(parent, "ibm,drc-indexes", NULL);
>> -		names   = of_get_property(parent, "ibm,drc-names", NULL);
>> -		types   = of_get_property(parent, "ibm,drc-types", NULL);
>> -		domains = of_get_property(parent, "ibm,drc-power-domains", NULL);
>> -
>> -		if (!indexes || !names || !types || !domains) {
>> -			of_node_put(parent);
>> -			continue;
>> -		}
>> +	struct hotplug_slot *php_slot = slot->hotplug_slot;
>> +	int retval = 0;
>>  
>> -		name = (char *)&names[1];
>> -		type = (char *)&types[1];
>> -		for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> -			if (be32_to_cpu(indexes[i + 1]) != drc_index) {
>> -				name += (strlen(name) + 1);
>> -				type += (strlen(type) + 1);
>> -				continue;
>> -			}
>> -
>> -			/* Matched index */
>> -			if (drc_name)
>> -				*drc_name = name;
>> -			if (drc_type)
>> -				*drc_type = type;
>> -			if (drc_power)
>> -				*drc_power = be32_to_cpu(domains[i + 1]);
>> -
>> -			of_node_put(parent);
>> -			return 0;
>> -		}
>> -
>> -		/* Next level parent */
>> -		of_node_put(parent);
>> -	}
>> +	dbg("%s - Entry: deregistering slot=%s\n",
>> +	    __func__, slot->name);
>>  
>> -	return -ENODEV;
>> -}
>> +	list_del(&slot->rpaphp_slot_list);
>>  
>> -/*
>> - * To get the DRC props describing the current node, first obtain it's
>> - * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
>> - * the my-drc-index for correlation, and obtain the requested properties.
>> - */
>> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> -			 char **drc_name, char **drc_type, int *drc_power)
>> -{
>> -	const u32 *my_index;
>> -
>> -	/* Check if node is capable of hotplug */
>> -	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> -	if (!my_index)
>> -		return -EINVAL;
>> -	if (drc_index)
>> -		*drc_index = be32_to_cpu(*my_index);
>> +	retval = pci_hp_deregister(php_slot);
>> +	if (retval)
>> +		err("Problem unregistering a slot %s\n", slot->name);
>>  
>> -	return parse_drc_props(dn, be32_to_cpu(*my_index),
>> -			       drc_name, drc_type, drc_power);
>> +	dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> +	return retval;
>>  }
>> -EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>> +EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>>  
>>  /**
>>   * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>> @@ -252,29 +176,22 @@ EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>>   */
>>  int rpaphp_add_slot(struct device_node *dn)
>>  {
>> -	char *name, *type, *endptr;
>> -	int index, power_domain;
>> -	struct slot *slot;
>> -	int val, ret;
>> -
>> -	/* Get and parse the hotplug properties */
>> -	ret = rpaphp_get_drc_props(dn, &index, &name, &type, &power_domain);
>> -	if (ret)
>> -		return ret;
>> -
>> -	/* PCI Hotplug nodes have an integer for drc_type */
>> -	val = simple_strtoul(type, &endptr, 10);
>> -	if (endptr == type)
>> -		return -EINVAL;
>> +	struct rpa_php_slot *slot = NULL;
>> +	int ret;
>>  
>> -	slot = alloc_slot_struct(dn, index, name, power_domain);
>> +	/* Create slot */
>> +	if (machine_is(pseries))
>> +		slot = rpaphp_rtas_add_slot(dn);
>>  	if (!slot)
>> -		return -ENOMEM;
>> +		return -EIO;
>>  
>> -	slot->type = val;
>> -	ret = rpaphp_enable_slot(slot);
>> -	if (!ret)
>> -		ret = rpaphp_register_slot(slot);
>> +	/* Enable slot */
>> +	ret = slot->hotplug_slot->ops->enable_slot(slot->hotplug_slot);
>> +	if (ret)
>> +		goto fail;
>> +
>> +	/* Register slot */
>> +	ret = rpaphp_register_slot(slot);
>>  	if (ret)
>>  		goto fail;
>>  
>> @@ -288,7 +205,7 @@ EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>>  static void __exit cleanup_slots(void)
>>  {
>>  	struct list_head *tmp, *n;
>> -	struct slot *slot;
>> +	struct rpa_php_slot *slot;
>>  
>>  	/*
>>  	 * Unregister all of our slots with the pci_hotplug subsystem,
>> @@ -297,7 +214,7 @@ static void __exit cleanup_slots(void)
>>  	 */
>>  
>>  	list_for_each_safe(tmp, n, &rpaphp_slot_head) {
>> -		slot = list_entry(tmp, struct slot, rpaphp_slot_list);
>> +		slot = list_entry(tmp, struct rpa_php_slot, rpaphp_slot_list);
>>  		list_del(&slot->rpaphp_slot_list);
>>  		pci_hp_deregister(slot->hotplug_slot);
>>  	}
>> @@ -320,59 +237,9 @@ static void __exit rpaphp_exit(void)
>>  	cleanup_slots();
>>  }
>>  
>> -static int enable_slot(struct hotplug_slot *hotplug_slot)
>> -{
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -	int state;
>> -	int retval;
>> -
>> -	if (slot->state == CONFIGURED)
>> -		return 0;
>> -
>> -	retval = rpaphp_get_sensor_state(slot, &state);
>> -	if (retval)
>> -		return retval;
>> -
>> -	if (state == PRESENT) {
>> -		pci_lock_rescan_remove();
>> -		pcibios_add_pci_devices(slot->bus);
>> -		pci_unlock_rescan_remove();
>> -		slot->state = CONFIGURED;
>> -	} else if (state == EMPTY) {
>> -		slot->state = EMPTY;
>> -	} else {
>> -		err("%s: slot[%s] is in invalid state\n", __func__, slot->name);
>> -		slot->state = NOT_VALID;
>> -		return -EINVAL;
>> -	}
>> -
>> -	slot->bus->max_bus_speed = get_max_bus_speed(slot);
>> -	return 0;
>> -}
>> -
>> -static int disable_slot(struct hotplug_slot *hotplug_slot)
>> -{
>> -	struct slot *slot = (struct slot *)hotplug_slot->private;
>> -	if (slot->state == NOT_CONFIGURED)
>> -		return -EINVAL;
>> -
>> -	pci_lock_rescan_remove();
>> -	pcibios_remove_pci_devices(slot->bus);
>> -	pci_unlock_rescan_remove();
>> -	vm_unmap_aliases();
>> -
>> -	slot->state = NOT_CONFIGURED;
>> -	return 0;
>> -}
>> -
>> -struct hotplug_slot_ops rpaphp_hotplug_slot_ops = {
>> -	.enable_slot = enable_slot,
>> -	.disable_slot = disable_slot,
>> -	.set_attention_status = set_attention_status,
>> -	.get_power_status = get_power_status,
>> -	.get_attention_status = get_attention_status,
>> -	.get_adapter_status = get_adapter_status,
>> -};
>> -
>>  module_init(rpaphp_init);
>>  module_exit(rpaphp_exit);
>> +
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
>> deleted file mode 100644
>> index a4aa65c..0000000
>> --- a/drivers/pci/hotplug/rpaphp_pci.c
>> +++ /dev/null
>> @@ -1,136 +0,0 @@
>> -/*
>> - * PCI Hot Plug Controller Driver for RPA-compliant PPC64 platform.
>> - * Copyright (C) 2003 Linda Xie <lxie@us.ibm.com>
>> - *
>> - * All rights reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or modify
>> - * it under the terms of the GNU General Public License as published by
>> - * the Free Software Foundation; either version 2 of the License, or (at
>> - * your option) any later version.
>> - *
>> - * This program is distributed in the hope that it will be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> - * NON INFRINGEMENT.  See the GNU General Public License for more
>> - * details.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program; if not, write to the Free Software
>> - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> - *
>> - * Send feedback to <lxie@us.ibm.com>
>> - *
>> - */
>> -#include <linux/pci.h>
>> -#include <linux/string.h>
>> -
>> -#include <asm/pci-bridge.h>
>> -#include <asm/rtas.h>
>> -#include <asm/machdep.h>
>> -
>> -#include "../pci.h"		/* for pci_add_new_bus */
>> -#include "rpaphp.h"
>> -
>> -int rpaphp_get_sensor_state(struct slot *slot, int *state)
>> -{
>> -	int rc;
>> -	int setlevel;
>> -
>> -	rc = rtas_get_sensor(DR_ENTITY_SENSE, slot->index, state);
>> -	if (rc >= 0)
>> -		return rc;
>> -	if (rc != -EFAULT && rc != -EEXIST) {
>> -		err("%s: Failure %d getting sensor state on slot[%s]\n",
>> -		    __func__, rc, slot->name);
>> -		return rc;
>> -	}
>> -
>> -
>> -	/*
>> -	 * Some slots have to be powered up before
>> -	 * get-sensor will succeed
>> -	 */
>> -	dbg("%s: Slot[%s] must be power up to get sensor-state\n",
>> -	    __func__, slot->name);
>> -	rc = rtas_set_power_level(slot->power_domain, POWER_ON,
>> -				  &setlevel);
>> -	if (rc < 0) {
>> -		dbg("%s: Failure %d powerng on slot[%s]\n",
>> -		    __func__, rc, slot->name);
>> -		return rc;
>> -	}
>> -
>> -	return rtas_get_sensor(DR_ENTITY_SENSE,
>> -			       slot->index, state);
>> -}
>> -
>> -/**
>> - * rpaphp_enable_slot - record slot state, config pci device
>> - * @slot: target &slot
>> - *
>> - * Initialize values in the slot, and the hotplug_slot info
>> - * structures to indicate if there is a pci card plugged into
>> - * the slot. If the slot is not empty, run the pcibios routine
>> - * to get pcibios stuff correctly set up.
>> - */
>> -int rpaphp_enable_slot(struct slot *slot)
>> -{
>> -	int rc, level, state;
>> -	struct pci_bus *bus;
>> -	struct hotplug_slot_info *info = slot->hotplug_slot->info;
>> -
>> -	info->adapter_status = NOT_VALID;
>> -	slot->state = EMPTY;
>> -
>> -	/* Find out if the power is turned on for the slot */
>> -	rc = rtas_get_power_level(slot->power_domain, &level);
>> -	if (rc)
>> -		return rc;
>> -	info->power_status = level;
>> -
>> -	/* Figure out if there is an adapter in the slot */
>> -	rc = rpaphp_get_sensor_state(slot, &state);
>> -	if (rc)
>> -		return rc;
>> -
>> -	bus = pcibios_find_pci_bus(slot->dn);
>> -	if (!bus) {
>> -		err("%s: no pci_bus for dn %s\n", __func__, slot->dn->full_name);
>> -		return -EINVAL;
>> -	}
>> -
>> -	info->adapter_status = EMPTY;
>> -	slot->bus = bus;
>> -	slot->pci_devs = &bus->devices;
>> -
>> -	/* if there's an adapter in the slot, go add the pci devices */
>> -	if (state == PRESENT) {
>> -		info->adapter_status = NOT_CONFIGURED;
>> -		slot->state = NOT_CONFIGURED;
>> -
>> -		/* non-empty slot has to have child */
>> -		if (!slot->dn->child) {
>> -			err("%s: slot[%s]'s device_node doesn't have child for adapter\n",
>> -			    __func__, slot->name);
>> -			return -EINVAL;
>> -		}
>> -
>> -		if (list_empty(&bus->devices))
>> -			pcibios_add_pci_devices(bus);
>> -
>> -		if (!list_empty(&bus->devices)) {
>> -			info->adapter_status = CONFIGURED;
>> -			slot->state = CONFIGURED;
>> -		}
>> -
>> -		if (rpaphp_debug) {
>> -			struct pci_dev *dev;
>> -			dbg("%s: pci_devs of slot[%s]\n", __func__, slot->dn->full_name);
>> -			list_for_each_entry (dev, &bus->devices, bus_list)
>> -				dbg("\t%s\n", pci_name(dev));
>> -		}
>> -	}
>> -
>> -	return 0;
>> -}
>> diff --git a/drivers/pci/hotplug/rpaphp_rtas.c b/drivers/pci/hotplug/rpaphp_rtas.c
>> new file mode 100644
>> index 0000000..74f024a
>> --- /dev/null
>> +++ b/drivers/pci/hotplug/rpaphp_rtas.c
>> @@ -0,0 +1,320 @@
>> +/*
>> + * RTAS backend for RPA-compliant PP64 platform
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or (at
>> + * your option) any later version.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci_hotplug.h>
>> +#include <linux/smp.h>
>> +#include <linux/init.h>
>> +#include <linux/vmalloc.h>
>> +#include <asm/eeh.h>
>> +#include <asm/rtas.h>
>> +#include <asm/pci-bridge.h>
>> +
>> +#include "../pci.h"
>> +#include "rpaphp.h"
>> +
>> +#define	RPA_PHP_DR_INDICATOR	9002
>> +#define RPA_PHP_DR_ENTITY_SENSE	9003
>> +
>> +static int get_power_status(struct hotplug_slot *hp_slot, u8 *val)
>> +{
>> +	struct rpa_php_slot *slot = hp_slot->private;
>> +	int state, ret;
>> +
>> +	/* By default, the power is on */
>> +	*val = RPA_PHP_SLOT_POWER_ON;
>> +
>> +	/* Retrieve power state from firmware, which might fail */
>> +	ret = rtas_get_power_level(slot->power_domain, &state);
>> +	if (!ret) {
>> +		if (state > 0)
>> +			hp_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
>> +		else
>> +			hp_slot->info->power_status = RPA_PHP_SLOT_POWER_OFF;
>> +		*val = hp_slot->info->power_status;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_adapter_status(struct hotplug_slot *hp_slot, u8 *val)
>> +{
>> +	struct rpa_php_slot *slot = hp_slot->private;
>> +        int state, ret;
>> +
>> +	/* By default, the slot is empty */
>> +	*val = RPA_PHP_SLOT_EMPTY;
>> +
>> +	/* Retrieve presence from firmware */
>> +	ret = rtas_get_sensor(RPA_PHP_DR_ENTITY_SENSE, slot->index, &state);
>> +	if (ret >= 0) {
>> +		if (state > 0)
>> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_PRESENT;
>> +		else
>> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
>> +		*val = hp_slot->info->adapter_status;
>> +		return 0;
>> +	}
>> +
>> +	/* Check if we need power slot on and retry */
>> +	if (ret != -EFAULT && ret != -EEXIST) {
>> +		err("%s: Error %d getting slot[%s] presence\n",
>> +		    __func__, ret, slot->name);
>> +		return ret;
>> +	}
>> +
>> +	/* Power slot on, which might fail */
>> +	ret = rtas_set_power_level(slot->power_domain,
>> +				   RPA_PHP_SLOT_POWER_ON, &state);
>> +	if (!ret)
>> +		hp_slot->info->power_status = RPA_PHP_SLOT_POWER_ON;
>> +
>> +	/* Recheck the presence */
>> +	ret = rtas_get_sensor(RPA_PHP_DR_ENTITY_SENSE, slot->index, &state);
>> +	if (ret >= 0) {
>> +		if (state > 0)
>> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_PRESENT;
>> +		else
>> +			hp_slot->info->adapter_status = RPA_PHP_SLOT_EMPTY;
>> +		*val = hp_slot->info->adapter_status;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int set_attention_status(struct hotplug_slot *hp_slot, u8 val)
>> +{
>> +	struct rpa_php_slot *slot = hp_slot->private;
>> +	int ret;
>> +
>> +	/*
>> +	 * The default operation would to turn on
>> +	 * the attention
>> +	 */
>> +	switch (val) {
>> +	case RPA_PHP_SLOT_ATTEN_OFF:
>> +	case RPA_PHP_SLOT_ATTEN_ON:
>> +	case RPA_PHP_SLOT_ATTEN_IND:
>> +	case RPA_PHP_SLOT_ATTEN_ACT:
>> +		break;
>> +	default:
>> +		val = RPA_PHP_SLOT_ATTEN_ON;
>> +	}
>> +
>> +	/* Set the attention */
>> +	ret = rtas_set_indicator(RPA_PHP_DR_INDICATOR, slot->index, val);
>> +	if (!ret)
>> +		hp_slot->info->attention_status = val;
>> +
>> +	return ret;
>> +}
>> +
>> +static enum pci_bus_speed get_max_bus_speed(struct rpa_php_slot *slot)
>> +{
>> +	enum pci_bus_speed speed;
>> +
>> +	switch (slot->type) {
>> +	case 1 ... 6:
>> +		speed = PCI_SPEED_33MHz;
>> +		break;
>> +	case 7 ... 8:
>> +		speed = PCI_SPEED_66MHz;
>> +		break;
>> +	case 11:
>> +	case 14:
>> +		speed = PCI_SPEED_66MHz_PCIX;
>> +		break;
>> +	case 12:
>> +	case 15:
>> +		speed = PCI_SPEED_100MHz_PCIX;
>> +		break;
>> +	case 13:
>> +	case 16:
>> +		speed = PCI_SPEED_133MHz_PCIX;
>> +		break;
>> +	default:
>> +		speed = PCI_SPEED_UNKNOWN;
>> +	}
>> +
>> +	return speed;
>> +}
>> +
>> +static int enable_slot(struct hotplug_slot *hp_slot)
>> +{
>> +	struct rpa_php_slot *slot = hp_slot->private;
>> +	uint8_t presence;
>> +	int ret;
>> +
>> +	/* Check if the slot has been configured */
>> +	if (slot->state == RPA_PHP_SLOT_CONFIGURED)
>> +		return 0;
>> +
>> +	/* Retrieve slot presence status */
>> +	ret = hp_slot->ops->get_adapter_status(hp_slot, &presence);
>> +	if (ret)
>> +		return ret;
>> +
>> +	switch (presence) {
>> +	case RPA_PHP_SLOT_PRESENT:
>> +		pci_lock_rescan_remove();
>> +		pcibios_add_pci_devices(slot->bus);
>> +		pci_unlock_rescan_remove();
>> +		slot->state = RPA_PHP_SLOT_CONFIGURED;
>> +		break;
>> +	case RPA_PHP_SLOT_EMPTY:
>> +		slot->state = RPA_PHP_SLOT_NOT_CONFIGURED;
>> +		break;
>> +	default:
>> +		slot->state = RPA_PHP_SLOT_NOT_VALID;
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Fix the bus maximal speed */
>> +	slot->bus->max_bus_speed = get_max_bus_speed(slot);
>> +	return 0;
>> +}
>> +
>> +static int disable_slot(struct hotplug_slot *hp_slot)
>> +{
>> +	struct rpa_php_slot *slot = hp_slot->private;
>> +
>> +	if (slot->state != RPA_PHP_SLOT_CONFIGURED)
>> +		return 0;
>> +
>> +	pci_lock_rescan_remove();
>> +	pcibios_remove_pci_devices(slot->bus);
>> +	pci_unlock_rescan_remove();
>> +	vm_unmap_aliases();
>> +
>> +	slot->state = RPA_PHP_SLOT_NOT_CONFIGURED;
>> +	return 0;
>> +}
>> +
>> +static struct hotplug_slot_ops rpaphp_rtas_ops = {
>> +	.enable_slot		= enable_slot,
>> +	.disable_slot		= disable_slot,
>> +	.set_attention_status	= set_attention_status,
>> +	.get_power_status	= get_power_status,
>> +	.get_adapter_status	= get_adapter_status,
>> +};
>> +
>> +static int parse_drc_props(struct device_node *dn, u32 drc_index,
>> +                           char **drc_name, char **drc_type, u32 *drc_power)
>> +{
>> +	const u32 *indexes, *names, *types, *domains;
>> +	char *name, *type;
>> +	struct device_node *parent = dn;
>> +	u32 i;
>> +
>> +	while ((parent = of_get_parent(parent))) {
>> +		indexes = of_get_property(parent, "ibm,drc-indexes", NULL);
>> +		names   = of_get_property(parent, "ibm,drc-names", NULL);
>> +		types   = of_get_property(parent, "ibm,drc-types", NULL);
>> +		domains = of_get_property(parent, "ibm,drc-power-domains", NULL);
>> +
>> +		if (!indexes || !names || !types || !domains) {
>> +			of_node_put(parent);
>> +			continue;
>> +		}
>> +
>> +		name = (char *)&names[1];
>> +		type = (char *)&types[1];
>> +		for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> +			if (be32_to_cpu(indexes[i + 1]) != drc_index) {
>> +				name += (strlen(name) + 1);
>> +				type += (strlen(type) + 1);
>> +				continue;
>> +			}
>> +
>> +			/* Matched index */
>> +			if (drc_name)
>> +				*drc_name = name;
>> +			if (drc_type)
>> +				*drc_type = type;
>> +			if (drc_power)
>> +				*drc_power = be32_to_cpu(domains[i + 1]);
>> +
>> +			of_node_put(parent);
>> +			return 0;
>> +		}
>> +
>> +		/* Next level parent */
>> +		of_node_put(parent);
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +
>> +/*
>> + * To get the DRC props describing the current node, first obtain it's
>> + * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
>> + * the my-drc-index for correlation, and obtain the requested properties.
>> + */
>> +int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> +                         char **drc_name, char **drc_type, int *drc_power)
>> +{
>> +	const u32 *my_index;
>> +
>> +	/* Check if node is capable of hotplug */
>> +	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> +	if (!my_index)
>> +		return -EINVAL;
>> +	if (drc_index)
>> +		*drc_index = be32_to_cpu(*my_index);
>> +
>> +	return parse_drc_props(dn, be32_to_cpu(*my_index),
>> +			       drc_name, drc_type, drc_power);
>> +}
>> +EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>> +
>> +struct rpa_php_slot *rpaphp_rtas_add_slot(struct device_node *dn)
>> +{
>> +	char *name, *type, *endptr;
>> +	int index, power_domain;
>> +	struct rpa_php_slot *slot;
>> +	struct pci_bus *bus;
>> +	int val, ret;
>> +
>> +	/* Get and parse the hotplug properties */
>> +	ret = rpaphp_get_drc_props(dn, &index, &name, &type, &power_domain);
>> +	if (ret)
>> +		return NULL;
>> +
>> +	/*
>> +	 * PCI hotplug slots have integer DRC type. That of
>> +	 * PHB slot is fixed to "PHB"
>> +	 */
>> +        val = simple_strtoul(type, &endptr, 10);
>> +	if (strcmp(type, "PHB") && (endptr == type))
>> +		return NULL;
>> +
>> +	slot = alloc_slot_struct(dn, index, name, power_domain);
>> +	if (!slot)
>> +		return NULL;
>> +
>> +        /* The slot should have an associated bus */
>> +	bus = pcibios_find_pci_bus(dn);
>> +	if (!bus) {
>> +		err("%s: No PCI bus for device node %s\n",
>> +			__func__, dn->full_name);
>> +		goto fail;
>> +	}
>> +
>> +	slot->hotplug_slot->ops = &rpaphp_rtas_ops;
>> +	slot->type     = val;
>> +	slot->bus      = bus;
>> +	slot->pci_devs = &bus->devices;
>> +	return slot;
>> +fail:
>> +	dealloc_slot_struct(slot);
>> +	return NULL;
>> +}
>> diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
>> deleted file mode 100644
>> index be48e69..0000000
>> --- a/drivers/pci/hotplug/rpaphp_slot.c
>> +++ /dev/null
>> @@ -1,140 +0,0 @@
>> -/*
>> - * RPA Virtual I/O device functions
>> - * Copyright (C) 2004 Linda Xie <lxie@us.ibm.com>
>> - *
>> - * All rights reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or modify
>> - * it under the terms of the GNU General Public License as published by
>> - * the Free Software Foundation; either version 2 of the License, or (at
>> - * your option) any later version.
>> - *
>> - * This program is distributed in the hope that it will be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>> - * NON INFRINGEMENT.  See the GNU General Public License for more
>> - * details.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program; if not, write to the Free Software
>> - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> - *
>> - * Send feedback to <lxie@us.ibm.com>
>> - *
>> - */
>> -#include <linux/kernel.h>
>> -#include <linux/module.h>
>> -#include <linux/sysfs.h>
>> -#include <linux/pci.h>
>> -#include <linux/string.h>
>> -#include <linux/slab.h>
>> -
>> -#include <asm/rtas.h>
>> -#include "rpaphp.h"
>> -
>> -/* free up the memory used by a slot */
>> -static void rpaphp_release_slot(struct hotplug_slot *hotplug_slot)
>> -{
>> -	struct slot *slot = (struct slot *) hotplug_slot->private;
>> -	dealloc_slot_struct(slot);
>> -}
>> -
>> -void dealloc_slot_struct(struct slot *slot)
>> -{
>> -	kfree(slot->hotplug_slot->info);
>> -	kfree(slot->name);
>> -	kfree(slot->hotplug_slot);
>> -	kfree(slot);
>> -}
>> -
>> -struct slot *alloc_slot_struct(struct device_node *dn,
>> -                       int drc_index, char *drc_name, int power_domain)
>> -{
>> -	struct slot *slot;
>> -
>> -	slot = kzalloc(sizeof(struct slot), GFP_KERNEL);
>> -	if (!slot)
>> -		goto error_nomem;
>> -	slot->hotplug_slot = kzalloc(sizeof(struct hotplug_slot), GFP_KERNEL);
>> -	if (!slot->hotplug_slot)
>> -		goto error_slot;
>> -	slot->hotplug_slot->info = kzalloc(sizeof(struct hotplug_slot_info),
>> -					   GFP_KERNEL);
>> -	if (!slot->hotplug_slot->info)
>> -		goto error_hpslot;
>> -	slot->name = kstrdup(drc_name, GFP_KERNEL);
>> -	if (!slot->name)
>> -		goto error_info;
>> -	slot->dn = dn;
>> -	slot->index = drc_index;
>> -	slot->power_domain = power_domain;
>> -	slot->hotplug_slot->private = slot;
>> -	slot->hotplug_slot->ops = &rpaphp_hotplug_slot_ops;
>> -	slot->hotplug_slot->release = &rpaphp_release_slot;
>> -
>> -	return slot;
>> -
>> -error_info:
>> -	kfree(slot->hotplug_slot->info);
>> -error_hpslot:
>> -	kfree(slot->hotplug_slot);
>> -error_slot:
>> -	kfree(slot);
>> -error_nomem:
>> -	return NULL;
>> -}
>> -
>> -int rpaphp_deregister_slot(struct slot *slot)
>> -{
>> -	int retval = 0;
>> -	struct hotplug_slot *php_slot = slot->hotplug_slot;
>> -
>> -	 dbg("%s - Entry: deregistering slot=%s\n",
>> -		__func__, slot->name);
>> -
>> -	list_del(&slot->rpaphp_slot_list);
>> -
>> -	retval = pci_hp_deregister(php_slot);
>> -	if (retval)
>> -		err("Problem unregistering a slot %s\n", slot->name);
>> -
>> -	dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> -	return retval;
>> -}
>> -EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>> -
>> -int rpaphp_register_slot(struct slot *slot)
>> -{
>> -	struct hotplug_slot *php_slot = slot->hotplug_slot;
>> -	struct slot *tmp;
>> -	int retval;
>> -	int slotno;
>> -
>> -	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
>> -		__func__, slot->dn->full_name, slot->index, slot->name,
>> -		slot->power_domain, slot->type);
>> -
>> -	/* Should not try to register the same slot twice */
>> -	list_for_each_entry(tmp, &rpaphp_slot_head, rpaphp_slot_list) {
>> -		if (!strcmp(tmp->name, slot->name)) {
>> -			err("%s: Slot[%s] is already registered\n",
>> -			    __func__, slot->name);
>> -			return -EAGAIN;
>> -		}
>> -	}
>> -
>> -	if (slot->dn->child)
>> -		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
>> -	else
>> -		slotno = -1;
>> -	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
>> -	if (retval) {
>> -		err("pci_hp_register failed with error %d\n", retval);
>> -		return retval;
>> -	}
>> -
>> -	/* add slot to our internal list */
>> -	list_add(&slot->rpaphp_slot_list, &rpaphp_slot_head);
>> -	info("Slot [%s] registered\n", slot->name);
>> -	return 0;
>> -}
>
>
>

^ permalink raw reply

* Re: [PATCH v1 0/5] powerpc/pci: PCI slot unified reset
From: Gavin Shan @ 2014-11-26  0:04 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <1416956093.5089.1.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 09:54:53AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:38 +1100, Gavin Shan wrote:
>> The patchset corresponds to skiboot changes, which manages PCI slots
>> in a unified way: OPAL APIs used to do slot reset, power management,
>> presence status retrival. The patchset shouldn't be merged before
>> the OPAL firmware counterpart is merged.
>
>This doesn't work. We need to have both backward and forward
>compatibility.
>
>IE. New kernels must work with old OPAL and vice-versa.
>

Yes, it's known issue as you remind me yesterday. I'll fix it in next
revision, from skiboot (I guess it would be official name to be used
now) side.

>> The kernel changes have been split into 2 parts: (A) Use the unified
>> PCI slot reset OPAL API - this patchset; (B) Changes to rpaphp.ko to
>> support PCI slot hotplug via sysfs entries for PowerPC PowerNV platform.
>
>I don't think we should be using rpaphp... we should be creating our new
>backend, there is almost no commonalities between RPA/PAPR and OPAL.
>

Ok. I'll create powernv-php as you suggested in another thread. Thank you
for your time on this.

Thanks,
Gavin

>> This patchset affects EEH, PCI reset logics, which is used mainly by
>> VFIO. A couple of cases have been tested out on P7/P8 boxes and looks
>> good: EEH on PowerNV, EEH support for guest, PCI passthrou.
>> 
>> Gavin Shan (5):
>>   powerpc/powernv: Use PCI slot reset infrastructure
>>   powerpc/powernv: Refactor ioda_eeh_reset()
>>   powerpc/powernv: Avoid do complete reset for twice
>>   powerpc/powernv: Issue fundamental reset if required
>>   powerpc/powernv: Drop opal_pci_reinit()
>> 
>>  arch/powerpc/include/asm/eeh.h                 |   2 +-
>>  arch/powerpc/include/asm/opal.h                |  31 +---
>>  arch/powerpc/kernel/eeh_pe.c                   |   3 -
>>  arch/powerpc/platforms/powernv/eeh-ioda.c      | 230 ++++++++++++++-----------
>>  arch/powerpc/platforms/powernv/eeh-powernv.c   |  24 +--
>>  arch/powerpc/platforms/powernv/opal-wrappers.S |   1 -
>>  arch/powerpc/platforms/powernv/pci-ioda.c      |   5 +-
>>  arch/powerpc/platforms/pseries/eeh_pseries.c   |   3 +-
>>  8 files changed, 139 insertions(+), 160 deletions(-)
>> 
>> Thanks,
>> Gavin
>
>

^ permalink raw reply

* Re: [PATCH 1/8] powerpc/pci: Move pcibios_find_pci_bus() around
From: Gavin Shan @ 2014-11-26  0:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev, Gavin Shan
In-Reply-To: <1416956310.5089.5.camel@kernel.crashing.org>

On Wed, Nov 26, 2014 at 09:58:30AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
>> The patch moves pcibios_find_pci_bus() to PPC kerenl directory so
>> that it can be reused by hotplug code for pSeries and PowerNV
>> platform at the same time.
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
>Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>

Thanks, Ben. But Michael needn't pick it up and merge it as the left
patches need reworks to some extent...

Thanks,
Gavin

>> ---
>>  arch/powerpc/kernel/pci-hotplug.c          | 36 ++++++++++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/pci_dlpar.c | 32 --------------------------
>>  2 files changed, 36 insertions(+), 32 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
>> index 5b78917..6e2b4e3 100644
>> --- a/arch/powerpc/kernel/pci-hotplug.c
>> +++ b/arch/powerpc/kernel/pci-hotplug.c
>> @@ -21,6 +21,42 @@
>>  #include <asm/firmware.h>
>>  #include <asm/eeh.h>
>>  
>> +static struct pci_bus *find_pci_bus(struct pci_bus *bus,
>> +				    struct device_node *dn)
>> +{
>> +	struct pci_bus *tmp, *child = NULL;
>> +	struct device_node *busdn;
>> +
>> +	busdn = pci_bus_to_OF_node(bus);
>> +	if (busdn == dn)
>> +		return bus;
>> +
>> +	list_for_each_entry(tmp, &bus->children, node) {
>> +		child = find_pci_bus(tmp, dn);
>> +		if (child)
>> +			break;
>> +	}
>> +
>> +	return child;
>> +}
>> +
>> +/**
>> + * pcibios_find_pci_bus - find PCI bus according to the given device node
>> + * @dn: Device node
>> + *
>> + * Find the corresponding PCI bus according to the given device node.
>> + */
>> +struct pci_bus *pcibios_find_pci_bus(struct device_node *dn)
>> +{
>> +	struct pci_dn *pdn = PCI_DN(dn);
>> +
>> +	if (!pdn  || !pdn->phb || !pdn->phb->bus)
>> +		return NULL;
>> +
>> +	return find_pci_bus(pdn->phb->bus, dn);
>> +}
>> +EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
>> +
>>  /**
>>   * pcibios_release_device - release PCI device
>>   * @dev: PCI device
>> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> index 89e2381..98c50bc 100644
>> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> @@ -32,38 +32,6 @@
>>  #include <asm/firmware.h>
>>  #include <asm/eeh.h>
>>  
>> -static struct pci_bus *
>> -find_bus_among_children(struct pci_bus *bus,
>> -                        struct device_node *dn)
>> -{
>> -	struct pci_bus *child = NULL;
>> -	struct pci_bus *tmp;
>> -	struct device_node *busdn;
>> -
>> -	busdn = pci_bus_to_OF_node(bus);
>> -	if (busdn == dn)
>> -		return bus;
>> -
>> -	list_for_each_entry(tmp, &bus->children, node) {
>> -		child = find_bus_among_children(tmp, dn);
>> -		if (child)
>> -			break;
>> -	};
>> -	return child;
>> -}
>> -
>> -struct pci_bus *
>> -pcibios_find_pci_bus(struct device_node *dn)
>> -{
>> -	struct pci_dn *pdn = dn->data;
>> -
>> -	if (!pdn  || !pdn->phb || !pdn->phb->bus)
>> -		return NULL;
>> -
>> -	return find_bus_among_children(pdn->phb->bus, dn);
>> -}
>> -EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
>> -
>>  struct pci_controller *init_phb_dynamic(struct device_node *dn)
>>  {
>>  	struct pci_controller *phb;
>
>

^ permalink raw reply

* Re: [PATCH 1/8] powerpc/pci: Move pcibios_find_pci_bus() around
From: Benjamin Herrenschmidt @ 2014-11-26  0:19 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <20141126000830.GA21766@shangw>

On Wed, 2014-11-26 at 11:08 +1100, Gavin Shan wrote:
> On Wed, Nov 26, 2014 at 09:58:30AM +1100, Benjamin Herrenschmidt wrote:
> >On Tue, 2014-11-25 at 09:49 +1100, Gavin Shan wrote:
> >> The patch moves pcibios_find_pci_bus() to PPC kerenl directory so
> >> that it can be reused by hotplug code for pSeries and PowerNV
> >> platform at the same time.
> >> 
> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >
> >Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >
> 
> Thanks, Ben. But Michael needn't pick it up and merge it as the left
> patches need reworks to some extent...

Sure but you can keep that ack around for that specific patch :P)

Cheers,
Ben.

> Thanks,
> Gavin
> 
> >> ---
> >>  arch/powerpc/kernel/pci-hotplug.c          | 36 ++++++++++++++++++++++++++++++
> >>  arch/powerpc/platforms/pseries/pci_dlpar.c | 32 --------------------------
> >>  2 files changed, 36 insertions(+), 32 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> >> index 5b78917..6e2b4e3 100644
> >> --- a/arch/powerpc/kernel/pci-hotplug.c
> >> +++ b/arch/powerpc/kernel/pci-hotplug.c
> >> @@ -21,6 +21,42 @@
> >>  #include <asm/firmware.h>
> >>  #include <asm/eeh.h>
> >>  
> >> +static struct pci_bus *find_pci_bus(struct pci_bus *bus,
> >> +				    struct device_node *dn)
> >> +{
> >> +	struct pci_bus *tmp, *child = NULL;
> >> +	struct device_node *busdn;
> >> +
> >> +	busdn = pci_bus_to_OF_node(bus);
> >> +	if (busdn == dn)
> >> +		return bus;
> >> +
> >> +	list_for_each_entry(tmp, &bus->children, node) {
> >> +		child = find_pci_bus(tmp, dn);
> >> +		if (child)
> >> +			break;
> >> +	}
> >> +
> >> +	return child;
> >> +}
> >> +
> >> +/**
> >> + * pcibios_find_pci_bus - find PCI bus according to the given device node
> >> + * @dn: Device node
> >> + *
> >> + * Find the corresponding PCI bus according to the given device node.
> >> + */
> >> +struct pci_bus *pcibios_find_pci_bus(struct device_node *dn)
> >> +{
> >> +	struct pci_dn *pdn = PCI_DN(dn);
> >> +
> >> +	if (!pdn  || !pdn->phb || !pdn->phb->bus)
> >> +		return NULL;
> >> +
> >> +	return find_pci_bus(pdn->phb->bus, dn);
> >> +}
> >> +EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
> >> +
> >>  /**
> >>   * pcibios_release_device - release PCI device
> >>   * @dev: PCI device
> >> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> >> index 89e2381..98c50bc 100644
> >> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> >> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> >> @@ -32,38 +32,6 @@
> >>  #include <asm/firmware.h>
> >>  #include <asm/eeh.h>
> >>  
> >> -static struct pci_bus *
> >> -find_bus_among_children(struct pci_bus *bus,
> >> -                        struct device_node *dn)
> >> -{
> >> -	struct pci_bus *child = NULL;
> >> -	struct pci_bus *tmp;
> >> -	struct device_node *busdn;
> >> -
> >> -	busdn = pci_bus_to_OF_node(bus);
> >> -	if (busdn == dn)
> >> -		return bus;
> >> -
> >> -	list_for_each_entry(tmp, &bus->children, node) {
> >> -		child = find_bus_among_children(tmp, dn);
> >> -		if (child)
> >> -			break;
> >> -	};
> >> -	return child;
> >> -}
> >> -
> >> -struct pci_bus *
> >> -pcibios_find_pci_bus(struct device_node *dn)
> >> -{
> >> -	struct pci_dn *pdn = dn->data;
> >> -
> >> -	if (!pdn  || !pdn->phb || !pdn->phb->bus)
> >> -		return NULL;
> >> -
> >> -	return find_bus_among_children(pdn->phb->bus, dn);
> >> -}
> >> -EXPORT_SYMBOL_GPL(pcibios_find_pci_bus);
> >> -
> >>  struct pci_controller *init_phb_dynamic(struct device_node *dn)
> >>  {
> >>  	struct pci_controller *phb;
> >
> >

^ permalink raw reply

* Re: [PATCH] i2c-qoriq: modified compatibility for correct prescaler
From: Scott Wood @ 2014-11-26  1:41 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Valentin Longchamp, Linux device trees, Boschung, Rainer,
	Brunck, Holger, Linux I2C, Linux PowerPC Kernel
In-Reply-To: <20141125181347.GC9716@katana>

On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote:
> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote:
> > On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote:
> > > > > 
> > > > > If we're going to change the device tree I'd rather just add a property
> > > > > to say what the prescaler is.
> > > > 
> > > >  We would however, leave the boards' device trees that use things like
> > > > "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it.
> > > > 
> > > > 
> > > > Now the drawback is that the driver would require a change, to parse this
> > > > prescaler new prescaler property. Would this be OK from your point of view
> > > > Wolfram ? If yes, I will send the patches for it.
> > > 
> > > I don't think it is OK.
> > 
> > Why?
> 
> Because I thought it could be deduced. Then, a seperate property would
> not be OK.
> 
> > >  I'd think it can be deduced from the compatible property.
> > 
> > For almost all existing device trees it cannot be.
> 
> Pity :( If we do introduce a new property, it should probably be
> "clock-div". Grepping through binding documentation, that seems
> accepted. We should ask DT maintainers, too, to be safe.
> 
> > If you want something that will work without changing device trees,
> > you'll need to use SVR to identify the SoC.
> 
> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno
> if it makes sense to add to it for consistency reasons?

That's not SVR, but sure.  Better to avoid messing with existing device
trees.

-Scott

^ permalink raw reply

* Re: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Scott Wood @ 2014-11-26  1:49 UTC (permalink / raw)
  To: shh.xie; +Cc: devicetree, linuxppc-dev, Shaohui Xie, Emilian.Medve,
	Igal.Liberman
In-Reply-To: <1415958800-9867-1-git-send-email-shh.xie@gmail.com>

On Fri, 2014-11-14 at 17:53 +0800, shh.xie@gmail.com wrote:
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
> 
> This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
> 
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---
> changes in V2:
> addressed comments from Scott in V1.
> 
>  .../devicetree/bindings/powerpc/fsl/fman.txt       | 69 ++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> index edeea16..1523a87 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> @@ -7,6 +7,7 @@ CONTENTS
>    - FMan MURAM Node
>    - FMan dTSEC/XGEC/mEMAC Node
>    - FMan IEEE 1588 Node
> +  - FMan MDIO Node
>    - Example
>  
>  =============================================================================
> @@ -357,6 +358,68 @@ ptp-timer@fe000 {
>  };
>  
>  =============================================================================
> +FMan MDIO Node
> +
> +DESCRIPTION
> +
> +The MDIO is a bus to which the PHY devices are connected.
> +
> +PROPERTIES
> +
> +- compatible
> +		Usage: required
> +		Value type: <stringlist>
> +		Definition: A standard property.
> +		Must include "fsl,fman-mdio" for 1 Gb/s MDIO from FMan v2.
> +		Must include "fsl,fman-xmdio" for 10 Gb/s MDIO from FMan v2.
> +		Must include "fsl,fman-memac-mdio" for 1/10 Gb/s MDIO from
> +		FMan v3.
> +
> +- reg
> +		Usage: required
> +		Value type: <prop-encoded-array>
> +		Definition: A standard property.
> +
> +- bus-frequency
> +		Usage: optional
> +		Value type: <u32>
> +		Definition: Specifies external MDIO bus clock speed which is
> +		different from MDIO standard 2.5MHz. Should be defined for SoCs
> +		on which the standard one cannot work.
> +
> +- interrupts
> +		Usage: optional
> +		Value type: <prop-encoded-array>
> +		Definition: Event interrupt of external MDIO controller.
> +		1 Gb/s MDIO and 10 Gb/s MDIO has one interrupt respectively.
> +
> +- fsl,fman-internal-mdio
> +		Usage: required for internal MDIO
> +		Value type: boolean
> +		Definition: Fman has internal MDIO for internal PCS(Physical
> +		Coding Sublayer) PHYs and external MDIO for external PHYs.
> +		The settings and programming routines for internal/external
> +		MDIO are different. Must be included for internal MDIO.
> +
> +EXAMPLE
> +
> +Example for FMan v2 external MDIO:
> +
> +mdio@f1000 {
> +	compatible = "fsl,fman-xmdio";
> +	reg = <0xf1000 0x1000>;
> +	bus-frequency = <20000>;
> +};

So the bus frequency is only 20 KHz?  Or is the unit supposed to be
something other than Hz?

-Scott

^ permalink raw reply

* Re: [v4,17/21] powerpc/8xx: set PTE bit 22 off TLBmiss
From: Scott Wood @ 2014-11-26  1:58 UTC (permalink / raw)
  To: christophe leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <546BA6F3.2070106@c-s.fr>

On Tue, 2014-11-18 at 21:07 +0100, christophe leroy wrote:
> Le 08/11/2014 01:08, Scott Wood a écrit :
> >
> > OK, so the _PAGE_KERNEL_RO(X) stuff is because initially setting the PTE
> > doesn't go through pte_update().
> >
> > I'll apply this, though it'd be cleaner to just have 8xx versions of the
> > relevant PTE accessor functions to maintain the PTE the way the hardware
> > wants (this would also eliminate the _PAGE_RW inversion that's still in
> > the TLB miss handler).
> >
> >
> 
> Yes, I've been looking at a simple way to also eliminate the _PAGE_RW 
> inversion, but i've not been able to find an easy solution up to now.
> 
> It seems that we have two functions that set PTE: set_pte_at() and 
> pte_update()
> I could perform the bit 22 (0x200) stuff and the _PAGE_RW invertion in 
> both functions, but then there are functions that read the PTE to make 
> decision based on PAGE_RW for instance.

Where?  The only relevant use of _PAGE_RW outside accessors that I see
is in gup_pte_range(), which shouldn't be too hard to fix up.

-Scott

^ permalink raw reply

* RE: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Shaohui Xie @ 2014-11-26  2:10 UTC (permalink / raw)
  To: Scott Wood
  Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Emilian Medve, Igal.Liberman@freescale.com
In-Reply-To: <1416966592.15957.172.camel@freescale.com>

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0K
PiBTZW50OiBXZWRuZXNkYXksIE5vdmVtYmVyIDI2LCAyMDE0IDk6NTAgQU0NCj4gVG86IHNoaC54
aWVAZ21haWwuY29tDQo+IENjOiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgZGV2aWNl
dHJlZUB2Z2VyLmtlcm5lbC5vcmc7IE1lZHZlDQo+IEVtaWxpYW4tRU1NRURWRTE7IExpYmVybWFu
IElnYWwtQjMxOTUwOyBYaWUgU2hhb2h1aS1CMjE5ODkNCj4gU3ViamVjdDogUmU6IFtQQVRDSF0g
W3YyXSBwb3dlci9mc2w6IGFkZCBNRElPIGR0IGJpbmRpbmcgZm9yIEZNYW4NCj4gDQo+IE9uIEZy
aSwgMjAxNC0xMS0xNCBhdCAxNzo1MyArMDgwMCwgc2hoLnhpZUBnbWFpbC5jb20gd3JvdGU6DQo+
ID4gRnJvbTogU2hhb2h1aSBYaWUgPFNoYW9odWkuWGllQGZyZWVzY2FsZS5jb20+DQo+ID4NCj4g
PiBUaGlzIGJpbmRpbmcgaXMgZm9yIEZNYW4gTURJTywgaXQgY292ZXJzIEZNYW4gdjIgJiBGTWFu
IHYzLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogU2hhb2h1aSBYaWUgPFNoYW9odWkuWGllQGZy
ZWVzY2FsZS5jb20+DQo+ID4gLS0tDQo+ID4gY2hhbmdlcyBpbiBWMjoNCj4gPiBhZGRyZXNzZWQg
Y29tbWVudHMgZnJvbSBTY290dCBpbiBWMS4NCj4gPg0KPiA+ICAuLi4vZGV2aWNldHJlZS9iaW5k
aW5ncy9wb3dlcnBjL2ZzbC9mbWFuLnR4dCAgICAgICB8IDY5DQo+ICsrKysrKysrKysrKysrKysr
KysrKysNCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDY5IGluc2VydGlvbnMoKykNCj4gPg0KPiA+IGRp
ZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvcG93ZXJwYy9mc2wv
Zm1hbi50eHQNCj4gPiBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9wb3dlcnBj
L2ZzbC9mbWFuLnR4dA0KPiA+IGluZGV4IGVkZWVhMTYuLjE1MjNhODcgMTAwNjQ0DQo+ID4gLS0t
IGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3Bvd2VycGMvZnNsL2ZtYW4udHh0
DQo+ID4gKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3Bvd2VycGMvZnNs
L2ZtYW4udHh0DQo+ID4gQEAgLTcsNiArNyw3IEBAIENPTlRFTlRTDQo+ID4gICAgLSBGTWFuIE1V
UkFNIE5vZGUNCj4gPiAgICAtIEZNYW4gZFRTRUMvWEdFQy9tRU1BQyBOb2RlDQo+ID4gICAgLSBG
TWFuIElFRUUgMTU4OCBOb2RlDQo+ID4gKyAgLSBGTWFuIE1ESU8gTm9kZQ0KPiA+ICAgIC0gRXhh
bXBsZQ0KPiA+DQo+ID4NCj4gPiA9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQo+ID4gPT09PT09PSBAQCAtMzU3LDYg
KzM1OCw2OCBAQCBwdHAtdGltZXJAZmUwMDAgeyAgfTsNCj4gPg0KPiA+DQo+ID4gPT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PQ0KPiA+ID09PT09PT0NCj4gPiArRk1hbiBNRElPIE5vZGUNCj4gPiArDQo+ID4gK0RFU0NS
SVBUSU9ODQo+ID4gKw0KPiA+ICtUaGUgTURJTyBpcyBhIGJ1cyB0byB3aGljaCB0aGUgUEhZIGRl
dmljZXMgYXJlIGNvbm5lY3RlZC4NCj4gPiArDQo+ID4gK1BST1BFUlRJRVMNCj4gPiArDQo+ID4g
Ky0gY29tcGF0aWJsZQ0KPiA+ICsJCVVzYWdlOiByZXF1aXJlZA0KPiA+ICsJCVZhbHVlIHR5cGU6
IDxzdHJpbmdsaXN0Pg0KPiA+ICsJCURlZmluaXRpb246IEEgc3RhbmRhcmQgcHJvcGVydHkuDQo+
ID4gKwkJTXVzdCBpbmNsdWRlICJmc2wsZm1hbi1tZGlvIiBmb3IgMSBHYi9zIE1ESU8gZnJvbSBG
TWFuIHYyLg0KPiA+ICsJCU11c3QgaW5jbHVkZSAiZnNsLGZtYW4teG1kaW8iIGZvciAxMCBHYi9z
IE1ESU8gZnJvbSBGTWFuIHYyLg0KPiA+ICsJCU11c3QgaW5jbHVkZSAiZnNsLGZtYW4tbWVtYWMt
bWRpbyIgZm9yIDEvMTAgR2IvcyBNRElPIGZyb20NCj4gPiArCQlGTWFuIHYzLg0KPiA+ICsNCj4g
PiArLSByZWcNCj4gPiArCQlVc2FnZTogcmVxdWlyZWQNCj4gPiArCQlWYWx1ZSB0eXBlOiA8cHJv
cC1lbmNvZGVkLWFycmF5Pg0KPiA+ICsJCURlZmluaXRpb246IEEgc3RhbmRhcmQgcHJvcGVydHku
DQo+ID4gKw0KPiA+ICstIGJ1cy1mcmVxdWVuY3kNCj4gPiArCQlVc2FnZTogb3B0aW9uYWwNCj4g
PiArCQlWYWx1ZSB0eXBlOiA8dTMyPg0KPiA+ICsJCURlZmluaXRpb246IFNwZWNpZmllcyBleHRl
cm5hbCBNRElPIGJ1cyBjbG9jayBzcGVlZCB3aGljaCBpcw0KPiA+ICsJCWRpZmZlcmVudCBmcm9t
IE1ESU8gc3RhbmRhcmQgMi41TUh6LiBTaG91bGQgYmUgZGVmaW5lZCBmb3INCj4gU29Dcw0KPiA+
ICsJCW9uIHdoaWNoIHRoZSBzdGFuZGFyZCBvbmUgY2Fubm90IHdvcmsuDQo+ID4gKw0KPiA+ICst
IGludGVycnVwdHMNCj4gPiArCQlVc2FnZTogb3B0aW9uYWwNCj4gPiArCQlWYWx1ZSB0eXBlOiA8
cHJvcC1lbmNvZGVkLWFycmF5Pg0KPiA+ICsJCURlZmluaXRpb246IEV2ZW50IGludGVycnVwdCBv
ZiBleHRlcm5hbCBNRElPIGNvbnRyb2xsZXIuDQo+ID4gKwkJMSBHYi9zIE1ESU8gYW5kIDEwIEdi
L3MgTURJTyBoYXMgb25lIGludGVycnVwdCByZXNwZWN0aXZlbHkuDQo+ID4gKw0KPiA+ICstIGZz
bCxmbWFuLWludGVybmFsLW1kaW8NCj4gPiArCQlVc2FnZTogcmVxdWlyZWQgZm9yIGludGVybmFs
IE1ESU8NCj4gPiArCQlWYWx1ZSB0eXBlOiBib29sZWFuDQo+ID4gKwkJRGVmaW5pdGlvbjogRm1h
biBoYXMgaW50ZXJuYWwgTURJTyBmb3IgaW50ZXJuYWwgUENTKFBoeXNpY2FsDQo+ID4gKwkJQ29k
aW5nIFN1YmxheWVyKSBQSFlzIGFuZCBleHRlcm5hbCBNRElPIGZvciBleHRlcm5hbCBQSFlzLg0K
PiA+ICsJCVRoZSBzZXR0aW5ncyBhbmQgcHJvZ3JhbW1pbmcgcm91dGluZXMgZm9yIGludGVybmFs
L2V4dGVybmFsDQo+ID4gKwkJTURJTyBhcmUgZGlmZmVyZW50LiBNdXN0IGJlIGluY2x1ZGVkIGZv
ciBpbnRlcm5hbCBNRElPLg0KPiA+ICsNCj4gPiArRVhBTVBMRQ0KPiA+ICsNCj4gPiArRXhhbXBs
ZSBmb3IgRk1hbiB2MiBleHRlcm5hbCBNRElPOg0KPiA+ICsNCj4gPiArbWRpb0BmMTAwMCB7DQo+
ID4gKwljb21wYXRpYmxlID0gImZzbCxmbWFuLXhtZGlvIjsNCj4gPiArCXJlZyA9IDwweGYxMDAw
IDB4MTAwMD47DQo+ID4gKwlidXMtZnJlcXVlbmN5ID0gPDIwMDAwPjsNCj4gPiArfTsNCj4gDQo+
IFNvIHRoZSBidXMgZnJlcXVlbmN5IGlzIG9ubHkgMjAgS0h6PyAgT3IgaXMgdGhlIHVuaXQgc3Vw
cG9zZWQgdG8gYmUNCj4gc29tZXRoaW5nIG90aGVyIHRoYW4gSHo/DQpbUy5IXSBpdCdzIG9ubHkg
YW4gZXhhbXBsZSwgaXQgY291bGQgYmUgZGlmZmVyZW50IG9uIHJlYWwgU29DcywgYnV0IHRoZXkg
YWx3YXlzIGxvd2VyIHRoYW4gdGhlIHN0YW5kYXJkIG9uZSwNClRoZSBzdGFuZGFyZCBvbmUgaXMg
Mi41TUh6LCBJIGhhdmUgdG8gdXNlIEh6IGZvciBpdC4NCg0KVGhhbmtzIQ0KU2hhb2h1aQ0K

^ permalink raw reply

* Re: [PATCH v2 07/14] of/reconfig: Always use the same structure for notifiers
From: Nathan Fontenot @ 2014-11-26  3:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Grant Likely
  Cc: devicetree, Wolfram Sang, Pantelis Antoniou, linux-kernel,
	Rob Herring, Mark Brown, linuxppc-dev
In-Reply-To: <1416956874.5089.13.camel@kernel.crashing.org>

On 11/25/2014 05:07 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2014-11-24 at 22:33 +0000, Grant Likely wrote:
>> The OF_RECONFIG notifier callback uses a different structure depending
>> on whether it is a node change or a property change. This is silly, and
>> not very safe. Rework the code to use the same data structure regardless
>> of the type of notifier.
> 
> I fell pretty good about this one except...
> 
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index b9d1dfdbe5bb..9fe6002c1d5a 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -1711,12 +1711,11 @@ static void stage_topology_update(int core_id)
>>  static int dt_update_callback(struct notifier_block *nb,
>>  				unsigned long action, void *data)
>>  {
>> -	struct of_prop_reconfig *update;
>> +	struct of_reconfig_data *update = data;
>>  	int rc = NOTIFY_DONE;
>>  
>>  	switch (action) {
>>  	case OF_RECONFIG_UPDATE_PROPERTY:
>> -		update = (struct of_prop_reconfig *)data;
> 
> Should we assert/bug on !update->dn / update->prop ?
> 
> (Same for the rest of the patch)
> 
> Or do you reckon it's pointless ?
> 

I'm not sure it's worth it, if those are NULL pointers the drivers/of
code would have tried to use them before invoking the notifier chain.
We won't make it this far if they're NULL.

Otherwise the patch looks good to me,

Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

-Nathan

^ permalink raw reply

* Re: pseries/le: Fix endiannes issue in RTAS call from xmon
From: Michael Ellerman @ 2014-11-26  3:31 UTC (permalink / raw)
  To: Laurent Dufour, linuxppc-dev, linux-kernel; +Cc: Laurent Dufour, paulus
In-Reply-To: <1416838073-16642-1-git-send-email-ldufour@linux.vnet.ibm.com>

On Mon, 2014-24-11 at 14:07:53 UTC, Laurent Dufour wrote:
> On pseries system (LPAR) xmon failed to enter when running in LE mode, system
> is hunging. Inititating xmon will lead to such an output on the console:

OK. You say "LPAR", by which you mean "under phyp" I think. I haven't seen this
under KVM, and it looks like KVM doesn't implement "set-indicator" so that
would explain that.

I'll take this as a bug fix and CC it to stable.

cheers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox