On Tue, Jun 8, 2021 at 4:18 PM Limonciello, Mario <mario.limonciello@xxxxxxx> wrote: > > On 6/8/2021 06:20, Rafael J. Wysocki wrote: > > On Tue, Jun 8, 2021 at 7:35 AM Christoph Hellwig <hch@xxxxxx> 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.
