linux-kernel - RE: [RESEND PATCH v2 1/2] device property: Add function to search for named child of device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2E89032DDAA8B9408CB92943514A0337D46421B6@SW-EX-MBX01.diasemi.com>
Date:	Fri, 10 Jun 2016 09:58:39 +0000
From:	"Opensource [Adam Thomson]" <Adam.Thomson.Opensource@...semi.com>
To:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	"Opensource [Adam Thomson]" <Adam.Thomson.Opensource@...semi.com>,
	Jaroslav Kysela <perex@...ex.cz>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	"Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
CC:	Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Takashi Iwai <tiwai@...e.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Robert Moore <robert.moore@...el.com>,
	Lv Zheng <lv.zheng@...el.com>, Len Brown <lenb@...nel.org>,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"devel@...ica.org" <devel@...ica.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Support Opensource <Support.Opensource@...semi.com>,
	Sathyanarayana Nujella <sathyanarayana.nujella@...el.com>,
	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Hanjun Guo" <hanjun.guo@...aro.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: RE: [RESEND PATCH v2 1/2] device property: Add function to search
 for named child of device

On 10 June 2016 00:11, Rafael J. Wysocki wrote:

> For some reason that didn't make it into the linux-acpi list, or at
> least I can't see it there.

That's strange. I'm not a subscriber to that mailing list, but I assume that
shouldn't matter here? Strangely though the only mailing list these seem to have
made it to is the ALSA one. :( Will see if I can find out why as I've not
seen this problem before.

> > + * device_get_named_child_node - Return first matching named child node
> handle
> > + * @dev: Device to find the named child node for.
> > + * @childname: String to match child node name against.
> > + */
> > +struct fwnode_handle *device_get_named_child_node(struct device *dev,
> > +						  const char *childname)
> > +{
> > +	struct fwnode_handle *child;
> > +
> > +	/*
> > +	 * Find first matching named child node of this device.
> > +	 * For ACPI this will be a data only sub-node.
> > +	 */
> > +	device_for_each_child_node(dev, child) {
> > +		if (is_of_node(child)) {
> > +			if (!strcasecmp(to_of_node(child)->name, childname))
>
> Why do you use strcasecmp() here?

DT node names are case insensitive. The of.h header does provide a helper macro
which is equivalent to this, but that macro is part of the '#ifdef CONFIG_OF'
block. If I were to use it then it would cause non-DT builds to fail. I opted
for strcasecmp() directly as I didn't think for just this one scenario it made
sense to reorganise the of.h header with regards to the helper macros. Of course
if there are other opinions on this then am happy to listen.

> > +static inline bool acpi_data_node_match(struct fwnode_handle *fwnode,
> > +					const char *name)
> > +{
> > +	return is_acpi_data_node(fwnode) ?
> > +		(!strcasecmp(to_acpi_data_node(fwnode)->name, name)) : false;
> > +}
>
> Is there any particular reason to introduce this function instead of
> doing the test in device_get_named_child_node() directly?

Again this is a build related design option (I mention it in the patch
description). In a non-DT build there is no access to the acpi_data_node struct
(returned by to_acpi_data_node() call) so if we call this in that scenario then
the build will fail. I could have added some #ifdefs to the
device_get_named_child_node() function directly, but that would have been a bit
messy I think. To me it made more sense to have this helper function which can
be called regardless of build type, and for non-ACPI builds its definition
always returns false.