[PATCH v2 1/2] ARM: mxc: Introduce imx_add_gpio_leds

[Uwe Kleine-König]

On Tue, Apr 05, 2011 at 08:47:54AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 05, 2011 at 09:43:15AM +0200, Uwe Kleine-König wrote:
> > On Tue, Apr 05, 2011 at 08:40:17AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Apr 05, 2011 at 09:30:30AM +0200, Uwe Kleine-König wrote:
> > > > I like my approach better, without using mxc specific functions it would
> > > > look as follows:
> > > > 
> > > > struct platform_device *__init gpio_led_register_device(
> > > > 		const gpio_led_platform_data *pdata)
> > > > {
> > > > 	struct platform_device *ret = ERR_PTR(-ENOMEM);
> > > > 	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", -1,
> > > > 		NULL, 0, _pdata, sizeof(_pdata));
> > > > 
> > > > 	if (IS_ERR(ret))
> > > > 		kfree(_pdata->leds);
> > > > 
> > > > 	return ret;
> > > > }
> > > 
> > > But this is inconsistent.  The gpio_led_platform_data isn't copied,
> > > but a sub-structure is - which makes this a candidate for people adding
> > > __initdata to the wrong thing.  Also marking the LEDs array as __initdata
> > > but leaving the gpio_led_platform_data without __initdata will lead to
> > > warnings.
> > platform_device_register_resndata does the copying.
> 
> Ok, but you're still modifying the leds pointer in the platform data,
> which you've also marked as const.  That's an error.
> 
> If you want gpio_led_platform_data to be const, you also have to make
> a copy of it here, even if you have to free it after
> platform_device_register_resndata().
This was intended by

	struct gpio_led_platform_data *_pdata = *pdata;

I'm just fighting with the compiler at this line because it doesn't like
_pdata being initialized by a const pointer.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |