On 18/09/2020 09:57, Heikki Krogerus wrote: > On Fri, Sep 18, 2020 at 10:57:41AM +0300, Sakari Ailus wrote: >> On Fri, Sep 18, 2020 at 08:46:52AM +0100, Dan Scally wrote: >>> On 18/09/2020 08:34, Sakari Ailus wrote: >>>> On Fri, Sep 18, 2020 at 07:49:31AM +0100, Dan Scally wrote: >>>>> Good morning >>>>> >>>>> On 18/09/2020 07:22, Sakari Ailus wrote: >>>>>> I wonder if this has something to do with replacing the device's fwnode >>>>>> in the cio2-bridge patch. >>>>>> >>>>>> It's the device that needs to be enabled, and it's not a software node. >>>>>> >>>>> I think it is because of that yes, but I don't see a way around it at >>>>> the moment - unless there's a way to attach the software_node port and >>>>> endpoints that cio2-bridge creates to the device's existing firmware >>>>> instead. >>>> I thought this was how it was meant to be used? >>>> >>>> The secondary field is there for this purpose. But it may be not all fwnode >>>> interface functions operate on fwnode->secondary? >>> Let me test it; it might just require some changes to >>> software_node_graph_get_port_parent() to check if the parent fwnode is a >>> secondary, and if it is to return the primary instead. >> Ah, indeed. I forgot this part. I wonder if it'd cause issues to return the >> primary if you've got the secondary swnode. >> >> Heikki, any idea? >> >> Code elsewhere (e.g. V4L2 fwnode framework + drivers) assume a device is >> identified by a single fwnode, not two --- currently the swnode graph >> function returning port parent returns the secondary so there's no match >> with the primary fwnode. > Sorry I don't think I understand the scenario here, but never return > the primary node when the software node is the secondary from the > software node API! The software node functions deal and return > software nodes, and nothing else, just like ACPI deals with ACPI nodes > only and DT deals with OF nodes only. We must never jump between the > fwnode types at this level. That also means that if you want to > describe the device graph with software nodes, then every node in the > graph, starting from the port parents, must be a software node. > Whether or not the node is secondary is irrelevant. But I guess this > is not a problem here (or is it?). > > Considering the secondary node will unfortunately need to be done by > the callers of fwnode API when the fwnode API can't take care of that. > Alright, so if we want to attach software nodes as secondaries to a devices existing fwnode we'd need to modify things like fwnode_graph_get_next_endpoint_by_id() [1] to consider whether the returned node was a software_node secondary when they try to get the device's node to run *is_available() I did sort of wonder whether this was the right approach before, but there's other comments [2] in the source that reassured me, for example device_add_properties(): > * WARNING: The callers should not use this function if it is known that there > * is no real firmware node associated with @dev! In that case the callers > * should create a software node and assign it to @dev directly. [1] https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L1126 [2] https://elixir.bootlin.com/linux/latest/source/drivers/base/property.c#L541
