Hi Kai, > On Oct 27, 2020, at 19:38, Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> wrote: > > 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"? :) Typos :) Will update them in v3. > >> 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)? Yes, I was worried that this cleanup may regress the user again. > >> @@ -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. Sure, I think we should use different terms. > >> + azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & >> + ~STATESTS_INT_MASK); > > This would fit to one line now. Ok, will concat the lines. Kai-Heng > > Br, Kai