RE: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

From: Joy Zou
Date: Thu Oct 19 2023 - 03:48:04 EST

Next message: Takashi Iwai: "Re: [PATCH v2] ALSA: virtio: use copy and fill_silence callbacks"
Previous message: Andrew Jones: "Re: [PATCH v2 2/8] RISC-V: KVM: Change the SBI specification version to v2.0"
In reply to: Alexander Stein: "Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> Sent: 2023年10月17日 22:02
> To: Jacky Bai <ping.bai@xxxxxxx>; lgirdwood@xxxxxxxxx;
> broonie@xxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> conor+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Joy Zou <joy.zou@xxxxxxx>
> Cc: kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> 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 Dienstag, 1. August 2023, 12:17:20 CEST schrieb Joy Zou:
> > > -----Original Message-----
> > > From: Alexander Stein
> > > <alexander.stein@xxxxxxxxxxxxxxx<mailto:alexander.stein@xxxxx-group.
> > > com>>
> > >
> Sent: 2023年7月5日 21:13
> > > To: Jacky Bai <ping.bai@xxxxxxx<mailto:ping.bai@xxxxxxx>>;
> > > lgirdwood@xxxxxxxxx<mailto:lgirdwood@xxxxxxxxx>;
> > > broonie@xxxxxxxxxx<mailto:broonie@xxxxxxxxxx>;
> > > robh+dt@xxxxxxxxxx<mailto:robh+dt@xxxxxxxxxx>;
> > > krzysztof.kozlowski+dt@xxxxxxxxxx<mailto:krzysztof.kozlowski+dt@lina
> > > ro.or
> > > g>; conor+dt@xxxxxxxxxx<mailto:conor+dt@xxxxxxxxxx>;
> > > shawnguo@xxxxxxxxxx<mailto:shawnguo@xxxxxxxxxx>;
> > > s.hauer@xxxxxxxxxxxxxx<mailto:s.hauer@xxxxxxxxxxxxxx>;
> > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx<mailto:linux-arm-kernel@lists.i
> > > nfrad ead.org>; Joy Zou <joy.zou@xxxxxxx<mailto:joy.zou@xxxxxxx>>
> > > Cc:
> > > kernel@xxxxxxxxxxxxxx<mailto:kernel@xxxxxxxxxxxxxx>;
> > > festevam@xxxxxxxxx<mailto:festevam@xxxxxxxxx>; dl-linux-imx
> > > <linux-imx@xxxxxxx<mailto:linux-imx@xxxxxxx>>;
> > > devicetree@xxxxxxxxxxxxxxx<mailto:devicetree@xxxxxxxxxxxxxxx>;
> > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx<mailto:linux-arm-kernel@lists.i
> > > nfrad
> > > ead.org>;
> > > linux-kernel@xxxxxxxxxxxxxxx<mailto:linux-kernel@xxxxxxxxxxxxxxx>
> > > 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
> > >
> > >
> > >
> > >
> > > Hello,
> > >
> > >
> > >
> > > Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Alexander Stein
> > > > > <alexander.stein@xxxxxxxxxxxxxxx<mailto:alexander.stein@xxxxx-gr
> > > > > oup.c
> > > > > om>>
> Sent: 2023年5月31日 19:35
> > > > > To: Jacky Bai <ping.bai@xxxxxxx<mailto:ping.bai@xxxxxxx>>;
> > > > > lgirdwood@xxxxxxxxx<mailto:lgirdwood@xxxxxxxxx>;
> > > > > broonie@xxxxxxxxxx<mailto:broonie@xxxxxxxxxx>;
> > > > > robh+dt@xxxxxxxxxx<mailto:robh+dt@xxxxxxxxxx>;
> > > > > krzysztof.kozlowski+dt@xxxxxxxxxx<mailto:krzysztof.kozlowski+dt@
> > > > > linar o.org>; conor+dt@xxxxxxxxxx<mailto:conor+dt@xxxxxxxxxx>;
> > > > > shawnguo@xxxxxxxxxx<mailto:shawnguo@xxxxxxxxxx>;
> > > > > s.hauer@xxxxxxxxxxxxxx<mailto:s.hauer@xxxxxxxxxxxxxx>;
> > > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx<mailto:linux-arm-kernel@lis
> > > > > ts.in fradead.org> Cc:
> > > > > kernel@xxxxxxxxxxxxxx<mailto:kernel@xxxxxxxxxxxxxx>;
> > > > > festevam@xxxxxxxxx<mailto:festevam@xxxxxxxxx>; dl-linux-imx
> > > > > <linux-imx@xxxxxxx<mailto:linux-imx@xxxxxxx>>;
> > > > > devicetree@xxxxxxxxxxxxxxx<mailto:devicetree@xxxxxxxxxxxxxxx>;
> > > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx<mailto:linux-arm-kernel@lis
> > > > > ts.in
> > > > > fradead.org>;
> > > > > linux-kernel@xxxxxxxxxxxxxxx<mailto:linux-kernel@xxxxxxxxxxxxxxx
> > > > > >;
> > > > > Joy Zou
> > >
> > > <joy.zou@xxxxxxx<mailto:joy.zou@xxxxxxx>>
> > >
> > > > > Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > > > support
> > > > >
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > > @@ -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),
> > > > > >
> > > > > >
> > > > > >
> > > > > > };
> >
> >
> >
> > > > > > @@ -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?
> >
> > Yes, there is only one part. We distinguish the new and old part by
> > this bit
> Toff_Deb of PCA9450_REG_PWRCTRL reset value. The reset value 0 means
> > it's old part, and the reset value 1 means it's new part.
>
> Is the "old" part by coincidence an unofficial prerelease/sample chip?
Yes, the "old" part is sample chip for the customer.
>
> > >
> > >
> > > > 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?
> > Yes, we don't consider modify the debounce bit case. Modify the
> > Toff_deb value
> will influence the old and new part judgement.
>
> This judgement seems broken to me. How can I know offline whether I have
> old or new parts? I would like to know if there is a difference on some my
> boards.
Yes, We can't differentiate the old and new parts offline. there is no
difference on some my boards. So we select the register bit default value as the
check value. Unfortunately, the register is readable and writable.
>
> > For example, this default value of Toff_deb is 1 in the new part, if
> > the customers
> change the Toff_deb value from 1 to 0, and then make the board
> > warm reset, the Toff_deb value still keep 0, if the Toff_deb value is
> > 0, the PMIC driver will think this part is old part. But this part is
> > new part in fact.
>
> This should show you it's a bad idea to decide the chip revision depending on
> Toff_deb.
Yes, I think so, so we have discussed that we firstly only new pmic. We will bypass
the register bit check. And send new patch again.
>
> > Have discussed this issue with our internal team member, we will add a
> > note to PCA9451
> datasheet – “Please contract NXP If you want to change
> > Toff_deb.” But till now, we am not aware any customers case which need
> > to adjust Toff_deb.
> >
> > Make it more clear: If customers do need to manually adjust Toff_deb,
> > It need PMIC driver
> update to bypass this bit check and directly apply
> > corresponding voltage config table old or new. Thank you very much for
> > your comments and efforts.
>
> In this case I need to know if I use old, new or both revision of these parts.
For the user, you don't need to know the revision. There is no function difference
about different version.
Thank you very much for your comments and efforts.
BR
Joy Zou
>
> Best regards,
> Alexander
> --
> 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%7C7195b18abca549c
> 7afa208dbcf19a8c9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638331481351694496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&sdata=9SxW%2FUPpKWt0HIUsPoQ54ykworKw7gOZIhKsHfEPZS0%3D&rese
> rved=0
>

Next message: Takashi Iwai: "Re: [PATCH v2] ALSA: virtio: use copy and fill_silence callbacks"
Previous message: Andrew Jones: "Re: [PATCH v2 2/8] RISC-V: KVM: Change the SBI specification version to v2.0"
In reply to: Alexander Stein: "Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]