All of lore.kernel.org
 help / color / mirror / Atom feed
From: sshtylyov@mvista.com (Sergei Shtylyov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 1/4] mfd: add syscon driver based on regmap
Date: Tue, 04 Sep 2012 16:16:59 +0400	[thread overview]
Message-ID: <5045F13B.8060409@mvista.com> (raw)
In-Reply-To: <20120904113545.GD4657@jack.whiskey>

Hello.

On 04-09-2012 15:35, Andi Shyti wrote:

>> +static int __devinit syscon_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;

> Do we really need this variable? Anyway you are using it only
> once in the dev_info.

    But Dong definitely could use it more than once, so it seems useful.

>> +	struct device_node *np = dev->of_node;
>> +	struct syscon *syscon;
>> +	struct resource res;
>> +	int ret;
>> +
>> +	if (!np)
>> +		return -ENOENT;
>> +
>> +	syscon = devm_kzalloc(&pdev->dev, sizeof(struct syscon),
>> +			    GFP_KERNEL);
>> +	if (!syscon)
>> +		return -ENOMEM;
>> +
>> +	syscon->base = of_iomap(np, 0);
>> +	if (!syscon->base)
>> +		return -EADDRNOTAVAIL;
>> +
>> +	ret = of_address_to_resource(np, 0, &res);
>> +	if (ret)
>> +		return ret;
>> +
>> +	syscon_regmap_config.max_register = res.end - res.start - 3;
>> +	syscon->regmap = devm_regmap_init_mmio(&pdev->dev, syscon->base,
>> +					&syscon_regmap_config);
>> +	if (IS_ERR(syscon->regmap)) {
>> +		dev_err(&pdev->dev, "regmap init failed\n");
>> +		return PTR_ERR(syscon->regmap);
>> +	}
>> +
>> +	syscon->dev = &pdev->dev;
>> +	platform_set_drvdata(pdev, syscon);
>> +
>> +	dev_info(dev, "syscon regmap start 0x%x end 0x%x registered\n",
>> +		res.start, res.end);
>> +
>> +	return 0;

> in case of error you are not freeing syscon.

    It's allocated using devm_kzalloc(), so is freed automaticelly upon probe 
error.

> Moreover, in my opinion, some dev_err more should not heart

    Hurt, you mean?

> Andi

WBR, Sergei

WARNING: multiple messages have this Message-ID (diff)
From: Sergei Shtylyov <sshtylyov@mvista.com>
To: Andi Shyti <andi@etezian.org>
Cc: Dong Aisheng <b29396@freescale.com>,
	linus.walleij@stericsson.com, swarren@wwwdotorg.org,
	devicetree-discuss@lists.ozlabs.org, s.hauer@pengutronix.de,
	broonie@opensource.wolfsonmicro.com,
	linux-kernel@vger.kernel.org, rob.herring@calxeda.com,
	grant.likely@secretlab.ca, richard.zhao@freescale.com,
	kernel@pengutronix.de, shawn.guo@linaro.org, lrg@ti.com,
	linux-arm-kernel@lists.infradead.org, sameo@linux.intel.com
Subject: Re: [PATCH v5 1/4] mfd: add syscon driver based on regmap
Date: Tue, 04 Sep 2012 16:16:59 +0400	[thread overview]
Message-ID: <5045F13B.8060409@mvista.com> (raw)
In-Reply-To: <20120904113545.GD4657@jack.whiskey>

Hello.

On 04-09-2012 15:35, Andi Shyti wrote:

>> +static int __devinit syscon_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;

> Do we really need this variable? Anyway you are using it only
> once in the dev_info.

    But Dong definitely could use it more than once, so it seems useful.

>> +	struct device_node *np = dev->of_node;
>> +	struct syscon *syscon;
>> +	struct resource res;
>> +	int ret;
>> +
>> +	if (!np)
>> +		return -ENOENT;
>> +
>> +	syscon = devm_kzalloc(&pdev->dev, sizeof(struct syscon),
>> +			    GFP_KERNEL);
>> +	if (!syscon)
>> +		return -ENOMEM;
>> +
>> +	syscon->base = of_iomap(np, 0);
>> +	if (!syscon->base)
>> +		return -EADDRNOTAVAIL;
>> +
>> +	ret = of_address_to_resource(np, 0, &res);
>> +	if (ret)
>> +		return ret;
>> +
>> +	syscon_regmap_config.max_register = res.end - res.start - 3;
>> +	syscon->regmap = devm_regmap_init_mmio(&pdev->dev, syscon->base,
>> +					&syscon_regmap_config);
>> +	if (IS_ERR(syscon->regmap)) {
>> +		dev_err(&pdev->dev, "regmap init failed\n");
>> +		return PTR_ERR(syscon->regmap);
>> +	}
>> +
>> +	syscon->dev = &pdev->dev;
>> +	platform_set_drvdata(pdev, syscon);
>> +
>> +	dev_info(dev, "syscon regmap start 0x%x end 0x%x registered\n",
>> +		res.start, res.end);
>> +
>> +	return 0;

> in case of error you are not freeing syscon.

    It's allocated using devm_kzalloc(), so is freed automaticelly upon probe 
error.

> Moreover, in my opinion, some dev_err more should not heart

    Hurt, you mean?

> Andi

WBR, Sergei

  parent reply	other threads:[~2012-09-04 12:16 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04  3:20 [PATCH v5 0/4] add syscon driver based on regmap for general registers access Dong Aisheng
2012-09-04  3:20 ` Dong Aisheng
2012-09-04  3:20 ` Dong Aisheng
2012-09-04  3:20 ` [PATCH v5 1/4] mfd: add syscon driver based on regmap Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04 11:35   ` Andi Shyti
2012-09-04 11:35     ` Andi Shyti
2012-09-04 11:46     ` Dong Aisheng
2012-09-04 11:46       ` Dong Aisheng
2012-09-04 15:44       ` Andi Shyti
2012-09-04 15:44         ` Andi Shyti
2012-09-04 12:16     ` Sergei Shtylyov [this message]
2012-09-04 12:16       ` Sergei Shtylyov
2012-09-04  3:20 ` [PATCH v5 2/4] ARM: imx6q: add iomuxc gpr support into syscon Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04  3:20 ` [PATCH v5 3/4] regulator: anatop-regulator: convert to use syscon to access anatop register Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04 11:34   ` Shawn Guo
2012-09-04 11:34     ` Shawn Guo
2012-09-04 11:27     ` Dong Aisheng
2012-09-04 11:27       ` Dong Aisheng
2012-09-04  3:20 ` [PATCH v5 4/4] mfd: anatop-mfd: remove anatop driver Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng
2012-09-04  3:20   ` Dong Aisheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5045F13B.8060409@mvista.com \
    --to=sshtylyov@mvista.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.