diff mbox

[3/3] edac/85xx: Enable the EDAC PCI err driver by device_initcall

Message ID 1348772523-17587-3-git-send-email-Chunhe.Lan@freescale.com (mailing list archive)
State Superseded
Delegated to: Kumar Gala
Headers show

Commit Message

Chunhe Lan Sept. 27, 2012, 7:02 p.m. UTC
Original process of call:
	The mpc85xx_pci_err_probe function completes to been registered
	and enabled of EDAC PCI err driver at the latter time stage of
	kernel boot in the mpc85xx_edac.c.
Current process of call:
	The mpc85xx_pci_err_probe function completes to been registered
	and enabled of EDAC PCI err driver at the first	time stage of
	kernel boot in the fsl_pci.c.

So in this case the following error messages appear in the boot log:

    PCI: Probing PCI hardware
    pci 0000:00:00.0: ignoring class b20 (doesn't match header type 01)
    PCIE error(s) detected
    PCIE ERR_DR register: 0x00020000
    PCIE ERR_CAP_STAT register: 0x80000001
    PCIE ERR_CAP_R0 register: 0x00000800
    PCIE ERR_CAP_R1 register: 0x00000000
    PCIE ERR_CAP_R2 register: 0x00000000
    PCIE ERR_CAP_R3 register: 0x00000000

Because the EDAC PCI err driver is registered and enabled earlier than
original point of call. But at this point of time, PCI hardware is not
probed and initialized, and it is in unknowable state.

So, move enable function into mpc85xx_pci_err_en which is called at the
middle time stage of kernel boot and after PCI hardware is probed and
initialized by device_initcall in the fsl_pci.c.

Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
---
 arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
 arch/powerpc/sysdev/fsl_pci.h |    5 ++++
 drivers/edac/mpc85xx_edac.c   |   47 ++++++++++++++++++++++++++++------------
 3 files changed, 50 insertions(+), 14 deletions(-)

Comments

Scott Wood Sept. 27, 2012, 4:09 p.m. UTC | #1
On 09/27/2012 02:02:03 PM, Chunhe Lan wrote:
> Original process of call:
> 	The mpc85xx_pci_err_probe function completes to been registered
> 	and enabled of EDAC PCI err driver at the latter time stage of
> 	kernel boot in the mpc85xx_edac.c.
> Current process of call:
> 	The mpc85xx_pci_err_probe function completes to been registered
> 	and enabled of EDAC PCI err driver at the first	time stage of
> 	kernel boot in the fsl_pci.c.
> 
> So in this case the following error messages appear in the boot log:
> 
>     PCI: Probing PCI hardware
>     pci 0000:00:00.0: ignoring class b20 (doesn't match header type  
> 01)
>     PCIE error(s) detected
>     PCIE ERR_DR register: 0x00020000
>     PCIE ERR_CAP_STAT register: 0x80000001
>     PCIE ERR_CAP_R0 register: 0x00000800
>     PCIE ERR_CAP_R1 register: 0x00000000
>     PCIE ERR_CAP_R2 register: 0x00000000
>     PCIE ERR_CAP_R3 register: 0x00000000
> 
> Because the EDAC PCI err driver is registered and enabled earlier than
> original point of call. But at this point of time, PCI hardware is not
> probed and initialized, and it is in unknowable state.
> 
> So, move enable function into mpc85xx_pci_err_en which is called at  
> the
> middle time stage of kernel boot and after PCI hardware is probed and
> initialized by device_initcall in the fsl_pci.c.
> 
> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> ---
>  arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
>  arch/powerpc/sysdev/fsl_pci.h |    5 ++++
>  drivers/edac/mpc85xx_edac.c   |   47  
> ++++++++++++++++++++++++++++------------
>  3 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_pci.c  
> b/arch/powerpc/sysdev/fsl_pci.c
> index 3d6f4d8..a591965 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -904,4 +904,16 @@ static int __init fsl_pci_init(void)
>  	return platform_driver_register(&fsl_pci_driver);
>  }
>  arch_initcall(fsl_pci_init);
> +
> +static int __init fsl_pci_err_en(void)
> +{
> +	struct device_node *np;
> +
> +	for_each_node_by_type(np, "pci")
> +		if (of_match_node(pci_ids, np))
> +			mpc85xx_pci_err_en(np);
> +
> +	return 0;
> +}
> +device_initcall(fsl_pci_err_en);

Why can't you call this from the normal PCIe controller init, instead  
of searching for the node independently?

-Scott
Gala Kumar-B11780 Sept. 27, 2012, 9:45 p.m. UTC | #2
On Sep 27, 2012, at 11:09 AM, Scott Wood wrote:

> On 09/27/2012 02:02:03 PM, Chunhe Lan wrote:
>> Original process of call:
>> 	The mpc85xx_pci_err_probe function completes to been registered
>> 	and enabled of EDAC PCI err driver at the latter time stage of
>> 	kernel boot in the mpc85xx_edac.c.
>> Current process of call:
>> 	The mpc85xx_pci_err_probe function completes to been registered
>> 	and enabled of EDAC PCI err driver at the first	time stage of
>> 	kernel boot in the fsl_pci.c.
>> So in this case the following error messages appear in the boot log:
>>    PCI: Probing PCI hardware
>>    pci 0000:00:00.0: ignoring class b20 (doesn't match header type 01)
>>    PCIE error(s) detected
>>    PCIE ERR_DR register: 0x00020000
>>    PCIE ERR_CAP_STAT register: 0x80000001
>>    PCIE ERR_CAP_R0 register: 0x00000800
>>    PCIE ERR_CAP_R1 register: 0x00000000
>>    PCIE ERR_CAP_R2 register: 0x00000000
>>    PCIE ERR_CAP_R3 register: 0x00000000
>> Because the EDAC PCI err driver is registered and enabled earlier than
>> original point of call. But at this point of time, PCI hardware is not
>> probed and initialized, and it is in unknowable state.
>> So, move enable function into mpc85xx_pci_err_en which is called at the
>> middle time stage of kernel boot and after PCI hardware is probed and
>> initialized by device_initcall in the fsl_pci.c.
>> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
>> ---
>> arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
>> arch/powerpc/sysdev/fsl_pci.h |    5 ++++
>> drivers/edac/mpc85xx_edac.c   |   47 ++++++++++++++++++++++++++++------------
>> 3 files changed, 50 insertions(+), 14 deletions(-)
>> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>> index 3d6f4d8..a591965 100644
>> --- a/arch/powerpc/sysdev/fsl_pci.c
>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>> @@ -904,4 +904,16 @@ static int __init fsl_pci_init(void)
>> 	return platform_driver_register(&fsl_pci_driver);
>> }
>> arch_initcall(fsl_pci_init);
>> +
>> +static int __init fsl_pci_err_en(void)
>> +{
>> +	struct device_node *np;
>> +
>> +	for_each_node_by_type(np, "pci")
>> +		if (of_match_node(pci_ids, np))
>> +			mpc85xx_pci_err_en(np);
>> +
>> +	return 0;
>> +}
>> +device_initcall(fsl_pci_err_en);
> 
> Why can't you call this from the normal PCIe controller init, instead of searching for the node independently?

Don't we have this now with mpc85xx_pci_err_probe() ??

- k
Scott Wood Sept. 27, 2012, 9:51 p.m. UTC | #3
On 09/27/2012 04:45:08 PM, Gala Kumar-B11780 wrote:
> 
> On Sep 27, 2012, at 11:09 AM, Scott Wood wrote:
> 
> > On 09/27/2012 02:02:03 PM, Chunhe Lan wrote:
> >> Original process of call:
> >> 	The mpc85xx_pci_err_probe function completes to been registered
> >> 	and enabled of EDAC PCI err driver at the latter time stage of
> >> 	kernel boot in the mpc85xx_edac.c.
> >> Current process of call:
> >> 	The mpc85xx_pci_err_probe function completes to been registered
> >> 	and enabled of EDAC PCI err driver at the first	time stage of
> >> 	kernel boot in the fsl_pci.c.
> >> So in this case the following error messages appear in the boot  
> log:
> >>    PCI: Probing PCI hardware
> >>    pci 0000:00:00.0: ignoring class b20 (doesn't match header type  
> 01)
> >>    PCIE error(s) detected
> >>    PCIE ERR_DR register: 0x00020000
> >>    PCIE ERR_CAP_STAT register: 0x80000001
> >>    PCIE ERR_CAP_R0 register: 0x00000800
> >>    PCIE ERR_CAP_R1 register: 0x00000000
> >>    PCIE ERR_CAP_R2 register: 0x00000000
> >>    PCIE ERR_CAP_R3 register: 0x00000000
> >> Because the EDAC PCI err driver is registered and enabled earlier  
> than
> >> original point of call. But at this point of time, PCI hardware is  
> not
> >> probed and initialized, and it is in unknowable state.
> >> So, move enable function into mpc85xx_pci_err_en which is called  
> at the
> >> middle time stage of kernel boot and after PCI hardware is probed  
> and
> >> initialized by device_initcall in the fsl_pci.c.
> >> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> >> ---
> >> arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
> >> arch/powerpc/sysdev/fsl_pci.h |    5 ++++
> >> drivers/edac/mpc85xx_edac.c   |   47  
> ++++++++++++++++++++++++++++------------
> >> 3 files changed, 50 insertions(+), 14 deletions(-)
> >> diff --git a/arch/powerpc/sysdev/fsl_pci.c  
> b/arch/powerpc/sysdev/fsl_pci.c
> >> index 3d6f4d8..a591965 100644
> >> --- a/arch/powerpc/sysdev/fsl_pci.c
> >> +++ b/arch/powerpc/sysdev/fsl_pci.c
> >> @@ -904,4 +904,16 @@ static int __init fsl_pci_init(void)
> >> 	return platform_driver_register(&fsl_pci_driver);
> >> }
> >> arch_initcall(fsl_pci_init);
> >> +
> >> +static int __init fsl_pci_err_en(void)
> >> +{
> >> +	struct device_node *np;
> >> +
> >> +	for_each_node_by_type(np, "pci")
> >> +		if (of_match_node(pci_ids, np))
> >> +			mpc85xx_pci_err_en(np);
> >> +
> >> +	return 0;
> >> +}
> >> +device_initcall(fsl_pci_err_en);
> >
> > Why can't you call this from the normal PCIe controller init,  
> instead of searching for the node independently?
> 
> Don't we have this now with mpc85xx_pci_err_probe() ??

What do you mean by "this"?

-Scott
Kumar Gala Sept. 27, 2012, 10:33 p.m. UTC | #4
On Sep 27, 2012, at 4:51 PM, Scott Wood wrote:

> On 09/27/2012 04:45:08 PM, Gala Kumar-B11780 wrote:
>> On Sep 27, 2012, at 11:09 AM, Scott Wood wrote:
>>> On 09/27/2012 02:02:03 PM, Chunhe Lan wrote:
>>>> Original process of call:
>>>> 	The mpc85xx_pci_err_probe function completes to been registered
>>>> 	and enabled of EDAC PCI err driver at the latter time stage of
>>>> 	kernel boot in the mpc85xx_edac.c.
>>>> Current process of call:
>>>> 	The mpc85xx_pci_err_probe function completes to been registered
>>>> 	and enabled of EDAC PCI err driver at the first	time stage of
>>>> 	kernel boot in the fsl_pci.c.
>>>> So in this case the following error messages appear in the boot log:
>>>>   PCI: Probing PCI hardware
>>>>   pci 0000:00:00.0: ignoring class b20 (doesn't match header type 01)
>>>>   PCIE error(s) detected
>>>>   PCIE ERR_DR register: 0x00020000
>>>>   PCIE ERR_CAP_STAT register: 0x80000001
>>>>   PCIE ERR_CAP_R0 register: 0x00000800
>>>>   PCIE ERR_CAP_R1 register: 0x00000000
>>>>   PCIE ERR_CAP_R2 register: 0x00000000
>>>>   PCIE ERR_CAP_R3 register: 0x00000000
>>>> Because the EDAC PCI err driver is registered and enabled earlier than
>>>> original point of call. But at this point of time, PCI hardware is not
>>>> probed and initialized, and it is in unknowable state.
>>>> So, move enable function into mpc85xx_pci_err_en which is called at the
>>>> middle time stage of kernel boot and after PCI hardware is probed and
>>>> initialized by device_initcall in the fsl_pci.c.
>>>> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
>>>> ---
>>>> arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
>>>> arch/powerpc/sysdev/fsl_pci.h |    5 ++++
>>>> drivers/edac/mpc85xx_edac.c   |   47 ++++++++++++++++++++++++++++------------
>>>> 3 files changed, 50 insertions(+), 14 deletions(-)
>>>> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>>>> index 3d6f4d8..a591965 100644
>>>> --- a/arch/powerpc/sysdev/fsl_pci.c
>>>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>>>> @@ -904,4 +904,16 @@ static int __init fsl_pci_init(void)
>>>> 	return platform_driver_register(&fsl_pci_driver);
>>>> }
>>>> arch_initcall(fsl_pci_init);
>>>> +
>>>> +static int __init fsl_pci_err_en(void)
>>>> +{
>>>> +	struct device_node *np;
>>>> +
>>>> +	for_each_node_by_type(np, "pci")
>>>> +		if (of_match_node(pci_ids, np))
>>>> +			mpc85xx_pci_err_en(np);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +device_initcall(fsl_pci_err_en);
>>> 
>>> Why can't you call this from the normal PCIe controller init, instead of searching for the node independently?
>> Don't we have this now with mpc85xx_pci_err_probe() ??
> 
> What do you mean by "this"?

I'm saying don't we replace fsl_pci_err_en() with mpc85xx_pci_err_probe()...

I need to look at this more, but not clear why mpc85xx_pci_err_en() can just be part of mpc85xx_pci_err_probe()

- k
Lan Chunhe-B25806 Sept. 28, 2012, 2:29 p.m. UTC | #5
On 09/27/2012 12:09 PM, Scott Wood wrote:
> On 09/27/2012 02:02:03 PM, Chunhe Lan wrote:
>> Original process of call:
>>     The mpc85xx_pci_err_probe function completes to been registered
>>     and enabled of EDAC PCI err driver at the latter time stage of
>>     kernel boot in the mpc85xx_edac.c.
>> Current process of call:
>>     The mpc85xx_pci_err_probe function completes to been registered
>>     and enabled of EDAC PCI err driver at the first    time stage of
>>     kernel boot in the fsl_pci.c.
>>
>> So in this case the following error messages appear in the boot log:
>>
>>     PCI: Probing PCI hardware
>>     pci 0000:00:00.0: ignoring class b20 (doesn't match header type 01)
>>     PCIE error(s) detected
>>     PCIE ERR_DR register: 0x00020000
>>     PCIE ERR_CAP_STAT register: 0x80000001
>>     PCIE ERR_CAP_R0 register: 0x00000800
>>     PCIE ERR_CAP_R1 register: 0x00000000
>>     PCIE ERR_CAP_R2 register: 0x00000000
>>     PCIE ERR_CAP_R3 register: 0x00000000
>>
>> Because the EDAC PCI err driver is registered and enabled earlier than
>> original point of call. But at this point of time, PCI hardware is not
>> probed and initialized, and it is in unknowable state.
>>
>> So, move enable function into mpc85xx_pci_err_en which is called at the
>> middle time stage of kernel boot and after PCI hardware is probed and
>> initialized by device_initcall in the fsl_pci.c.
>>
>> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
>> ---
>>  arch/powerpc/sysdev/fsl_pci.c |   12 ++++++++++
>>  arch/powerpc/sysdev/fsl_pci.h |    5 ++++
>>  drivers/edac/mpc85xx_edac.c   |   47 
>> ++++++++++++++++++++++++++++------------
>>  3 files changed, 50 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/fsl_pci.c 
>> b/arch/powerpc/sysdev/fsl_pci.c
>> index 3d6f4d8..a591965 100644
>> --- a/arch/powerpc/sysdev/fsl_pci.c
>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>> @@ -904,4 +904,16 @@ static int __init fsl_pci_init(void)
>>      return platform_driver_register(&fsl_pci_driver);
>>  }
>>  arch_initcall(fsl_pci_init);
>> +
>> +static int __init fsl_pci_err_en(void)
>> +{
>> +    struct device_node *np;
>> +
>> +    for_each_node_by_type(np, "pci")
>> +        if (of_match_node(pci_ids, np))
>> +            mpc85xx_pci_err_en(np);
>> +
>> +    return 0;
>> +}
>> +device_initcall(fsl_pci_err_en);
>
> Why can't you call this from the normal PCIe controller init, instead 
> of searching for the node independently?
     Because PCIe controller init is earlier than device_initcall, and 
it does not fix this issue.
     Do you indicate that which function calls fsl_pci_err_en in the 
which file ?

     Thanks,
     Chunhe
>
> -Scott
diff mbox

Patch

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 3d6f4d8..a591965 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -904,4 +904,16 @@  static int __init fsl_pci_init(void)
 	return platform_driver_register(&fsl_pci_driver);
 }
 arch_initcall(fsl_pci_init);
+
+static int __init fsl_pci_err_en(void)
+{
+	struct device_node *np;
+
+	for_each_node_by_type(np, "pci")
+		if (of_match_node(pci_ids, np))
+			mpc85xx_pci_err_en(np);
+
+	return 0;
+}
+device_initcall(fsl_pci_err_en);
 #endif
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index 796fe55..62a7323 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -136,11 +136,16 @@  static inline void fsl_pci_assign_primary(void) {}
 
 #ifdef CONFIG_EDAC_MPC85XX
 int mpc85xx_pci_err_probe(struct platform_device *op);
+int __init mpc85xx_pci_err_en(struct device_node *np);
 #else
 static inline int mpc85xx_pci_err_probe(struct platform_device *op)
 {
 	return -ENOTSUPP;
 }
+static inline int __init mpc85xx_pci_err_en(struct device_node *np)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #endif /* __POWERPC_FSL_PCI_H */
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index 05ef1f2..c317683 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -394,9 +394,6 @@  int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
 	pdata->orig_pci_err_en = in_be32(&reg->pex_err_en);
 	out_be32(&reg->pex_err_en, 0);
 
-	/* clear all error bits */
-	out_be32(&reg->pex_err_dr, ~0);
-
 	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
 		edac_dbg(3, "failed edac_pci_add_device()\n");
 		goto err;
@@ -420,17 +417,6 @@  int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
 		       pdata->irq);
 	}
 
-	if (pdata->pcie_flag) {
-		/* enable all pcie error interrupt & error detect */
-		out_be32(&reg->pex_err_en, ~0);
-		out_be32(&reg->pex_err_disr, 0);
-	} else {
-		/* PCI master abort is expected during config cycles */
-		out_be32(&reg->pex_err_cap_dr, PCI_ERR_CAP_DR_DIS_MST);
-		/* disable master abort reporting */
-		out_be32(&reg->pex_err_en, PCI_ERR_EN_DIS_MST);
-	}
-
 	devres_remove_group(&op->dev, mpc85xx_pci_err_probe);
 	edac_dbg(3, "success\n");
 	printk(KERN_INFO EDAC_MOD_STR " PCI err registered\n");
@@ -446,6 +432,39 @@  err:
 }
 EXPORT_SYMBOL(mpc85xx_pci_err_probe);
 
+int __init mpc85xx_pci_err_en(struct device_node *np)
+{
+	struct mpc85xx_pci_pdata pdata;
+	struct resource res;
+	struct ccsr_pci *reg;
+
+	if (mpc85xx_pcie_find_capability(np) > 0)
+		pdata.pcie_flag = 1;
+
+	of_address_to_resource(np, 0, &res);
+	pdata.pci_reg = ioremap(res.start, (res.end - res.start + 1));
+	reg = pdata.pci_reg;
+
+	/* clear all error bits */
+	out_be32(&reg->pex_err_dr, ~0);
+
+	if (pdata.pcie_flag) {
+		/* enable all pcie error interrupt & error detect */
+		out_be32(&reg->pex_err_en, ~0);
+		out_be32(&reg->pex_err_disr, 0);
+	} else {
+		/* PCI master abort is expected during config cycles */
+		out_be32(&reg->pex_err_cap_dr, PCI_ERR_CAP_DR_DIS_MST);
+		/* disable master abort reporting */
+		out_be32(&reg->pex_err_en, PCI_ERR_EN_DIS_MST);
+	}
+
+	pr_info(EDAC_MOD_STR " PCI err enabled\n");
+
+	return 0;
+}
+EXPORT_SYMBOL(mpc85xx_pci_err_en);
+
 static int mpc85xx_pci_err_remove(struct platform_device *op)
 {
 	struct edac_pci_ctl_info *pci = dev_get_drvdata(&op->dev);