-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Implement video control framework #82158
base: main
Are you sure you want to change the base?
Changes from 1 commit
6fc1d3e
8b3c3c5
b98753e
4080680
0fe00c7
a7ea463
81be3e1
356e98b
1583d64
62695e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,118 @@ | ||||||||||
/* | ||||||||||
* Copyright 2024 NXP | ||||||||||
* | ||||||||||
* SPDX-License-Identifier: Apache-2.0 | ||||||||||
*/ | ||||||||||
|
||||||||||
#include "video_async.h" | ||||||||||
#include "video_device.h" | ||||||||||
|
||||||||||
#include <zephyr/device.h> | ||||||||||
#include <zephyr/logging/log.h> | ||||||||||
#include <zephyr/sys/slist.h> | ||||||||||
|
||||||||||
LOG_MODULE_REGISTER(video_async, CONFIG_VIDEO_LOG_LEVEL); | ||||||||||
|
||||||||||
static sys_slist_t notifiers_list; | ||||||||||
|
||||||||||
/* Find a registered notifier associated with this device */ | ||||||||||
struct video_notifier *video_async_find_notifier(const struct device *dev) | ||||||||||
{ | ||||||||||
struct video_notifier *nf; | ||||||||||
|
||||||||||
SYS_SLIST_FOR_EACH_CONTAINER(¬ifiers_list, nf, node) { | ||||||||||
if (nf->dev == dev) { | ||||||||||
return nf; | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
return NULL; | ||||||||||
} | ||||||||||
Comment on lines
+19
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be in favor of a struct video_device_data *vdata = dev->data;
struct video_notifier *nf = &vdata->notifier;
struct thisdriver_data *data = vdata->data; Is there anywhere where this would not work? |
||||||||||
|
||||||||||
/* Find the video device of the notifier tree that this notifier is part of */ | ||||||||||
static struct video_device *video_async_find_vdev(struct video_notifier *notifier) | ||||||||||
{ | ||||||||||
if (!notifier) { | ||||||||||
return NULL; | ||||||||||
} | ||||||||||
|
||||||||||
while (notifier->parent) { | ||||||||||
notifier = notifier->parent; | ||||||||||
} | ||||||||||
|
||||||||||
return notifier->vdev; | ||||||||||
} | ||||||||||
|
||||||||||
void video_async_init(struct video_notifier *notifier, const struct device *dev, | ||||||||||
struct video_device *vdev, const struct device **children_devs, | ||||||||||
uint8_t children_num) | ||||||||||
{ | ||||||||||
if (!notifier) { | ||||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
notifier->dev = dev; | ||||||||||
notifier->vdev = vdev; | ||||||||||
notifier->children_devs = (const struct device **)children_devs; | ||||||||||
notifier->children_num = children_num; | ||||||||||
} | ||||||||||
|
||||||||||
int video_async_register(struct video_notifier *notifier) | ||||||||||
{ | ||||||||||
uint8_t i = 0; | ||||||||||
bool found_parent = false; | ||||||||||
struct video_notifier *nf; | ||||||||||
struct video_device *vdev = NULL; | ||||||||||
|
||||||||||
if (!notifier) { | ||||||||||
return -EINVAL; | ||||||||||
} | ||||||||||
|
||||||||||
if (sys_slist_find(¬ifiers_list, ¬ifier->node, NULL)) { | ||||||||||
LOG_ERR("Notifier of device %s is already registered", notifier->dev->name); | ||||||||||
return -EALREADY; | ||||||||||
} | ||||||||||
|
||||||||||
SYS_SLIST_FOR_EACH_CONTAINER(¬ifiers_list, nf, node) { | ||||||||||
/* Find the parent notifier in the global notifiers registry */ | ||||||||||
if (!found_parent) { | ||||||||||
for (i = 0; i < nf->children_num; ++i) { | ||||||||||
if (nf->children_devs[i] == notifier->dev) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trying to reduce nested ifs :)
Suggested change
|
||||||||||
notifier->parent = nf; | ||||||||||
|
||||||||||
vdev = video_async_find_vdev(notifier); | ||||||||||
if (vdev) { | ||||||||||
notifier->vdev = vdev; | ||||||||||
} | ||||||||||
|
||||||||||
found_parent = true; | ||||||||||
break; | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/* Find all children notifiers in the notifiers registry */ | ||||||||||
for (i = 0; i < notifier->children_num; ++i) { | ||||||||||
if (notifier->children_devs[i] == nf->dev) { | ||||||||||
nf->parent = notifier; | ||||||||||
break; | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/* Add the notifier to the notifiers registry */ | ||||||||||
sys_slist_append(¬ifiers_list, ¬ifier->node); | ||||||||||
|
||||||||||
LOG_DBG("Notifier of device %s is registered", notifier->dev->name); | ||||||||||
|
||||||||||
return 0; | ||||||||||
} | ||||||||||
|
||||||||||
void video_async_unregister(struct video_notifier *notifier) | ||||||||||
{ | ||||||||||
if (!notifier) { | ||||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
sys_slist_find_and_remove(¬ifiers_list, ¬ifier->node); | ||||||||||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this supposed to be a public header ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, there are no public header in this PR. They are all internal, for driver usage only. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright 2024 NXP | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
#ifndef ZEPHYR_INCLUDE_VIDEO_ASYNC_H_ | ||
#define ZEPHYR_INCLUDE_VIDEO_ASYNC_H_ | ||
|
||
#include <zephyr/device.h> | ||
#include <zephyr/sys/slist.h> | ||
|
||
struct video_device; | ||
|
||
struct video_notifier { | ||
struct video_device *vdev; | ||
const struct device *dev; | ||
struct video_notifier *parent; | ||
const struct device **children_devs; | ||
uint8_t children_num; | ||
sys_snode_t node; | ||
}; | ||
|
||
void video_async_init(struct video_notifier *notifier, const struct device *dev, | ||
struct video_device *vdev, const struct device **children_devs, | ||
uint8_t children_num); | ||
|
||
int video_async_register(struct video_notifier *notifier); | ||
|
||
void video_async_unregister(struct video_notifier *notifier); | ||
|
||
struct video_notifier *video_async_find_notifier(const struct device *dev); | ||
|
||
#endif /* ZEPHYR_INCLUDE_VIDEO_ASYNC_H_ */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some of this can be replaced by this new device API.
Each of Zephyr
struct device *dev
is on a static array, and it is possible to get that array at runtime withz_device_get_all_static()
.Combined with this new API that allows to check which API a device belongs to, it is already possible to loop through all video devices.
Would that allow to remove some/all looping mechanisms and list to maintain?