Background
We have a custom carrier board with connectors for 3 Leopard Imaging IMX185 sensors. We have modified the Device Tree to support our carrier board and have successfully used all of the sensors.
We have implemented Device Tree overlay support using a very recent version of U-Boot. I won’t waste anyone’s time going into the details.
One feature of our modified U-Boot design is the ability to probe the attached cameras and then dynamically enable the appropriate vi/nvcsi/tegra-camera-platform configurations.
My setup allows me to quickly switch between a standard Device Tree boot and a Device Tree boot with an Overlay.
To confirm the two different setups were the same I used the DTC to generate DTSs for both the ‘Device Tree’ and ‘Overlay’ versions and then compared the results and they are identical (except for the phandles).
Problem
The behavior of the two different setups were identical except for one aspect. The camera sensors. On the normal Device Tree version they worked fine; I could run the ‘argus_camera’ and see live video, while on the Overlay version the application threw an error. The problem had me stumped for a while until I started digging into the kernel and enabling/adding debug messages.
When I compared the boot logs of the two setups I found that that the Overlay version didn’t order the entries in the device tree the same way as the normal Device Tree version had.
To be more specific, this effected the function tegra_vi_graph_init that can be found in the kernel source:
<kernel source>/nvidia/drivers/media/platform/tegra/camera/vi/graph.c
git commit hash: df1762cda0e4d3137a7198e0ade3c8435b8a4bb5
or
git tag tegra-l4t-r32.3.1
One aspect that seemed to be amiss was the line 744:
next = of_graph_get_next_endpoint(vi->dev->of_node, ep);
The function of_graph_get_next_endpoint gets the next endpoint regardless of what channel it is on, so you can create a graph with a channel and endpoints that are not related to each other.
There is a very good chance, while using the normal Device Tree, that the channel and endpoints will be aligned but it’s not a guarantee, so instead of using this function I used this one:
of_graph_get_endpoint_by_regs(vi->dev->of_node, chan->id, -1);
To simplify things, here is a patch that can be aplied to graph.c to enable this. Unfortunately I only have one IMX185 sensor to test this out and I would like to see if it is usable with multiple sensors.
This should be applied to the base of the ‘nvidia’ kernel
diff --git a/drivers/media/platform/tegra/camera/vi/graph.c b/drivers/media/platform/tegra/camera/vi/graph.c
index 8deb42186..32dbb8614 100644
--- a/drivers/media/platform/tegra/camera/vi/graph.c
+++ b/drivers/media/platform/tegra/camera/vi/graph.c
@@ -725,9 +725,8 @@ int tegra_vi_graph_init(struct tegra_mc_vi *vi)
unsigned int num_subdevs = 0;
int ret = 0, i;
struct device_node *ep = NULL;
- struct device_node *next;
struct device_node *remote = NULL;
- struct tegra_channel *chan;
+ struct tegra_channel *chan = NULL;
/*
* Walk the links to parse the full graph. Each struct tegra_channel
@@ -738,22 +737,22 @@ int tegra_vi_graph_init(struct tegra_mc_vi *vi)
* channel in case of something wrong during graph parsing and try
* the next channel. Return error only if memory allocation is failed.
*/
- chan = list_first_entry(&vi->vi_chans, struct tegra_channel, list);
- do {
- /* Get the next endpoint and parse its entities. */
- next = of_graph_get_next_endpoint(vi->dev->of_node, ep);
- if (next == NULL)
- break;
-
- ep = next;
+ while ((chan == NULL) || !list_is_last(&chan->list, &vi->vi_chans)){
+ if (chan == NULL) /* First Channel */
+ chan = list_first_entry(&vi->vi_chans, struct tegra_channel, list);
+ else /* Next Channel */
+ chan = list_next_entry(chan, list);
+
+ dev_dbg(vi->dev, "Working on Channel: %d\n", chan->id);
+ ep = of_graph_get_endpoint_by_regs(vi->dev->of_node, chan->id, -1);
+ if (ep == NULL){
+ dev_info(vi->dev, "Channel: %d EP == NULL!\n", chan->id);
+ continue;
+ }
if (!of_device_is_available(ep)) {
dev_info(vi->dev, "ep of_device is not enabled %s.\n",
ep->full_name);
- if (list_is_last(&chan->list, &vi->vi_chans))
- break;
- /* Try the next channel */
- chan = list_next_entry(chan, list);
continue;
}
@@ -768,20 +767,12 @@ int tegra_vi_graph_init(struct tegra_mc_vi *vi)
remote = of_graph_get_remote_port_parent(ep);
if (!remote) {
dev_info(vi->dev, "cannot find remote port parent\n");
- if (list_is_last(&chan->list, &vi->vi_chans))
- break;
- /* Try the next channel */
- chan = list_next_entry(chan, list);
continue;
}
if (!of_device_is_available(remote)) {
dev_info(vi->dev, "remote of_device is not enabled %s.\n",
ep->full_name);
- if (list_is_last(&chan->list, &vi->vi_chans))
- break;
- /* Try the next channel */
- chan = list_next_entry(chan, list);
continue;
}
@@ -802,10 +793,6 @@ int tegra_vi_graph_init(struct tegra_mc_vi *vi)
if (ret < 0) {
dev_info(vi->dev, "graph parse error: %s.\n",
entity->node->full_name);
- if (list_is_last(&chan->list, &vi->vi_chans))
- break;
- /* Try the next channel */
- chan = list_next_entry(chan, list);
continue;
}
@@ -837,11 +824,7 @@ int tegra_vi_graph_init(struct tegra_mc_vi *vi)
goto done;
}
- if (list_is_last(&chan->list, &vi->vi_chans))
- break;
- /* One endpoint for each vi channel, go with the next channel */
- chan = list_next_entry(chan, list);
- } while (next);
+ }
done:
if (ret == -ENOMEM) {
I would have uploaded a patch file but the upload tool wouldn’t let me.
I’d like to know if this works out for multiple sensors and if there is any issue with this approach.
Thanks,
Dave