From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753590AbbCWWFj (ORCPT <rfc822;w@1wt.eu>);
	Mon, 23 Mar 2015 18:05:39 -0400
Received: from mail-wi0-f173.google.com ([209.85.212.173]:38665 "EHLO
	mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753546AbbCWWFg (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 23 Mar 2015 18:05:36 -0400
Message-ID: <55108E2B.7050305@linaro.org>
Date: Mon, 23 Mar 2015 22:05:31 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
MIME-Version: 1.0
To: Mark Brown <broonie@kernel.org>
CC: linux-arm-kernel@lists.infradead.org,
        Maxime Ripard <maxime.ripard@free-electrons.com>,
        Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
        Kumar Gala <galak@codeaurora.org>, linux-api@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        Stephen Boyd <sboyd@codeaurora.org>, Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v2 1/7] eeprom: Add a simple EEPROM framework for eeprom
 providers
References: <1426240157-2383-1-git-send-email-srinivas.kandagatla@linaro.org> <1426240214-2434-1-git-send-email-srinivas.kandagatla@linaro.org> <20150323210918.GS14954@sirena.org.uk>
In-Reply-To: <20150323210918.GS14954@sirena.org.uk>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org



On 23/03/15 21:09, Mark Brown wrote:
> On Fri, Mar 13, 2015 at 09:50:14AM +0000, Srinivas Kandagatla wrote:
>
> A couple of *very* minor points below, otherwise this looks OK to me.
>
Thankyou for the review.

>> +struct eeprom_device *eeprom_register(struct eeprom_config *config)
>> +{
>> +	struct eeprom_device *eeprom;
>> +	int rval;
>> +
>> +	if (!config->regmap || !config->size) {
>> +		dev_err(config->dev, "Regmap not found\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>
> You have a struct device in the config and the regmap API has
> dev_get_regmap() which for most devices that don't have multiple regmaps
> will give the right regmap.  It would be nice to support this as a
> convenience for users.
Yes, sure that makes sense, I will give it a try.

>
>> +	eeprom = kzalloc(sizeof(*eeprom), GFP_KERNEL);
>> +	if (!eeprom)
>> +		return ERR_PTR(-ENOMEM);
>
> ...
>
>> +	rval = device_add(&eeprom->dev);
>> +	if (rval)
>> +		return ERR_PTR(rval);
>
> Don't you need a kfree() if device_add() fails?
I will fix it in next version.

--srini
>