[PATCH v2 5/5] ALSA: korg1212: snd_korg1212_prepare: Rename del_timer in comment

WangYuli posted 5 patches 8 months, 1 week ago
[PATCH v2 5/5] ALSA: korg1212: snd_korg1212_prepare: Rename del_timer in comment
Posted by WangYuli 8 months, 1 week ago
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;
 		*/
 	}
-- 
2.49.0
Re: [PATCH v2 5/5] ALSA: korg1212: snd_korg1212_prepare: Rename del_timer in comment
Posted by Philipp Stanner 8 months, 1 week ago
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.


>  	}
Re: [PATCH v2 5/5] ALSA: korg1212: snd_korg1212_prepare: Rename del_timer in comment
Posted by WangYuli 8 months, 1 week ago
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,

-- 
WangYuli
Re: [PATCH v2 5/5] ALSA: korg1212: snd_korg1212_prepare: Rename del_timer in comment
Posted by Takashi Iwai 8 months, 1 week ago
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