Linux-Media Archive on
 help / color / Atom feed
From: Dan Scally <>
To: Sakari Ailus <>,
	Heikki Krogerus <>
Subject: Re: [PATCH v2] software_node: Add support for fwnode_graph*() family of functions
Date: Fri, 18 Sep 2020 10:22:34 +0100
Message-ID: <> (raw)
In-Reply-To: <>

On 18/09/2020 10:15, Sakari Ailus wrote:
> On Fri, Sep 18, 2020 at 11:57:09AM +0300, 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:
>>>>>>> Hi Dan,
>>>>>>> On Wed, Sep 16, 2020 at 02:22:10PM +0100, Dan Scally wrote:
>>>>>>>> Hi Sakari - thanks for the comments
>>>>>>>> On 16/09/2020 10:17, Sakari Ailus wrote:
>>>>>>>>> Moi Daniel and Heikki,
>>>>>>>>> On Wed, Sep 16, 2020 at 12:28:27AM +0100, Daniel Scally wrote:
>>>>>>>>>> From: Heikki Krogerus <>
>>>>>>>>>> This implements the remaining .graph_* callbacks in the
>>>>>>>>>> fwnode operations vector for the software nodes. That makes
>>>>>>>>>> the fwnode_graph*() functions available in the drivers also
>>>>>>>>>> when software nodes are used.
>>>>>>>>>> The implementation tries to mimic the "OF graph" as much as
>>>>>>>>>> possible, but there is no support for the "reg" device
>>>>>>>>>> property. The ports will need to have the index in their
>>>>>>>>>> name which starts with "port" (for example "port0", "port1",
>>>>>>>>>> ...) and endpoints will use the index of the software node
>>>>>>>>>> that is given to them during creation. The port nodes can
>>>>>>>>>> also be grouped under a specially named "ports" subnode,
>>>>>>>>>> just like in DT, if necessary.
>>>>>>>>>> The remote-endpoints are reference properties under the
>>>>>>>>>> endpoint nodes that are named "remote-endpoint". 
>>>>>>>>>> Signed-off-by: Heikki Krogerus <>
>>>>>>>>>> Co-developed-by: Daniel Scally <>
>>>>>>>>>> Signed-off-by: Daniel Scally <>
>>>>>>>>>> ---
>>>>>>>>>> changes in v2:
>>>>>>>>>> 	- added software_node_device_is_available
>>>>>>>>>> 	- altered software_node_get_next_child to get references
>>>>>>>>>> 	- altered software_node_get_next_endpoint to release references
>>>>>>>>>> 	to ports and avoid passing invalid combinations of swnodes to
>>>>>>>>>> 	software_node_get_next_child
>>>>>>>>>> 	- altered swnode_graph_find_next_port to release port rather than
>>>>>>>>>> 	old
>>>>>>>>>>  drivers/base/swnode.c | 129 +++++++++++++++++++++++++++++++++++++++++-
>>>>>>>>>>  1 file changed, 127 insertions(+), 2 deletions(-)
>>>>>>>>>> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
>>>>>>>>>> index 010828fc785b..d69034b807e3 100644
>>>>>>>>>> --- a/drivers/base/swnode.c
>>>>>>>>>> +++ b/drivers/base/swnode.c
>>>>>>>>>> @@ -363,6 +363,11 @@ static void software_node_put(struct fwnode_handle *fwnode)
>>>>>>>>>>  	kobject_put(&swnode->kobj);
>>>>>>>>>>  }
>>>>>>>>>> +static bool software_node_device_is_available(const struct fwnode_handle *fwnode)
>>>>>>>>>> +{
>>>>>>>>>> +	return is_software_node(fwnode);
>>>>>>>>> This basically tells whether the device is there. Are there software node
>>>>>>>>> based devices, i.e. do you need this?
>>>>>>>>> If you do really need this, then I guess this could just return true for
>>>>>>>>> now as if you somehow get here, the node is a software node anyway.
>>>>>>>> I do think its better to include it; I'm targeting using this with
>>>>>>>> ipu3-cio2; the cio2_parse_firmware() call there doesn't pass
>>>>>>>> FWNODE_GRAPH_DEVICE_DISABLED to fwnode_graph_get_endpoint_by_id() so
>>>>>>> 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?).
> The way software nodes work (as in this patch) is not consistent with DT or
> ACPI. For instance, the parent of the port node, returned by
> software_node_graph_get_port_parent() is fwnode->secondary of the device,
> not device's fwnode.
At the moment this isn't the case; at least in the cio2-bridge, I've
been setting the device's _primary_ fwnode to the software_node that the
driver creates. Sorry to confuse things; I thought you were suggesting I
set the software node as fwnode->secondary of the device instead, and
arrange it so that when other bits of code fetch the "device node" via
software_node_get_port_parent() it returns the primary rather than the
software_node secondary, meaning we wouldn't need
software_node_device_is_available() because when something calls
fwnode_device_is_available() it would be using the existing device
firmware node instead of the software node.
> This is not expected by the users of the fwnode property API.
> Also, it leads to drivers only seeing the software nodes while DT and ACPI
> nodes as well as properties would be hidden.
>> 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.
> What problems would there be in returning the primary fwnode?

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15 23:28 Daniel Scally
2020-09-16  9:17 ` Sakari Ailus
2020-09-16 13:22   ` Dan Scally
2020-09-16 14:32     ` Andy Shevchenko
2020-09-16 15:06     ` Kieran Bingham
2020-09-16 16:10       ` Andy Shevchenko
2020-09-18  6:22     ` Sakari Ailus
2020-09-18  6:49       ` Dan Scally
2020-09-18  7:34         ` Sakari Ailus
2020-09-18  7:46           ` Dan Scally
2020-09-18  7:57             ` Sakari Ailus
     [not found]               ` <>
2020-09-18  9:10                 ` Dan Scally
2020-09-18  9:15                 ` Sakari Ailus
2020-09-18  9:22                   ` Dan Scally [this message]
2020-09-18 12:42               ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on

Archives are clonable:
	git clone --mirror linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ \
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone