Skip to content

Commit 10deca2

Browse files
ngphibangdkalowsk
ngphibang
authored and
dkalowsk
committed
drivers: video: Fix skipped CIDs when enumerating controls
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 <[email protected]> Signed-off-by: Josuah Demangeon <[email protected]>
1 parent 635a006 commit 10deca2

File tree

6 files changed

+40
-36
lines changed

6 files changed

+40
-36
lines changed

‎drivers/video/video_common.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ int64_t video_get_csi_link_freq(const struct device *dev, uint8_t bpp, uint8_t l
400400
.id = VIDEO_CID_LINK_FREQ,
401401
};
402402
struct video_ctrl_query ctrl_query = {
403+
.dev = dev,
403404
.id = VIDEO_CID_LINK_FREQ,
404405
};
405406
int ret;
@@ -410,7 +411,7 @@ int64_t video_get_csi_link_freq(const struct device *dev, uint8_t bpp, uint8_t l
410411
goto fallback;
411412
}
412413

413-
ret = video_query_ctrl(dev, &ctrl_query);
414+
ret = video_query_ctrl(&ctrl_query);
414415
if (ret < 0) {
415416
return ret;
416417
}

‎drivers/video/video_ctrls.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -549,30 +549,32 @@ static inline const char *video_get_ctrl_name(uint32_t id)
549549
}
550550
}
551551

552-
int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq)
552+
int video_query_ctrl(struct video_ctrl_query *cq)
553553
{
554554
int ret;
555555
struct video_device *vdev;
556556
struct video_ctrl *ctrl = NULL;
557557

558-
__ASSERT_NO_MSG(dev != NULL);
559558
__ASSERT_NO_MSG(cq != NULL);
559+
__ASSERT_NO_MSG(cq->dev != NULL);
560560

561561
if (cq->id & VIDEO_CTRL_FLAG_NEXT_CTRL) {
562-
vdev = video_find_vdev(dev);
563562
cq->id &= ~VIDEO_CTRL_FLAG_NEXT_CTRL;
564-
while (vdev) {
563+
vdev = video_find_vdev(cq->dev);
564+
while (vdev != NULL) {
565565
SYS_DLIST_FOR_EACH_CONTAINER(&vdev->ctrls, ctrl, node) {
566566
if (ctrl->id > cq->id) {
567567
goto fill_query;
568568
}
569569
}
570-
vdev = video_find_vdev(vdev->src_dev);
570+
cq->id = 0;
571+
cq->dev = vdev->src_dev;
572+
vdev = video_find_vdev(cq->dev);
571573
}
572574
return -ENOTSUP;
573575
}
574576

575-
ret = video_find_ctrl(dev, cq->id, &ctrl);
577+
ret = video_find_ctrl(cq->dev, cq->id, &ctrl);
576578
if (ret) {
577579
return ret;
578580
}
@@ -592,14 +594,14 @@ int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq)
592594
return 0;
593595
}
594596

595-
void video_print_ctrl(const struct device *const dev, const struct video_ctrl_query *const cq)
597+
void video_print_ctrl(const struct video_ctrl_query *const cq)
596598
{
597599
uint8_t i = 0;
598600
const char *type = NULL;
599601
char typebuf[8];
600602

601-
__ASSERT_NO_MSG(dev != NULL);
602603
__ASSERT_NO_MSG(cq != NULL);
604+
__ASSERT_NO_MSG(cq->dev != NULL);
603605

604606
/* Get type of the control */
605607
switch (cq->type) {
@@ -629,7 +631,7 @@ void video_print_ctrl(const struct device *const dev, const struct video_ctrl_qu
629631
/* Get current value of the control */
630632
struct video_control vc = {.id = cq->id};
631633

632-
video_get_ctrl(dev, &vc);
634+
video_get_ctrl(cq->dev, &vc);
633635

634636
/* Print the control information */
635637
if (cq->type == VIDEO_CTRL_TYPE_INTEGER64) {

‎drivers/video/video_shell.c

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,7 @@ static void video_shell_convert_ctrl_name(char const *src, char *dst, size_t dst
637637
} while (o-- > 0 && !isalnum(dst[o]));
638638
}
639639

640-
static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_ctrl_query *cq,
641-
char const *name)
640+
static int video_shell_get_ctrl_by_name(struct video_ctrl_query *cq, char const *name)
642641
{
643642
char ctrl_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
644643
int ret;
@@ -647,7 +646,7 @@ static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_c
647646
for (size_t i = 0;; i++) {
648647
cq->id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
649648

650-
ret = video_query_ctrl(dev, cq);
649+
ret = video_query_ctrl(cq);
651650
if (ret < 0) {
652651
return -ENOENT;
653652
}
@@ -661,16 +660,15 @@ static int video_shell_get_ctrl_by_name(const struct device *dev, struct video_c
661660
return 0;
662661
}
663662

664-
static int video_shell_get_ctrl_by_num(const struct device *dev, struct video_ctrl_query *cq,
665-
int num)
663+
static int video_shell_get_ctrl_by_num(struct video_ctrl_query *cq, int num)
666664
{
667665
int ret;
668666

669667
cq->id = 0;
670668
for (size_t i = 0; i <= num; i++) {
671669
cq->id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
672670

673-
ret = video_query_ctrl(dev, cq);
671+
ret = video_query_ctrl(cq);
674672
if (ret < 0) {
675673
return -ENOENT;
676674
}
@@ -682,7 +680,7 @@ static int video_shell_get_ctrl_by_num(const struct device *dev, struct video_ct
682680
static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev, size_t argc,
683681
char **argv)
684682
{
685-
struct video_ctrl_query cq = {0};
683+
struct video_ctrl_query cq = {.dev = dev};
686684
struct video_control ctrl;
687685
char menu_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
688686
char *arg_ctrl = argv[0];
@@ -691,7 +689,7 @@ static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev
691689
size_t i;
692690
int ret;
693691

694-
ret = video_shell_get_ctrl_by_name(dev, &cq, arg_ctrl);
692+
ret = video_shell_get_ctrl_by_name(&cq, arg_ctrl);
695693
if (ret < 0) {
696694
shell_error(sh, "Video control '%s' not found for this device", arg_ctrl);
697695
return ret;
@@ -752,15 +750,15 @@ static int video_shell_set_ctrl(const struct shell *sh, const struct device *dev
752750

753751
static int video_shell_print_ctrls(const struct shell *sh, const struct device *dev)
754752
{
755-
struct video_ctrl_query cq = {.id = VIDEO_CTRL_FLAG_NEXT_CTRL};
753+
struct video_ctrl_query cq = {.dev = dev, .id = VIDEO_CTRL_FLAG_NEXT_CTRL};
756754
char ctrl_name[CONFIG_VIDEO_SHELL_CTRL_NAME_SIZE];
757755

758-
while (video_query_ctrl(dev, &cq) == 0) {
756+
while (video_query_ctrl(&cq) == 0) {
759757
/* Convert from human-friendly form to machine-friendly */
760758
video_shell_convert_ctrl_name(cq.name, ctrl_name, sizeof(ctrl_name));
761759
cq.name = ctrl_name;
762760

763-
video_print_ctrl(dev, &cq);
761+
video_print_ctrl(&cq);
764762
cq.id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
765763
}
766764

@@ -812,22 +810,21 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_video_ctrl_boolean, complete_video_ctrl_boolean);
812810
static void complete_video_ctrl_menu_name(size_t idx, struct shell_static_entry *entry, int ctrln)
813811
{
814812
const struct device *dev = video_shell_dev;
815-
struct video_ctrl_query cq = {0};
813+
struct video_ctrl_query cq = {.dev = dev};
816814
int ret;
817815

818816
entry->handler = NULL;
819817
entry->help = NULL;
820818
entry->subcmd = NULL;
821819
entry->syntax = NULL;
822820

823-
dev = video_shell_dev;
824821
if (!device_is_ready(dev)) {
825822
return;
826823
}
827824

828825
/* Check which control name was selected */
829826

830-
ret = video_shell_get_ctrl_by_num(dev, &cq, ctrln);
827+
ret = video_shell_get_ctrl_by_num(&cq, ctrln);
831828
if (ret < 0) {
832829
return;
833830
}
@@ -874,13 +871,13 @@ static void complete_video_ctrl_name_dev(size_t idx, struct shell_static_entry *
874871
}
875872

876873
/* Then complete the control name */
877-
878-
ret = video_shell_get_ctrl_by_num(dev, &video_shell_cq, idx);
874+
cq->dev = dev;
875+
ret = video_shell_get_ctrl_by_num(cq, idx);
879876
if (ret < 0) {
880877
return;
881878
}
882879

883-
video_shell_convert_ctrl_name(video_shell_cq.name, video_shell_ctrl_name,
880+
video_shell_convert_ctrl_name(cq->name, video_shell_ctrl_name,
884881
sizeof(video_shell_ctrl_name));
885882
entry->syntax = video_shell_ctrl_name;
886883

‎include/zephyr/drivers/video-controls.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,9 @@ struct video_ctrl_range {
455455
* Used to query information about a control.
456456
*/
457457
struct video_ctrl_query {
458-
/** control id */
458+
/** device being queried, application needs to set this field */
459+
const struct device *dev;
460+
/** control id, application needs to set this field */
459461
uint32_t id;
460462
/** control type */
461463
uint32_t type;

‎include/zephyr/drivers/video.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -708,14 +708,13 @@ struct video_ctrl_query;
708708
* difficult. Hence, the best way to enumerate all kinds of device's supported controls is to
709709
* iterate with VIDEO_CTRL_FLAG_NEXT_CTRL.
710710
*
711-
* @param dev Pointer to the device structure.
712711
* @param cq Pointer to the control query struct.
713712
*
714713
* @retval 0 If successful.
715714
* @retval -EINVAL If the control id is invalid.
716715
* @retval -ENOTSUP If the control id is not supported.
717716
*/
718-
int video_query_ctrl(const struct device *dev, struct video_ctrl_query *cq);
717+
int video_query_ctrl(struct video_ctrl_query *cq);
719718

720719
/**
721720
* @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);
724723
* menu (if any) and current value, i.e. by invoking the video_get_ctrl(), in a
725724
* human readble format.
726725
*
727-
* @param dev Pointer to the device structure.
728726
* @param cq Pointer to the control query struct.
729727
*/
730-
void video_print_ctrl(const struct device *const dev, const struct video_ctrl_query *const cq);
728+
void video_print_ctrl(const struct video_ctrl_query *const cq);
731729

732730
/**
733731
* @brief Register/Unregister k_poll signal for a video endpoint.

‎samples/drivers/video/capture/src/main.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,15 @@ int main(void)
181181

182182
/* Get supported controls */
183183
LOG_INF("- Supported controls:");
184+
const struct device *last_dev = NULL;
185+
struct video_ctrl_query cq = {.dev = video_dev, .id = VIDEO_CTRL_FLAG_NEXT_CTRL};
184186

185-
struct video_ctrl_query cq = {.id = VIDEO_CTRL_FLAG_NEXT_CTRL};
186-
187-
while (!video_query_ctrl(video_dev, &cq)) {
188-
video_print_ctrl(video_dev, &cq);
187+
while (!video_query_ctrl(&cq)) {
188+
if (cq.dev != last_dev) {
189+
last_dev = cq.dev;
190+
LOG_INF("\t\tdevice: %s", cq.dev->name);
191+
}
192+
video_print_ctrl(&cq);
189193
cq.id |= VIDEO_CTRL_FLAG_NEXT_CTRL;
190194
}
191195

0 commit comments

Comments
 (0)