linux-kernel - RE: [PATCH v3 RESEND] regulator: Add S5M8767A regulator driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 02 Jan 2012 10:30:31 +0900
From:	Sangbeom Kim <sbkim73@...sung.com>
To:	'Mark Brown' <broonie@...nsource.wolfsonmicro.com>
Cc:	lrg@...com, sameo@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: RE: [PATCH v3 RESEND] regulator: Add S5M8767A regulator driver

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/