linux-kernel - Re: [PATCH 1/2] mfd: da9052: add device-tree support for i2c driver

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: <20120412155341.GB3195@opensource.wolfsonmicro.com>
Date:	Thu, 12 Apr 2012 16:53:41 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	"Ying-Chun Liu (PaulLiu)" <paul.liu@...aro.org>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linaro-dev@...ts.linaro.org, patches@...aro.org,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Shawn Guo <shawn.guo@...aro.org>,
	Ashish Jangam <ashish.jangam@...tcummins.com>
Subject: Re: [PATCH 1/2] mfd: da9052: add device-tree support for i2c driver

On Thu, Apr 12, 2012 at 11:39:41PM +0800, Ying-Chun Liu (PaulLiu) wrote:

> +- compatible : Should be "dialog,da9052", "dialog,da9053-aa",
> +			 "dialog,da9053-ab", or "dialog,da9053-bb"

This is generally the stock ticker symbol so DLG for Dialog.

> +Sub-nodes:
> +- regulators : Contain the regulator nodes.  The DA9052 regulators are
> +  sorted as following:

This seems poor from a usability point of view, the user shouldn't have
to care about the order in which they list things in their device tree
and they should be able to only list the regulators they use.

> +    buck_core : regulator BUCK_CORE (id: 0 )

Having to care about numeric IDs also seems bad for usability.

> +#ifdef CONFIG_OF
> +	if (!id) {
> +		int i;
> +		struct device_node *np = client->dev.of_node;
> +
> +		for (i = 0;
> +		     i < sizeof(dialog_dt_ids)/sizeof(dialog_dt_ids[0]);
> +		     i++)
> +			if (of_device_is_compatible(np,
> +						dialog_dt_ids[i].compatible))
> +				id = &da9052_i2c_id[i];
> +	}
> +#endif

Ick, this doesn't look terribly nice - are you sure this is idiomatic?
The bound for the iteration also looks a lot like ARRAY_SIZE().

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)