Skip to content

Commit

Permalink
Asserts and removed unused code
Browse files Browse the repository at this point in the history
  • Loading branch information
doki-nordic authored and lstnl committed Dec 19, 2024
1 parent e511099 commit 5ffb19c
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 44 deletions.
76 changes: 44 additions & 32 deletions subsys/ipc/ipc_service/backends/ipc_icmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,38 +56,50 @@ static int backend_init(const struct device *instance)

#define UNBOUND_MODE(i) CONCAT(ICMSG_UNBOUND_MODE_, DT_INST_STRING_UPPER_TOKEN(i, unbound))

#define DEFINE_BACKEND_DEVICE(i) \
static const struct icmsg_config_t backend_config_##i = { \
.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx), \
.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx), \
.unbound_mode = UNBOUND_MODE(i), \
}; \
\
PBUF_DEFINE(tx_pb_##i, \
DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \
DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \
DT_INST_PROP_OR(i, dcache_alignment, 0), \
UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \
UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \
PBUF_DEFINE(rx_pb_##i, \
DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \
DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \
DT_INST_PROP_OR(i, dcache_alignment, 0), \
UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \
UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \
\
static struct icmsg_data_t backend_data_##i = { \
.tx_pb = &tx_pb_##i, \
.rx_pb = &rx_pb_##i, \
}; \
\
DEVICE_DT_INST_DEFINE(i, \
&backend_init, \
NULL, \
&backend_data_##i, \
&backend_config_##i, \
POST_KERNEL, \
CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY, \
#define DEFINE_BACKEND_DEVICE(i) \
static const struct icmsg_config_t backend_config_##i = { \
.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx), \
.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx), \
.unbound_mode = UNBOUND_MODE(i), \
}; \
\
PBUF_DEFINE(tx_pb_##i, \
DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \
DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \
DT_INST_PROP_OR(i, dcache_alignment, 0), \
UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \
UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \
PBUF_DEFINE(rx_pb_##i, \
DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \
DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \
DT_INST_PROP_OR(i, dcache_alignment, 0), \
UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \
UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \
\
BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE || \
IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DISABLED_ALLOWED), \
"Unbound mode \"disabled\" is was forbidden in Kconfig."); \
\
BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_ENABLE || \
IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABLED_ALLOWED), \
"Unbound mode \"enabled\" is was forbidden in Kconfig."); \
\
BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DETECT || \
IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DETECT_ALLOWED), \
"Unbound mode \"detect\" is was forbidden in Kconfig."); \
\
static struct icmsg_data_t backend_data_##i = { \
.tx_pb = &tx_pb_##i, \
.rx_pb = &rx_pb_##i, \
}; \
\
DEVICE_DT_INST_DEFINE(i, \
&backend_init, \
NULL, \
&backend_data_##i, \
&backend_config_##i, \
POST_KERNEL, \
CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY, \
&backend_ops);

Check notice on line 104 in subsys/ipc/ipc_service/backends/ipc_icmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/ipc/ipc_service/backends/ipc_icmsg.c:104 -#define DEFINE_BACKEND_DEVICE(i) \ - static const struct icmsg_config_t backend_config_##i = { \ - .mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx), \ - .mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx), \ - .unbound_mode = UNBOUND_MODE(i), \ - }; \ - \ - PBUF_DEFINE(tx_pb_##i, \ - DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ - DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ - DT_INST_PROP_OR(i, dcache_alignment, 0), \ - UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \ - UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \ - PBUF_DEFINE(rx_pb_##i, \ - DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ - DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ - DT_INST_PROP_OR(i, dcache_alignment, 0), \ - UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \ - UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \ - \ - BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE || \ - IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DISABLED_ALLOWED), \ - "Unbound mode \"disabled\" is was forbidden in Kconfig."); \ - \ - BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_ENABLE || \ - IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABLED_ALLOWED), \ - "Unbound mode \"enabled\" is was forbidden in Kconfig."); \ - \ - BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DETECT || \ - IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DETECT_ALLOWED), \ - "Unbound mode \"detect\" is was forbidden in Kconfig."); \ - \ - static struct icmsg_data_t backend_data_##i = { \ - .tx_pb = &tx_pb_##i, \ - .rx_pb = &rx_pb_##i, \ - }; \ - \ - DEVICE_DT_INST_DEFINE(i, \ - &backend_init, \ - NULL, \ - &backend_data_##i, \ - &backend_config_##i, \ - POST_KERNEL, \ - CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY, \ - &backend_ops); +#define DEFINE_BACKEND_DEVICE(i) \ + static const struct icmsg_config_t backend_config_##i = { \ + .mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx), \ + .mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx), \ + .unbound_mode = UNBOUND_MODE(i), \ + }; \ + \ + PBUF_DEFINE(tx_pb_##i, DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)), \ + DT_INST_PROP_OR(i, dcache_alignment, 0), \ + UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \ + UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \ + PBUF_DEFINE(rx_pb_##i, DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)), \ + DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)), \ + DT_INST_PROP_OR(i, dcache_alignment, 0), \ + UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE, \ + UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_DETECT); \ + \ + BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE || \ + IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DISABLED_ALLOWED), \ + "Unbound mode \"disabled\" is was forbidden in Kconfig."); \ + \ + BUILD_ASSERT(UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_ENABLE || \ + IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABL
DT_INST_FOREACH_STATUS_OKAY(DEFINE_BACKEND_DEVICE)
9 changes: 8 additions & 1 deletion subsys/ipc/ipc_service/lib/icmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#include <zephyr/init.h>


#define UNBOUND_ENABLED IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABLED_ALLOWED)
#define UNBOUND_DISABLED IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DISABLED_ALLOWED)
#define UNBOUND_ENABLED IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_ENABLED_ALLOWED)
#define UNBOUND_DETECT IS_ENABLED(CONFIG_IPC_SERVICE_ICMSG_UNBOUND_DETECT_ALLOWED)

/** Get local session id request from RX handshake word.
Expand Down Expand Up @@ -384,6 +384,13 @@ int icmsg_open(const struct icmsg_config_t *conf,
int ret;
enum icmsg_state old_state;

__ASSERT(conf->unbound_mode != ICMSG_UNBOUND_MODE_DISABLE || UNBOUND_DISABLED,
"Unbound mode \"disabled\" is was forbidden in Kconfig.");
__ASSERT(conf->unbound_mode != ICMSG_UNBOUND_MODE_ENABLE || UNBOUND_ENABLED,
"Unbound mode \"enabled\" is was forbidden in Kconfig.");
__ASSERT(conf->unbound_mode != ICMSG_UNBOUND_MODE_DETECT || UNBOUND_DETECT,
"Unbound mode \"detect\" is was forbidden in Kconfig.");

if (conf->unbound_mode == ICMSG_UNBOUND_MODE_DISABLE ||
!(UNBOUND_ENABLED || UNBOUND_DETECT)) {
if (!atomic_cas(&dev_data->state, ICMSG_STATE_OFF,
Expand Down
13 changes: 2 additions & 11 deletions subsys/ipc/ipc_service/lib/pbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ static int validate_cfg(const struct pbuf_cfg *cfg)
{
/* Validate pointers. */
if (!cfg || !cfg->rd_idx_loc || !cfg->wr_idx_loc || !cfg->data_loc) {
printk("Invalid pointers\n");
return -EINVAL;
}

Expand All @@ -41,13 +40,11 @@ static int validate_cfg(const struct pbuf_cfg *cfg)
!IS_PTR_ALIGNED_BYTES(cfg->wr_idx_loc, MAX(cfg->dcache_alignment, _PBUF_IDX_SIZE)) ||
!IS_PTR_ALIGNED_BYTES(cfg->handshake_loc, _PBUF_IDX_SIZE) ||
!IS_PTR_ALIGNED_BYTES(cfg->data_loc, _PBUF_IDX_SIZE)) {
printk("Invalid alignment\n");
return -EINVAL;
}

/* Validate len. */
if (cfg->len < _PBUF_MIN_DATA_LEN || !IS_PTR_ALIGNED_BYTES(cfg->len, _PBUF_IDX_SIZE)) {
printk("Invalid length\n");
return -EINVAL;
}

Expand All @@ -58,12 +55,6 @@ static int validate_cfg(const struct pbuf_cfg *cfg)
!((uint8_t *)cfg->wr_idx_loc < cfg->data_loc) ||
!(((uint8_t *)cfg->rd_idx_loc + MAX(_PBUF_IDX_SIZE, cfg->dcache_alignment)) ==
(uint8_t *)cfg->wr_idx_loc)) {
printk("Invalid pointer values\n");
printk("rd_idx_loc: 0x%08X\n", (uintptr_t)cfg->rd_idx_loc);
printk("wr_idx_loc: 0x%08X\n", (uintptr_t)cfg->wr_idx_loc);
printk("handshake_loc: 0x%08X\n", (uintptr_t)cfg->handshake_loc);
printk("data_loc: 0x%08X\n", (uintptr_t)cfg->data_loc);
printk("dcache_alignment: 0x%08X\n", (uintptr_t)cfg->dcache_alignment);
return -EINVAL;
}

Expand Down Expand Up @@ -203,7 +194,7 @@ int pbuf_get_initial_buf(struct pbuf *pb, volatile char **buf, uint16_t *len)

wr_idx = *(pb->cfg->wr_idx_loc);
if (wr_idx >= pb->cfg->len || wr_idx > 0xFFFF || wr_idx == 0) {
/* Incorrect index - probably pbuf was not initialized or message was not send yet. */
/* Wrong index - probably pbuf was not initialized or message was not send yet. */
return -EINVAL;
}

Expand All @@ -213,7 +204,7 @@ int pbuf_get_initial_buf(struct pbuf *pb, volatile char **buf, uint16_t *len)
plen = sys_get_be16(&pb->cfg->data_loc[0]);

if (plen + 4 > wr_idx) {
/* Incorrect length - probably pbuf was not initialized or message was not send yet. */
/* Wrong length - probably pbuf was not initialized or message was not send yet. */
return -EINVAL;
}

Expand Down

0 comments on commit 5ffb19c

Please sign in to comment.