michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: video: Fix skipped CIDs when enumerating controls

Browse source 
When enumerating controls with VIDEO_CTRL_FLAG_NEXT_CTRL, if child devices
have controls with IDs lower or equal to the ones in the parent devices,
those controls will be accidentally skipped.

Fix this by resetting the query's ID and tracking of the queried device in
the query when moving to the next device in the pipeline.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Signed-off-by: Josuah Demangeon <me@josuah.net>
This commit is contained in:
Phi Bang Nguyen 2025-06-08 19:25:03 +02:00 • committed by Dan Kalowsky
commit 10deca2ef6
6 changed files with 40 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

3
drivers/video/video_common.c
View file

@ -400,6 +400,7 @@ int64_t video_get_csi_link_freq(const struct device *dev, uint8_t bpp, uint8_t l
.id = VIDEO_CID_LINK_FREQ,
};
struct video_ctrl_query ctrl_query = {
.dev = dev,
.id = VIDEO_CID_LINK_FREQ,
};
int ret;
@ -410,7 +411,7 @@ int64_t video_get_csi_link_freq(const struct device *dev, uint8_t bpp, uint8_t l
goto fallback;
}
ret = video_query_ctrl(dev, &ctrl_query);
ret = video_query_ctrl(&ctrl_query);
if (ret < 0) {
return ret;
}

20
drivers/video/video_ctrls.c
View file

@ -549,30 +549,32 @@ static inline const char *video_get_ctrl_name(uint32_t id)
}
}
int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq)
int video_query_ctrl(struct video_ctrl_query *cq)
{
int ret;
struct video_device *vdev;
struct video_ctrl *ctrl = NULL;
__ASSERT_NO_MSG(dev != NULL);
__ASSERT_NO_MSG(cq != NULL);
__ASSERT_NO_MSG(cq->dev != NULL);
if (cq->id & VIDEO_CTRL_FLAG_NEXT_CTRL) {
vdev = video_find_vdev(dev);
cq->id &= ~VIDEO_CTRL_FLAG_NEXT_CTRL;
while (vdev) {
vdev = video_find_vdev(cq->dev);
while (vdev != NULL) {
SYS_DLIST_FOR_EACH_CONTAINER(&vdev->ctrls, ctrl, node) {
if (ctrl->id > cq->id) {
goto fill_query;
}
}
vdev = video_find_vdev(vdev->src_dev);
cq->id = 0;
cq->dev = vdev->src_dev;
vdev = video_find_vdev(cq->dev);
}
return -ENOTSUP;
}
ret = video_find_ctrl(dev, cq->id, &ctrl);
ret = video_find_ctrl(cq->dev, cq->id, &ctrl);
if (ret) {
return ret;
}
@ -592,14 +594,14 @@ fill_query:
return 0;
}
void video_print_ctrl(const struct device *const dev, const struct video_ctrl_query *const cq)
void video_print_ctrl(const struct video_ctrl_query *const cq)
{
uint8_t i = 0;
const char *type = NULL;
char typebuf[8];
__ASSERT_NO_MSG(dev != NULL);
__ASSERT_NO_MSG(cq != NULL);
__ASSERT_NO_MSG(cq->dev != NULL);
/* Get type of the control */
switch (cq->type) {
@ -629,7 +631,7 @@ void video_print_ctrl(const struct device *const dev, const struct video_ctrl_qu
/* Get current value of the control */
struct video_control vc = {.id = cq->id};
video_get_ctrl(dev, &vc);
video_get_ctrl(cq->dev, &vc);
/* Print the control information */
if (cq->type == VIDEO_CTRL_TYPE_INTEGER64) {

31
drivers/video/video_shell.c
View file

@ -637,8 +637,7 @@ static void video_shell_convert_ctrl_name(char const *src, char *dst, size_t dst
} while (o-- > 0 && !isalnum(dst[o]));
}
static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_ctrl_query *cq,
char const *name)
static int video_shell_get_ctrl_by_name(struct video_ctrl_query *cq, char const *name)
{
char ctrl_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
int ret;
@ -647,7 +646,7 @@ static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_c
for (size_t i = 0;; i++) {
cq->id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
ret = video_query_ctrl(dev, cq);
ret = video_query_ctrl(cq);
if (ret < 0) {
return -ENOENT;
}
@ -661,8 +660,7 @@ static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_c
return 0;
}
static int video_shell_get_ctrl_by_num(const struct device *dev, struct video_ctrl_query *cq,
int num)
static int video_shell_get_ctrl_by_num(struct video_ctrl_query *cq, int num)
{
int ret;
@ -670,7 +668,7 @@ static int video_shell_get_ctrl_by_num(const struct device *dev, struct video_ct
for (size_t i = 0; i <= num; i++) {
cq->id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
ret = video_query_ctrl(dev, cq);
ret = video_query_ctrl(cq);
if (ret < 0) {
return -ENOENT;
}
@ -682,7 +680,7 @@ static int video_shell_get_ctrl_by_num(const struct device *dev, struct video_ct
static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev, size_t argc,
char **argv)
{
struct video_ctrl_query cq = {0};
struct video_ctrl_query cq = {.dev = dev};
struct video_control ctrl;
char menu_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
char *arg_ctrl = argv[0];
@ -691,7 +689,7 @@ static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev
size_t i;
int ret;
ret = video_shell_get_ctrl_by_name(dev, &cq, arg_ctrl);
ret = video_shell_get_ctrl_by_name(&cq, arg_ctrl);
if (ret < 0) {
shell_error(sh, "Video control '%s' not found for this device", arg_ctrl);
return ret;
@ -752,15 +750,15 @@ static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev
static int video_shell_print_ctrls(const struct shell *sh, const struct device *dev)
{
struct video_ctrl_query cq = {.id = VIDEO_CTRL_FLAG_NEXT_CTRL};
struct video_ctrl_query cq = {.dev = dev, .id = VIDEO_CTRL_FLAG_NEXT_CTRL};
char ctrl_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
while (video_query_ctrl(dev, &cq) == 0) {
while (video_query_ctrl(&cq) == 0) {
/* Convert from human-friendly form to machine-friendly */
video_shell_convert_ctrl_name(cq.name, ctrl_name, sizeof(ctrl_name));
cq.name = ctrl_name;
video_print_ctrl(dev, &cq);
video_print_ctrl(&cq);
cq.id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
}
@ -812,7 +810,7 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_video_ctrl_boolean, complete_video_ctrl_boolean);
static void complete_video_ctrl_menu_name(size_t idx, struct shell_static_entry *entry, int ctrln)
{
const struct device *dev = video_shell_dev;
struct video_ctrl_query cq = {0};
struct video_ctrl_query cq = {.dev = dev};
int ret;
entry->handler = NULL;
@ -820,14 +818,13 @@ static void complete_video_ctrl_menu_name(size_t idx, struct shell_static_entry
entry->subcmd = NULL;
entry->syntax = NULL;
dev = video_shell_dev;
if (!device_is_ready(dev)) {
return;
}
/* Check which control name was selected */
ret = video_shell_get_ctrl_by_num(dev, &cq, ctrln);
ret = video_shell_get_ctrl_by_num(&cq, ctrln);
if (ret < 0) {
return;
}
@ -874,13 +871,13 @@ static void complete_video_ctrl_name_dev(size_t idx, struct shell_static_entry *
}
/* Then complete the control name */
ret = video_shell_get_ctrl_by_num(dev, &video_shell_cq, idx);
cq->dev = dev;
ret = video_shell_get_ctrl_by_num(cq, idx);
if (ret < 0) {
return;
}
video_shell_convert_ctrl_name(video_shell_cq.name, video_shell_ctrl_name,
video_shell_convert_ctrl_name(cq->name, video_shell_ctrl_name,
sizeof(video_shell_ctrl_name));
entry->syntax = video_shell_ctrl_name;

4
include/zephyr/drivers/video-controls.h
View file

@ -455,7 +455,9 @@ struct video_ctrl_range {
* Used to query information about a control.
*/
struct video_ctrl_query {
/** control id */
/** device being queried, application needs to set this field */
const struct device *dev;
/** control id, application needs to set this field */
uint32_t id;
/** control type */
uint32_t type;

6
include/zephyr/drivers/video.h
View file

@ -708,14 +708,13 @@ struct video_ctrl_query;
* difficult. Hence, the best way to enumerate all kinds of device's supported controls is to
* iterate with VIDEO_CTRL_FLAG_NEXT_CTRL.
*
* @param dev Pointer to the device structure.
* @param cq Pointer to the control query struct.
*
* @retval 0 If successful.
* @retval -EINVAL If the control id is invalid.
* @retval -ENOTSUP If the control id is not supported.
*/
int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq);
int video_query_ctrl(struct video_ctrl_query *cq);
/**
* @brief Print all the information of a control.
@ -724,10 +723,9 @@ int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq);
* menu (if any) and current value, i.e. by invoking the video_get_ctrl(), in a
* human readble format.
*
* @param dev Pointer to the device structure.
* @param cq Pointer to the control query struct.
*/
void video_print_ctrl(const struct device *const dev, const struct video_ctrl_query *const cq);
void video_print_ctrl(const struct video_ctrl_query *const cq);
/**
* @brief Register/Unregister k_poll signal for a video endpoint.

12
samples/drivers/video/capture/src/main.c
View file

@ -181,11 +181,15 @@ int main(void)
/* Get supported controls */
LOG_INF("- Supported controls:");
const struct device *last_dev = NULL;
struct video_ctrl_query cq = {.dev = video_dev, .id = VIDEO_CTRL_FLAG_NEXT_CTRL};
struct video_ctrl_query cq = {.id = VIDEO_CTRL_FLAG_NEXT_CTRL};
while (!video_query_ctrl(video_dev, &cq)) {
video_print_ctrl(video_dev, &cq);
while (!video_query_ctrl(&cq)) {
if (cq.dev != last_dev) {
last_dev = cq.dev;
LOG_INF("\t\tdevice: %s", cq.dev->name);
}
video_print_ctrl(&cq);
cq.id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
}