1. Patchew
  2. linux
  3. View series

[PATCH v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions

Ethan Carter Edwards posted 1 patch 1 year, 4 months ago
There is a newer version of this series
sound/pci/ctxfi/ctdaio.c | 50 ++++++++++++++--------------------------
1 file changed, 17 insertions(+), 33 deletions(-)
[PATCH v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions
Posted by Ethan Carter Edwards 1 year, 4 months ago 
There was a lote of code duplication in the dao_clear_left_input() and
dao_clear_right_input() functions. A new function, dao_clear_input(),
was created and now the left and right functions call it instead of
repeating themselves.

Link: https://lore.kernel.org/lkml/NyKCr2VHK_xCQDwNxFKKx2LVd2d_AC2f2j4eAvnD9uRPtb50i2AruCLOp6mHxsGiyYJ0Tgd3Z50Oy1JTi5gPhjd2WQM2skrv7asp3fLl8HU=@ethancedwards.com/

Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
Co-developed-by: David Laight <david.laight.linux@gmail.com>
---
 Changes in V2: Fixed formatting caused by email client. Sorry!
 sound/pci/ctxfi/ctdaio.c | 50 ++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/sound/pci/ctxfi/ctdaio.c b/sound/pci/ctxfi/ctdaio.c
index 83aaf9441ef3..d46542d0b454 100644
--- a/sound/pci/ctxfi/ctdaio.c
+++ b/sound/pci/ctxfi/ctdaio.c
@@ -211,52 +211,36 @@ static int dao_set_right_input(struct dao *dao, struct rsc *input)
 	return 0;
 }
 
-static int dao_clear_left_input(struct dao *dao)
+static int dao_clear_input(struct dao *dao, unsigned int start, unsigned int end)
 {
-	struct imapper *entry;
-	struct daio *daio = &dao->daio;
-	int i;
+	struct imapper *to_free = dao->imappers[start];
+	unsigned int i;
 
-	if (!dao->imappers[0])
+	if (!to_free)
 		return 0;
-
-	entry = dao->imappers[0];
-	dao->mgr->imap_delete(dao->mgr, entry);
-	/* Program conjugate resources */
-	for (i = 1; i < daio->rscl.msr; i++) {
-		entry = dao->imappers[i];
-		dao->mgr->imap_delete(dao->mgr, entry);
+	for (i = start; i < end; i++) {
+		dao->mgr->imap_delete(dao->mgr, dao->imappers[i]);
 		dao->imappers[i] = NULL;
 	}
 
-	kfree(dao->imappers[0]);
-	dao->imappers[0] = NULL;
-
+	kfree(to_free);
 	return 0;
 }
 
-static int dao_clear_right_input(struct dao *dao)
-{
-	struct imapper *entry;
-	struct daio *daio = &dao->daio;
-	int i;
 
-	if (!dao->imappers[daio->rscl.msr])
-		return 0;
+static int dao_clear_left_input(struct dao *dao)
+{
+	u32 offset = dao->daio.rscl.msr;
 
-	entry = dao->imappers[daio->rscl.msr];
-	dao->mgr->imap_delete(dao->mgr, entry);
-	/* Program conjugate resources */
-	for (i = 1; i < daio->rscr.msr; i++) {
-		entry = dao->imappers[daio->rscl.msr + i];
-		dao->mgr->imap_delete(dao->mgr, entry);
-		dao->imappers[daio->rscl.msr + i] = NULL;
-	}
+	return dao_clear_input(dao, 0, 0 + offset);
+}
 
-	kfree(dao->imappers[daio->rscl.msr]);
-	dao->imappers[daio->rscl.msr] = NULL;
+static int dao_clear_right_input(struct dao *dao)
+{
+	u32 start = dao->daio.rscl.msr;
+	u32 offset = dao->daio.rscr.msr;
 
-	return 0;
+	return dao_clear_input(dao, start, start + offset);
 }
 
 static const struct dao_rsc_ops dao_ops = {
-- 
2.47.1
Re: [PATCH v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions
Posted by David Laight 1 year, 4 months ago 
On Mon, 30 Dec 2024 15:32:58 +0000
Ethan Carter Edwards <ethan@ethancedwards.com> wrote:

> There was a lote of code duplication in the dao_clear_left_input() and
> dao_clear_right_input() functions. A new function, dao_clear_input(),
> was created and now the left and right functions call it instead of
> repeating themselves.
> 
> Link: https://lore.kernel.org/lkml/NyKCr2VHK_xCQDwNxFKKx2LVd2d_AC2f2j4eAvnD9uRPtb50i2AruCLOp6mHxsGiyYJ0Tgd3Z50Oy1JTi5gPhjd2WQM2skrv7asp3fLl8HU=@ethancedwards.com/
> 
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> Co-developed-by: David Laight <david.laight.linux@gmail.com>
> ---
>  Changes in V2: Fixed formatting caused by email client. Sorry!
>  sound/pci/ctxfi/ctdaio.c | 50 ++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/sound/pci/ctxfi/ctdaio.c b/sound/pci/ctxfi/ctdaio.c
> index 83aaf9441ef3..d46542d0b454 100644
> --- a/sound/pci/ctxfi/ctdaio.c
> +++ b/sound/pci/ctxfi/ctdaio.c
> @@ -211,52 +211,36 @@ static int dao_set_right_input(struct dao *dao, struct rsc *input)
>  	return 0;
>  }
>  
> -static int dao_clear_left_input(struct dao *dao)
> +static int dao_clear_input(struct dao *dao, unsigned int start, unsigned int end)
>  {
> -	struct imapper *entry;
> -	struct daio *daio = &dao->daio;
> -	int i;
> +	struct imapper *to_free = dao->imappers[start];
> +	unsigned int i;
>  
> -	if (!dao->imappers[0])
> +	if (!to_free)
>  		return 0;
> -
> -	entry = dao->imappers[0];
> -	dao->mgr->imap_delete(dao->mgr, entry);
> -	/* Program conjugate resources */
> -	for (i = 1; i < daio->rscl.msr; i++) {
> -		entry = dao->imappers[i];
> -		dao->mgr->imap_delete(dao->mgr, entry);
> +	for (i = start; i < end; i++) {
> +		dao->mgr->imap_delete(dao->mgr, dao->imappers[i]);
>  		dao->imappers[i] = NULL;
>  	}
>  
> -	kfree(dao->imappers[0]);
> -	dao->imappers[0] = NULL;
> -
> +	kfree(to_free);
>  	return 0;
>  }
>  
> -static int dao_clear_right_input(struct dao *dao)
> -{
> -	struct imapper *entry;
> -	struct daio *daio = &dao->daio;
> -	int i;
>  
> -	if (!dao->imappers[daio->rscl.msr])
> -		return 0;
> +static int dao_clear_left_input(struct dao *dao)
> +{
> +	u32 offset = dao->daio.rscl.msr;
>  
> -	entry = dao->imappers[daio->rscl.msr];
> -	dao->mgr->imap_delete(dao->mgr, entry);
> -	/* Program conjugate resources */
> -	for (i = 1; i < daio->rscr.msr; i++) {
> -		entry = dao->imappers[daio->rscl.msr + i];
> -		dao->mgr->imap_delete(dao->mgr, entry);
> -		dao->imappers[daio->rscl.msr + i] = NULL;
> -	}
> +	return dao_clear_input(dao, 0, 0 + offset);

I really wouldn't bother with the temporary variable.
It is also a 'length' not an offset and you can't decide on the type.
'unsigned int' is fine.

But I suspect it really is worth checking if that *imappers[] is needed at all.
(Rename and see where the build breaks!)
You might just need struct imappers *imappers_left and *imappers_right to hold
base of the kmalloc()ed block.
At that point the functions collapse down even more.

	David

> +}
>  
> -	kfree(dao->imappers[daio->rscl.msr]);
> -	dao->imappers[daio->rscl.msr] = NULL;
> +static int dao_clear_right_input(struct dao *dao)
> +{
> +	u32 start = dao->daio.rscl.msr;
> +	u32 offset = dao->daio.rscr.msr;
>  
> -	return 0;
> +	return dao_clear_input(dao, start, start + offset);
>  }
>  
>  static const struct dao_rsc_ops dao_ops = {
Re: [PATCH v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions
Posted by Kuan-Wei Chiu 1 year, 4 months ago 
Hi Ethan,

Thank you for your patch. However, there are a few issues:

On Mon, Dec 30, 2024 at 03:32:58PM +0000, Ethan Carter Edwards wrote:
> There was a lote of code duplication in the dao_clear_left_input() and
> dao_clear_right_input() functions. A new function, dao_clear_input(),
> was created and now the left and right functions call it instead of
> repeating themselves.
> 
> Link: https://lore.kernel.org/lkml/NyKCr2VHK_xCQDwNxFKKx2LVd2d_AC2f2j4eAvnD9uRPtb50i2AruCLOp6mHxsGiyYJ0Tgd3Z50Oy1JTi5gPhjd2WQM2skrv7asp3fLl8HU=@ethancedwards.com/
>
Please move the Link: tag to the SoB area without an empty line.

> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> Co-developed-by: David Laight <david.laight.linux@gmail.com>

The Co-developed-by: tag should be immediately followed by the
co-author's Signed-off-by: tag. Additionally, the last Signed-off-by:
tag must always belong to the patch submitter.

See: https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Regards,
Kuan-Wei
Re: [PATCH v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions
Posted by Ethan Carter Edwards 1 year, 4 months ago 
On 24/12/30 03:32PM, Ethan Carter Edwards wrote:
> There was a lote of code duplication in the dao_clear_left_input() and
> dao_clear_right_input() functions. A new function, dao_clear_input(),
> was created and now the left and right functions call it instead of
> repeating themselves.
> 
> Link: https://lore.kernel.org/lkml/NyKCr2VHK_xCQDwNxFKKx2LVd2d_AC2f2j4eAvnD9uRPtb50i2AruCLOp6mHxsGiyYJ0Tgd3Z50Oy1JTi5gPhjd2WQM2skrv7asp3fLl8HU=@ethancedwards.com/
> 
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> Co-developed-by: David Laight <david.laight.linux@gmail.com>
> ---
>  Changes in V2: Fixed formatting caused by email client. Sorry!
>  sound/pci/ctxfi/ctdaio.c | 50 ++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 33 deletions(-)

Alright. It appears my email server is mangling my patches somehow. I
have tried using both mutt and git send-email, and it appears that they
are still messed up.

I'm using Protonmail as my server with its bridge locally. Does anyone
have any experience with working around this? I can't immediately find
anything online.

Apologies.
Ethan
-- 
Ethan Carter Edwards
CompTIA A+, Security+, and ISC2 (CC)
Ham Radio: AE4CE
Website: https://ethancedwards.com
GPG: 2E51 F618 39D1 FA94 7A73  00C2 34C0 4305 D581 DBFE

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.