All of lore.kernel.org
 help / color / mirror / Atom feed
From: lrg@slimlogic.co.uk (Liam Girdwood)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] AB3100 regulator support v4
Date: Thu, 03 Sep 2009 15:15:48 +0100	[thread overview]
Message-ID: <1251987348.6735.49.camel@odin> (raw)
In-Reply-To: <1251982396-23790-1-git-send-email-linus.walleij@stericsson.com>

On Thu, 2009-09-03 at 14:53 +0200, Linus Walleij wrote:
> This adds support for the regulators found in the AB3100
> Mixed-Signal IC.
> 
> It further also defines platform data for the ST-Ericsson
> U300 platform and extends the AB3100 MFD driver so that
> platform/board data with regulation constraints and an init
> function can be passed down all the way from the board to
> the regulators.
> 

<snip>

> +static int ab3100_disable_regulator(struct regulator_dev *reg)
> +{
> +	struct ab3100_regulator *abreg = reg->reg_data;
> +	int err;
> +	u8 regval;
> +
> +	/*
> +	 * LDO D is a special regulator. When it is disabled, the entire
> +	 * system is shut down. So this is handled specially.
> +	 */
> +	if (abreg->regreg == AB3100_LDO_D) {
> +		int i;
> +
> +		dev_info(&reg->dev, "disabling LDO D - shut down system\n");
> +		/*
> +		 * Set regulators to default values, ignore any errors,
> +		 * we're going DOWN
> +		 */
> +		for (i = 0; i < ARRAY_SIZE(ab3100_reg_init_order); i++) {
> +			(void) ab3100_set_register_interruptible(abreg->ab3100,
> +					ab3100_reg_init_order[i],
> +					abreg->plfdata->reg_initvals[i]);
> +		}
> +
> +		/* Setting LDO D to 0x00 cuts the power to the SoC */
> +		return ab3100_set_register_interruptible(abreg->ab3100,
> +							 AB3100_LDO_D, 0x00U);
> +
> +	}
> +
> +	/*
> +	 * All other regulators are handled here
> +	 */
> +	err = ab3100_get_register_interruptible(abreg->ab3100, abreg->regreg,
> +						&regval);
> +	if (err) {
> +		dev_err(&reg->dev, "unable to get register 0x%x\n",
> +			abreg->regreg);
> +		return err;
> +	}
> +	regval &= ~AB3100_REG_ON_MASK;
> +	return ab3100_set_register_interruptible(abreg->ab3100, abreg->regreg,
> +						 regval);
> +}

Just wondering if you had looked at the regulator supplier field in
struct regulator_init_data. It's intention was to allow LDO D to be
switched OFF only when all it's consumer regulators were OFF. Likewise
it would also switch this regulator ON if any of it's children were
switched ON.

I assume for this PMIC it would not be useful as it looks like this LDO
is supplying all other PMIC regulators and is ultimately being used as
master power ON/OFF switch ? 

Samuel, are you happy for this to go via mfd for simpler merging ?
If so :-

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

Thanks

Liam

WARNING: multiple messages have this Message-ID (diff)
From: Liam Girdwood <lrg@slimlogic.co.uk>
To: Linus Walleij <linus.walleij@stericsson.com>,
	Samuel Ortiz <sameo@linux.intel.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz <sameo@linux.intel.com>,
	Russell King <linux@arm.linux.org.uk>
Subject: Re: [PATCH 1/2] AB3100 regulator support v4
Date: Thu, 03 Sep 2009 15:15:48 +0100	[thread overview]
Message-ID: <1251987348.6735.49.camel@odin> (raw)
In-Reply-To: <1251982396-23790-1-git-send-email-linus.walleij@stericsson.com>

On Thu, 2009-09-03 at 14:53 +0200, Linus Walleij wrote:
> This adds support for the regulators found in the AB3100
> Mixed-Signal IC.
> 
> It further also defines platform data for the ST-Ericsson
> U300 platform and extends the AB3100 MFD driver so that
> platform/board data with regulation constraints and an init
> function can be passed down all the way from the board to
> the regulators.
> 

<snip>

> +static int ab3100_disable_regulator(struct regulator_dev *reg)
> +{
> +	struct ab3100_regulator *abreg = reg->reg_data;
> +	int err;
> +	u8 regval;
> +
> +	/*
> +	 * LDO D is a special regulator. When it is disabled, the entire
> +	 * system is shut down. So this is handled specially.
> +	 */
> +	if (abreg->regreg == AB3100_LDO_D) {
> +		int i;
> +
> +		dev_info(&reg->dev, "disabling LDO D - shut down system\n");
> +		/*
> +		 * Set regulators to default values, ignore any errors,
> +		 * we're going DOWN
> +		 */
> +		for (i = 0; i < ARRAY_SIZE(ab3100_reg_init_order); i++) {
> +			(void) ab3100_set_register_interruptible(abreg->ab3100,
> +					ab3100_reg_init_order[i],
> +					abreg->plfdata->reg_initvals[i]);
> +		}
> +
> +		/* Setting LDO D to 0x00 cuts the power to the SoC */
> +		return ab3100_set_register_interruptible(abreg->ab3100,
> +							 AB3100_LDO_D, 0x00U);
> +
> +	}
> +
> +	/*
> +	 * All other regulators are handled here
> +	 */
> +	err = ab3100_get_register_interruptible(abreg->ab3100, abreg->regreg,
> +						&regval);
> +	if (err) {
> +		dev_err(&reg->dev, "unable to get register 0x%x\n",
> +			abreg->regreg);
> +		return err;
> +	}
> +	regval &= ~AB3100_REG_ON_MASK;
> +	return ab3100_set_register_interruptible(abreg->ab3100, abreg->regreg,
> +						 regval);
> +}

Just wondering if you had looked at the regulator supplier field in
struct regulator_init_data. It's intention was to allow LDO D to be
switched OFF only when all it's consumer regulators were OFF. Likewise
it would also switch this regulator ON if any of it's children were
switched ON.

I assume for this PMIC it would not be useful as it looks like this LDO
is supplying all other PMIC regulators and is ultimately being used as
master power ON/OFF switch ? 

Samuel, are you happy for this to go via mfd for simpler merging ?
If so :-

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

Thanks

Liam


  parent reply	other threads:[~2009-09-03 14:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-03 12:53 [PATCH 1/2] AB3100 regulator support v4 Linus Walleij
2009-09-03 12:53 ` Linus Walleij
2009-09-03 13:36 ` Mark Brown
2009-09-03 13:36   ` Mark Brown
2009-09-03 14:15 ` Liam Girdwood [this message]
2009-09-03 14:15   ` Liam Girdwood
2009-09-08 22:07   ` Linus Walleij
2009-09-08 22:07     ` Linus Walleij
2009-09-09  9:35     ` Samuel Ortiz
2009-09-09  9:35       ` Samuel Ortiz
2009-09-09 11:17       ` Linus Walleij
2009-09-09 11:17         ` Linus Walleij

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=1251987348.6735.49.camel@odin \
    --to=lrg@slimlogic.co.uk \
    --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.