From 14923f41312707fa6bcdee53d6152c71fdbbeeb6 Mon Sep 17 00:00:00 2001 From: Andrei Emeltchenko Date: Fri, 4 Nov 2022 18:03:32 +0200 Subject: [PATCH] smbus: Refactor callbacks API The callbacks API was similar to gpio / espi callbacks API. Refactor it to use more intuitive names for set / remove callbacks. Signed-off-by: Andrei Emeltchenko --- drivers/smbus/intel_pch_smbus.c | 44 ++++++--- drivers/smbus/smbus_handlers.c | 36 +++++-- include/zephyr/drivers/smbus.h | 98 ++++++++++++++----- include/zephyr/drivers/smbus_utils.h | 44 +++++---- .../drivers/smbus/smbus_api/src/test_smbus.c | 22 ++++- tests/drivers/smbus/smbus_emul/src/smbus.c | 8 +- 6 files changed, 180 insertions(+), 72 deletions(-) diff --git a/drivers/smbus/intel_pch_smbus.c b/drivers/smbus/intel_pch_smbus.c index 2916217cee0..c43f6e41ec8 100644 --- a/drivers/smbus/intel_pch_smbus.c +++ b/drivers/smbus/intel_pch_smbus.c @@ -148,26 +148,44 @@ static void smbalert_work(struct k_work *work) } while (true); } -static int pch_smbus_manage_smbalert_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static int pch_smbus_smbalert_set_sb(const struct device *dev, + struct smbus_callback *cb) { struct pch_data *data = dev->data; - LOG_DBG("dev %p cb %p set %d", dev, cb, set); + LOG_DBG("dev %p cb %p", dev, cb); - return smbus_manage_smbus_callback(&data->smbalert_cbs, cb, set); + return smbus_callback_set(&data->smbalert_cbs, cb); } -static int pch_smbus_manage_host_notify_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static int pch_smbus_smbalert_remove_sb(const struct device *dev, + struct smbus_callback *cb) { struct pch_data *data = dev->data; - LOG_DBG("dev %p cb %p set %d", dev, cb, set); + LOG_DBG("dev %p cb %p", dev, cb); - return smbus_manage_smbus_callback(&data->host_notify_cbs, cb, set); + return smbus_callback_remove(&data->smbalert_cbs, cb); +} + +static int pch_smbus_host_notify_set_cb(const struct device *dev, + struct smbus_callback *cb) +{ + struct pch_data *data = dev->data; + + LOG_DBG("dev %p cb %p", dev, cb); + + return smbus_callback_set(&data->host_notify_cbs, cb); +} + +static int pch_smbus_host_notify_remove_cb(const struct device *dev, + struct smbus_callback *cb) +{ + struct pch_data *data = dev->data; + + LOG_DBG("dev %p cb %p", dev, cb); + + return smbus_callback_remove(&data->host_notify_cbs, cb); } static int pch_configure(const struct device *dev, uint32_t config) @@ -902,8 +920,10 @@ static const struct smbus_driver_api funcs = { .smbus_block_write = pch_smbus_block_write, .smbus_block_read = pch_smbus_block_read, .smbus_block_pcall = pch_smbus_block_pcall, - .smbus_manage_smbalert_cb = pch_smbus_manage_smbalert_cb, - .smbus_manage_host_notify_cb = pch_smbus_manage_host_notify_cb, + .smbus_smbalert_set_cb = pch_smbus_smbalert_set_sb, + .smbus_smbalert_remove_cb = pch_smbus_smbalert_remove_sb, + .smbus_host_notify_set_cb = pch_smbus_host_notify_set_cb, + .smbus_host_notify_remove_cb = pch_smbus_host_notify_remove_cb, }; static void smbus_isr(const struct device *dev) diff --git a/drivers/smbus/smbus_handlers.c b/drivers/smbus/smbus_handlers.c index fd03a7cad48..0c979788830 100644 --- a/drivers/smbus/smbus_handlers.c +++ b/drivers/smbus/smbus_handlers.c @@ -144,22 +144,38 @@ static inline int z_vrfy_smbus_block_pcall(const struct device *dev, } #include -static inline int z_vrfy_smbus_manage_smbalert_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static inline int z_vrfy_smbus_smbalert_set_cb(const struct device *dev, + struct smbus_callback *cb) { Z_OOPS(Z_SYSCALL_OBJ(dev, K_OBJ_DRIVER_SMBUS)); - return z_impl_smbus_manage_smbalert_cb(dev, cb, set); + return z_impl_smbus_smbalert_set_cb(dev, cb); } -#include +#include -static inline int z_vrfy_smbus_manage_host_notify_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static inline int z_vrfy_smbus_smbalert_remove_cb(const struct device *dev, + struct smbus_callback *cb) { Z_OOPS(Z_SYSCALL_OBJ(dev, K_OBJ_DRIVER_SMBUS)); - return z_impl_smbus_manage_host_notify_cb(dev, cb, set); + return z_impl_smbus_smbalert_remove_cb(dev, cb); } -#include +#include + +static inline int z_vrfy_smbus_host_notify_set_cb(const struct device *dev, + struct smbus_callback *cb) +{ + Z_OOPS(Z_SYSCALL_OBJ(dev, K_OBJ_DRIVER_SMBUS)); + + return z_impl_smbus_host_notify_set_cb(dev, cb); +} +#include + +static inline int z_vrfy_smbus_host_notify_remove_cb(const struct device *dev, + struct smbus_callback *cb) +{ + Z_OOPS(Z_SYSCALL_OBJ(dev, K_OBJ_DRIVER_SMBUS)); + + return z_impl_smbus_host_notify_remove_cb(dev, cb); +} +#include diff --git a/include/zephyr/drivers/smbus.h b/include/zephyr/drivers/smbus.h index d20786f7d11..02e673c1e47 100644 --- a/include/zephyr/drivers/smbus.h +++ b/include/zephyr/drivers/smbus.h @@ -358,12 +358,10 @@ typedef int (*smbus_api_block_pcall_t)(const struct device *dev, uint16_t addr, uint8_t cmd, uint8_t send_count, uint8_t *send_buf, uint8_t *recv_count, uint8_t *recv_buf); -typedef int (*smbus_api_manage_smbalert_cb_t)(const struct device *dev, - struct smbus_callback *cb, - bool set); -typedef int (*smbus_api_manage_host_notify_cb_t)(const struct device *dev, - struct smbus_callback *cb, - bool set); +typedef int (*smbus_api_smbalert_cb_t)(const struct device *dev, + struct smbus_callback *cb); +typedef int (*smbus_api_host_notify_cb_t)(const struct device *dev, + struct smbus_callback *cb); __subsystem struct smbus_driver_api { smbus_api_configure_t configure; @@ -379,8 +377,10 @@ __subsystem struct smbus_driver_api { smbus_api_block_write_t smbus_block_write; smbus_api_block_read_t smbus_block_read; smbus_api_block_pcall_t smbus_block_pcall; - smbus_api_manage_smbalert_cb_t smbus_manage_smbalert_cb; - smbus_api_manage_host_notify_cb_t smbus_manage_host_notify_cb; + smbus_api_smbalert_cb_t smbus_smbalert_set_cb; + smbus_api_smbalert_cb_t smbus_smbalert_remove_cb; + smbus_api_host_notify_cb_t smbus_host_notify_set_cb; + smbus_api_host_notify_cb_t smbus_host_notify_remove_cb; }; /** @@ -592,28 +592,51 @@ static inline int z_impl_smbus_get_config(const struct device *dev, * * @param dev Pointer to the device structure for the driver instance. * @param cb Pointer to a callback structure. - * @param set A boolean indicating insertion or removal of the callback. * * @retval 0 If successful. * @retval -EIO General input / output error, failed to configure device. * @retval -ENOSYS If get config is not implemented */ -__syscall int smbus_manage_smbalert_cb(const struct device *dev, - struct smbus_callback *cb, - bool set); +__syscall int smbus_smbalert_set_cb(const struct device *dev, + struct smbus_callback *cb); -static inline int z_impl_smbus_manage_smbalert_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static inline int z_impl_smbus_smbalert_set_cb(const struct device *dev, + struct smbus_callback *cb) { const struct smbus_driver_api *api = (const struct smbus_driver_api *)dev->api; - if (api->smbus_manage_smbalert_cb == NULL) { + if (api->smbus_smbalert_set_cb == NULL) { return -ENOSYS; } - return api->smbus_manage_smbalert_cb(dev, cb, set); + return api->smbus_smbalert_set_cb(dev, cb); +} + +/** + * @brief Remove SMBUSALERT callback from a SMBus host controller. + * + * @param dev Pointer to the device structure for the driver instance. + * @param cb Pointer to a callback structure. + * + * @retval 0 If successful. + * @retval -EIO General input / output error, failed to configure device. + * @retval -ENOSYS If get config is not implemented + */ +__syscall int smbus_smbalert_remove_cb(const struct device *dev, + struct smbus_callback *cb); + +static inline int z_impl_smbus_smbalert_remove_cb(const struct device *dev, + struct smbus_callback *cb) +{ + const struct smbus_driver_api *api = + (const struct smbus_driver_api *)dev->api; + + if (api->smbus_smbalert_remove_cb == NULL) { + return -ENOSYS; + } + + return api->smbus_smbalert_remove_cb(dev, cb); } /** @@ -621,28 +644,51 @@ static inline int z_impl_smbus_manage_smbalert_cb(const struct device *dev, * * @param dev Pointer to the device structure for the driver instance. * @param cb Pointer to a callback structure. - * @param set A boolean indicating insertion or removal of the callback. * * @retval 0 If successful. * @retval -EIO General input / output error, failed to configure device. * @retval -ENOSYS If get config is not implemented */ -__syscall int smbus_manage_host_notify_cb(const struct device *dev, - struct smbus_callback *cb, - bool set); +__syscall int smbus_host_notify_set_cb(const struct device *dev, + struct smbus_callback *cb); -static inline int z_impl_smbus_manage_host_notify_cb(const struct device *dev, - struct smbus_callback *cb, - bool set) +static inline int z_impl_smbus_host_notify_set_cb(const struct device *dev, + struct smbus_callback *cb) { const struct smbus_driver_api *api = (const struct smbus_driver_api *)dev->api; - if (api->smbus_manage_host_notify_cb == NULL) { + if (api->smbus_host_notify_set_cb == NULL) { return -ENOSYS; } - return api->smbus_manage_host_notify_cb(dev, cb, set); + return api->smbus_host_notify_set_cb(dev, cb); +} + +/** + * @brief Remove Host Notify callback from a SMBus host controller. + * + * @param dev Pointer to the device structure for the driver instance. + * @param cb Pointer to a callback structure. + * + * @retval 0 If successful. + * @retval -EIO General input / output error, failed to configure device. + * @retval -ENOSYS If get config is not implemented + */ +__syscall int smbus_host_notify_remove_cb(const struct device *dev, + struct smbus_callback *cb); + +static inline int z_impl_smbus_host_notify_remove_cb(const struct device *dev, + struct smbus_callback *cb) +{ + const struct smbus_driver_api *api = + (const struct smbus_driver_api *)dev->api; + + if (api->smbus_host_notify_remove_cb == NULL) { + return -ENOSYS; + } + + return api->smbus_host_notify_remove_cb(dev, cb); } /** diff --git a/include/zephyr/drivers/smbus_utils.h b/include/zephyr/drivers/smbus_utils.h index 426891dfdf6..0158d2c5a52 100644 --- a/include/zephyr/drivers/smbus_utils.h +++ b/include/zephyr/drivers/smbus_utils.h @@ -8,35 +8,45 @@ #define ZEPHYR_DRIVERS_SMBUS_SMBUS_UTILS_H_ /** - * @brief Generic function to insert or remove a callback from a callback list + * @brief Generic function to insert a callback to a callback list * * @param callbacks A pointer to the original list of callbacks (can be NULL) - * @param callback A pointer of the callback to insert or remove from the list - * @param set A boolean indicating insertion or removal of the callback + * @param callback A pointer of the callback to insert to the list * * @return 0 on success, negative errno otherwise. */ -static inline int smbus_manage_smbus_callback(sys_slist_t *callbacks, - struct smbus_callback *callback, - bool set) +static inline int smbus_callback_set(sys_slist_t *callbacks, + struct smbus_callback *callback) { __ASSERT(callback, "No callback!"); __ASSERT(callback->handler, "No callback handler!"); if (!sys_slist_is_empty(callbacks)) { - if (!sys_slist_find_and_remove(callbacks, &callback->node)) { - if (!set) { - return -ENOENT; - } - } - } else { - if (!set) { - return -ENOENT; - } + sys_slist_find_and_remove(callbacks, &callback->node); } - if (set) { - sys_slist_prepend(callbacks, &callback->node); + sys_slist_prepend(callbacks, &callback->node); + + return 0; +} + +/** + * @brief Generic function to remove a callback from a callback list + * + * @param callbacks A pointer to the original list of callbacks (can be NULL) + * @param callback A pointer of the callback to remove from the list + * + * @return 0 on success, negative errno otherwise. + */ +static inline int smbus_callback_remove(sys_slist_t *callbacks, + struct smbus_callback *callback) +{ + __ASSERT(callback, "No callback!"); + __ASSERT(callback->handler, "No callback handler!"); + + if (sys_slist_is_empty(callbacks) || + !sys_slist_find_and_remove(callbacks, &callback->node)) { + return -ENOENT; } return 0; diff --git a/tests/drivers/smbus/smbus_api/src/test_smbus.c b/tests/drivers/smbus/smbus_api/src/test_smbus.c index 74b3d27250d..e3eda7ceee3 100644 --- a/tests/drivers/smbus/smbus_api/src/test_smbus.c +++ b/tests/drivers/smbus/smbus_api/src/test_smbus.c @@ -58,16 +58,32 @@ ZTEST_USER(test_smbus_general, test_smbus_callback_api) zassert_true(device_is_ready(dev), "Device is not ready"); + /* Smbalert callbacks */ + /* Try to remove not existing callback */ - ret = smbus_manage_smbalert_cb(dev, &callback, false); + ret = smbus_smbalert_remove_cb(dev, &callback); zassert_equal(ret, -ENOENT, "Callback remove failed"); /* Set callback */ - ret = smbus_manage_smbalert_cb(dev, &callback, true); + ret = smbus_smbalert_set_cb(dev, &callback); zassert_ok(ret, "Callback set failed"); /* Remove existing callback */ - ret = smbus_manage_smbalert_cb(dev, &callback, false); + ret = smbus_smbalert_remove_cb(dev, &callback); + zassert_ok(ret, "Callback remove failed"); + + /* Host Notify callbacks */ + + /* Try to remove not existing callback */ + ret = smbus_host_notify_remove_cb(dev, &callback); + zassert_equal(ret, -ENOENT, "Callback remove failed"); + + /* Set callback */ + ret = smbus_host_notify_set_cb(dev, &callback); + zassert_ok(ret, "Callback set failed"); + + /* Remove existing callback */ + ret = smbus_host_notify_remove_cb(dev, &callback); zassert_ok(ret, "Callback remove failed"); } diff --git a/tests/drivers/smbus/smbus_emul/src/smbus.c b/tests/drivers/smbus/smbus_emul/src/smbus.c index 4c62a1a9c55..451859c841e 100644 --- a/tests/drivers/smbus/smbus_emul/src/smbus.c +++ b/tests/drivers/smbus/smbus_emul/src/smbus.c @@ -294,11 +294,11 @@ ZTEST(test_smbus_emul, test_alert) zassert_not_null(dev, "Device not found"); /* Try to remove not existing callback */ - ret = smbus_manage_smbalert_cb(dev, &smbalert_callback, false); + ret = smbus_smbalert_remove_cb(dev, &smbalert_callback); zassert_equal(ret, -ENOENT, "Callback remove failed"); /* Set callback */ - ret = smbus_manage_smbalert_cb(dev, &smbalert_callback, true); + ret = smbus_smbalert_set_cb(dev, &smbalert_callback); zassert_ok(ret, "Callback set failed"); /* Emulate SMBus alert from peripheral device */ @@ -338,11 +338,11 @@ ZTEST(test_smbus_emul, test_host_notify) zassert_not_null(dev, "Device not found"); /* Try to remove not existing callback */ - ret = smbus_manage_host_notify_cb(dev, ¬ify_callback, false); + ret = smbus_host_notify_remove_cb(dev, ¬ify_callback); zassert_equal(ret, -ENOENT, "Callback remove failed"); /* Set callback */ - ret = smbus_manage_host_notify_cb(dev, ¬ify_callback, true); + ret = smbus_host_notify_set_cb(dev, ¬ify_callback); zassert_ok(ret, "Callback set failed"); /* Emulate SMBus alert from peripheral device */