On 07/21/2016 11:36 PM, Gavin Shan wrote:
> On Fri, Jul 22, 2016 at 03:23:36PM +1000, Russell Currey wrote:
>> On EEH events the kernel will print a dump of relevant registers.
>> If EEH is unavailable (i.e. CONFIG_EEH is disabled, a new platform
>> doesn't have EEH support, etc) this information isn't readily available.
>>
>> Add a new debugfs handler to trigger a PHB register dump, so that this
>> information can be made available on demand.
>>
>> Signed-off-by: Russell Currey <rus...@russell.cc>
>
> Reviewed-by: Gavin Shan <gws...@linux.vnet.ibm.com>
>
>> ---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 35
>> +++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
>> b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 891fc4a..ada2f3c 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -3018,6 +3018,38 @@ static void pnv_ioda_setup_pe_seg(struct pnv_ioda_pe
>> *pe)
>> }
>> }
>>
>> +#ifdef CONFIG_DEBUG_FS
>> +static ssize_t pnv_pci_debug_write(struct file *filp,
>> + const char __user *user_buf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct pci_controller *hose = filp->private_data;
>> + struct pnv_phb *phb;
>> + int ret = 0;
>
> Needn't initialize @ret in advance. The code might be simpler, but it's
> only a personal preference:
I believe its actually preferred that it not be initialized in advance
so that the tooling can warn you about conditional code paths where you
may have forgotten to set a value. Or as Gavin suggests to explicitly
use error values in the return statements.
-Tyrel
>
> struct pci_controller *hose = filp->private_data;
> struct pnv_phb *phb = hose ? hose->private_data : NULL;
>
> if (!phb)
> return -ENODEV;
>
>> +
>> + if (!hose)
>> + return -EFAULT;
>> +
>> + phb = hose->private_data;
>> + if (!phb)
>> + return -EFAULT;
>> +
>> + ret = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
>> + PNV_PCI_DIAG_BUF_SIZE);
>> +
>> + if (!ret)
>> + pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob);
>> +
>> + return ret < 0 ? ret : count;
>
> return ret == OPAL_SUCCESS ? count : -EIO;
>
>> +}
>> +
>> +static const struct file_operations pnv_pci_debug_ops = {
>> + .open = simple_open,
>> + .llseek = no_llseek,
>> + .write = pnv_pci_debug_write,
>
> It might be reasonable to dump the diag-data on read if it is trying
> to do it on write.
>
>> +};
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>> static void pnv_pci_ioda_create_dbgfs(void)
>> {
>> #ifdef CONFIG_DEBUG_FS
>> @@ -3036,6 +3068,9 @@ static void pnv_pci_ioda_create_dbgfs(void)
>> if (!phb->dbgfs)
>> pr_warning("%s: Error on creating debugfs on PHB#%x\n",
>> __func__, hose->global_number);
>> +
>> + debugfs_create_file("regdump", 0200, phb->dbgfs, hose,
>> + &pnv_pci_debug_ops);
>
> "diag-data" might be indicating or a better one you can name :)
>
> Thanks,
> Gavin
>
>> }
>> #endif /* CONFIG_DEBUG_FS */
>> }
>> --
>> 2.9.0
>>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
