From 81df174f8ab3c6f3d112c305e45c2a9fa460da51 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 20 Feb 2019 09:03:33 +0100 Subject: [PATCH 01/12] idc: add a missing preamble Add a missing preamble in a recently created idc.c Signed-off-by: Guennadi Liakhovetski --- src/arch/xtensa/smp/idc.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/arch/xtensa/smp/idc.c b/src/arch/xtensa/smp/idc.c index 18815367fcfa..5c4add87047d 100644 --- a/src/arch/xtensa/smp/idc.c +++ b/src/arch/xtensa/smp/idc.c @@ -1,3 +1,32 @@ +/* + * Copyright (c) 2018, Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the Intel Corporation nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * Author: Tomasz Lauda + */ #include #include From 0e0e650893e2f4e45f9d4942f8b18cfd7c4e7b79 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 20 Feb 2019 09:04:51 +0100 Subject: [PATCH 02/12] idc: check an interrupt registration return code Interrupt registration can fail, check its return code in idc.c and propagate error to the caller. Signed-off-by: Guennadi Liakhovetski --- src/arch/xtensa/smp/idc.c | 11 ++++++++--- src/arch/xtensa/smp/include/arch/idc.h | 2 +- src/arch/xtensa/smp/init.c | 4 +++- src/arch/xtensa/up/include/arch/idc.h | 2 +- src/library/include/platform/idc.h | 3 ++- src/platform/baytrail/include/platform/idc.h | 2 +- src/platform/haswell/include/platform/idc.h | 2 +- src/platform/intel/cavs/include/cavs/idc.h | 4 ++-- src/platform/intel/cavs/platform.c | 4 +++- 9 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/arch/xtensa/smp/idc.c b/src/arch/xtensa/smp/idc.c index 5c4add87047d..b87117ff2a33 100644 --- a/src/arch/xtensa/smp/idc.c +++ b/src/arch/xtensa/smp/idc.c @@ -328,9 +328,10 @@ static uint32_t idc_get_done_bit_mask(int core) /** * \brief Initializes IDC data and registers for interrupt. */ -void arch_idc_init(void) +int arch_idc_init(void) { int core = arch_cpu_get_id(); + int ret; trace_idc("arch_idc_init()"); @@ -346,13 +347,17 @@ void arch_idc_init(void) schedule_task_config(&(*idc)->idc_task, TASK_PRI_IDC, core); /* configure interrupt */ - interrupt_register(PLATFORM_IDC_INTERRUPT(core), IRQ_AUTO_UNMASK, - idc_irq_handler, *idc); + ret = interrupt_register(PLATFORM_IDC_INTERRUPT(core), IRQ_AUTO_UNMASK, + idc_irq_handler, *idc); + if (ret < 0) + return ret; interrupt_enable(PLATFORM_IDC_INTERRUPT(core)); /* enable BUSY and DONE (only for master core) interrupts */ idc_write(IPC_IDCCTL, core, (*idc)->busy_bit_mask | (*idc)->done_bit_mask); + + return 0; } /** diff --git a/src/arch/xtensa/smp/include/arch/idc.h b/src/arch/xtensa/smp/include/arch/idc.h index 5b7c07ad4feb..9b77b5cdc995 100644 --- a/src/arch/xtensa/smp/include/arch/idc.h +++ b/src/arch/xtensa/smp/include/arch/idc.h @@ -43,7 +43,7 @@ void cpu_power_down_core(void); void idc_enable_interrupts(int target_core, int source_core); int arch_idc_send_msg(struct idc_msg *msg, uint32_t mode); -void arch_idc_init(void); +int arch_idc_init(void); void idc_free(void); #endif diff --git a/src/arch/xtensa/smp/init.c b/src/arch/xtensa/smp/init.c index d91132c14fa4..06e720ac531f 100644 --- a/src/arch/xtensa/smp/init.c +++ b/src/arch/xtensa/smp/init.c @@ -131,7 +131,9 @@ int slave_core_init(struct sof *sof) /* initialize IDC mechanism */ trace_point(TRACE_BOOT_PLATFORM_IDC); - idc_init(); + err = idc_init(); + if (err < 0) + return err; trace_point(TRACE_BOOT_PLATFORM); diff --git a/src/arch/xtensa/up/include/arch/idc.h b/src/arch/xtensa/up/include/arch/idc.h index 585a57125d92..6d71f2d9055d 100644 --- a/src/arch/xtensa/up/include/arch/idc.h +++ b/src/arch/xtensa/up/include/arch/idc.h @@ -51,6 +51,6 @@ static inline int arch_idc_send_msg(struct idc_msg *msg, /** * \brief Initializes IDC data and registers for interrupt. */ -static inline void arch_idc_init(void) { } +static inline int arch_idc_init(void) { return 0; } #endif diff --git a/src/library/include/platform/idc.h b/src/library/include/platform/idc.h index 6228b634d64c..7007077164e8 100644 --- a/src/library/include/platform/idc.h +++ b/src/library/include/platform/idc.h @@ -40,8 +40,9 @@ static inline void idc_process_msg_queue(void) { } -static inline void idc_init(void) +static inline int idc_init(void) { + return 0; } #endif diff --git a/src/platform/baytrail/include/platform/idc.h b/src/platform/baytrail/include/platform/idc.h index 613ffa37c5a9..6669b8d677d1 100644 --- a/src/platform/baytrail/include/platform/idc.h +++ b/src/platform/baytrail/include/platform/idc.h @@ -36,6 +36,6 @@ struct idc_msg; static inline int idc_send_msg(struct idc_msg *msg, uint32_t mode) { return 0; } -static inline void idc_init(void) { } +static inline int idc_init(void) { return 0; } #endif diff --git a/src/platform/haswell/include/platform/idc.h b/src/platform/haswell/include/platform/idc.h index 613ffa37c5a9..6669b8d677d1 100644 --- a/src/platform/haswell/include/platform/idc.h +++ b/src/platform/haswell/include/platform/idc.h @@ -36,6 +36,6 @@ struct idc_msg; static inline int idc_send_msg(struct idc_msg *msg, uint32_t mode) { return 0; } -static inline void idc_init(void) { } +static inline int idc_init(void) { return 0; } #endif diff --git a/src/platform/intel/cavs/include/cavs/idc.h b/src/platform/intel/cavs/include/cavs/idc.h index 6cf29648521b..9d7b55e052ab 100644 --- a/src/platform/intel/cavs/include/cavs/idc.h +++ b/src/platform/intel/cavs/include/cavs/idc.h @@ -38,9 +38,9 @@ static inline int idc_send_msg(struct idc_msg *msg, uint32_t mode) return arch_idc_send_msg(msg, mode); } -static inline void idc_init(void) +static inline int idc_init(void) { - arch_idc_init(); + return arch_idc_init(); } #endif diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index 52dd0d6b9327..1c3a389dc721 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -513,7 +513,9 @@ int platform_init(struct sof *sof) /* initialize IDC mechanism */ trace_point(TRACE_BOOT_PLATFORM_IDC); - idc_init(); + ret = idc_init(); + if (ret < 0) + return ret; #if defined(CONFIG_DW_SPI) /* initialize the SPI slave */ From efc660036242a9289cdbf0e7fb49a6ae7af0631a Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 20 Feb 2019 09:05:41 +0100 Subject: [PATCH 03/12] init: (cosmetic) add panic.h where needed and remove where not The SMP version of init.c calls panic(), therefore it needs panic.h, OTOH the UP version doesn't call panic(), so the header isn't needed there. Signed-off-by: Guennadi Liakhovetski --- src/arch/xtensa/smp/init.c | 1 + src/arch/xtensa/up/init.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arch/xtensa/smp/init.c b/src/arch/xtensa/smp/init.c index 06e720ac531f..b5a0cee1f2f6 100644 --- a/src/arch/xtensa/smp/init.c +++ b/src/arch/xtensa/smp/init.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include diff --git a/src/arch/xtensa/up/init.c b/src/arch/xtensa/up/init.c index 1aeac1c9b0a5..1a013eaad07e 100644 --- a/src/arch/xtensa/up/init.c +++ b/src/arch/xtensa/up/init.c @@ -36,7 +36,6 @@ #include #include #include -#include #include /** From 8efa113a8aa8e54a11716e4036b356cd7697631c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 20 Feb 2019 09:13:18 +0100 Subject: [PATCH 04/12] cavs: propagate error codes instead of overriding them When returning an error, it's usually better to propagate the error code, that caused the termination, than overriding it. Signed-off-by: Guennadi Liakhovetski --- src/platform/intel/cavs/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/intel/cavs/platform.c b/src/platform/intel/cavs/platform.c index 1c3a389dc721..df3aede69aca 100644 --- a/src/platform/intel/cavs/platform.c +++ b/src/platform/intel/cavs/platform.c @@ -500,7 +500,7 @@ int platform_init(struct sof *sof) trace_point(TRACE_BOOT_PLATFORM_DMA); ret = dmac_init(); if (ret < 0) - return -ENODEV; + return ret; /* initialize the host IPC mechanisms */ trace_point(TRACE_BOOT_PLATFORM_IPC); @@ -509,7 +509,7 @@ int platform_init(struct sof *sof) /* init DAIs */ ret = dai_init(); if (ret < 0) - return -ENODEV; + return ret; /* initialize IDC mechanism */ trace_point(TRACE_BOOT_PLATFORM_IDC); From 8fe531f3b05bb0df451ffbd1ad5237243ab67f59 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 22 Feb 2019 11:30:36 +0100 Subject: [PATCH 05/12] trace: make _log_message() derived macros usable as functions All trace_event() style macros resolve to _log_message() or __log_message() eventually. This message makes those macros usable as a function, e.g. within "if () else" clauses with or without curly braces. Signed-off-by: Guennadi Liakhovetski --- src/audio/host.c | 2 +- src/drivers/intel/cavs/dmic.c | 2 +- src/include/sof/audio/kpb.h | 4 ++-- src/include/sof/trace.h | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/audio/host.c b/src/audio/host.c index 3b922fad8a1a..4eeab8bf14ee 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -298,7 +298,7 @@ static int host_trigger(struct comp_dev *dev, int cmd) ret = dma_stop(hd->dma, hd->chan); if (ret < 0) trace_host_error("host_trigger(): dma stop failed: %d", - ret) + ret); #endif break; case COMP_TRIGGER_START: diff --git a/src/drivers/intel/cavs/dmic.c b/src/drivers/intel/cavs/dmic.c index 3215b16ad45f..1cc1f53e7c9d 100644 --- a/src/drivers/intel/cavs/dmic.c +++ b/src/drivers/intel/cavs/dmic.c @@ -1239,7 +1239,7 @@ static int dmic_set_config(struct dai *dai, struct sof_ipc_dai_config *config) cfg.clkdiv, cfg.mcic); trace_dmic("dmic_set_config(), cfg mfir_a = %u, mfir_b = %u", cfg.mfir_a, cfg.mfir_b); - trace_dmic("dmic_set_config(), cfg cic_shift = %u", cfg.cic_shift) + trace_dmic("dmic_set_config(), cfg cic_shift = %u", cfg.cic_shift); trace_dmic("dmic_set_config(), cfg fir_a_shift = %u, " "cfg.fir_b_shift = %u", cfg.fir_a_shift, cfg.fir_b_shift); trace_dmic("dmic_set_config(), cfg fir_a_length = %u, " diff --git a/src/include/sof/audio/kpb.h b/src/include/sof/audio/kpb.h index 881c2129b418..5830b89a8183 100644 --- a/src/include/sof/audio/kpb.h +++ b/src/include/sof/audio/kpb.h @@ -39,8 +39,8 @@ /* KPB tracing */ #define trace_kpb(__e, ...) trace_event(TRACE_CLASS_KPB, __e, ##__VA_ARGS__) -#define trace_kpb_error(__e, ...) (trace_error(TRACE_CLASS_KPB, __e, \ - ##__VA_ARGS__)) +#define trace_kpb_error(__e, ...) trace_error(TRACE_CLASS_KPB, __e, \ + ##__VA_ARGS__) #define tracev_kpb(__e, ...) tracev_event(TRACE_CLASS_KPB, __e, ##__VA_ARGS__) /* KPB internal defines */ #define KPB_MAX_BUFF_TIME 2100 /**< time of buffering in miliseconds */ diff --git a/src/include/sof/trace.h b/src/include/sof/trace.h index c07c7eb7d61b..6eb423e051d0 100644 --- a/src/include/sof/trace.h +++ b/src/include/sof/trace.h @@ -124,13 +124,13 @@ extern int test_bench_trace; char *get_trace_class(uint32_t trace_class); #define _log_message(mbox, atomic, level, comp_class, id_0, id_1, \ has_ids, format, ...) \ -{ \ +do { \ if (test_bench_trace) { \ char *msg = "%s " format; \ fprintf(stderr, msg, get_trace_class(comp_class), \ ##__VA_ARGS__); \ } \ -} +} while (0) #endif #define _TRACE_EVENT_NTH_PARAMS(id_count, param_count) \ @@ -348,11 +348,11 @@ _thrown_from_macro_BASE_LOG_in_trace_h #define __log_message(func_name, lvl, comp_class, id_0, id_1, has_ids, \ format, ...) \ -{ \ +do { \ _DECLARE_LOG_ENTRY(lvl, format, comp_class, \ PP_NARG(__VA_ARGS__), has_ids); \ BASE_LOG(func_name, id_0, id_1, &log_entry, ##__VA_ARGS__) \ -} +} while (0) #define _log_message(mbox, atomic, level, comp_class, id_0, id_1, \ has_ids, format, ...) \ From 71e50f955270f11c46a4b2e870a73148a1607a53 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 20 Feb 2019 10:03:48 +0100 Subject: [PATCH 06/12] math: replace MIN() and MAX() macros with safer versions Using macro arguments multiple times within the macro definition body can lead to repeated expressioon evaluation. To avoid that replace MIN() and MAX() macros with safer versions. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/math/numbers.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/include/sof/math/numbers.h b/src/include/sof/math/numbers.h index d98b52831dbf..87000800ff81 100644 --- a/src/include/sof/math/numbers.h +++ b/src/include/sof/math/numbers.h @@ -35,8 +35,16 @@ #include -#define MIN(a, b) (((a) < (b)) ? (a) : (b)) -#define MAX(a, b) (((a) > (b)) ? (a) : (b)) +#define MIN(a, b) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + __a > __b ? __b : __a; \ +}) +#define MAX(a, b) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + __a < __b ? __b : __a; \ +}) int gcd(int a, int b); /* Calculate greatest common divisor for a and b */ From 7969661764b0d3ea618e746b0984a4ccf613fa75 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 26 Feb 2019 18:57:39 +0100 Subject: [PATCH 07/12] dma: fix a macro Function-type macros are usually called as "M(...);" with a semicolon in the end. Therefore defining such macros with a semicolon isn't a good practice. This breaks constructs like if (...) M(...); else ...; Fix the dma_set_drvdata() macro to avoid such problems. Signed-off-by: Guennadi Liakhovetski --- src/include/sof/dma.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/sof/dma.h b/src/include/sof/dma.h index c0f17d8a6d8a..08843f18abec 100644 --- a/src/include/sof/dma.h +++ b/src/include/sof/dma.h @@ -223,7 +223,7 @@ struct dma *dma_get(uint32_t dir, uint32_t caps, uint32_t dev, uint32_t flags); void dma_put(struct dma *dma); #define dma_set_drvdata(dma, data) \ - dma->private = data; + do { dma->private = data; } while (0) #define dma_get_drvdata(dma) \ dma->private #define dma_base(dma) \ From 3e71493f47e16b4b2bf0986a8f3fcf91207af19c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 5 Mar 2019 12:41:08 +0100 Subject: [PATCH 08/12] ipc: (cosmetic) stop confusing editors Some editors (e.g. emacs) do not take into account preprocessor conditionals when performing syntax highlighting. I.e. patterns like if (a) { if (b) { do_things(); } cause those editors to miscalculate the number of opening and closing braces and thus break code highlighting. Move the opening brace to after the preprocessor conditional to avoid that. Signed-off-by: Guennadi Liakhovetski --- src/drivers/intel/cavs/ipc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/drivers/intel/cavs/ipc.c b/src/drivers/intel/cavs/ipc.c index c05717534e15..0bbbd7cfe01e 100644 --- a/src/drivers/intel/cavs/ipc.c +++ b/src/drivers/intel/cavs/ipc.c @@ -85,10 +85,11 @@ static void ipc_irq_handler(void *arg) /* new message from host */ #if CAVS_VERSION == CAVS_VERSION_1_5 - if (dipct & IPC_DIPCT_BUSY && dipcctl & IPC_DIPCCTL_IPCTBIE) { + if (dipct & IPC_DIPCT_BUSY && dipcctl & IPC_DIPCCTL_IPCTBIE) #else - if (dipctdr & IPC_DIPCTDR_BUSY && dipcctl & IPC_DIPCCTL_IPCTBIE) { + if (dipctdr & IPC_DIPCTDR_BUSY && dipcctl & IPC_DIPCCTL_IPCTBIE) #endif + { /* mask Busy interrupt */ ipc_write(IPC_DIPCCTL, dipcctl & ~IPC_DIPCCTL_IPCTBIE); @@ -112,10 +113,11 @@ static void ipc_irq_handler(void *arg) /* reply message(done) from host */ #if CAVS_VERSION == CAVS_VERSION_1_5 - if (dipcie & IPC_DIPCIE_DONE && dipcctl & IPC_DIPCCTL_IPCIDIE) { + if (dipcie & IPC_DIPCIE_DONE && dipcctl & IPC_DIPCCTL_IPCIDIE) #else - if (dipcida & IPC_DIPCIDA_DONE) { + if (dipcida & IPC_DIPCIDA_DONE) #endif + { /* mask Done interrupt */ ipc_write(IPC_DIPCCTL, ipc_read(IPC_DIPCCTL) & ~IPC_DIPCCTL_IPCIDIE); From c8a04417fcc9f906a52537c3a5fca252ad1dc3a4 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 5 Mar 2019 12:46:02 +0100 Subject: [PATCH 09/12] ipc: (cosmetic) simplify a conditional statement Rearrange a conditional to reduce the number of branches and eliminate a goto. Signed-off-by: Guennadi Liakhovetski --- src/drivers/intel/cavs/ipc.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/drivers/intel/cavs/ipc.c b/src/drivers/intel/cavs/ipc.c index 0bbbd7cfe01e..21085e32054b 100644 --- a/src/drivers/intel/cavs/ipc.c +++ b/src/drivers/intel/cavs/ipc.c @@ -145,22 +145,17 @@ void ipc_platform_do_cmd(struct ipc *ipc) /* perform command and return any error */ err = ipc_cmd(); - if (err > 0) { - goto done; /* reply created and copied by cmd() */ - } else if (err < 0) { - /* send std error reply */ + + /* if err > 0, reply created and copied by cmd() */ + if (err <= 0) { + /* send std error/ok reply */ reply.error = err; - } else if (err == 0) { - /* send std reply */ - reply.error = 0; - } - /* send std error/ok reply */ - reply.hdr.cmd = SOF_IPC_GLB_REPLY; - reply.hdr.size = sizeof(reply); - mailbox_hostbox_write(0, &reply, sizeof(reply)); + reply.hdr.cmd = SOF_IPC_GLB_REPLY; + reply.hdr.size = sizeof(reply); + mailbox_hostbox_write(0, &reply, sizeof(reply)); + } -done: ipc->host_pending = 0; /* are we about to enter D3 ? */ From f03667df2cde904f9d650b4e8bb0706c0453f01e Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 5 Mar 2019 12:53:01 +0100 Subject: [PATCH 10/12] ipc: simplify multiple comparisons As long as the low bits of two numbers are 0, there's no need to right shift those numbers to compare them. Signed-off-by: Guennadi Liakhovetski --- src/ipc/handler.c | 144 +++++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/src/ipc/handler.c b/src/ipc/handler.c index d1a188e5f3d8..cd63d24bfdbc 100644 --- a/src/ipc/handler.c +++ b/src/ipc/handler.c @@ -70,8 +70,8 @@ #include #include -#define iGS(x) ((x >> SOF_GLB_TYPE_SHIFT) & 0xf) -#define iCS(x) ((x >> SOF_CMD_TYPE_SHIFT) & 0xfff) +#define iGS(x) ((x) & SOF_GLB_TYPE_MASK) +#define iCS(x) ((x) & SOF_CMD_TYPE_MASK) /* * IPC ABI version compatibility rules :- @@ -454,14 +454,14 @@ static int ipc_stream_trigger(uint32_t header) { struct ipc_comp_dev *pcm_dev; struct sof_ipc_stream stream; - uint32_t ipc_cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t ipc_cmd = iCS(header); uint32_t cmd; int ret; /* copy message with ABI safe method */ IPC_COPY_CMD(stream, _ipc->comp_data); - trace_ipc("ipc: comp %d -> trigger cmd %d", stream.comp_id, ipc_cmd); + trace_ipc("ipc: comp %d -> trigger cmd 0x%x", stream.comp_id, ipc_cmd); /* get the pcm_dev */ pcm_dev = ipc_get_comp(_ipc, stream.comp_id); @@ -471,30 +471,30 @@ static int ipc_stream_trigger(uint32_t header) } switch (ipc_cmd) { - case iCS(SOF_IPC_STREAM_TRIG_START): + case SOF_IPC_STREAM_TRIG_START: cmd = COMP_TRIGGER_START; break; - case iCS(SOF_IPC_STREAM_TRIG_STOP): + case SOF_IPC_STREAM_TRIG_STOP: cmd = COMP_TRIGGER_STOP; break; - case iCS(SOF_IPC_STREAM_TRIG_PAUSE): + case SOF_IPC_STREAM_TRIG_PAUSE: cmd = COMP_TRIGGER_PAUSE; break; - case iCS(SOF_IPC_STREAM_TRIG_RELEASE): + case SOF_IPC_STREAM_TRIG_RELEASE: cmd = COMP_TRIGGER_RELEASE; break; /* XRUN is special case- TODO */ - case iCS(SOF_IPC_STREAM_TRIG_XRUN): + case SOF_IPC_STREAM_TRIG_XRUN: return 0; default: - trace_ipc_error("ipc: invalid trigger cmd %d", ipc_cmd); + trace_ipc_error("ipc: invalid trigger cmd 0x%x", ipc_cmd); return -ENODEV; } /* trigger the component */ ret = pipeline_trigger(pcm_dev->cd->pipeline, pcm_dev->cd, cmd); if (ret < 0) { - trace_ipc_error("ipc: comp %d trigger %d failed %d", + trace_ipc_error("ipc: comp %d trigger 0x%x failed %d", stream.comp_id, ipc_cmd, ret); } @@ -503,24 +503,24 @@ static int ipc_stream_trigger(uint32_t header) static int ipc_glb_stream_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); switch (cmd) { - case iCS(SOF_IPC_STREAM_PCM_PARAMS): + case SOF_IPC_STREAM_PCM_PARAMS: return ipc_stream_pcm_params(header); - case iCS(SOF_IPC_STREAM_PCM_FREE): + case SOF_IPC_STREAM_PCM_FREE: return ipc_stream_pcm_free(header); - case iCS(SOF_IPC_STREAM_TRIG_START): - case iCS(SOF_IPC_STREAM_TRIG_STOP): - case iCS(SOF_IPC_STREAM_TRIG_PAUSE): - case iCS(SOF_IPC_STREAM_TRIG_RELEASE): - case iCS(SOF_IPC_STREAM_TRIG_DRAIN): - case iCS(SOF_IPC_STREAM_TRIG_XRUN): + case SOF_IPC_STREAM_TRIG_START: + case SOF_IPC_STREAM_TRIG_STOP: + case SOF_IPC_STREAM_TRIG_PAUSE: + case SOF_IPC_STREAM_TRIG_RELEASE: + case SOF_IPC_STREAM_TRIG_DRAIN: + case SOF_IPC_STREAM_TRIG_XRUN: return ipc_stream_trigger(header); - case iCS(SOF_IPC_STREAM_POSITION): + case SOF_IPC_STREAM_POSITION: return ipc_stream_position(header); default: - trace_ipc_error("ipc: unknown stream cmd %u", cmd); + trace_ipc_error("ipc: unknown stream cmd 0x%x", cmd); return -EINVAL; } } @@ -566,15 +566,15 @@ static int ipc_dai_config(uint32_t header) static int ipc_glb_dai_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); switch (cmd) { - case iCS(SOF_IPC_DAI_CONFIG): + case SOF_IPC_DAI_CONFIG: return ipc_dai_config(header); - case iCS(SOF_IPC_DAI_LOOPBACK): + case SOF_IPC_DAI_LOOPBACK: //return ipc_comp_set_value(header, COMP_CMD_LOOPBACK); default: - trace_ipc_error("ipc: unknown DAI cmd %u", cmd); + trace_ipc_error("ipc: unknown DAI cmd 0x%x", cmd); return -EINVAL; } } @@ -673,22 +673,22 @@ static int ipc_pm_core_enable(uint32_t header) static int ipc_glb_pm_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); switch (cmd) { - case iCS(SOF_IPC_PM_CTX_SAVE): + case SOF_IPC_PM_CTX_SAVE: return ipc_pm_context_save(header); - case iCS(SOF_IPC_PM_CTX_RESTORE): + case SOF_IPC_PM_CTX_RESTORE: return ipc_pm_context_restore(header); - case iCS(SOF_IPC_PM_CTX_SIZE): + case SOF_IPC_PM_CTX_SIZE: return ipc_pm_context_size(header); - case iCS(SOF_IPC_PM_CORE_ENABLE): + case SOF_IPC_PM_CORE_ENABLE: return ipc_pm_core_enable(header); - case iCS(SOF_IPC_PM_CLK_SET): - case iCS(SOF_IPC_PM_CLK_GET): - case iCS(SOF_IPC_PM_CLK_REQ): + case SOF_IPC_PM_CLK_SET: + case SOF_IPC_PM_CLK_GET: + case SOF_IPC_PM_CLK_REQ: default: - trace_ipc_error("ipc: unknown pm cmd %u", cmd); + trace_ipc_error("ipc: unknown pm cmd 0x%x", cmd); return -EINVAL; } } @@ -784,15 +784,15 @@ int ipc_dma_trace_send_position(void) static int ipc_glb_debug_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); trace_ipc("ipc: debug cmd 0x%x", cmd); switch (cmd) { - case iCS(SOF_IPC_TRACE_DMA_PARAMS): + case SOF_IPC_TRACE_DMA_PARAMS: return ipc_dma_trace_config(header); default: - trace_ipc_error("ipc: unknown debug cmd %u", cmd); + trace_ipc_error("ipc: unknown debug cmd 0x%x", cmd); return -EINVAL; } } @@ -890,19 +890,19 @@ static int ipc_comp_value(uint32_t header, uint32_t cmd) static int ipc_glb_comp_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); switch (cmd) { - case iCS(SOF_IPC_COMP_SET_VALUE): + case SOF_IPC_COMP_SET_VALUE: return ipc_comp_value(header, COMP_CMD_SET_VALUE); - case iCS(SOF_IPC_COMP_GET_VALUE): + case SOF_IPC_COMP_GET_VALUE: return ipc_comp_value(header, COMP_CMD_GET_VALUE); - case iCS(SOF_IPC_COMP_SET_DATA): + case SOF_IPC_COMP_SET_DATA: return ipc_comp_value(header, COMP_CMD_SET_DATA); - case iCS(SOF_IPC_COMP_GET_DATA): + case SOF_IPC_COMP_GET_DATA: return ipc_comp_value(header, COMP_CMD_GET_DATA); default: - trace_ipc_error("ipc: unknown comp cmd %u", cmd); + trace_ipc_error("ipc: unknown comp cmd 0x%x", cmd); return -EINVAL; } } @@ -1044,27 +1044,27 @@ static int ipc_glb_tplg_free(uint32_t header, static int ipc_glb_tplg_message(uint32_t header) { - uint32_t cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + uint32_t cmd = iCS(header); switch (cmd) { - case iCS(SOF_IPC_TPLG_COMP_NEW): + case SOF_IPC_TPLG_COMP_NEW: return ipc_glb_tplg_comp_new(header); - case iCS(SOF_IPC_TPLG_COMP_FREE): + case SOF_IPC_TPLG_COMP_FREE: return ipc_glb_tplg_free(header, ipc_comp_free); - case iCS(SOF_IPC_TPLG_COMP_CONNECT): + case SOF_IPC_TPLG_COMP_CONNECT: return ipc_glb_tplg_comp_connect(header); - case iCS(SOF_IPC_TPLG_PIPE_NEW): + case SOF_IPC_TPLG_PIPE_NEW: return ipc_glb_tplg_pipe_new(header); - case iCS(SOF_IPC_TPLG_PIPE_COMPLETE): + case SOF_IPC_TPLG_PIPE_COMPLETE: return ipc_glb_tplg_pipe_complete(header); - case iCS(SOF_IPC_TPLG_PIPE_FREE): + case SOF_IPC_TPLG_PIPE_FREE: return ipc_glb_tplg_free(header, ipc_pipeline_free); - case iCS(SOF_IPC_TPLG_BUFFER_NEW): + case SOF_IPC_TPLG_BUFFER_NEW: return ipc_glb_tplg_buffer_new(header); - case iCS(SOF_IPC_TPLG_BUFFER_FREE): + case SOF_IPC_TPLG_BUFFER_FREE: return ipc_glb_tplg_free(header, ipc_buffer_free); default: - trace_ipc_error("ipc: unknown tplg header %u", header); + trace_ipc_error("ipc: unknown tplg header 0x%x", header); return -EINVAL; } } @@ -1084,26 +1084,26 @@ int ipc_cmd(void) return -EINVAL; } - type = (hdr->cmd & SOF_GLB_TYPE_MASK) >> SOF_GLB_TYPE_SHIFT; + type = iGS(hdr->cmd); switch (type) { - case iGS(SOF_IPC_GLB_REPLY): + case SOF_IPC_GLB_REPLY: return 0; - case iGS(SOF_IPC_GLB_COMPOUND): + case SOF_IPC_GLB_COMPOUND: return -EINVAL; /* TODO */ - case iGS(SOF_IPC_GLB_TPLG_MSG): + case SOF_IPC_GLB_TPLG_MSG: return ipc_glb_tplg_message(hdr->cmd); - case iGS(SOF_IPC_GLB_PM_MSG): + case SOF_IPC_GLB_PM_MSG: return ipc_glb_pm_message(hdr->cmd); - case iGS(SOF_IPC_GLB_COMP_MSG): + case SOF_IPC_GLB_COMP_MSG: return ipc_glb_comp_message(hdr->cmd); - case iGS(SOF_IPC_GLB_STREAM_MSG): + case SOF_IPC_GLB_STREAM_MSG: return ipc_glb_stream_message(hdr->cmd); - case iGS(SOF_IPC_GLB_DAI_MSG): + case SOF_IPC_GLB_DAI_MSG: return ipc_glb_dai_message(hdr->cmd); - case iGS(SOF_IPC_GLB_TRACE_MSG): + case SOF_IPC_GLB_TRACE_MSG: return ipc_glb_debug_message(hdr->cmd); - case iGS(SOF_IPC_GLB_GDB_DEBUG): + case SOF_IPC_GLB_GDB_DEBUG: return ipc_glb_gdb_debug(hdr->cmd); default: trace_ipc_error("ipc: unknown command type %u", type); @@ -1134,11 +1134,11 @@ static inline struct ipc_msg *ipc_glb_stream_message_find(struct ipc *ipc, uint32_t cmd; /* Check whether the command is expected */ - cmd = (posn->rhdr.hdr.cmd & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + cmd = iCS(posn->rhdr.hdr.cmd); switch (cmd) { - case iCS(SOF_IPC_STREAM_TRIG_XRUN): - case iCS(SOF_IPC_STREAM_POSITION): + case SOF_IPC_STREAM_TRIG_XRUN: + case SOF_IPC_STREAM_POSITION: /* iterate host message list for searching */ list_for_item(plist, &ipc->shared_ctx->msg_list) { @@ -1166,10 +1166,10 @@ static inline struct ipc_msg *ipc_glb_trace_message_find(struct ipc *ipc, uint32_t cmd; /* Check whether the command is expected */ - cmd = (posn->rhdr.hdr.cmd & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT; + cmd = iCS(posn->rhdr.hdr.cmd); switch (cmd) { - case iCS(SOF_IPC_TRACE_DMA_POSITION): + case SOF_IPC_TRACE_DMA_POSITION: /* iterate host message list for searching */ list_for_item(plist, &ipc->shared_ctx->msg_list) { msg = container_of(plist, struct ipc_msg, list); @@ -1191,13 +1191,13 @@ static inline struct ipc_msg *msg_find(struct ipc *ipc, uint32_t header, uint32_t type; /* use different sub function for different global message type */ - type = (header & SOF_GLB_TYPE_MASK) >> SOF_GLB_TYPE_SHIFT; + type = iGS(header); switch (type) { - case iGS(SOF_IPC_GLB_STREAM_MSG): + case SOF_IPC_GLB_STREAM_MSG: return ipc_glb_stream_message_find(ipc, (struct sof_ipc_stream_posn *)tx_data); - case iGS(SOF_IPC_GLB_TRACE_MSG): + case SOF_IPC_GLB_TRACE_MSG: return ipc_glb_trace_message_find(ipc, (struct sof_ipc_dma_trace_posn *)tx_data); default: From 21a0219cbff236c8ba5cd71b58483e2fd218b2d0 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 6 Mar 2019 13:04:35 +0100 Subject: [PATCH 11/12] dmic: improve readability Use a "for" loop instead of simulating it, using a "while" loop. Also avoid needlessly setting a variable inside that loop, where it can just be set once after the loop termination. Signed-off-by: Guennadi Liakhovetski --- src/drivers/intel/cavs/dmic.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/drivers/intel/cavs/dmic.c b/src/drivers/intel/cavs/dmic.c index 1cc1f53e7c9d..ba2ce85c9325 100644 --- a/src/drivers/intel/cavs/dmic.c +++ b/src/drivers/intel/cavs/dmic.c @@ -383,14 +383,11 @@ static void find_modes(struct decim_modes *modes, uint32_t fs, int di) * is possible. Then check that CIC decimation constraints * are met. The passed decimation modes are added to array. */ - j = 0; - /* Loop until NULL */ - while (fir_list[j]) { + for (j = 0; fir_list[j]; j++) { mfir = fir_list[j]->decim_factor; - j++; /* Skip if previous decimation factor was the same */ - if (j > 2 && fir_list[j - 2]->decim_factor == mfir) + if (j > 1 && fir_list[j - 1]->decim_factor == mfir) continue; mcic = osr / mfir; @@ -404,11 +401,12 @@ static void find_modes(struct decim_modes *modes, uint32_t fs, int di) modes->mcic[i] = mcic; modes->mfir[i] = mfir; i++; - modes->num_of_modes = i; } - } } + + modes->num_of_modes = i; + #if defined MODULE_TEST printf("# Found %d modes\n", i); #endif From 58fed5c2793c993c07d24e7b02da10893b025d83 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 6 Mar 2019 13:23:08 +0100 Subject: [PATCH 12/12] tools: fix a typo "Formate" and "format" are two different words, in this case it's "format" that is meant. Signed-off-by: Guennadi Liakhovetski --- tools/test/qa/tests/stability_test/multiplex_pipeline-test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/test/qa/tests/stability_test/multiplex_pipeline-test.sh b/tools/test/qa/tests/stability_test/multiplex_pipeline-test.sh index e47fa1bc6d6e..1f4263ecddb2 100755 --- a/tools/test/qa/tests/stability_test/multiplex_pipeline-test.sh +++ b/tools/test/qa/tests/stability_test/multiplex_pipeline-test.sh @@ -33,7 +33,7 @@ NUMBER=$# # number of test pipeline PARAMS=$@ # name of test pipeline MAXLOOPS=1000 # loop number -FORMATE=s16_le # sample formate +FORMAT=s16_le # sample format CHANNEL=2 # test channel number FREQENCY=48000 # sample frequency T_TIME=3 # time for each test @@ -61,7 +61,7 @@ pipeline_test() fi # run pipeline - $cmd -Dhw:0,$pcmid -f $FORMATE -c $CHANNEL -r $FREQENCY $data & + $cmd -Dhw:0,$pcmid -f $FORMAT -c $CHANNEL -r $FREQENCY $data & PID[$index]=$! let index++ done