Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Dan Scally <djrscally@gmail.com>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Sakari Ailus <sakari.ailus@iki.fi>
Cc: gregkh@linuxfoundation.org, rafael@kernel.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	andriy.shevchenko@linux.intel.com,
	kieran.bingham@ideasonboard.com, jorhand@linux.microsoft.com,
	kitakar@gmail.com
Subject: Re: [PATCH v2] software_node: Add support for fwnode_graph*() family of functions
Date: Fri, 18 Sep 2020 10:10:24 +0100
Message-ID: <a4040e77-dc07-51d8-1970-76900d937722@gmail.com> (raw)
In-Reply-To: <20200918085709.GA1630537@kuha.fi.intel.com>


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


  parent 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]               ` <20200918085709.GA1630537@kuha.fi.intel.com>
2020-09-18  9:10                 ` Dan Scally [this message]
2020-09-18  9:15                 ` Sakari Ailus
2020-09-18  9:22                   ` Dan Scally
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=a4040e77-dc07-51d8-1970-76900d937722@gmail.com \
    --to=djrscally@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=jorhand@linux.microsoft.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=kitakar@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 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/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git