linux-kernel - Re: [PATCH v2 3/4] ALSA: hda: Separate runtime and system suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Oct 2020 13:38:56 +0200 (EET)
From:   Kai Vehmanen <kai.vehmanen@...ux.intel.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
cc:     tiwai@...e.com, alsa-devel@...a-project.org,
        kai.vehmanen@...ux.intel.com, linux-kernel@...r.kernel.org,
        hui.wang@...onical.com
Subject: Re: [PATCH v2 3/4] ALSA: hda: Separate runtime and system suspend

Hi,

thanks, this looks like a good improvement! Some minor notes:

On Tue, 27 Oct 2020, Kai-Heng Feng wrote:

> Both pm_runtime_force_suspend() and pm_runtime_force_resume() have
> some implicit checks, so it can make code flow more straightfoward if we
> separate runtime and systemd suspend callbacks.

straightforward -> straightforward

and systemd? Maybe just "system suspend"? :)

> While at it, also remove AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP, as the
> original bug commit a6630529aecb ("ALSA: hda: Workaround for spurious
> wakeups on some Intel platforms") solves doesn't happen with this
> patch.

Hmm, so this was gone already with the v1 version (so not related to 
programming the WAKEEN when going to system suspend)?

> @@ -143,6 +143,7 @@ struct azx {
>  	unsigned int align_buffer_size:1;
>  	unsigned int region_requested:1;
>  	unsigned int disabled:1; /* disabled by vga_switcheroo */
> +	unsigned int prepared:1;

I wonder if "pm_prepared" would be better as ALSA API has a prepare method 
as well and this is not related. OTOH, if ok to Takashi, ok for me as 
well. 

> +	azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
> +		   ~STATESTS_INT_MASK);

This would fit to one line now. 

Br, Kai