[Sound-open-firmware] [PATCH 0/2] macros and register cleanups
Found those issues while randomly looking at register configurations No functionality change, just better looking/safer code
Pierre-Louis Bossart (2): sof: protect macro parameters dma: dw: reorder register definitions by sequential address
src/drivers/dw-dma.c | 24 ++++++++++++------------ src/include/reef/ssp.h | 10 +++++----- src/include/uapi/intel-ipc.h | 20 ++++++++++---------- src/include/uapi/ipc.h | 4 ++-- src/platform/baytrail/include/platform/dma.h | 8 ++++---- 5 files changed, 33 insertions(+), 33 deletions(-)
Make sure all macro parameters are protected with parentheses to avoid unintended expansion issues
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/dw-dma.c | 14 +++++++------- src/include/reef/ssp.h | 10 +++++----- src/include/uapi/intel-ipc.h | 20 ++++++++++---------- src/include/uapi/ipc.h | 4 ++-- src/platform/baytrail/include/platform/dma.h | 8 ++++---- 5 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index 05791e6..ebecbe2 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -124,23 +124,23 @@
/* CTL_LO */ #define DW_CTLL_INT_EN (1 << 0) -#define DW_CTLL_DST_WIDTH(x) (x << 1) -#define DW_CTLL_SRC_WIDTH(x) (x << 4) +#define DW_CTLL_DST_WIDTH(x) ((x) << 1) +#define DW_CTLL_SRC_WIDTH(x) ((x) << 4) #define DW_CTLL_DST_INC (0 << 7) #define DW_CTLL_DST_DEC (1 << 7) #define DW_CTLL_DST_FIX (2 << 7) #define DW_CTLL_SRC_INC (0 << 9) #define DW_CTLL_SRC_DEC (1 << 9) #define DW_CTLL_SRC_FIX (2 << 9) -#define DW_CTLL_DST_MSIZE(x) (x << 11) -#define DW_CTLL_SRC_MSIZE(x) (x << 14) -#define DW_CTLL_FC(x) (x << 20) +#define DW_CTLL_DST_MSIZE(x) ((x) << 11) +#define DW_CTLL_SRC_MSIZE(x) ((x) << 14) +#define DW_CTLL_FC(x) ((x) << 20) #define DW_CTLL_FC_M2M (0 << 20) #define DW_CTLL_FC_M2P (1 << 20) #define DW_CTLL_FC_P2M (2 << 20) #define DW_CTLL_FC_P2P (3 << 20) -#define DW_CTLL_DMS(x) (x << 23) -#define DW_CTLL_SMS(x) (x << 25) +#define DW_CTLL_DMS(x) ((x) << 23) +#define DW_CTLL_SMS(x) ((x) << 25) #define DW_CTLL_LLP_D_EN (1 << 27) #define DW_CTLL_LLP_S_EN (1 << 28) #define DW_CTLL_RELOAD_SRC (1 << 30) diff --git a/src/include/reef/ssp.h b/src/include/reef/ssp.h index 448a0dd..e3da690 100644 --- a/src/include/reef/ssp.h +++ b/src/include/reef/ssp.h @@ -79,7 +79,7 @@ extern const struct dai_ops ssp_ops; #define SSCR0_NCS (1 << 21) #define SSCR0_RIM (1 << 22) #define SSCR0_TIM (1 << 23) -#define SSCR0_FRDC(x) ((x - 1) << 24) +#define SSCR0_FRDC(x) (((x) - 1) << 24) #define SSCR0_ACS (1 << 30) #define SSCR0_MOD (1 << 31)
@@ -140,14 +140,14 @@ extern const struct dai_ops ssp_ops; #define SSCR3_I2S_CLK_MST (1 << 16)
/* SSCR4 bits */ -#define SSCR4_FRM_CLOCKS(x) (x << 7) +#define SSCR4_FRM_CLOCKS(x) ((x) << 7)
/* SSCR5 bits */ -#define SSCR5_FRM_ASRT_CLOCKS(x) ((x - 1) << 1) +#define SSCR5_FRM_ASRT_CLOCKS(x) (((x) - 1) << 1)
/* SFIFOTT bits */ -#define SFIFOTT_TX(x) (x - 1) -#define SFIFOTT_RX(x) ((x - 1) << 16) +#define SFIFOTT_TX(x) ((x) - 1) +#define SFIFOTT_RX(x) (((x) - 1) << 16)
/* tracing */ #define trace_ssp(__e) trace_event(TRACE_CLASS_SSP, __e) diff --git a/src/include/uapi/intel-ipc.h b/src/include/uapi/intel-ipc.h index 5fae423..a0d4c80 100644 --- a/src/include/uapi/intel-ipc.h +++ b/src/include/uapi/intel-ipc.h @@ -58,20 +58,20 @@ /* Global Message - Generic */ #define IPC_INTEL_GLB_TYPE_SHIFT 24 #define IPC_INTEL_GLB_TYPE_MASK (0x1f << IPC_INTEL_GLB_TYPE_SHIFT) -#define IPC_INTEL_GLB_TYPE(x) (x << IPC_INTEL_GLB_TYPE_SHIFT) +#define IPC_INTEL_GLB_TYPE(x) ((x) << IPC_INTEL_GLB_TYPE_SHIFT)
/* Global Message - Reply */ #define IPC_INTEL_GLB_REPLY_SHIFT 0 #define IPC_INTEL_GLB_REPLY_MASK (0x1f << IPC_INTEL_GLB_REPLY_SHIFT) -#define IPC_INTEL_GLB_REPLY_TYPE(x) (x << IPC_INTEL_GLB_REPLY_TYPE_SHIFT) +#define IPC_INTEL_GLB_REPLY_TYPE(x) ((x) << IPC_INTEL_GLB_REPLY_TYPE_SHIFT)
/* Stream Message - Generic */ #define IPC_INTEL_STR_TYPE_SHIFT 20 #define IPC_INTEL_STR_TYPE_MASK (0xf << IPC_INTEL_STR_TYPE_SHIFT) -#define IPC_INTEL_STR_TYPE(x) (x << IPC_INTEL_STR_TYPE_SHIFT) +#define IPC_INTEL_STR_TYPE(x) ((x) << IPC_INTEL_STR_TYPE_SHIFT) #define IPC_INTEL_STR_ID_SHIFT 16 #define IPC_INTEL_STR_ID_MASK (0xf << IPC_INTEL_STR_ID_SHIFT) -#define IPC_INTEL_STR_ID(x) (x << IPC_INTEL_STR_ID_SHIFT) +#define IPC_INTEL_STR_ID(x) ((x) << IPC_INTEL_STR_ID_SHIFT)
/* Stream Message - Reply */ #define IPC_INTEL_STR_REPLY_SHIFT 0 @@ -80,10 +80,10 @@ /* Stream Stage Message - Generic */ #define IPC_INTEL_STG_TYPE_SHIFT 12 #define IPC_INTEL_STG_TYPE_MASK (0xf << IPC_INTEL_STG_TYPE_SHIFT) -#define IPC_INTEL_STG_TYPE(x) (x << IPC_INTEL_STG_TYPE_SHIFT) +#define IPC_INTEL_STG_TYPE(x) ((x) << IPC_INTEL_STG_TYPE_SHIFT) #define IPC_INTEL_STG_ID_SHIFT 10 #define IPC_INTEL_STG_ID_MASK (0x3 << IPC_INTEL_STG_ID_SHIFT) -#define IPC_INTEL_STG_ID(x) (x << IPC_INTEL_STG_ID_SHIFT) +#define IPC_INTEL_STG_ID(x) ((x) << IPC_INTEL_STG_ID_SHIFT)
/* Stream Stage Message - Reply */ #define IPC_INTEL_STG_REPLY_SHIFT 0 @@ -92,19 +92,19 @@ /* Debug Log Message - Generic */ #define IPC_INTEL_LOG_OP_SHIFT 20 #define IPC_INTEL_LOG_OP_MASK (0xf << IPC_INTEL_LOG_OP_SHIFT) -#define IPC_INTEL_LOG_OP_TYPE(x) (x << IPC_INTEL_LOG_OP_SHIFT) +#define IPC_INTEL_LOG_OP_TYPE(x) ((x) << IPC_INTEL_LOG_OP_SHIFT) #define IPC_INTEL_LOG_ID_SHIFT 16 #define IPC_INTEL_LOG_ID_MASK (0xf << IPC_INTEL_LOG_ID_SHIFT) -#define IPC_INTEL_LOG_ID(x) (x << IPC_INTEL_LOG_ID_SHIFT) +#define IPC_INTEL_LOG_ID(x) ((x) << IPC_INTEL_LOG_ID_SHIFT)
/* Module Message */ #define IPC_INTEL_MODULE_OPERATION_SHIFT 20 #define IPC_INTEL_MODULE_OPERATION_MASK (0xf << IPC_INTEL_MODULE_OPERATION_SHIFT) -#define IPC_INTEL_MODULE_OPERATION(x) (x << IPC_INTEL_MODULE_OPERATION_SHIFT) +#define IPC_INTEL_MODULE_OPERATION(x) ((x) << IPC_INTEL_MODULE_OPERATION_SHIFT)
#define IPC_INTEL_MODULE_ID_SHIFT 16 #define IPC_INTEL_MODULE_ID_MASK (0xf << IPC_INTEL_MODULE_ID_SHIFT) -#define IPC_INTEL_MODULE_ID(x) (x << IPC_INTEL_MODULE_ID_SHIFT) +#define IPC_INTEL_MODULE_ID(x) ((x) << IPC_INTEL_MODULE_ID_SHIFT)
/* IPC message timeout (msecs) */ #define IPC_INTEL_TIMEOUT_MSECS 300 diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index fc83721..ec41483 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -49,12 +49,12 @@ /* Global Message - Generic */ #define SOF_GLB_TYPE_SHIFT 28 #define SOF_GLB_TYPE_MASK (0xf << SOF_GLB_TYPE_SHIFT) -#define SOF_GLB_TYPE(x) (x << SOF_GLB_TYPE_SHIFT) +#define SOF_GLB_TYPE(x) ((x) << SOF_GLB_TYPE_SHIFT)
/* Command Message - Generic */ #define SOF_CMD_TYPE_SHIFT 16 #define SOF_CMD_TYPE_MASK (0xfff << SOF_CMD_TYPE_SHIFT) -#define SOF_CMD_TYPE(x) (x << SOF_CMD_TYPE_SHIFT) +#define SOF_CMD_TYPE(x) ((x) << SOF_CMD_TYPE_SHIFT)
/* Global Message Types */ #define SOF_IPC_GLB_REPLY SOF_GLB_TYPE(0x1U) diff --git a/src/platform/baytrail/include/platform/dma.h b/src/platform/baytrail/include/platform/dma.h index 5f5c52a..37ae067 100644 --- a/src/platform/baytrail/include/platform/dma.h +++ b/src/platform/baytrail/include/platform/dma.h @@ -45,13 +45,13 @@ /* CTL_HI */ #define DW_CTLH_DONE 0x00020000 #define DW_CTLH_BLOCK_TS_MASK 0x0001ffff -#define DW_CTLH_CLASS(x) (x << 29) -#define DW_CTLH_WEIGHT(x) (x << 18) +#define DW_CTLH_CLASS(x) ((x) << 29) +#define DW_CTLH_WEIGHT(x) ((x) << 18) /* CFG_LO */ #define DW_CFG_CH_DRAIN 0x400 /* CFG_HI */ -#define DW_CFGH_SRC_PER(x) (x << 0) -#define DW_CFGH_DST_PER(x) (x << 4) +#define DW_CFGH_SRC_PER(x) ((x) << 0) +#define DW_CFGH_DST_PER(x) ((x) << 4) /* FIFO Partition */ #define DW_FIFO_PARTITION #define DW_FIFO_PART0_LO 0x0400
Cosmetic change, for some reason the register definition was not sequential as in the data sheet
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/drivers/dw-dma.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index ebecbe2..4bda45b 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -87,16 +87,16 @@ (0x0044 + BYT_CHAN_OFFSET(chan))
/* registers */ -#define DW_STATUS_TFR 0x02E8 -#define DW_STATUS_BLOCK 0x02F0 -#define DW_STATUS_SRC_TRAN 0x02F8 -#define DW_STATUS_DST_TRAN 0x0300 -#define DW_STATUS_ERR 0x0308 #define DW_RAW_TFR 0x02C0 #define DW_RAW_BLOCK 0x02C8 #define DW_RAW_SRC_TRAN 0x02D0 #define DW_RAW_DST_TRAN 0x02D8 #define DW_RAW_ERR 0x02E0 +#define DW_STATUS_TFR 0x02E8 +#define DW_STATUS_BLOCK 0x02F0 +#define DW_STATUS_SRC_TRAN 0x02F8 +#define DW_STATUS_DST_TRAN 0x0300 +#define DW_STATUS_ERR 0x0308 #define DW_MASK_TFR 0x0310 #define DW_MASK_BLOCK 0x0318 #define DW_MASK_SRC_TRAN 0x0320
On Wed, 2017-11-08 at 17:46 -0600, Pierre-Louis Bossart wrote:
Found those issues while randomly looking at register configurations No functionality change, just better looking/safer code
Pierre-Louis Bossart (2): sof: protect macro parameters dma: dw: reorder register definitions by sequential address
All applied.
Thanks
Liam
participants (2)
-
Liam Girdwood -
Pierre-Louis Bossart