On 05/08/2018 03:28 AM, Sakari Ailus wrote:
Hi Steve,
Again, sorry about the delay. This thread got buried in my inbox. :-(
Please see my reply below.
On Mon, Apr 23, 2018 at 11:00:22AM -0700, Steve Longerbeam wrote:
On 04/23/2018 12:14 AM, Sakari Ailus wrote:
Hi Steve,
On Tue, Mar 20, 2018 at 05:37:21PM -0700, Steve Longerbeam wrote:
Adds v4l2_async_register_fwnode_subdev(), which is a convenience function
for parsing a sub-device's fwnode port endpoints for connected remote
sub-devices, registering a sub-device notifier, and then registering
the sub-device itself.
Signed-off-by: Steve Longerbeam <steve_longerb...@mentor.com>
---
Changes since v2:
- fix error-out path in v4l2_async_register_fwnode_subdev() that forgot
to put device.
Changes since v1:
- add #include <media/v4l2-subdev.h> to v4l2-fwnode.h for
'struct v4l2_subdev' declaration.
---
drivers/media/v4l2-core/v4l2-fwnode.c | 101
++++++++++++++++++++++++++++++++++
include/media/v4l2-fwnode.h | 43 +++++++++++++++
2 files changed, 144 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 99198b9..d42024d 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -880,6 +880,107 @@ int v4l2_async_register_subdev_sensor_common(struct
v4l2_subdev *sd)
}
EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
+int v4l2_async_register_fwnode_subdev(
+ struct v4l2_subdev *sd, size_t asd_struct_size,
+ unsigned int *ports, unsigned int num_ports,
+ int (*parse_endpoint)(struct device *dev,
+ struct v4l2_fwnode_endpoint *vep,
+ struct v4l2_async_subdev *asd))
+{
+ struct v4l2_async_notifier *notifier;
+ struct device *dev = sd->dev;
+ struct fwnode_handle *fwnode;
+ unsigned int subdev_port;
+ bool is_port;
+ int ret;
+
+ if (WARN_ON(!dev))
+ return -ENODEV;
+
+ fwnode = dev_fwnode(dev);
+ if (!fwnode_device_is_available(fwnode))
+ return -ENODEV;
+
+ is_port = (is_of_node(fwnode) &&
+ of_node_cmp(to_of_node(fwnode)->name, "port") == 0);
What's the intent of this and the code below? You may not parse the graph
data structure here, it should be done in the actual firmware
implementation instead.
The i.MX6 CSI sub-device registers itself from a port fwnode, so
the intent of the is_port code below is to support the i.MX6 CSI.
I can remove the is_port checks, but it means
v4l2_async_register_fwnode_subdev() won't be usable by the CSI
sub-device.
This won't scale.
The vast majority of sub-devices register themselves as
port parent nodes. So for now at least, I think
v4l2_async_register_fwnode_subdev() could be useful to many
platforms.
Instead, I think we'd need to separate registering
sub-devices (through async sub-devices) and binding them with the driver
that registered the notifier. Or at least change how that process works: a
single sub-device can well be bound to multiple notifiers,
Ok, that is certainly not the case now, a sub-device can only
be bound to a single notifier.
or multiple
times to the same notifier while it may be registered only once.
Anyway, this is a future generalization if I understand you
correctly. Not something to deal with here.
Also, sub-devices generally do not match ports.
Yes that's generally true, sub-devices generally match to port parent
nodes. But I do know of one other sub-device that buck that trend.
The ADV748x CSI-2 output sub-devices match against endpoint nodes.
Endpoints, yes, but not ports.
Well, the imx CSI registers from a port node.
How sub-devices generally
correspond to fwnodes is up to the device.
What do you think of adding a v4l2_async_register_port_fwnode_subdev(),
and a v4l2_async_register_endpoint_fwnode_subdev() to support such
sub-devices?
The endpoint is more specific than a port, so why the port and not the
endpoint?
Do you mean there should be a
v4l2_async_register_endpoint_fwnode_subdev() but not
v4l2_async_register_endpoint_port_subdev()?
Steve
