| 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
| ||
|
Date: Wed, 31 May 2023 13:34:41 +0200
From: Alexander Stein <alexander.stein@...tq-group.com>
To: ping.bai@....com, lgirdwood@...il.com, broonie@...nel.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
conor+dt@...nel.org, shawnguo@...nel.org, s.hauer@...gutronix.de,
linux-arm-kernel@...ts.infradead.org
Cc: kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Joy Zou <joy.zou@....com>
Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
Hi,
Am Mittwoch, 31. Mai 2023, 08:57:23 CEST schrieb Joy Zou:
> Adding support for pmic pca9451a.
>
> This patch support old and new pmic pca9451a. The new pmic trimed BUCK1.
> The default value of Toff_Deb is used to distinguish the old and new pmic.
>
> Signed-off-by: Joy Zou <joy.zou@....com>
> ---
> drivers/regulator/pca9450-regulator.c | 262 ++++++++++++++++++++++++--
> include/linux/regulator/pca9450.h | 2 +
> 2 files changed, 252 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/regulator/pca9450-regulator.c
> b/drivers/regulator/pca9450-regulator.c index 91bfb7e026c9..654aa4fbe494
> 100644
> --- a/drivers/regulator/pca9450-regulator.c
> +++ b/drivers/regulator/pca9450-regulator.c
> @@ -104,7 +104,15 @@ static const struct regulator_ops
> pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> */
> static const struct linear_range pca9450_dvs_buck_volts[] = {
> - REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500),
> + REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500),
> +};
> +
> +/*
> + * BUCK1/3
> + * 0.65 to 2.2375V (12.5mV step)
Reading this comment, it seems the same distinction needs to be done for BUCK3
as well, no?
> + */
> +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> + REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> };
>
> /*
> @@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc
> pca9450bc_regulators[] = { },
> };
>
> +static const struct pca9450_regulator_desc pca9451a_regulators[] = {
> + {
> + .desc = {
> + .name = "buck1",
> + .of_match = of_match_ptr("BUCK1"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK1,
> + .ops = &pca9450_dvs_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> + .linear_ranges = pca9450_dvs_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_dvs_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> + .vsel_mask = BUCK1OUT_DVS0_MASK,
> + .enable_reg = PCA9450_REG_BUCK1CTRL,
> + .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> + .ramp_mask = BUCK1_RAMP_MASK,
> + .ramp_delay_table = pca9450_dvs_buck_ramp_table,
> + .n_ramp_values =
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> + .owner = THIS_MODULE,
> + .of_parse_cb = pca9450_set_dvs_levels,
> + },
> + .dvs = {
> + .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> + .run_mask = BUCK1OUT_DVS0_MASK,
> + .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> + .standby_mask = BUCK1OUT_DVS1_MASK,
> + },
> + },
> + {
> + .desc = {
> + .name = "buck1_trim",
> + .of_match = of_match_ptr("BUCK1"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK1,
> + .ops = &pca9450_dvs_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> + .linear_ranges = pca9450_trim_dvs_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> + .vsel_mask = BUCK1OUT_DVS0_MASK,
> + .enable_reg = PCA9450_REG_BUCK1CTRL,
> + .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> + .ramp_mask = BUCK1_RAMP_MASK,
> + .ramp_delay_table = pca9450_dvs_buck_ramp_table,
> + .n_ramp_values =
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> + .owner = THIS_MODULE,
> + .of_parse_cb = pca9450_set_dvs_levels,
> + },
> + .dvs = {
> + .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> + .run_mask = BUCK1OUT_DVS0_MASK,
> + .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> + .standby_mask = BUCK1OUT_DVS1_MASK,
> + },
> + },
> + {
> + .desc = {
> + .name = "buck2",
> + .of_match = of_match_ptr("BUCK2"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK2,
> + .ops = &pca9450_dvs_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
> + .linear_ranges = pca9450_dvs_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_dvs_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> + .vsel_mask = BUCK2OUT_DVS0_MASK,
> + .enable_reg = PCA9450_REG_BUCK2CTRL,
> + .enable_mask = BUCK2_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> + .ramp_mask = BUCK2_RAMP_MASK,
> + .ramp_delay_table = pca9450_dvs_buck_ramp_table,
> + .n_ramp_values =
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> + .owner = THIS_MODULE,
> + .of_parse_cb = pca9450_set_dvs_levels,
> + },
> + .dvs = {
> + .run_reg = PCA9450_REG_BUCK2OUT_DVS0,
> + .run_mask = BUCK2OUT_DVS0_MASK,
> + .standby_reg = PCA9450_REG_BUCK2OUT_DVS1,
> + .standby_mask = BUCK2OUT_DVS1_MASK,
> + },
> + },
> + {
> + .desc = {
> + .name = "buck4",
> + .of_match = of_match_ptr("BUCK4"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK4,
> + .ops = &pca9450_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
> + .linear_ranges = pca9450_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK4OUT,
> + .vsel_mask = BUCK4OUT_MASK,
> + .enable_reg = PCA9450_REG_BUCK4CTRL,
> + .enable_mask = BUCK4_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> + .owner = THIS_MODULE,
> + },
> + },
> + {
> + .desc = {
> + .name = "buck5",
> + .of_match = of_match_ptr("BUCK5"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK5,
> + .ops = &pca9450_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
> + .linear_ranges = pca9450_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK5OUT,
> + .vsel_mask = BUCK5OUT_MASK,
> + .enable_reg = PCA9450_REG_BUCK5CTRL,
> + .enable_mask = BUCK5_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> + .owner = THIS_MODULE,
> + },
> + },
> + {
> + .desc = {
> + .name = "buck6",
> + .of_match = of_match_ptr("BUCK6"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_BUCK6,
> + .ops = &pca9450_buck_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
> + .linear_ranges = pca9450_buck_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_buck_volts),
> + .vsel_reg = PCA9450_REG_BUCK6OUT,
> + .vsel_mask = BUCK6OUT_MASK,
> + .enable_reg = PCA9450_REG_BUCK6CTRL,
> + .enable_mask = BUCK6_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> + .owner = THIS_MODULE,
> + },
> + },
> + {
> + .desc = {
> + .name = "ldo1",
> + .of_match = of_match_ptr("LDO1"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_LDO1,
> + .ops = &pca9450_ldo_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
> + .linear_ranges = pca9450_ldo1_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_ldo1_volts),
> + .vsel_reg = PCA9450_REG_LDO1CTRL,
> + .vsel_mask = LDO1OUT_MASK,
> + .enable_reg = PCA9450_REG_LDO1CTRL,
> + .enable_mask = LDO1_EN_MASK,
> + .owner = THIS_MODULE,
> + },
> + },
> + {
> + .desc = {
> + .name = "ldo4",
> + .of_match = of_match_ptr("LDO4"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_LDO4,
> + .ops = &pca9450_ldo_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
> + .linear_ranges = pca9450_ldo34_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_ldo34_volts),
> + .vsel_reg = PCA9450_REG_LDO4CTRL,
> + .vsel_mask = LDO4OUT_MASK,
> + .enable_reg = PCA9450_REG_LDO4CTRL,
> + .enable_mask = LDO4_EN_MASK,
> + .owner = THIS_MODULE,
> + },
> + },
> + {
> + .desc = {
> + .name = "ldo5",
> + .of_match = of_match_ptr("LDO5"),
> + .regulators_node = of_match_ptr("regulators"),
> + .id = PCA9450_LDO5,
> + .ops = &pca9450_ldo_regulator_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
> + .linear_ranges = pca9450_ldo5_volts,
> + .n_linear_ranges =
ARRAY_SIZE(pca9450_ldo5_volts),
> + .vsel_reg = PCA9450_REG_LDO5CTRL_H,
> + .vsel_mask = LDO5HOUT_MASK,
> + .enable_reg = PCA9450_REG_LDO5CTRL_H,
> + .enable_mask = LDO5H_EN_MASK,
> + .owner = THIS_MODULE,
> + },
> + },
> +};
> +
> static irqreturn_t pca9450_irq_handler(int irq, void *data)
> {
> struct pca9450 *pca9450 = data;
> @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> const struct pca9450_regulator_desc *regulator_desc;
> struct regulator_config config = { };
> struct pca9450 *pca9450;
> - unsigned int device_id, i;
> + unsigned int device_id, i, val;
> unsigned int reset_ctrl;
> + bool pmic_trim = false;
> int ret;
>
> if (!i2c->irq) {
> @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> if (!pca9450)
> return -ENOMEM;
>
> + pca9450->regmap = devm_regmap_init_i2c(i2c,
> +
&pca9450_regmap_config);
> + if (IS_ERR(pca9450->regmap)) {
> + dev_err(&i2c->dev, "regmap initialization failed\n");
> + return PTR_ERR(pca9450->regmap);
> + }
> +
> + ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL, &val);
> + if (ret) {
> + dev_err(&i2c->dev, "Read device id error\n");
> + return ret;
> + }
> +
> + if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> + pmic_trim = true;
PCA9450_REG_PWRCTRL is a read/write register. How is it possible to detect a
chip revision using a bit which can be changed by software e.g. bootloader?
Despite that this bit sets debounce time for PMIC_ON_REQ, how is this related
to BUCK1 voltage range?
> +
> switch (type) {
> case PCA9450_TYPE_PCA9450A:
> regulator_desc = pca9450a_regulators;
> @@ -730,6 +956,10 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> regulator_desc = pca9450bc_regulators;
> pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
> break;
> + case PCA9450_TYPE_PCA9451A:
> + regulator_desc = pca9451a_regulators;
> + pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> + break;
> default:
> dev_err(&i2c->dev, "Unknown device type");
> return -EINVAL;
> @@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>
> dev_set_drvdata(&i2c->dev, pca9450);
>
> - pca9450->regmap = devm_regmap_init_i2c(i2c,
> -
&pca9450_regmap_config);
> - if (IS_ERR(pca9450->regmap)) {
> - dev_err(&i2c->dev, "regmap initialization failed\n");
> - return PTR_ERR(pca9450->regmap);
> - }
> -
> ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID, &device_id);
> if (ret) {
> dev_err(&i2c->dev, "Read device id error\n");
> @@ -756,7 +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>
> /* Check your board and dts for match the right pmic */
> if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
> - ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)) {
> + ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC) ||
> + ((device_id >> 4) != 0x9 && type == PCA9450_TYPE_PCA9451A)) {
> dev_err(&i2c->dev, "Device id(%x) mismatched\n",
> device_id >> 4);
> return -EINVAL;
> @@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> struct regulator_dev *rdev;
> const struct pca9450_regulator_desc *r;
>
> - r = ®ulator_desc[i];
> + if (type == PCA9450_TYPE_PCA9451A &&
> + !strcmp((®ulator_desc[i])->desc.name, "buck1") &&
pmic_trim) {
> + r = ®ulator_desc[i + 1];
> + i = i + 1;
> + } else if (type == PCA9450_TYPE_PCA9451A &&
> + !strcmp((®ulator_desc[i])->desc.name,
"buck1")) {
> + r = ®ulator_desc[i];
> + i = i + 1;
I would put this in a single 'type == PCA9450_TYPE_PCA9451A' branch, to
indicate that only PCA9451A needs some kind of special handling.
> + } else
> + r = ®ulator_desc[i];
> desc = &r->desc;
>
> config.regmap = pca9450->regmap;
> @@ -847,7 +1080,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> }
>
> dev_info(&i2c->dev, "%s probed.\n",
> - type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
> + type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> + (type == PCA9450_TYPE_PCA9451A ? "pca9451a" :
"pca9450bc"));
>
> return 0;
> }
> @@ -865,6 +1099,10 @@ static const struct of_device_id pca9450_of_match[] =
> { .compatible = "nxp,pca9450c",
> .data = (void *)PCA9450_TYPE_PCA9450BC,
> },
> + {
> + .compatible = "nxp,pca9451a",
> + .data = (void *)PCA9450_TYPE_PCA9451A,
> + },
> { }
> };
> MODULE_DEVICE_TABLE(of, pca9450_of_match);
> diff --git a/include/linux/regulator/pca9450.h
> b/include/linux/regulator/pca9450.h index 3c01c2bf84f5..5dd79f52165a 100644
> --- a/include/linux/regulator/pca9450.h
> +++ b/include/linux/regulator/pca9450.h
> @@ -9,6 +9,7 @@
> enum pca9450_chip_type {
> PCA9450_TYPE_PCA9450A = 0,
> PCA9450_TYPE_PCA9450BC,
> + PCA9450_TYPE_PCA9451A,
> PCA9450_TYPE_AMOUNT,
> };
>
> @@ -93,6 +94,7 @@ enum {
> PCA9450_MAX_REGISTER = 0x2F,
> };
>
> +#define PCA9450_REG_PWRCTRL_TOFF_DEB BIT(5)
Newline here please.
Best regards,
Alexander
> /* PCA9450 BUCK ENMODE bits */
> #define BUCK_ENMODE_OFF 0x00
> #define BUCK_ENMODE_ONREQ 0x01
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Powered by blists - more mailing lists