| 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: <2213718.72vocr9iq0@steina-w>
Date: Tue, 17 Oct 2023 16:02:09 +0200
From: Alexander Stein <alexander.stein@...tq-group.com>
To: Jacky Bai <ping.bai@....com>,
"lgirdwood@...il.com" <lgirdwood@...il.com>,
"broonie@...nel.org" <broonie@...nel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, Joy Zou <joy.zou@....com>
Cc: "kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
Hi,
Am Dienstag, 1. August 2023, 12:17:20 CEST schrieb Joy Zou:
> > -----Original Message-----
> > From: Alexander Stein
> > <alexander.stein@...tq-group.com<mailto:alexander.stein@...tq-group.com>>
> >
Sent: 2023年7月5日 21:13
> > To: Jacky Bai <ping.bai@....com<mailto:ping.bai@....com>>;
> > lgirdwood@...il.com<mailto:lgirdwood@...il.com>;
> > broonie@...nel.org<mailto:broonie@...nel.org>;
> > robh+dt@...nel.org<mailto:robh+dt@...nel.org>;
> > krzysztof.kozlowski+dt@...aro.org<mailto:krzysztof.kozlowski+dt@...aro.or
> > g>; conor+dt@...nel.org<mailto:conor+dt@...nel.org>;
> > shawnguo@...nel.org<mailto:shawnguo@...nel.org>;
> > s.hauer@...gutronix.de<mailto:s.hauer@...gutronix.de>;
> > linux-arm-kernel@...ts.infradead.org<mailto:linux-arm-kernel@...ts.infrad
> > ead.org>; Joy Zou <joy.zou@....com<mailto:joy.zou@....com>> Cc:
> > kernel@...gutronix.de<mailto:kernel@...gutronix.de>;
> > festevam@...il.com<mailto:festevam@...il.com>; dl-linux-imx
> > <linux-imx@....com<mailto:linux-imx@....com>>;
> > devicetree@...r.kernel.org<mailto:devicetree@...r.kernel.org>;
> > linux-arm-kernel@...ts.infradead.org<mailto:linux-arm-kernel@...ts.infrad
> > ead.org>;
> > linux-kernel@...r.kernel.org<mailto:linux-kernel@...r.kernel.org>
> > 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@...tq-group.com<mailto:alexander.stein@...tq-group.c
> > > > om>>
Sent: 2023年5月31日 19:35
> > > > To: Jacky Bai <ping.bai@....com<mailto:ping.bai@....com>>;
> > > > lgirdwood@...il.com<mailto:lgirdwood@...il.com>;
> > > > broonie@...nel.org<mailto:broonie@...nel.org>;
> > > > robh+dt@...nel.org<mailto:robh+dt@...nel.org>;
> > > > krzysztof.kozlowski+dt@...aro.org<mailto:krzysztof.kozlowski+dt@...ar
> > > > o.org>; conor+dt@...nel.org<mailto:conor+dt@...nel.org>;
> > > > shawnguo@...nel.org<mailto:shawnguo@...nel.org>;
> > > > s.hauer@...gutronix.de<mailto:s.hauer@...gutronix.de>;
> > > > linux-arm-kernel@...ts.infradead.org<mailto:linux-arm-kernel@...ts.in
> > > > fradead.org> Cc: kernel@...gutronix.de<mailto:kernel@...gutronix.de>;
> > > > festevam@...il.com<mailto:festevam@...il.com>; dl-linux-imx
> > > > <linux-imx@....com<mailto:linux-imx@....com>>;
> > > > devicetree@...r.kernel.org<mailto:devicetree@...r.kernel.org>;
> > > > linux-arm-kernel@...ts.infradead.org<mailto:linux-arm-kernel@...ts.in
> > > > fradead.org>;
> > > > linux-kernel@...r.kernel.org<mailto:linux-kernel@...r.kernel.org>;
> > > > Joy Zou
> >
> > <joy.zou@....com<mailto:joy.zou@....com>>
> >
> > > > 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?
> >
> >
> > > 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.
> 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.
> 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.
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/
Powered by blists - more mailing lists