Date: Thu, 11 Dec 2008 13:33:51 -0800 (PST)
From: Trent Piepho <tpiepho@freescale.com>
To: Anton Vorontsov <avorontsov@ru.mvista.com>
cc: linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
       Richard Purdie <rpurdie@rpsys.net>, devicetree-discuss@ozlabs.org,
       Grant Likely <grant.likely@secretlab.ca>,
       Sean MacLennan <smaclennan@pikatech.com>
Subject: Re: [PATCH 1/4] leds: Support OpenFirmware led bindings
In-Reply-To: <20081210163217.GA20150@oksana.dev.rtsoft.ru>
Message-ID: <Pine.LNX.4.64.0812111326480.28806@t2.domain.actdsltmp>
References: <1228923703-16370-1-git-send-email-tpiepho@freescale.com>
 <20081210163217.GA20150@oksana.dev.rtsoft.ru>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 963
Lines: 29

On Wed, 10 Dec 2008, Anton Vorontsov wrote:
>> +	gpio_direction_output(led_dat->gpio, led_dat->active_low);
>
> This can fail (yeah, the original code didn't check return value
> either).

I've added a check.

>> +		unsigned int flags;
>
> I think it would be better to use `enum of_gpio_flags' type here,
> just for clarity.

You're right, I forgot to change this after the of_get_gpio patch was
changed.

>> +		ret = create_gpio_led(&led, &pdata->led_data[pdata->num_leds++],
>> +				      &ofdev->dev, NULL);
>> +		if (ret < 0)
>
> of_node_put(np); is missing here.

of_node_put(child) you mean.  It's easy to forget this when one exits one
of these iterators early, since there's no explicit get or put otherwise.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/