Hi Mats,
On Fri, Jun 15, 2018 at 11:51:17PM +0200, Mats Karrman wrote:
> I hacked away to see if I could make some sense of this but I didn't get far.
> Do you mean that the device_connection_find_match() function should be
> extended with code for finding matches in devicetrees as well?
Yes.
> I tried and ended up creating temporary device_connection instances just
> to try to satisfy the match function you supply from usb_role_switch_get(),
That's how I planned to use it.
> only the OF graph connection does not have a name to compare with.
I think this is about the problem I explained below. So let's just add the
fwnode member to the struct device_connection.
> > > (Ugh, just realized that the "reg" numeric value of the endpoint node is
> > > put in the "id" field of the endpoint struct, bad luck...).
> > Isn't the "compatible" property the one that should be used in this
> > case? The remote endpoint just has to have compatible property
> > matching the con_id, no?
>
> By endpoint you mean the OF node named "endpoint" as described by graph
> documentation or something else?
> I have not seen or heard of any "endpoint" nodes having "compatible" properties,
> only the nodes containing the "port(s)" nodes. The USB controller node
> won't have compatible set to "usb-role-switch" so which node do you mean?
"compatible" was just a suggestion, but why couldn't for example USB
controller have the "compatible" set also to "usb-role-switch" if it
really acts as the role switch on your system?
compatible = "<your_platform>-dwc3", "usb-role-switch"
> > My worry is that currently there is no function to convert a fwnode
> > to device. With ACPI it is possible already, but only to the first
> > device bind to the node (ACPI device node can be bind to multiple
> > devices). I don't think there is a function to convert OF node to
> > device.
> >
> > We could of course workaround this problem and add fwnode member(s) to
> > the device_lookup structure. The callers would then have to check is
> > fwnode or the endpoint (device) name being used, which is not ideal
> > IMO.
>
> "device_connection" structure?
Yes, sorry about that.
You know, I'm not completely sure what is the benefit in using device
graph with USB connectors? I don't think we need to describe a real
device graph where we could have multiple levels, or do we?
In any case, I guess we are stuck with it since it's used in
Documentation/devicetree/bindings/connector/usb-connector.txt, but I
think in device_connection_find_match() we should still first simply
try to find a named reference (so in case of device tree call
of_parse_phandle(node, con_id, 0)), if that fails try to parse the
graph, and finally if that also fails, check if we have a build-in
description of the connection. Something like this:
void *device_connection_find_match(struct device *dev, const char *con_id,
...
struct fwnode_handle *fwnode = dev_fwnode(dev);
struct device_connection connection = { };
struct fwnode_handle *endpoint = NULL;
struct fwnode_referece_args args;
...
/* First let's check if there is a named reference. */
if (!fwnode_property_get_reference_args(fwnode, con_id, NULL, 0, 0,
&args) {
connection.fwnode = args.fwnode;
ret = match(&connection, 0, data);
...
}
/* Let's try device graph */
while ((endpoint = fwnode_graph_get_next_endpoint(fwnode, endpoint))) {
struct fwnode_handle *remote;
remote = fwnode_graph_get_remote_port_parent(endpoint);
if (!remote)
break;
/* In this example I'm using "compatible" */
if (!fwnode_property_match_string(remote, "compatible",
con_id)) {
connection.fwnode = remote;
ret = match(&connection, 0, data);
...
}
}
/* Finally check if there is a build-in connection description */
...
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
