| Message ID | 590769506CF46967+20250414042629.63019-5-wangyuli@uniontech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Mon, 2025-04-14 at 12:26 +0800, WangYuli wrote: > Commit 8fa7292fee5c ("treewide: Switch/rename to > timer_delete[_sync]()") > switched del_timer to timer_delete, but did not modify the comment > for > snd_korg1212_prepare(). Now fix it. > > Cc: Jaroslav Kysela <perex@perex.cz> > Cc: Takashi Iwai <tiwai@suse.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Philipp Stanner <pstanner@redhat.com> > Cc: SOUND <linux-sound@vger.kernel.org> > Cc: linux-kernel@vger.kernel.org > Signed-off-by: WangYuli <wangyuli@uniontech.com> > --- > sound/pci/korg1212/korg1212.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/pci/korg1212/korg1212.c > b/sound/pci/korg1212/korg1212.c > index 49b71082c485..5e69bb25d420 100644 > --- a/sound/pci/korg1212/korg1212.c > +++ b/sound/pci/korg1212/korg1212.c > @@ -1553,7 +1553,7 @@ static int snd_korg1212_prepare(struct > snd_pcm_substream *substream) > return -EAGAIN; > /* > korg1212->sharedBufferPtr->cardCommand = 0; > - del_timer(&korg1212->timer); > + timer_delete(&korg1212->timer); > korg1212->stop_pending_cnt = 0; > */ Wouldn't it be better to just remove all that? Or at least document what it is good for? P. > }
Hi Philipp Stanner, On 2025/4/14 16:34, Philipp Stanner wrote: > Wouldn't it be better to just remove all that? Or at least document > what it is good for? This code comes from commit 4d6dcf60b9 ("ALSA update - documentation - control API - added multi-elements to reduce memory usage - improved preallocation of DMA buffers - CS46xx driver - added support for secondary codec - HDSP driver - big update - firmware is loaded with hdsptool now - pmac driver updates (fixed oops and beep stuff) - VIA82xx driver updated - ymfpci driver updated - drivers updated to new PnP layer - wavefront, ad1816a, cs423x, es18xx, interwave, opl3sa2, cmi8330") in the history tree [1] as part of a huge patch. I couldn't find any useful information about this specific code within that commit. Since I'm uncertain if we truly don't need it (this code is marked with "FIXME" above), I've kept this comment. Furthermore, this driver has other sections of commented-out code as well, so a more comprehensive cleanup of this driver might be a better approach in the future. If possible, I'd also be happy to contribute to that effort. However, this commit isn't intended for that purpose. Its sole aim is to clean up the comment that were missed by commit 8fa7292fee5c ("treewide: Switch/rename to timer_delete_sync") and should have been updated. [1]. https://web.git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=4d6dcf60b98000c204b309b529c10b2a3884789e Thanks,
On Mon, 14 Apr 2025 13:10:39 +0200, WangYuli wrote: > > Hi Philipp Stanner, > > On 2025/4/14 16:34, Philipp Stanner wrote: > > Wouldn't it be better to just remove all that? Or at least document > > what it is good for? > > This code comes from commit 4d6dcf60b9 ("ALSA update - documentation > - control API - added multi-elements to reduce memory usage - > improved preallocation of DMA buffers - CS46xx driver - added > support for secondary codec - HDSP driver - big update - > firmware is loaded with hdsptool now - pmac driver updates (fixed > oops and beep stuff) - VIA82xx driver updated - ymfpci driver > updated - drivers updated to new PnP layer - wavefront, ad1816a, > cs423x, es18xx, interwave, opl3sa2, cmi8330") in the history tree [1] > as part of a huge patch. > > I couldn't find any useful information about this specific code within > that commit. > > Since I'm uncertain if we truly don't need it (this code is marked > with "FIXME" above), I've kept this comment. > > Furthermore, this driver has other sections of commented-out code as > well, so a more comprehensive cleanup of this driver might be a better > approach in the future. > > If possible, I'd also be happy to contribute to that effort. > > However, this commit isn't intended for that purpose. > > Its sole aim is to clean up the comment that were missed by commit > 8fa7292fee5c ("treewide: Switch/rename to timer_delete_sync") and > should have been updated. > > > [1]. https://web.git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=4d6dcf60b98000c204b309b529c10b2a3884789e Yeah, the code is quite old and dirty, and this timer stuff can be omitted completely by introducing the proper sync_stop PCM ops. Let me rewrite the code instead. thanks, Takashi
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c index 49b71082c485..5e69bb25d420 100644 --- a/sound/pci/korg1212/korg1212.c +++ b/sound/pci/korg1212/korg1212.c @@ -1553,7 +1553,7 @@ static int snd_korg1212_prepare(struct snd_pcm_substream *substream) return -EAGAIN; /* korg1212->sharedBufferPtr->cardCommand = 0; - del_timer(&korg1212->timer); + timer_delete(&korg1212->timer); korg1212->stop_pending_cnt = 0; */ }
Commit 8fa7292fee5c ("treewide: Switch/rename to timer_delete[_sync]()") switched del_timer to timer_delete, but did not modify the comment for snd_korg1212_prepare(). Now fix it. Cc: Jaroslav Kysela <perex@perex.cz> Cc: Takashi Iwai <tiwai@suse.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Philipp Stanner <pstanner@redhat.com> Cc: SOUND <linux-sound@vger.kernel.org> Cc: linux-kernel@vger.kernel.org Signed-off-by: WangYuli <wangyuli@uniontech.com> --- sound/pci/korg1212/korg1212.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)