From: broonie@opensource.wolfsonmicro.com (Mark Brown)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2] make mc13783 regulator code generic
Date: Wed, 1 Dec 2010 11:25:16 +0000 [thread overview]
Message-ID: <20101201112515.GC24176@rakim.wolfsonmicro.main> (raw)
In-Reply-To: <AANLkTi=E8etAOPTCBw8ccTZ9LMU-Yf=wELn+VQkg2eom@mail.gmail.com>
On Wed, Dec 01, 2010 at 03:15:55PM +0800, Yong Shen wrote:
> move some common functions and micros of mc13783 regulaor driver to
> a seperate file, which makes it possible for mc13892 to share code.
You've done way more than this in the patch - you've also renamed a lot
of things and done other restructurings. I'd suggest splitting out the
big mechanical changes for easier review. For example, a patch moving
code from one place to another, another patch renaming things and so on.
> +config REGULATOR_MC13XXX_CORE
> + tristate "Support regulators on Freescale MC13xxx PMIC"
> +
This doesn't need to be user visible - the user only cares about the
individual regulator drivers.
> /* Power Gate enable value is 0 */
> - if (id == MC13783_REGU_PWGT1SPI ||
> - id == MC13783_REGU_PWGT2SPI)
> + if (id == MC13783_REG_PWGT1SPI ||
> + id == MC13783_REG_PWGT2SPI)
> en_val = 0;
I can't tell what the actual change is here?
> +int mc13xxx_sw_regulator(struct regulator_dev *rdev)
> +{
> + return 0;
> +}
> +
Eh?
> +MODULE_ALIAS("platform:mc13xxx-regulator-core");
Is there really going to be a platform device for this?
> +++ b/include/linux/regulator/mc13xxx.h
Pretty much everything in this file is internal to the driver and
shouldn't be in include/linux.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Yong Shen <yong.shen@linaro.org>
Cc: "Linaro Dev" <linaro-dev@lists.linaro.org>,
"List Linux Arm Kernel" <linux-arm-kernel@lists.infradead.org>,
"Sascha Hauer" <s.hauer@pengutronix.de>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
lrg@slimlogic.co.uk, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2] make mc13783 regulator code generic
Date: Wed, 1 Dec 2010 11:25:16 +0000 [thread overview]
Message-ID: <20101201112515.GC24176@rakim.wolfsonmicro.main> (raw)
In-Reply-To: <AANLkTi=E8etAOPTCBw8ccTZ9LMU-Yf=wELn+VQkg2eom@mail.gmail.com>
On Wed, Dec 01, 2010 at 03:15:55PM +0800, Yong Shen wrote:
> move some common functions and micros of mc13783 regulaor driver to
> a seperate file, which makes it possible for mc13892 to share code.
You've done way more than this in the patch - you've also renamed a lot
of things and done other restructurings. I'd suggest splitting out the
big mechanical changes for easier review. For example, a patch moving
code from one place to another, another patch renaming things and so on.
> +config REGULATOR_MC13XXX_CORE
> + tristate "Support regulators on Freescale MC13xxx PMIC"
> +
This doesn't need to be user visible - the user only cares about the
individual regulator drivers.
> /* Power Gate enable value is 0 */
> - if (id == MC13783_REGU_PWGT1SPI ||
> - id == MC13783_REGU_PWGT2SPI)
> + if (id == MC13783_REG_PWGT1SPI ||
> + id == MC13783_REG_PWGT2SPI)
> en_val = 0;
I can't tell what the actual change is here?
> +int mc13xxx_sw_regulator(struct regulator_dev *rdev)
> +{
> + return 0;
> +}
> +
Eh?
> +MODULE_ALIAS("platform:mc13xxx-regulator-core");
Is there really going to be a platform device for this?
> +++ b/include/linux/regulator/mc13xxx.h
Pretty much everything in this file is internal to the driver and
shouldn't be in include/linux.
next prev parent reply other threads:[~2010-12-01 11:25 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-01 7:15 [PATCHv2] make mc13783 regulator code generic Yong Shen
2010-12-01 7:15 ` Yong Shen
2010-12-01 7:50 ` Sascha Hauer
2010-12-01 7:50 ` Sascha Hauer
2010-12-02 2:36 ` Yong Shen
2010-12-02 2:36 ` Yong Shen
2010-12-01 11:25 ` Mark Brown [this message]
2010-12-01 11:25 ` Mark Brown
2010-12-02 2:41 ` Yong Shen
2010-12-02 2:41 ` Yong Shen
2010-12-02 10:45 ` [PATCH 2/2] " Yong Shen
2010-12-02 10:45 ` Yong Shen
2010-12-02 10:57 ` Uwe Kleine-König
2010-12-02 10:57 ` Uwe Kleine-König
2010-12-02 11:15 ` Yong Shen
2010-12-02 11:15 ` Yong Shen
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=20101201112515.GC24176@rakim.wolfsonmicro.main \
--to=broonie@opensource.wolfsonmicro.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.