[PATCH v6 1/2] ACPI: Move check for _DSD StorageD3Enable property to acpi

[Rafael J. Wysocki]

On Tue, Jun 8, 2021 at 4:18 PM Limonciello, Mario
<mario.limonciello at amd.com> wrote:
>
> On 6/8/2021 06:20, Rafael J. Wysocki wrote:
> > On Tue, Jun 8, 2021 at 7:35 AM Christoph Hellwig <hch at lst.de> wrote:
> >>
> >> On Mon, Jun 07, 2021 at 12:31:55PM -0500, Mario Limonciello wrote:
> >>> +/**
> >>> + * acpi_storage_d3 - Check if a storage device should use D3.
> >
> > Let's be specific about what D3 means here in the first place and
> > that's D3hot AFAICS.
> >
> > And the comment should be something like "Check whether or not to use
> > D3hot in the suspend path".
>
> Actually it can be D3hot or D3cold.  Microsoft's documentation doesn't
> indicate it's D3hot.  On the AMD platforms that prompted some of these
> changes it's D3cold.

So say "D3" in the one-line description above and "D3hot or D3cold (if
supported)" in the more detailed comment below.

> >
> >>> + * @dev: Device to check
> >>> + *
> >>> + * Returns %true if @dev should be put into D3 when the ->suspend method is
> >>> + * called, else %false.  The name of this function is somewhat misleading
> >>> + * as it has nothing to do with storage except for the name of the ACPI
> >>> + * property.  On some platforms resume will not work if this hint is ignored.
> >
> > I would write it this way:
> >
> > "Return %true if the platform firmware wants @dev to be programmed
> > into D3hot in the suspend path, or %false when there is no specific
> > preference. On some platforms, if this hint is ignored, @dev may
> > remain unresponsive after suspending the platform as a whole."
> >
> > And I'm not sure if it is necessary to mention "storage" in this comment at all.
> >
>
> Is your thought here in not mentioning "storage" that this symbol may be
> overloaded in the future to look at more than just the StorageD3Enable
> property and used for other things too?

Well, the property itself is not about storage any more anyway.