linux-kernel - Re: [PATCH] media: i2c: thp7312: Don't require node availability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7286121f-e3f4-48bb-9fea-1d14e695f203@gmail.com>
Date: Fri, 21 Mar 2025 10:29:46 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Paul Elder <paul.elder@...asonboard.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: i2c: thp7312: Don't require node availability

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