Quoting Russell Currey (2016-07-22 15:23:36)
> 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.

This is a bit weird.

It's a debugfs file, but when you read from it you get nothing (I think,
you have no read() defined).

When you write to it, regardless of what you write, the kernel spits
some stuff out to dmesg and throws away whatever you wrote.

Ideally pnv_pci_dump_phb_diag_data() would write its output to a buffer,
which we could then either send to dmesg, or give to debugfs. But that
might be more work than we want to do for this.

If we just want a trigger file, then I think it'd be preferable to just
use a simple attribute, with a set and no show, eg. something like:

static int foo_set(void *data, u64 val)
{
        if (val != 1)
                return -EINVAL;

        ...

        return 0;
}

DEFINE_SIMPLE_ATTRIBUTE(fops_foo, NULL, foo_set, "%llu\n");

That requires that you write "1" to the file to trigger the reg dump.


> 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
> @@ -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);
>         }

You shouldn't be trying to create the file if the directory create failed. So
the check for (!phb->dbgfs) should probably print and then continue.

And a better name would be "dump-regs", because it indicates that the file does
something, rather than is something.

cheers
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to