linux-kernel - Re: [PATCH V3 3/6] pinctrl: core device tree mapping table parsing support

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: <4F6BF846.6010109@wwwdotorg.org>
Date:	Thu, 22 Mar 2012 22:12:54 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Dong Aisheng <aisheng.dong@...escale.com>
CC:	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"linus.walleij@...ricsson.com" <linus.walleij@...ricsson.com>,
	Dong Aisheng-B29396 <B29396@...escale.com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"dongas86@...il.com" <dongas86@...il.com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"thomas.abraham@...aro.org" <thomas.abraham@...aro.org>,
	"tony@...mide.com" <tony@...mide.com>,
	"sjg@...omium.org" <sjg@...omium.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH V3 3/6] pinctrl: core device tree mapping table parsing
 support

On 03/22/2012 09:55 PM, Dong Aisheng wrote:
> On Fri, Mar 23, 2012 at 02:27:19AM +0800, Stephen Warren wrote:
>> During pinctrl_get(), if the client device has a device tree node, look
>> for the common pinctrl properties there. If found, parse the referenced
>> device tree nodes, with the help of the pinctrl drivers, and generate
>> mapping table entries from them.
>>
>> During pinctrl_put(), free any results of device tree parsing.
...
>> +int pinctrl_dt_to_map(struct pinctrl *p)
...
>> +		propname = kasprintf(GFP_KERNEL, "pinctrl-%d", state);
>> +		prop = of_find_property(np, propname, &size);
>> +		kfree(propname);
>> +		if (!prop)
>> +			break;
>> +		list = prop->value;
>> +		size /= sizeof(*list);
>> +
>> +		/* Determine whether pinctrl-names property names the state */
>> +		ret = of_property_read_string_index(np, "pinctrl-names",
>> +						    state, &statename);
>> +		/*
>> +		 * If not, statename is just the integer state ID. But rather
>> +		 * than dynamically allocate it and have to free it later,
>> +		 * just point part way into the property name for the string.
>> +		 */
>> +		if (ret < 0) {
>> +			/* strlen("pinctrl-") == 8 */
>> +			if (strlen(prop->name) < 8) {
> You thought this checking is still needed?
> prop->name should be exactly what you search above ("pinctrl-%d"), right?

Oops. I accidentally blew this away when I dropped the change the change
to replace of_find_node_by_phandle() with of_parse_phandle(). I guess
I'll post V4:-(

>> +				dev_err(p->dev, "prop %s inconsistent length\n",
>> +					prop->name);
>> +				ret = -EINVAL;
>> +				goto err;
>> +			}
>> +			statename = prop->name + 8;
>> +		}
>> +
>> +		/* For every referenced pin configuration node in it */
>> +		for (config = 0; config < size; config++) {
>> +			phandle = be32_to_cpup(list++);
>> +
>> +			/* Look up the pin configuration node */
>> +			np_config = of_find_node_by_phandle(phandle);
>> +			if (!np_config) {
>> +				dev_err(p->dev,
>> +					"prop %s index %i invalid phandle\n",
>> +					prop->name, config);
>> +				ret = -EINVAL;
>> +				goto err;
>> +			}
>> +
>> +			/* Parse the node */
>> +			ret = dt_to_map_one_config(p, statename, np_config);
>> +			of_node_put(np_config);
>> +			if (ret < 0)
>> +				goto err;
>> +		}
>> +
>> +		/* No entries in DT? Generate a dummy state table entry */
>> +		if (!size) {
>> +			ret = dt_remember_dummy_state(p, statename);
>
> It seems we're still keeping the intermediate pinctrl_dt_map, although
> not know the result you found,
> but i'm ok we just do it currently.

Sorry, I'm not sure what the issue is here? We need some struct to
remember all the mapping table chunks that are parsed from DT so that
when pinctrl_put() is executed, they can be freed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/