All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sangbeom Kim <sbkim73@samsung.com>
To: "'Mark Brown'" <broonie@opensource.wolfsonmicro.com>
Cc: lrg@ti.com, sameo@linux.intel.com, linux-kernel@vger.kernel.org
Subject: RE: [PATCH v3 RESEND] regulator: Add S5M8767A regulator driver
Date: Mon, 02 Jan 2012 10:30:31 +0900	[thread overview]
Message-ID: <006301ccc8ee$1de26650$59a732f0$@com> (raw)
In-Reply-To: <20111230103725.GB2766@opensource.wolfsonmicro.com>

Hi,
> On Fri, Dec 30, 2011 at 7:37 PM +0900, Mark Brown wrote:
> > > > +	ret = s5m8767_get_register(rdev, &reg);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	return s5m_reg_update(s5m8767->iodev, reg, ~pattern, mask);
> 
> > > The above looks like it's going to update the same registers as the
> > > regular enable and disable which isn't right, this function is for
> > > updating suspend mode configuration.
> 
> > What's exact meaning of updating suspend mode configuration?
> > As I know, The purpose of set_suspend_enable is that
> > enabling voltage output during suspend mode.
> > So, I implement same register enable and disable.
> 
> If there's only the one set of registers you should just not implement
> this function.  Some regulators have separate registers for configuring
> the state they go into when suspended so they need a separate call to
> configure that mode.
Ok, I will remove another implementation of the same register 
for set_suspend_*.

> > > > +	do {
> > > > +		ret = s5m_reg_read(s5m8767->iodev, reg, &val);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +
> > > > +		s5m_reg_update(s5m8767->iodev, reg,
> > > > +				((val & 0x3f) | 0x40), 0xff);
> > > > +		reg++;
> > > > +		if ((reg == S5M8767_REG_LDO9CTRL) ||
> > > > +			(reg == S5M8767_REG_LDO11CTRL) ||
> > > > +			(reg == S5M8767_REG_LDO13CTRL) ||
> > > > +			(reg == S5M8767_REG_LDO17CTRL))
> > > > +			reg++;
> > > > +	} while (reg <= S5M8767_REG_LDO16CTRL);
> 
> > > What does this do?
> 
> > This is SMDK4412 specific code.
> > I will delete it for the general purpose.
> 
> OK.  Is it something that's missing from the framework or was it just
> done this way for a quick test?
S5M8767A has various power mode.
Especially, It can synchronize with AP power mode.
This initialization code is related to AP synchronization.
I have plan to add regulator framework for this function.
After merging basic S5M8767A driver, I will submit regulator framework.

Thanks,
Happy new year!

Sangbeom.


      reply	other threads:[~2012-01-02  1:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-28 22:55 [PATCH v3 RESEND] regulator: Add S5M8767A regulator driver Sangbeom Kim
2011-12-29 10:57 ` Mark Brown
2011-12-30  5:50   ` Sangbeom Kim
2011-12-30 10:37     ` Mark Brown
2012-01-02  1:30       ` Sangbeom Kim [this message]

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='006301ccc8ee$1de26650$59a732f0$@com' \
    --to=sbkim73@samsung.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@ti.com \
    --cc=sameo@linux.intel.com \
    /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.