[PATCH] powernv/pci: Add PHB register dump debugfs handle

[PATCH] powernv/pci: Add PHB register dump debugfs handle

[Russell Currey]

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 <ruscur@russell.cc>
---
 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;
+
+	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;
+}
+
+static const struct file_operations pnv_pci_debug_ops = {
+	.open	= simple_open,
+	.llseek	= no_llseek,
+	.write	= pnv_pci_debug_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);
 	}
 #endif /* CONFIG_DEBUG_FS */
 }
-- 
2.9.0

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Gavin Shan]

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 <ruscur@russell.cc>

Reviewed-by: Gavin Shan <gwshan@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:

	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
>

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Tyrel Datwyler]

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 <ruscur@russell.cc>
> 
> Reviewed-by: Gavin Shan <gwshan@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
> 

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Gavin Shan]

On Mon, Jul 25, 2016 at 10:53:49AM -0700, Tyrel Datwyler wrote:
>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 <ruscur@russell.cc>
>> 
>> Reviewed-by: Gavin Shan <gwshan@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.
>

Yeah, the data type should be int64_t as well.

Thanks,
Gavin

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Russell Currey]

On Fri, 2016-07-22 at 16:36 +1000, 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 <ruscur@russell.cc>
> 
> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Hi Gavin, thanks for the review.

> 
> > 
> > ---
> > 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:
> 
> 	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;

Yeah, that's much better.

> 
> > 
> > +}
> > +
> > +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.

I'm not sure about this one.  I went with write since (at least, in my mind)
writing to a file feels like triggering an action, although we're not actually
reading any input.  It also means that it works the same way as the other PHB
debugfs entries (i.e. errinjct).

I could rework it into a read that said something like "PHB#%x diag data dumped,
check the kernel log", what do you think?

> 
> > 
> > +};
> > +#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
> > 

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Michael Ellerman]

Tyrel Datwyler <tyreld@linux.vnet.ibm.com> writes:

> On 07/21/2016 11:36 PM, Gavin Shan wrote:
>> On Fri, Jul 22, 2016 at 03:23:36PM +1000, Russell Currey wrote:
>>> 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.

Yeah that's right, it's preferable not to initialise it.

It helps for complex if/else/switch cases, where you might accidentally
have a path where you return without giving ret the right value.

The other case is when someone modifies your code. For example if you
have:

	int ret;

        if (foo)
                ret = do_foo();
        else
                ret = 1;

        return ret;

And then you add a case to the if:

        if (foo)
                ret = do_foo();
        else if (bar)
        	do_bar();
        else
                ret = 1;

The compiler will warn you that in the bar case you forget to initialise
ret. Whereas if you initialised ret at the start then the compiler can't
help you.

There are times when it's cleaner to initialise the value at the start,
eg. if you have many error cases and only one success case. But that
should be a deliberate choice.

cheers

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Michael Ellerman]

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 !=3D 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/pla=
tforms/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

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Russell Currey]

On Tue, 2016-07-26 at 11:45 +1000, Michael Ellerman wrote:
> 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.

I don't think I can use this here.  Triggering the diag dump on the given PHB
(these are in /sys/kernel/debug/powerpc/PCI####), and that PHB is retrieved from
the file handler.  It looks like I have no access to the file struct if using a
simple getter/setter.

> 
> 
> > 
> > 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.

Good catch.

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

That is indeed better.

> 
> cheers

Re: [PATCH] powernv/pci: Add PHB register dump debugfs handle

[Michael Ellerman]

Russell Currey <ruscur@russell.cc> writes:

> On Tue, 2016-07-26 at 11:45 +1000, Michael Ellerman wrote:
>> Quoting Russell Currey (2016-07-22 15:23:36)
>>=20
>> DEFINE_SIMPLE_ATTRIBUTE(fops_foo, NULL, foo_set, "%llu\n");
>>=20
>> That requires that you write "1" to the file to trigger the reg dump.
>
> I don't think I can use this here. =C2=A0Triggering the diag dump on the =
given PHB
> (these are in /sys/kernel/debug/powerpc/PCI####), and that PHB is retriev=
ed from
> the file handler. =C2=A0It looks like I have no access to the file struct=
 if using a
> simple getter/setter.

You don't have access to the file struct, but you don't need it, can
register the fops with a data pointer.

So the DEFINE_SIMPLE_ATTRIBUTE() gives you a fops_foo, which you can
then do:

  debugfs_create_file("dump-regs", 0200, phb->dbgfs, hose, &fops_foo))

And then in foo_set() data =3D=3D hose.

cheers