All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matti Vaittinen <mazziesaccount@gmail.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	Paul Elder <paul.elder@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: i2c: thp7312: Don't require node availability
Date: Fri, 21 Mar 2025 10:29:46 +0200	[thread overview]
Message-ID: <7286121f-e3f4-48bb-9fea-1d14e695f203@gmail.com> (raw)
In-Reply-To: <ffa5ae6d-a925-41da-9826-4bb376ca0fbe@gmail.com>

On 21/03/2025 08:35, Matti Vaittinen wrote:
> Hi dee Ho Laurent,
> 
> On 20/03/2025 16:26, Laurent Pinchart wrote:
>> Hi Matti,
>>
>> On Thu, Mar 20, 2025 at 10:35:35AM +0200, Matti Vaittinen wrote:
>>> It appears that the concept of available firmware nodes is not really
>>> applicable to the scenarios where a specific name is required from a
>>> node.
>>>
>>> As explained[1] by Sakari:
>>> "OF only enumerates available nodes via the fwnode API, software nodes
>>> don't have the concept but on ACPI I guess you could have a difference
>>> in nodes where you have device sub-nodes that aren't available. Still,
>>> these ACPI device nodes don't have meaningful names in this context
>>> (they're 4-character object names) so you wouldn't use them like this
>>> anyway."
>>>
>>> Use the fwnode_for_each_child_node() instead of the
>>> fwnode_for_each_available_child_node() In order to make it clearly
>>> visible that the 'availability' of the nodes does not need to be
>>> considered here.
>>
>> Why not ? Node availability is a concept that exists in DT, and this
>> driver has only been tested on DT-based systems.
> 
> I admit I need to study this then. I just took what Sakari said for 
> granted, without taking any further look at this.
> 

I took a peek in the 'availability' concept and found:
https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/of/base.c#L468

So, the availability indeed has a well defined meaning in the DT, 
boiling down to the value of the 'status' -property.

Then I took further look at the fwnode_for_each_child_node(), and if I'm 
not mistaken, it calls:

fwnode_for_each_child_node()
	fwnode_get_next_child_node()
		fwnode_call_ptr_op(fwnode, get_next_child_node, child);
			 of_fwnode_get_next_child_node() (dt-based)
				of_get_next_available_child() (dt-based)

where the of_get_next_available_child() skips all the disabled nodes.

So, in that regard I agree with Sakari. On DT based systems, the

fwnode_for_each_child_node() seems to equal the
fwnode_for_each_available_child_node().

And, since the 'thp7312' driver requires specific names for the nodes, 
it indeed seems to me that only the device-tree use-case needs to be 
considered.

After all this I'd say this patch is still valid - but the commit 
message is misleading. If no one objects I'll rewrite the commit msg and 
respin :)

Yours,
	-- Matti

      reply	other threads:[~2025-03-21  8:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-20  8:35 [PATCH] media: i2c: thp7312: Don't require node availability Matti Vaittinen
2025-03-20 14:26 ` Laurent Pinchart
2025-03-21  6:35   ` Matti Vaittinen
2025-03-21  8:29     ` Matti Vaittinen [this message]

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=7286121f-e3f4-48bb-9fea-1d14e695f203@gmail.com \
    --to=mazziesaccount@gmail.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=matti.vaittinen@fi.rohmeurope.com \
    --cc=mchehab@kernel.org \
    --cc=paul.elder@ideasonboard.com \
    --cc=sakari.ailus@linux.intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.