| 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
| ||
|
Message-ID: <53B0F168.7050709@ti.com>
Date: Mon, 30 Jun 2014 10:41:04 +0530
From: Keerthy <a0393675@...com>
To: Nishanth Menon <nm@...com>
CC: Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Keerthy <j-keerthy@...com>, <linux-omap@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <t-kristo@...com>
Subject: Re: [PATCH] regulator: tps65917: Fix SMPS enable/disable/is_enable
Hi Nishanth,
On Friday 27 June 2014 10:45 PM, Nishanth Menon wrote:
> On 14:48-20140627, Keerthy wrote:
>> Hello Nishanth,
>>
>> On Friday 27 June 2014 12:01 AM, Nishanth Menon wrote:
>>> We use regmap regulator ops to enable/disable and check if regulator
>>> is enabled for various SMPS. However, these depend on valid
>>> enable_reg, enable_mask and enable_value in regulator descriptor.
>>>
>>> So, similar to fix we did in commit 318dbb02b50c
>>> ("regulator: palmas: Fix SMPS enable/disable/is_enabled"), populate the
>>> same for TPS65917 SMPS registration. LDO definitions are already in place.
>>>
>>> Fixes: d6f83370ed97 ("regulator: palmas: Add tps65917 PMIC support")
>>> Signed-off-by: Nishanth Menon <nm@...com>
>>> ---
>>>
>>> Applies on:
>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
>>> branch: topic/palmas (4c0c9ca Merge remote-tracking branch 'regulator/fix/palmas' into regulator-palmas)
>>>
>>> Note: Ignored the minor style check from checkpatch --strict as fixing
>>> it would create an 80 char warning
>>>
>>> drivers/regulator/palmas-regulator.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
>>> index 7c8b441..c7aa1b1 100644
>>> --- a/drivers/regulator/palmas-regulator.c
>>> +++ b/drivers/regulator/palmas-regulator.c
>>> @@ -1333,6 +1333,14 @@ static int tps65917_smps_registration(struct palmas_pmic *pmic,
>>> pmic->current_reg_mode[id] = reg &
>>> PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
>>> + pmic->desc[id].enable_reg =
>>> + PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
>>> + palmas_regs_info[id].ctrl_addr);
>> This is wrong. Please change palmas_regs_info[id].ctrl_addr to
>> ddata->palmas_regs_info[id].ctrl_addr. The palmas_regs_info
>> should come from the driver data for specific instances as the regmap
>> is different for the different PMICs we support.
>>
>> Once you make the above changes please feel free to add
> Uggh.. just realized... since I was testing CPUFREQ with SMPS1 never
> realized it. and searched elsewhere on topic/palmas branch as well..
> Looks like we should make the following changes? since the register data
> is now a parameter and it so happened that palmas_reg_info was mapping
> back to a valid variable and the error is easily missed - so rename the
> variable as well - I can break this up as proper series of patches for
> topic/palmas - let me know what you think..
>
>
> diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
> index 7c8b441..86fefdfb 100644
> --- a/drivers/regulator/palmas-regulator.c
> +++ b/drivers/regulator/palmas-regulator.c
> @@ -41,7 +41,7 @@ static const struct regulator_linear_range smps_high_ranges[] = {
> REGULATOR_LINEAR_RANGE(3300000, 0x7A, 0x7f, 0),
> };
>
> -static struct regs_info palmas_regs_info[] = {
> +static struct regs_info palmas_generic_regs_info[] = {
Good renaming but You need to make one more change i guess. Otherwise
there is a compilation break.
@@ -643,7 +643,8 @@
static int palmas_regulator_config_external(struct palmas *palmas, int id,
struct palmas_reg_init *reg_init)
{
- int sleep_id = palmas_regs_info[id].sleep_id;
+ struct palmas_pmic_driver_data *ddata = palmas->pmic_ddata;
+ int sleep_id = ddata->palmas_regs_info[id].sleep_id;
int ret;
Including the above change.
Tested-by: Keerthy <j-keerthy@...com>.
> {
> .name = "SMPS12",
> .sname = "smps1-in",
> @@ -1165,18 +1165,18 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
> pmic->desc[id].ops = &palmas_ops_smps10;
> pmic->desc[id].vsel_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - PALMAS_SMPS10_CTRL);
> + ddata->palmas_regs_info[id].ctrl_addr);
> pmic->desc[id].vsel_mask = SMPS10_VSEL;
> pmic->desc[id].enable_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - PALMAS_SMPS10_CTRL);
> + ddata->palmas_regs_info[id].ctrl_addr);
> if (id == PALMAS_REG_SMPS10_OUT1)
> pmic->desc[id].enable_mask = SMPS10_SWITCH_EN;
> else
> pmic->desc[id].enable_mask = SMPS10_BOOST_EN;
> pmic->desc[id].bypass_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - PALMAS_SMPS10_CTRL);
> + ddata->palmas_regs_info[id].ctrl_addr);
> pmic->desc[id].bypass_mask = SMPS10_BYPASS_EN;
> pmic->desc[id].min_uV = 3750000;
> pmic->desc[id].uV_step = 1250000;
> @@ -1188,7 +1188,7 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
> * otherwise we error in probe with unsupportable
> * ranges. Read the current smps mode for later use.
> */
> - addr = palmas_regs_info[id].vsel_addr;
> + addr = ddata->palmas_regs_info[id].vsel_addr;
> pmic->desc[id].n_linear_ranges = 3;
>
> ret = palmas_smps_read(pmic->palmas, addr, ®);
> @@ -1209,12 +1209,12 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
> pmic->desc[id].n_voltages = PALMAS_SMPS_NUM_VOLTAGES;
> pmic->desc[id].vsel_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - palmas_regs_info[id].vsel_addr);
> + ddata->palmas_regs_info[id].vsel_addr);
> pmic->desc[id].vsel_mask =
> PALMAS_SMPS12_VOLTAGE_VSEL_MASK;
>
> /* Read the smps mode for later use. */
> - addr = palmas_regs_info[id].ctrl_addr;
> + addr = ddata->palmas_regs_info[id].ctrl_addr;
> ret = palmas_smps_read(pmic->palmas, addr, ®);
> if (ret)
> return ret;
> @@ -1223,7 +1223,7 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
>
> pmic->desc[id].enable_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - palmas_regs_info[id].ctrl_addr);
> + ddata->palmas_regs_info[id].ctrl_addr);
> pmic->desc[id].enable_mask =
> PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
> /* set_mode overrides this value */
> @@ -1319,7 +1319,7 @@ static int tps65917_smps_registration(struct palmas_pmic *pmic,
> pmic->desc[id].n_voltages = PALMAS_SMPS_NUM_VOLTAGES;
> pmic->desc[id].vsel_reg =
> PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> - tps65917_regs_info[id].vsel_addr);
> + ddata->palmas_regs_info[id].vsel_addr);
> pmic->desc[id].vsel_mask =
> PALMAS_SMPS12_VOLTAGE_VSEL_MASK;
>
> @@ -1414,7 +1414,7 @@ struct palmas_pmic_driver_data palmas_ddata = {
> .ldo_begin = PALMAS_REG_LDO1,
> .ldo_end = PALMAS_REG_LDOUSB,
> .max_reg = PALMAS_NUM_REGS,
> - .palmas_regs_info = palmas_regs_info,
> + .palmas_regs_info = palmas_generic_regs_info,
> .palmas_matches = palmas_matches,
> .sleep_req_info = palma_sleep_req_info,
> .smps_register = palmas_smps_registration,
Regards,
Keerthy
--
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/
Powered by blists - more mailing lists