diff --git a/include/zephyr/mgmt/mcumgr/transport/smp.h b/include/zephyr/mgmt/mcumgr/transport/smp.h index bc12f250c2..55c9c7b082 100644 --- a/include/zephyr/mgmt/mcumgr/transport/smp.h +++ b/include/zephyr/mgmt/mcumgr/transport/smp.h @@ -1,6 +1,6 @@ /* * Copyright Runtime.io 2018. All rights reserved. - * Copyright (c) 2022 Nordic Semiconductor ASA + * Copyright (c) 2022-2023 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ @@ -35,8 +35,6 @@ struct net_buf; * @return 0 on success, #mcumgr_err_t code on failure. */ typedef int (*smp_transport_out_fn)(struct net_buf *nb); -/* use smp_transport_out_fn instead */ -typedef int zephyr_smp_transport_out_fn(struct net_buf *nb); /** @typedef smp_transport_get_mtu_fn * @brief SMP MTU query callback for transport @@ -52,8 +50,6 @@ typedef int zephyr_smp_transport_out_fn(struct net_buf *nb); * 0 if transmission is currently not possible. */ typedef uint16_t (*smp_transport_get_mtu_fn)(const struct net_buf *nb); -/* use smp_transport_get_mtu_fn instead */ -typedef uint16_t zephyr_smp_transport_get_mtu_fn(const struct net_buf *nb); /** @typedef smp_transport_ud_copy_fn * @brief SMP copy user_data callback @@ -70,9 +66,6 @@ typedef uint16_t zephyr_smp_transport_get_mtu_fn(const struct net_buf *nb); */ typedef int (*smp_transport_ud_copy_fn)(struct net_buf *dst, const struct net_buf *src); -/* use smp_transport_ud_copy_fn instead */ -typedef int zephyr_smp_transport_ud_copy_fn(struct net_buf *dst, - const struct net_buf *src); /** @typedef smp_transport_ud_free_fn * @brief SMP free user_data callback @@ -84,8 +77,6 @@ typedef int zephyr_smp_transport_ud_copy_fn(struct net_buf *dst, * @param ud Contains a user_data pointer to be freed. */ typedef void (*smp_transport_ud_free_fn)(void *ud); -/* use smp_transport_ud_free_fn instead */ -typedef void zephyr_smp_transport_ud_free_fn(void *ud); /** @typedef smp_transport_query_valid_check_fn * @brief Function for checking if queued data is still valid. @@ -100,6 +91,27 @@ typedef void zephyr_smp_transport_ud_free_fn(void *ud); */ typedef bool (*smp_transport_query_valid_check_fn)(struct net_buf *nb, void *arg); +/** + * @brief Function pointers of SMP transport functions, if a handler is NULL then it is not + * supported/implemented. + */ +struct smp_transport_api_t { + /** Transport's send function. */ + smp_transport_out_fn output; + + /** Transport's get-MTU function. */ + smp_transport_get_mtu_fn get_mtu; + + /** Transport buffer user_data copy function. */ + smp_transport_ud_copy_fn ud_copy; + + /** Transport buffer user_data free function. */ + smp_transport_ud_free_fn ud_free; + + /** Transport's check function for if a query is valid. */ + smp_transport_query_valid_check_fn query_valid_check; +}; + /** * @brief SMP transport object for sending SMP responses. */ @@ -110,33 +122,8 @@ struct smp_transport { /* FIFO containing incoming requests to be processed. */ struct k_fifo fifo; - smp_transport_out_fn output; - smp_transport_get_mtu_fn get_mtu; - smp_transport_ud_copy_fn ud_copy; - smp_transport_ud_free_fn ud_free; - smp_transport_query_valid_check_fn query_valid_check; - -#ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY - /* Packet reassembly internal data, API access only */ - struct { - struct net_buf *current; /* net_buf used for reassembly */ - uint16_t expected; /* expected bytes to come */ - } __reassembly; -#endif -}; - -/* Deprecated, use smp_transport instead */ -struct zephyr_smp_transport { - /* Must be the first member. */ - struct k_work zst_work; - - /* FIFO containing incoming requests to be processed. */ - struct k_fifo zst_fifo; - - smp_transport_out_fn zst_output; - smp_transport_get_mtu_fn zst_get_mtu; - smp_transport_ud_copy_fn zst_ud_copy; - smp_transport_ud_free_fn zst_ud_free; + /* Function pointers */ + struct smp_transport_api_t functions; #ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY /* Packet reassembly internal data, API access only */ @@ -150,33 +137,12 @@ struct zephyr_smp_transport { /** * @brief Initializes a Zephyr SMP transport object. * - * @param smpt The transport to construct. - * @param output_func The transport's send function. - * @param get_mtu_func The transport's get-MTU function. - * @param ud_copy_func The transport buffer user_data copy function. - * @param ud_free_func The transport buffer user_data free function. - * @param query_valid_check_func The transport's check function for if a query is valid. + * @param smpt The transport to construct. + * + * @return 0 If successful + * @return Negative errno code if failure. */ -void smp_transport_init(struct smp_transport *smpt, - smp_transport_out_fn output_func, - smp_transport_get_mtu_fn get_mtu_func, - smp_transport_ud_copy_fn ud_copy_func, - smp_transport_ud_free_fn ud_free_func, - smp_transport_query_valid_check_fn query_valid_check_func); - -__deprecated static inline -void zephyr_smp_transport_init(struct zephyr_smp_transport *smpt, - zephyr_smp_transport_out_fn *output_func, - zephyr_smp_transport_get_mtu_fn *get_mtu_func, - zephyr_smp_transport_ud_copy_fn *ud_copy_func, - zephyr_smp_transport_ud_free_fn *ud_free_func) -{ - smp_transport_init((struct smp_transport *)smpt, - (smp_transport_out_fn)output_func, - (smp_transport_get_mtu_fn)get_mtu_func, - (smp_transport_ud_copy_fn)ud_copy_func, - (smp_transport_ud_free_fn)ud_free_func, NULL); -} +int smp_transport_init(struct smp_transport *smpt); /** * @brief Used to remove queued requests for an SMP transport that are no longer valid. A diff --git a/subsys/mgmt/mcumgr/smp/src/smp.c b/subsys/mgmt/mcumgr/smp/src/smp.c index 6907a59413..20d7c01eb4 100644 --- a/subsys/mgmt/mcumgr/smp/src/smp.c +++ b/subsys/mgmt/mcumgr/smp/src/smp.c @@ -334,7 +334,7 @@ static void smp_on_err(struct smp_streamer *streamer, const struct smp_hdr *req_ /* Build and transmit the error response. */ rc = smp_build_err_rsp(streamer, req_hdr, status, rsn); if (rc == 0) { - streamer->smpt->output(rsp); + streamer->smpt->functions.output(rsp); rsp = NULL; } @@ -415,7 +415,7 @@ int smp_process_request_packet(struct smp_streamer *streamer, void *vreq) } /* Send the response. */ - rc = streamer->smpt->output(rsp); + rc = streamer->smpt->functions.output(rsp); rsp = NULL; if (rc != 0) { break; diff --git a/subsys/mgmt/mcumgr/transport/src/smp.c b/subsys/mgmt/mcumgr/transport/src/smp.c index 45755a0b78..1052b82cd0 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp.c +++ b/subsys/mgmt/mcumgr/transport/src/smp.c @@ -5,6 +5,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include #include #include #include @@ -72,8 +73,8 @@ void *smp_alloc_rsp(const void *req, void *arg) return NULL; } - if (smpt->ud_copy) { - smpt->ud_copy(rsp_nb, req_nb); + if (smpt->functions.ud_copy) { + smpt->functions.ud_copy(rsp_nb, req_nb); } else { memcpy(net_buf_user_data(rsp_nb), net_buf_user_data((void *)req_nb), @@ -91,8 +92,8 @@ void smp_free_buf(void *buf, void *arg) return; } - if (smpt->ud_free) { - smpt->ud_free(net_buf_user_data((struct net_buf *)buf)); + if (smpt->functions.ud_free) { + smpt->functions.ud_free(net_buf_user_data((struct net_buf *)buf)); } smp_packet_free(buf); @@ -135,21 +136,14 @@ smp_handle_reqs(struct k_work *work) } } -void -smp_transport_init(struct smp_transport *smpt, - smp_transport_out_fn output_func, - smp_transport_get_mtu_fn get_mtu_func, - smp_transport_ud_copy_fn ud_copy_func, - smp_transport_ud_free_fn ud_free_func, - smp_transport_query_valid_check_fn query_valid_check_func) +int smp_transport_init(struct smp_transport *smpt) { - *smpt = (struct smp_transport) { - .output = output_func, - .get_mtu = get_mtu_func, - .ud_copy = ud_copy_func, - .ud_free = ud_free_func, - .query_valid_check = query_valid_check_func, - }; + __ASSERT((smpt->functions.output != NULL), + "Required transport output function pointer cannot be NULL"); + + if (smpt->functions.output == NULL) { + return -EINVAL; + } #ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY smp_reassembly_init(smpt); @@ -157,6 +151,8 @@ smp_transport_init(struct smp_transport *smpt, k_work_init(&smpt->work, smp_handle_reqs); k_fifo_init(&smpt->fifo); + + return 0; } /** @@ -180,7 +176,7 @@ void smp_rx_remove_invalid(struct smp_transport *zst, void *arg) struct net_buf *nb; struct k_fifo temp_fifo; - if (zst->query_valid_check == NULL) { + if (zst->functions.query_valid_check == NULL) { /* No check check function registered, abort check */ return; } @@ -196,7 +192,7 @@ void smp_rx_remove_invalid(struct smp_transport *zst, void *arg) k_fifo_init(&temp_fifo); while ((nb = net_buf_get(&zst->fifo, K_NO_WAIT)) != NULL) { - if (!zst->query_valid_check(nb, arg)) { + if (!zst->functions.query_valid_check(nb, arg)) { smp_free_buf(nb, zst); } else { net_buf_put(&temp_fifo, nb); diff --git a/subsys/mgmt/mcumgr/transport/src/smp_bt.c b/subsys/mgmt/mcumgr/transport/src/smp_bt.c index 4a8846cd01..6cd2d53d9f 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp_bt.c +++ b/subsys/mgmt/mcumgr/transport/src/smp_bt.c @@ -653,11 +653,17 @@ static void smp_bt_setup(void) ++i; } - smp_transport_init(&smp_bt_transport, smp_bt_tx_pkt, - smp_bt_get_mtu, smp_bt_ud_copy, - smp_bt_ud_free, smp_bt_query_valid_check); + smp_bt_transport.functions.output = smp_bt_tx_pkt; + smp_bt_transport.functions.get_mtu = smp_bt_get_mtu; + smp_bt_transport.functions.ud_copy = smp_bt_ud_copy; + smp_bt_transport.functions.ud_free = smp_bt_ud_free; + smp_bt_transport.functions.query_valid_check = smp_bt_query_valid_check; - rc = smp_bt_register(); + rc = smp_transport_init(&smp_bt_transport); + + if (rc == 0) { + rc = smp_bt_register(); + } if (rc != 0) { LOG_ERR("Bluetooth SMP transport register failed (err %d)", rc); diff --git a/subsys/mgmt/mcumgr/transport/src/smp_dummy.c b/subsys/mgmt/mcumgr/transport/src/smp_dummy.c index 238c99a3ff..d4b58899e7 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp_dummy.c +++ b/subsys/mgmt/mcumgr/transport/src/smp_dummy.c @@ -187,17 +187,24 @@ static int smp_dummy_tx_pkt_int(struct net_buf *nb) static int smp_dummy_init(void) { + int rc; k_sem_init(&smp_data_ready_sem, 0, 1); - smp_transport_init(&smp_dummy_transport, smp_dummy_tx_pkt_int, - smp_dummy_get_mtu, NULL, NULL, NULL); + smp_dummy_transport.functions.output = smp_dummy_tx_pkt_int; + smp_dummy_transport.functions.get_mtu = smp_dummy_get_mtu; + + rc = smp_transport_init(&smp_dummy_transport); + + if (rc != 0) { + return rc; + } + dummy_mgumgr_recv_cb = smp_dummy_rx_frag; return 0; } - static struct uart_mcumgr_rx_buf *dummy_mcumgr_alloc_rx_buf(void) { struct uart_mcumgr_rx_buf *rx_buf; diff --git a/subsys/mgmt/mcumgr/transport/src/smp_shell.c b/subsys/mgmt/mcumgr/transport/src/smp_shell.c index facae45de2..ac196c1008 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp_shell.c +++ b/subsys/mgmt/mcumgr/transport/src/smp_shell.c @@ -235,8 +235,12 @@ static int smp_shell_tx_pkt(struct net_buf *nb) int smp_shell_init(void) { - smp_transport_init(&smp_shell_transport, smp_shell_tx_pkt, - smp_shell_get_mtu, NULL, NULL, NULL); + int rc; - return 0; + smp_shell_transport.functions.output = smp_shell_tx_pkt; + smp_shell_transport.functions.get_mtu = smp_shell_get_mtu; + + rc = smp_transport_init(&smp_shell_transport); + + return rc; } diff --git a/subsys/mgmt/mcumgr/transport/src/smp_uart.c b/subsys/mgmt/mcumgr/transport/src/smp_uart.c index 21e4b6a9a0..9c3dade2f8 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp_uart.c +++ b/subsys/mgmt/mcumgr/transport/src/smp_uart.c @@ -92,12 +92,18 @@ static int smp_uart_tx_pkt(struct net_buf *nb) static int smp_uart_init(void) { + int rc; - smp_transport_init(&smp_uart_transport, smp_uart_tx_pkt, - smp_uart_get_mtu, NULL, NULL, NULL); - uart_mcumgr_register(smp_uart_rx_frag); + smp_uart_transport.functions.output = smp_uart_tx_pkt; + smp_uart_transport.functions.get_mtu = smp_uart_get_mtu; - return 0; + rc = smp_transport_init(&smp_uart_transport); + + if (rc == 0) { + uart_mcumgr_register(smp_uart_rx_frag); + } + + return rc; } SYS_INIT(smp_uart_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); diff --git a/subsys/mgmt/mcumgr/transport/src/smp_udp.c b/subsys/mgmt/mcumgr/transport/src/smp_udp.c index ef01ac8515..46617dbe74 100644 --- a/subsys/mgmt/mcumgr/transport/src/smp_udp.c +++ b/subsys/mgmt/mcumgr/transport/src/smp_udp.c @@ -174,20 +174,25 @@ static void smp_udp_receive_thread(void *p1, void *p2, void *p3) static int smp_udp_init(void) { + int rc; #ifdef CONFIG_MCUMGR_TRANSPORT_UDP_IPV4 - smp_transport_init(&configs.ipv4.smp_transport, - smp_udp4_tx, smp_udp_get_mtu, - smp_udp_ud_copy, NULL, NULL); + configs.ipv4.smp_transport.functions.output = smp_udp4_tx; + configs.ipv4.smp_transport.functions.get_mtu = smp_udp_get_mtu; + configs.ipv4.smp_transport.functions.ud_copy = smp_udp_ud_copy; + + rc = smp_transport_init(&configs.ipv4.smp_transport); #endif #ifdef CONFIG_MCUMGR_TRANSPORT_UDP_IPV6 - smp_transport_init(&configs.ipv6.smp_transport, - smp_udp6_tx, smp_udp_get_mtu, - smp_udp_ud_copy, NULL, NULL); + configs.ipv6.smp_transport.functions.output = smp_udp6_tx; + configs.ipv6.smp_transport.functions.get_mtu = smp_udp_get_mtu; + configs.ipv6.smp_transport.functions.ud_copy = smp_udp_ud_copy; + + rc = smp_transport_init(&configs.ipv6.smp_transport); #endif - return MGMT_ERR_EOK; + return rc; } static int create_socket(struct sockaddr *addr, const char *proto)