[PATCH] Documentation: PM: Drop pme_interrupt reference

[PATCH] Documentation: PM: Drop pme_interrupt reference

[Mario Limonciello]

`pme_interrupt` was dropped from `struct pci_dev` as part of commit
8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev"),
but the Documentation still includes this member.

Remove it from the documentation as well and update it to have the missing
`pme_poll` member instead.

Fixes: 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/power/pci.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index b04fb18cc4e2..a125544b4cb6 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -315,7 +315,7 @@ that these callbacks operate on::
 					   configuration space */
 	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
 					   can be generated */
-	unsigned int	pme_interrupt:1;/* Is native PCIe PME signaling used? */
+	unsigned int	pme_poll:1;	/* Poll device's PME status bit */
 	unsigned int	d1_support:1;	/* Low power state D1 is supported */
 	unsigned int	d2_support:1;	/* Low power state D2 is supported */
 	unsigned int	no_d1d2:1;	/* D1 and D2 are forbidden */
-- 
2.34.1

Re: [PATCH] Documentation: PM: Drop pme_interrupt reference

[Mika Westerberg]

On Thu, Jun 02, 2022 at 11:33:30AM -0500, Mario Limonciello wrote:
> `pme_interrupt` was dropped from `struct pci_dev` as part of commit
> 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev"),
> but the Documentation still includes this member.
> 
> Remove it from the documentation as well and update it to have the missing
> `pme_poll` member instead.
> 
> Fixes: 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH] Documentation: PM: Drop pme_interrupt reference

[Bjorn Helgaas]

On Thu, Jun 02, 2022 at 11:33:30AM -0500, Mario Limonciello wrote:
> `pme_interrupt` was dropped from `struct pci_dev` as part of commit
> 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev"),
> but the Documentation still includes this member.
> 
> Remove it from the documentation as well and update it to have the missing
> `pme_poll` member instead.
> 
> Fixes: 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  Documentation/power/pci.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> index b04fb18cc4e2..a125544b4cb6 100644
> --- a/Documentation/power/pci.rst
> +++ b/Documentation/power/pci.rst
> @@ -315,7 +315,7 @@ that these callbacks operate on::
>  					   configuration space */
>  	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
>  					   can be generated */
> -	unsigned int	pme_interrupt:1;/* Is native PCIe PME signaling used? */
> +	unsigned int	pme_poll:1;	/* Poll device's PME status bit */
>  	unsigned int	d1_support:1;	/* Low power state D1 is supported */
>  	unsigned int	d2_support:1;	/* Low power state D2 is supported */
>  	unsigned int	no_d1d2:1;	/* D1 and D2 are forbidden */

I'm OK with this patch if Rafael wants to take it.

But I'm not sure how much value this section of the doc really adds.
The doc basically says "the PCI PM callbacks operate on several of
these fields of the struct pci_dev" and goes on to quote part of the
struct pci_dev.

But "pm_cap" is the only one of those fields that is mentioned
elsewhere in the doc, and that one is only incidental.

For example, is it really useful to say "the PCI PM callbacks use
pci_dev.pme_poll" without any other details about pme_poll?

I think I would consider just removing everything from "The structure
representing a PCI device ..." to the end of the section, i.e., lines
308-329 at [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/power/pci.rst?id=v5.18#n308

RE: [PATCH] Documentation: PM: Drop pme_interrupt reference

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Wednesday, June 8, 2022 17:29
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Len Brown <len.brown@intel.com>;
> Pavel Machek <pavel@ucw.cz>; Bjorn Helgaas <bhelgaas@google.com>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; open list:SUSPEND TO RAM
> <linux-pm@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] Documentation: PM: Drop pme_interrupt reference
> 
> On Thu, Jun 02, 2022 at 11:33:30AM -0500, Mario Limonciello wrote:
> > `pme_interrupt` was dropped from `struct pci_dev` as part of commit
> > 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev"),
> > but the Documentation still includes this member.
> >
> > Remove it from the documentation as well and update it to have the missing
> > `pme_poll` member instead.
> >
> > Fixes: 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev")
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >  Documentation/power/pci.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> > index b04fb18cc4e2..a125544b4cb6 100644
> > --- a/Documentation/power/pci.rst
> > +++ b/Documentation/power/pci.rst
> > @@ -315,7 +315,7 @@ that these callbacks operate on::
> >  					   configuration space */
> >  	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
> >  					   can be generated */
> > -	unsigned int	pme_interrupt:1;/* Is native PCIe PME signaling used?
> */
> > +	unsigned int	pme_poll:1;	/* Poll device's PME status bit */
> >  	unsigned int	d1_support:1;	/* Low power state D1 is supported */
> >  	unsigned int	d2_support:1;	/* Low power state D2 is supported */
> >  	unsigned int	no_d1d2:1;	/* D1 and D2 are forbidden */
> 
> I'm OK with this patch if Rafael wants to take it.
> 
> But I'm not sure how much value this section of the doc really adds.
> The doc basically says "the PCI PM callbacks operate on several of
> these fields of the struct pci_dev" and goes on to quote part of the
> struct pci_dev.

The reason that the patch came up is that someone who doesn't normally look
at kernel code but looked at documentation reached out asking questions about
these variables at the time a bug was occurring.  I was baffled how they were referring
to pme_interrupt until I found that it's only mentioned in documentation since the linked
Fixes tag.  So I figured we should make the documentation match the code.

> 
> But "pm_cap" is the only one of those fields that is mentioned
> elsewhere in the doc, and that one is only incidental.
> 
> For example, is it really useful to say "the PCI PM callbacks use
> pci_dev.pme_poll" without any other details about pme_poll?
> 
> I think I would consider just removing everything from "The structure
> representing a PCI device ..." to the end of the section, i.e., lines
> 308-329 at [1].
> 

That's perfectly fine to me too.

> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree
> %2FDocumentation%2Fpower%2Fpci.rst%3Fid%3Dv5.18%23n308&amp;data=05
> %7C01%7Cmario.limonciello%40amd.com%7C2302a7c692c545aa56f808da499
> e5ade%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637903241734
> 399580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=qP0
> NdVaGMl2wzbiWzc8t4eMmilwogsLhEvzn6aJtsD8%3D&amp;reserved=0

Re: [PATCH] Documentation: PM: Drop pme_interrupt reference

[Rafael J. Wysocki]

Sorry for the delay.

On Mon, Jun 13, 2022 at 5:57 PM Limonciello, Mario
<Mario.Limonciello@amd.com> wrote:
>
> [Public]
>
>
>
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: Wednesday, June 8, 2022 17:29
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Cc: Rafael J. Wysocki <rafael@kernel.org>; Len Brown <len.brown@intel.com>;
> > Pavel Machek <pavel@ucw.cz>; Bjorn Helgaas <bhelgaas@google.com>; Mika
> > Westerberg <mika.westerberg@linux.intel.com>; open list:SUSPEND TO RAM
> > <linux-pm@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH] Documentation: PM: Drop pme_interrupt reference
> >
> > On Thu, Jun 02, 2022 at 11:33:30AM -0500, Mario Limonciello wrote:
> > > `pme_interrupt` was dropped from `struct pci_dev` as part of commit
> > > 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev"),
> > > but the Documentation still includes this member.
> > >
> > > Remove it from the documentation as well and update it to have the missing
> > > `pme_poll` member instead.
> > >
> > > Fixes: 8370c2dc4c7b ("PCI / PM: Drop pme_interrupt flag from struct pci_dev")
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >  Documentation/power/pci.rst | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> > > index b04fb18cc4e2..a125544b4cb6 100644
> > > --- a/Documentation/power/pci.rst
> > > +++ b/Documentation/power/pci.rst
> > > @@ -315,7 +315,7 @@ that these callbacks operate on::
> > >                                        configuration space */
> > >     unsigned int    pme_support:5;  /* Bitmask of states from which PME#
> > >                                        can be generated */
> > > -   unsigned int    pme_interrupt:1;/* Is native PCIe PME signaling used?
> > */
> > > +   unsigned int    pme_poll:1;     /* Poll device's PME status bit */
> > >     unsigned int    d1_support:1;   /* Low power state D1 is supported */
> > >     unsigned int    d2_support:1;   /* Low power state D2 is supported */
> > >     unsigned int    no_d1d2:1;      /* D1 and D2 are forbidden */
> >
> > I'm OK with this patch if Rafael wants to take it.
> >
> > But I'm not sure how much value this section of the doc really adds.
> > The doc basically says "the PCI PM callbacks operate on several of
> > these fields of the struct pci_dev" and goes on to quote part of the
> > struct pci_dev.
>
> The reason that the patch came up is that someone who doesn't normally look
> at kernel code but looked at documentation reached out asking questions about
> these variables at the time a bug was occurring.  I was baffled how they were referring
> to pme_interrupt until I found that it's only mentioned in documentation since the linked
> Fixes tag.  So I figured we should make the documentation match the code.
>
> >
> > But "pm_cap" is the only one of those fields that is mentioned
> > elsewhere in the doc, and that one is only incidental.
> >
> > For example, is it really useful to say "the PCI PM callbacks use
> > pci_dev.pme_poll" without any other details about pme_poll?
> >
> > I think I would consider just removing everything from "The structure
> > representing a PCI device ..." to the end of the section, i.e., lines
> > 308-329 at [1].
> >
>
> That's perfectly fine to me too.

I've decided to apply the patch, because it is fine as is and the
cleanup mentioned above can be done on top of it just fine.

Applied as 5.20 material, thanks!