| Message ID | 20241230153243.2874102-1-ethan@ethancedwards.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ALSA: ctxfi: Simplify dao_clear_{left,right}_input() functions | expand |
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
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
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 = {
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 = {
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(-)