All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: Jacky Bai <ping.bai@nxp.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, Joy Zou <joy.zou@nxp.com>
Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
Date: Wed, 05 Jul 2023 15:12:45 +0200	[thread overview]
Message-ID: <4630917.iIbC2pHGDl@steina-w> (raw)
In-Reply-To: <VI1PR04MB5936370C11D1523558437D9BE12FA@VI1PR04MB5936.eurprd04.prod.outlook.com>

Hello,

Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> 
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: 2023年5月31日 19:35
> > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > broonie@kernel.org; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > linux-arm-kernel@lists.infradead.org
> > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Joy
> > Zou
 <joy.zou@nxp.com>
> > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > support
> >
> >
> >
> > Caution: This is an external email. Please take care when clicking links
> > or
 opening attachments. When in doubt, report the message using the
> > 'Report this email' button
> >
> >
> >
> >
> > 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@nxp.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?
> 
> Sorry for the late reply!
> The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
> 
> >
> >
> > > + */
> > > +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?
> 
> There are old and new two kind PMIC pca9451a. 

There is only one part mentioned in the ordering options. How can I 
distinguish them? Any chip ID, date codes, markings?

> This bit sets debounce time in
> PCA9450_REG_PWRCTRL was set different value by hardware in order to only
> distinguish the old and new PMIC. This bit isn't related to the BUCK1
> voltage range. If the pmic_trim is true that means it's new pca9451a.

But this bit is writable. How do you know it has not been modified since 
reset?

Best regards,
Alexander

> > > +
> > > 
> > >       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 = &regulator_desc[i];
> > > +             if (type == PCA9450_TYPE_PCA9451A &&
> > > +                 !strcmp((&regulator_desc[i])->desc.name, "buck1") &&
> > 
> > pmic_trim) {
> > 
> > > +                     r = &regulator_desc[i + 1];
> > > +                     i = i + 1;
> > > +             } else if (type == PCA9450_TYPE_PCA9451A &&
> > > +                        !strcmp((&regulator_desc[i])->desc.name,
> > 
> > "buck1")) {
> > 
> > > +                     r = &regulator_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.
> 
> Yes, this makes the logic clearer. I will change in next version.
> 
> >
> >
> > > +             } else
> > > +                     r = &regulator_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.
> 
> Ok, will modify it.
> Thank you very much for your comments and efforts.
> BR
> Joy Zou
> 
> >
> >
> > 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%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C1ecbe75c432d464
> > cb45f08db61cb08bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C638211296883052738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> > 7C&sdata=mz7vtBiDvAxY9FO7tTZO3oPBA8zvYS6f8hLgAwrU8uk%3D&reserved
> > =0
> >
> >
> 
> 


-- 
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/



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: Jacky Bai <ping.bai@nxp.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org" 
	<krzysztof.kozlowski+dt@linaro.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>, Joy Zou <joy.zou@nxp.com>
Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
Date: Wed, 05 Jul 2023 15:12:45 +0200	[thread overview]
Message-ID: <4630917.iIbC2pHGDl@steina-w> (raw)
In-Reply-To: <VI1PR04MB5936370C11D1523558437D9BE12FA@VI1PR04MB5936.eurprd04.prod.outlook.com>

Hello,

Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> 
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: 2023年5月31日 19:35
> > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > broonie@kernel.org; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > linux-arm-kernel@lists.infradead.org
> > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Joy
> > Zou
 <joy.zou@nxp.com>
> > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > support
> >
> >
> >
> > Caution: This is an external email. Please take care when clicking links
> > or
 opening attachments. When in doubt, report the message using the
> > 'Report this email' button
> >
> >
> >
> >
> > 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@nxp.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?
> 
> Sorry for the late reply!
> The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
> 
> >
> >
> > > + */
> > > +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?
> 
> There are old and new two kind PMIC pca9451a. 

There is only one part mentioned in the ordering options. How can I 
distinguish them? Any chip ID, date codes, markings?

> This bit sets debounce time in
> PCA9450_REG_PWRCTRL was set different value by hardware in order to only
> distinguish the old and new PMIC. This bit isn't related to the BUCK1
> voltage range. If the pmic_trim is true that means it's new pca9451a.

But this bit is writable. How do you know it has not been modified since 
reset?

Best regards,
Alexander

> > > +
> > > 
> > >       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 = &regulator_desc[i];
> > > +             if (type == PCA9450_TYPE_PCA9451A &&
> > > +                 !strcmp((&regulator_desc[i])->desc.name, "buck1") &&
> > 
> > pmic_trim) {
> > 
> > > +                     r = &regulator_desc[i + 1];
> > > +                     i = i + 1;
> > > +             } else if (type == PCA9450_TYPE_PCA9451A &&
> > > +                        !strcmp((&regulator_desc[i])->desc.name,
> > 
> > "buck1")) {
> > 
> > > +                     r = &regulator_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.
> 
> Yes, this makes the logic clearer. I will change in next version.
> 
> >
> >
> > > +             } else
> > > +                     r = &regulator_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.
> 
> Ok, will modify it.
> Thank you very much for your comments and efforts.
> BR
> Joy Zou
> 
> >
> >
> > 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%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C1ecbe75c432d464
> > cb45f08db61cb08bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C638211296883052738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> > 7C&sdata=mz7vtBiDvAxY9FO7tTZO3oPBA8zvYS6f8hLgAwrU8uk%3D&reserved
> > =0
> >
> >
> 
> 


-- 
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/



  reply	other threads:[~2023-07-05 13:13 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-31  6:57 [PATCH v1 0/3] add pmic pca9451a support Joy Zou
2023-05-31  6:57 ` Joy Zou
2023-05-31  6:57 ` [PATCH v1 1/3] dt-bindings: regulator: pca9450: add " Joy Zou
2023-05-31  6:57   ` Joy Zou
2023-05-31  6:56   ` Krzysztof Kozlowski
2023-05-31  6:56     ` Krzysztof Kozlowski
2023-05-31  7:00     ` [EXT] " Joy Zou
2023-05-31  7:00       ` Joy Zou
2023-05-31  7:22     ` Frieder Schrempf
2023-05-31  7:22       ` Frieder Schrempf
2023-05-31  9:11       ` Krzysztof Kozlowski
2023-05-31  9:11         ` Krzysztof Kozlowski
2023-05-31  9:20         ` Frieder Schrempf
2023-05-31  9:20           ` Frieder Schrempf
2023-05-31 10:41         ` [EXT] " Joy Zou
2023-05-31 10:41           ` Joy Zou
2023-05-31  6:57 ` [PATCH v1 2/3] " Joy Zou
2023-05-31  6:57   ` Joy Zou
2023-05-31 11:34   ` Alexander Stein
2023-05-31 11:34     ` Alexander Stein
2023-07-05  6:50     ` [EXT] " Joy Zou
2023-07-05  6:50       ` Joy Zou
2023-07-05 13:12       ` Alexander Stein [this message]
2023-07-05 13:12         ` Alexander Stein
2023-07-17  9:53         ` [EXT] " Joy Zou
2023-07-17  9:53           ` Joy Zou
2023-07-17 13:33           ` Mark Brown
2023-07-17 13:33             ` Mark Brown
2023-08-01 10:17             ` Joy Zou
2023-08-01 10:17               ` Joy Zou
     [not found]         ` <AM6PR04MB59255766CED30160FB552F94E10AA@AM6PR04MB5925.eurprd04.prod.outlook.com>
2023-10-17 14:02           ` Alexander Stein
2023-10-17 14:02             ` Alexander Stein
2023-10-19  7:47             ` [EXT] " Joy Zou
2023-10-19  7:47               ` Joy Zou
2023-05-31  6:57 ` [PATCH v1 3/3] arm64: dts: imx93-11x11-evk: " Joy Zou
2023-05-31  6:57   ` Joy Zou
2023-05-31 11:18   ` Mark Brown
2023-05-31 11:18     ` Mark Brown
2023-08-01 10:17     ` [EXT] " Joy Zou
2023-08-01 10:17       ` Joy Zou
2024-03-25 17:44 ` (subset) [PATCH v1 0/3] add pmic " Mark Brown
2024-03-25 17:44   ` Mark Brown

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=4630917.iIbC2pHGDl@steina-w \
    --to=alexander.stein@ew.tq-group.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=joy.zou@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ping.bai@nxp.com \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.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.