[PATCH v2] EDAC, mpc85xx: Make mpc85xx-pci-edac a platform device

2015-12-09 Thread Scott Wood
Originally the mpc85xx-pci-edac driver bound directly to the PCI
controller node.

Commit 905e75c46dba5f30 ("powerpc/fsl-pci: Unify pci/pcie
initialization code") turned the PCI controller code into a platform
device.  Since we can't have two drivers binding to the same device,
the edac code was changed to be called into as a library-style
submodule.  However, this doesn't work if the edac driver is built as a
module.

Commit 8d8fcba6d1eab ("EDAC: Rip out the edac_subsys reference
counting") exposed another problem with this approach --
mpc85xx_pci_err_probe() was being called in the same early boot phase
that the PCI controller is initialized, rather than in the
device_initcall phase that the EDAC layer expects.  This caused a crash
on boot.

To fix this, the PCI controller code now creates a child platform
device specifically for EDAC, which the mpc85xx-pci-edac driver binds
to.

Signed-off-by: Scott Wood 
Cc: Jia Hongtao 
Cc: Borislav Petkov 
Cc: Johannes Thumshirn 
Cc: Michael Ellerman 
---
v2: Make mpc85xx_pci_err_probe() static again.

 arch/powerpc/sysdev/fsl_pci.c | 28 +++-
 arch/powerpc/sysdev/fsl_pci.h |  9 -
 drivers/edac/mpc85xx_edac.c   | 36 +++-
 include/linux/fsl/edac.h  |  8 
 4 files changed, 66 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/fsl/edac.h

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 610f472..a1ac80b 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -21,10 +21,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1255,6 +1257,25 @@ void fsl_pcibios_fixup_phb(struct pci_controller *phb)
 #endif
 }
 
+static int add_err_dev(struct platform_device *pdev)
+{
+   struct platform_device *errdev;
+   struct mpc85xx_edac_pci_plat_data pd = {
+   .of_node = pdev->dev.of_node
+   };
+
+   errdev = platform_device_register_resndata(>dev,
+  "mpc85xx-pci-edac",
+  PLATFORM_DEVID_AUTO,
+  pdev->resource,
+  pdev->num_resources,
+  , sizeof(pd));
+   if (IS_ERR(errdev))
+   return PTR_ERR(errdev);
+
+   return 0;
+}
+
 static int fsl_pci_probe(struct platform_device *pdev)
 {
struct device_node *node;
@@ -1262,8 +1283,13 @@ static int fsl_pci_probe(struct platform_device *pdev)
 
node = pdev->dev.of_node;
ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
+   if (ret)
+   return ret;
 
-   mpc85xx_pci_err_probe(pdev);
+   ret = add_err_dev(pdev);
+   if (ret)
+   dev_err(>dev, "couldn't register error device: %d\n",
+   ret);
 
return 0;
 }
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index c1cec77..1515885 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -130,15 +130,6 @@ void fsl_pci_assign_primary(void);
 static inline void fsl_pci_assign_primary(void) {}
 #endif
 
-#ifdef CONFIG_EDAC_MPC85XX
-int mpc85xx_pci_err_probe(struct platform_device *op);
-#else
-static inline int mpc85xx_pci_err_probe(struct platform_device *op)
-{
-   return -ENOTSUPP;
-}
-#endif
-
 #ifdef CONFIG_FSL_PCI
 extern int fsl_pci_mcheck_exception(struct pt_regs *);
 #else
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index 3eab063..406d75a 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -238,10 +239,12 @@ static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id)
return IRQ_HANDLED;
 }
 
-int mpc85xx_pci_err_probe(struct platform_device *op)
+static int mpc85xx_pci_err_probe(struct platform_device *op)
 {
struct edac_pci_ctl_info *pci;
struct mpc85xx_pci_pdata *pdata;
+   struct mpc85xx_edac_pci_plat_data *plat_data;
+   struct device_node *of_node;
struct resource r;
int res = 0;
 
@@ -266,7 +269,15 @@ int mpc85xx_pci_err_probe(struct platform_device *op)
pdata->name = "mpc85xx_pci_err";
pdata->irq = NO_IRQ;
 
-   if (mpc85xx_pcie_find_capability(op->dev.of_node) > 0)
+   plat_data = op->dev.platform_data;
+   if (!plat_data) {
+   dev_err(>dev, "no platform data");
+   res = -ENXIO;
+   goto err;
+   }
+   of_node = plat_data->of_node;
+
+   if (mpc85xx_pcie_find_capability(of_node) > 0)
pdata->is_pcie = true;
 
dev_set_drvdata(>dev, pci);
@@ 

Re: [PATCH v2] EDAC, mpc85xx: Make mpc85xx-pci-edac a platform device

2015-12-09 Thread Johannes Thumshirn
On Wed, 2015-12-09 at 18:02 -0600, Scott Wood wrote:
> Originally the mpc85xx-pci-edac driver bound directly to the PCI
> controller node.
> 
> Commit 905e75c46dba5f30 ("powerpc/fsl-pci: Unify pci/pcie
> initialization code") turned the PCI controller code into a platform
> device.  Since we can't have two drivers binding to the same device,
> the edac code was changed to be called into as a library-style
> submodule.  However, this doesn't work if the edac driver is built as a
> module.
> 
> Commit 8d8fcba6d1eab ("EDAC: Rip out the edac_subsys reference
> counting") exposed another problem with this approach --
> mpc85xx_pci_err_probe() was being called in the same early boot phase
> that the PCI controller is initialized, rather than in the
> device_initcall phase that the EDAC layer expects.  This caused a crash
> on boot.
> 
> To fix this, the PCI controller code now creates a child platform
> device specifically for EDAC, which the mpc85xx-pci-edac driver binds
> to.
> 
> Signed-off-by: Scott Wood 
> Cc: Jia Hongtao 
> Cc: Borislav Petkov 
> Cc: Johannes Thumshirn 
> Cc: Michael Ellerman 
> ---
> v2: Make mpc85xx_pci_err_probe() static again.
> 
>  arch/powerpc/sysdev/fsl_pci.c | 28 +++-
>  arch/powerpc/sysdev/fsl_pci.h |  9 -
>  drivers/edac/mpc85xx_edac.c   | 36 +++-
>  include/linux/fsl/edac.h  |  8 
>  4 files changed, 66 insertions(+), 15 deletions(-)
>  create mode 100644 include/linux/fsl/edac.h
> 
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 610f472..a1ac80b 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -21,10 +21,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1255,6 +1257,25 @@ void fsl_pcibios_fixup_phb(struct pci_controller *phb)
>  #endif
>  }
>  
> +static int add_err_dev(struct platform_device *pdev)
> +{
> + struct platform_device *errdev;
> + struct mpc85xx_edac_pci_plat_data pd = {
> + .of_node = pdev->dev.of_node
> + };
> +
> + errdev = platform_device_register_resndata(>dev,
> +    "mpc85xx-pci-edac",
> +    PLATFORM_DEVID_AUTO,
> +    pdev->resource,
> +    pdev->num_resources,
> +    , sizeof(pd));
> + if (IS_ERR(errdev))
> + return PTR_ERR(errdev);
> +
> + return 0;
> +}
> +
>  static int fsl_pci_probe(struct platform_device *pdev)
>  {
>   struct device_node *node;
> @@ -1262,8 +1283,13 @@ static int fsl_pci_probe(struct platform_device *pdev)
>  
>   node = pdev->dev.of_node;
>   ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
> + if (ret)
> + return ret;
>  
> - mpc85xx_pci_err_probe(pdev);
> + ret = add_err_dev(pdev);
> + if (ret)
> + dev_err(>dev, "couldn't register error device: %d\n",
> + ret);
>  
>   return 0;
>  }
> diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
> index c1cec77..1515885 100644
> --- a/arch/powerpc/sysdev/fsl_pci.h
> +++ b/arch/powerpc/sysdev/fsl_pci.h
> @@ -130,15 +130,6 @@ void fsl_pci_assign_primary(void);
>  static inline void fsl_pci_assign_primary(void) {}
>  #endif
>  
> -#ifdef CONFIG_EDAC_MPC85XX
> -int mpc85xx_pci_err_probe(struct platform_device *op);
> -#else
> -static inline int mpc85xx_pci_err_probe(struct platform_device *op)
> -{
> - return -ENOTSUPP;
> -}
> -#endif
> -
>  #ifdef CONFIG_FSL_PCI
>  extern int fsl_pci_mcheck_exception(struct pt_regs *);
>  #else
> diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> index 3eab063..406d75a 100644
> --- a/drivers/edac/mpc85xx_edac.c
> +++ b/drivers/edac/mpc85xx_edac.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -238,10 +239,12 @@ static irqreturn_t mpc85xx_pci_isr(int irq, void
> *dev_id)
>   return IRQ_HANDLED;
>  }
>  
> -int mpc85xx_pci_err_probe(struct platform_device *op)
> +static int mpc85xx_pci_err_probe(struct platform_device *op)
>  {
>   struct edac_pci_ctl_info *pci;
>   struct mpc85xx_pci_pdata *pdata;
> + struct mpc85xx_edac_pci_plat_data *plat_data;
> + struct device_node *of_node;
>   struct resource r;
>   int res = 0;
>  
> @@ -266,7 +269,15 @@ int mpc85xx_pci_err_probe(struct platform_device *op)
>   pdata->name = "mpc85xx_pci_err";
>   pdata->irq = NO_IRQ;
>  
> - if (mpc85xx_pcie_find_capability(op->dev.of_node) > 0)
> + plat_data = op->dev.platform_data;
> + if (!plat_data) {
> +