Hi Sakari, On Thu, Dec 19, 2024 at 2:37 PM Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > Hi Rafael, > > On Thu, Dec 19, 2024 at 12:25:22PM +0100, Rafael J. Wysocki wrote: > > Hi Sakari, > > > > On Thu, Dec 19, 2024 at 11:32 AM Sakari Ailus > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > Hi Rafael, > > > > > > Thanks for the review. > > > > > > On Wed, Dec 18, 2024 at 12:07:52PM +0100, Rafael J. Wysocki wrote: > > > > On Wed, Dec 18, 2024 at 10:16 AM Sakari Ailus > > > > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > > > > > > > > > Years after fwnode_device_is_available() was introduced, new functions > > > > > making use of the function on data nodes have been added, such as > > > > > fwnode_for_each_available_child_node(), it becomes apparent that returning > > > > > "false" for all child nodes on ACPI wasn't a workable option. > > > > > > > > Can you describe the problem in a bit more detail? > > > > > > How about: > > > > This is better IMV. At least it actually matches my understanding of the issue. > > > > > Years after fwnode_device_is_available() was introduced, new functions > > > making use of the data node availability information have been added, such > > > as fwnode_for_each_available_child_node(). > > > > I would rephrase the sentence above as follows: > > > > "New functions making use of the data node availability information, > > like fwnode_for_each_available_child_node(), have been added years > > after fwnode_device_is_available() was introduced." > > Looks good to me. > > > > > > To enumerate the data nodes in > > > various ways specific to those functions, the node availability test needs > > > to pass. > > > > > > On ACPI, there is no explicit information on this > > > > I guess by "this" you mean the availability of the data (non-device) nodes? > > Yes. I'll use: > > On ACPI, there is no explicit data node availbility information in the > first place and the original fwnode_device_is_available() implementation > simply returns false. > > > > > > in the first place and > > > the original fwnode_device_is_available() implementation simply returnes > > > > returns > > > > > false. This leads to the new functions enumerating only available nodes to > > > never return any nodes on ACPI. > > > > I'd say "This causes new functions that only enumerate available nodes > > to never return any nodes on ACPI for leaf devices that have child > > data nodes." > > Ack. > > > > > > On DT side most access functions, even > > > those without "_available" part, did only operate on available nodes. > > > > On the DT side, :device_is_available points to > > of_device_is_available() that calls __of_device_is_available() which > > returns "true" for all nodes without the "status" property, so if I'm > > not mistaken, on the DT side fwnode_device_is_available() will return > > "true" for any nodes without the "status" property. > > > > I would say something like this: > > > > "However, on the DT side, fwnode_device_is_available() returns "true" > > for all nodes without the "status" property which are analogous to the > > ACPI data nodes, so there is a difference in behavior between DT and > > ACPI in that respect." > > That's also true. I'll use that, dropping the quotes from "true". > > The commit message would be, in its entirety: > > New functions making use of the data node availability information, like > fwnode_for_each_available_child_node(), have been added years after > fwnode_device_is_available() was introduced. To enumerate the data nodes > in various ways specific to those functions, the node availability test > needs to pass. > > On ACPI, there is no explicit data node availbility information in the > first place and the original fwnode_device_is_available() implementation > simply returns false. This causes new functions that only enumerate > available nodes to never return any nodes on ACPI for leaf devices that > have child data nodes. > > However, on the DT side, fwnode_device_is_available() returns true > for all nodes without the "status" property which are analogous to the > ACPI data nodes, so there is a difference in behavior between DT and > ACPI in that respect. > > Thus from now on, return true from fwnode_device_is_available() on all ACPI > data nodes. Looks good to me now, thanks!