linux-kernel - Re: [PATCH v3] leds: provide helper to register "leds-gpio" devices

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]

Date:	Tue, 10 May 2011 08:45:04 +0200
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Richard Purdie <rpurdie@...ys.net>,
	Fabio Estevam <fabio.estevam@...escale.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	linux-kernel@...r.kernel.org, kernel@...gutronix.de,
	H Hartley Sweeten <hartleys@...ionengravers.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3] leds: provide helper to register "leds-gpio" devices

On Mon, May 09, 2011 at 11:17:19PM +0100, Russell King - ARM Linux wrote:
> On Mon, May 09, 2011 at 03:02:54PM -0700, Andrew Morton wrote:
> > On Mon, 11 Apr 2011 22:35:57 +0200
> > Uwe Kleine-K__nig  <u.kleine-koenig@...gutronix.de> wrote:
> > > +#if defined(CONFIG_LED_REGISTER_GPIO)
> > > +struct platform_device *__init gpio_led_register_device(
> > > +		int id, const struct gpio_led_platform_data *pdata)
> > > +{
> > > +	struct platform_device *ret;
> > > +	struct gpio_led_platform_data _pdata = *pdata;
> > > +
> > > +	_pdata.leds = kmemdup(pdata->leds,
> > > +			pdata->num_leds * sizeof(*pdata->leds), GFP_KERNEL);
> > > +	if (!_pdata.leds)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	ret = platform_device_register_resndata(NULL, "leds-gpio", id,
> > > +			NULL, 0, &_pdata, sizeof(_pdata));
> > > +	if (IS_ERR(ret))
> > > +		kfree(_pdata.leds);
> > > +
> > > +	return ret;
> > > +}
> > > +#endif
> ...
> > The comment doesn't document return values.
> 
> Two further comments.
> 
> 1. Why is this .c file always built, but _all_ the containing code is
> wrapped up in an ifdef?  It seems a waste of resources to compile a .c
> file with all code #ifdef'd out.
This was done because I thought that the .c file could contain other
registration routines later. Richard requested to use

	obj-$(CONFIG_LED_REGISTER_GPIO) += ...

then the #ifdef can go away, too. (@Andrew: Richard's ack was only for a
patch that used that. You took the patch anyhow and added his ack.)

> 2. What is the point of returning the platform device structure?  You've
> already registered it, so you must _not_ modify any data in that structure
> which may be used by the driver.  The only thing which you can safely do
> with it is unregister it.
pdev->device can be used as parent for another device. (OK, probably not
an led device. The origin of this function is the imx device
registration stuff, and all these functions return a platform device).
The other case where I needed to have the device created was to fill a
struct regulator_consumer_supply, but nowadays this is done using just
the name.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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/