Hi Heikki
On 09/09/2020 14:40, Heikki Krogerus wrote:
Hi,
On Sat, Sep 05, 2020 at 09:19:51AM +0100, Daniel Scally wrote:
Hello
Following on from this thread:
https://www.spinics.net/lists/linux-driver-devel/msg135122.html -
apologies, can't see a way to reply to it directly.
Myself and others [1] have been trying to get cameras working on
Microsoft Surface and similar platforms, currently I'm working on
expanding Jordan's module connecting cameras to the cio2
infrastructure (which works - we can use it to take images), aiming to
discover connection information from SSDB in the DSDT, as well as
connect as many supported sensors as are found on the device. I'm just
struggling with a problem I've encountered using the swnode patch that
Heikki linked in that thread; the module's working ok when I only
attempt to connect a single one of my sensors (by preventing the
driver for the other sensor from loading, in which case this new
module ignores the sensor), but when I attempt to connect both cameras
at the same time I get a kernel oops as part of
software_node_get_next_child. The module is creating software_nodes
like this...
/sys/kernel/software_node/INT343E/port0/endpoint0
/sys/kernel/software_node/INT343E/port1/endpoint0
/sys/kernel/software_node/OVTI2680/port0/endpoint0
/sys/kernel/software_node/OVTI5648/port0/endpoint0
And that's the structure that I expect to see, but it seems like the
call to list_next_entry in that function is returning something that
isn't quite a valid swnode. Printing the address of c->fwnode after
that point returns "3", which isn't a valid address of course, and
that's causing the oops when it's either returned (in the version of
the function without the patches) or when the program flow tries to
call the "get" op against that fwnode (in the patched version). I've
been trying to track it down for a while now without success, so I
posted the problem on SO[2] and it was suggested that I mail these
addressees for advice - hope that that is ok.
My copy of Jordan's module is parked in my git repo [3] for now, and
requires a batch of patches from Jordan's repo [4] (one made by Heikki
to fill in the missing swnode graph pieces, and some further tweaks) -
I've been applying those against 5.8.0-rc7. Any other criticism more
than welcome - I'm new to both c and kernel programming so I'm happy
to take all the advice people have the time to give.
I'm almost certain that my graph patch is broken. My tests did not
cover nearly as much that is needed to properly test the patch. I
think at least the refcount handling is totally broken in
software_node_graph_get_next_endpoint(), so that function at least
needs to be rewritten.
Unfortunately I do not have time to work on that patch right now.
thanks,
Alright no problem; I shall persevere with what we have and see if I can
figure out why the linking isn't working. I think I actually found the
problem with refcount handling in software_node_graph_get_endpoint();
see
https://lore.kernel.org/linux-media/2d4f1abb-c617-476a-1005-0ed91906a5f5@xxxxxxxxx/T/#mf0dcf7dd78ae0cd40af998bc25a5a775c7e3f93d
- that function seems to behave as expected after a small tweak (at
least as I expect it to...it doesn't get and hold 4x references to the
port fwnode_handle now), but there might be one more refcount handling
issue somewhere (I see one more reference held than I expect to see at
the moment...but it could be from my code too of course).
If I get to the bottom of the problems I'll let you know.
Regards
Dan
