Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
>[ cc Richard ]
>
>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>in eeh_subsystem_flags.
>>
>>In PowerNV platform, it try to create sysfs entry before
>>EEH_ENABLED been set, when boot up. So nothing will be
>>created for eeh in sysfs.
>>
>
>Could you please make the commit log more clear? :-)
>
>I guess the issue is introduced by commit 2213fb1 ("
>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>commit checks EEH is enabled while creating PCI device
>EEH sysfs files. If not, the sysfs files won't be created.
>That's to avoid warning reported during PCI hotplug.
>
>The problem you're reporting (if I understand completely):
>You don't see the sysfs files after the system boots up.
>If it's the case, you probably need following changes in
>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>Could you have a try with it?
>
>#ifdef CONFIG_EEH
>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>-	eeh_addr_cache_build();
>	eeh_init();
>+	eeh_addr_cache_build();
>#endif
>

I think this is a more proper fix.

BTW, I have one confusion in this mode set.

eeh_init()
  -> eeh_ops->dev_probe()
     -> powernv_eeh_dev_probe()
        -> eeh_set_enable(true)   <- here the eeh is marked enabled

We can see this flag would be set for each pci_dev. So is it possible to make
this "set" only once?

>Eventually PowerNV/pSeries have same function call sequence:
>
>- Set EEH probe mode
>- Doing probe (with device node or PCI device)
>- Build address cache.
>
>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 8ad0c5b..5f95581 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>>+	/* Creat sysfs after EEH_ENABLED been set */
>>+	eeh_add_sysfs_files(hose->bus);
>>+
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me