Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: video: wrap the CONFIG_POLL #ifdefs #86983

Open
josuah opened this issue Mar 12, 2025 · 0 comments
Open

drivers: video: wrap the CONFIG_POLL #ifdefs #86983

josuah opened this issue Mar 12, 2025 · 0 comments
Assignees
Labels
area: Video Video subsystem Enhancement Changes/Updates/Additions to existing features

Comments

@josuah
Copy link
Collaborator

josuah commented Mar 12, 2025

Is your enhancement proposal related to a problem? Please describe.

Video drivers use k_polll_signal_raise() to communicate events, this is wrapped in #ifdef CONFIG_POLL.

if (IS_ENABLED(CONFIG_POLL) && data->signal) {
k_poll_signal_raise(data->signal, VIDEO_BUF_DONE);
}

if (IS_ENABLED(CONFIG_POLL) && data->signal) {
k_poll_signal_raise(data->signal, VIDEO_BUF_ABORTED);
}

#ifdef CONFIG_POLL
static int video_sw_generator_set_signal(const struct device *dev, enum video_endpoint_id ep,
struct k_poll_signal *signal)
{
struct video_sw_generator_data *data = dev->data;
if (data->signal && signal != NULL) {
return -EALREADY;
}
data->signal = signal;
return 0;
}
#endif

#ifdef CONFIG_POLL
.set_signal = video_sw_generator_set_signal,
#endif

Describe the solution you'd like

Like I2C and SPI, add a wrapper around the k_poll_signal_raise() to avoid drivers to have to add #ifdef CONFIG_POLL every time:

#if defined(CONFIG_POLL) || defined(__DOXYGEN__)
/** @cond INTERNAL_HIDDEN */
void z_i2c_transfer_signal_cb(const struct device *dev, int result, void *userdata);
/** @endcond */
/**
* @brief Perform data transfer to another I2C device in controller mode.
*
* This routine provides a generic interface to perform data transfer
* to another I2C device asynchronously with a k_poll_signal completion.
*
* @see i2c_transfer_cb()
* @funcprops \isr_ok
*
* @param dev Pointer to the device structure for an I2C controller
* driver configured in controller mode.
* @param msgs Array of messages to transfer, must live until callback completes.
* @param num_msgs Number of messages to transfer.
* @param addr Address of the I2C target device.
* @param sig Signal to notify of transfer completion.
*
* @retval 0 If successful.
* @retval -EIO General input / output error.
* @retval -ENOSYS If transfer async is not implemented
* @retval -EWOULDBLOCK If the device is temporarily busy doing another transfer
*/
static inline int i2c_transfer_signal(const struct device *dev,
struct i2c_msg *msgs,
uint8_t num_msgs,
uint16_t addr,
struct k_poll_signal *sig)
{
const struct i2c_driver_api *api = (const struct i2c_driver_api *)dev->api;
if (api->transfer_cb == NULL) {
return -ENOSYS;
}
return api->transfer_cb(dev, msgs, num_msgs, addr, z_i2c_transfer_signal_cb, sig);
}
#endif /* CONFIG_POLL */

#if defined(CONFIG_POLL) || defined(__DOXYGEN__)
/** @cond INTERNAL_HIDDEN */
void z_spi_transfer_signal_cb(const struct device *dev, int result, void *userdata);
/** @endcond */
/**
* @brief Read/write the specified amount of data from the SPI driver.
*
* @note This function is asynchronous.
*
* @note This function is available only if @kconfig{CONFIG_SPI_ASYNC}
* and @kconfig{CONFIG_POLL} are selected.
*
* @param dev Pointer to the device structure for the driver instance
* @param config Pointer to a valid spi_config structure instance.
* Pointer-comparison may be used to detect changes from
* previous operations.
* @param tx_bufs Buffer array where data to be sent originates from,
* or NULL if none.
* @param rx_bufs Buffer array where data to be read will be written to,
* or NULL if none.
* @param sig A pointer to a valid and ready to be signaled
* struct k_poll_signal. (Note: if NULL this function will not
* notify the end of the transaction, and whether it went
* successfully or not).
*
* @retval frames Positive number of frames received in slave mode.
* @retval 0 If successful in master mode.
* @retval -errno Negative errno code on failure.
*/
static inline int spi_transceive_signal(const struct device *dev,
const struct spi_config *config,
const struct spi_buf_set *tx_bufs,
const struct spi_buf_set *rx_bufs,
struct k_poll_signal *sig)
{
const struct spi_driver_api *api =
(const struct spi_driver_api *)dev->api;
spi_callback_t cb = (sig == NULL) ? NULL : z_spi_transfer_signal_cb;
return api->transceive_async(dev, config, tx_bufs, rx_bufs, cb, sig);
}

Describe alternatives you've considered

Advise to keep struct k_poll_signal in the dev->data struct, allowing more lightweight idioms like (IF_ENABLED(CONFIG_POLL) && data->signal != NULL) instead of #ifdefs

bool ctrl_vflip;
struct k_poll_signal *signal;
uint32_t frame_rate;

Additional context

Example of extra work contributors would not need to do inside of #86915

@josuah josuah added Enhancement Changes/Updates/Additions to existing features area: Video Video subsystem labels Mar 12, 2025
@josuah josuah self-assigned this Mar 12, 2025
@josuah josuah changed the title drivers: video: wrap the CONFIG_POLL #ifdefs to help readability drivers: video: wrap the CONFIG_POLL #ifdefs Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Video Video subsystem Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

1 participant