Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Robin Gong <yibin.gong@nxp.com>
To:     Marco Felsch <m.felsch@pengutronix.de>
CC:     "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "festevam@gmail.com" <festevam@gmail.com>,
        "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
        "broonie@kernel.org" <broonie@kernel.org>,
        Anson Huang <anson.huang@nxp.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v1 1/2] regulator: pca9450: add enable_val for all bucks
Thread-Topic: [PATCH v1 1/2] regulator: pca9450: add enable_val for all bucks
Thread-Index: AQHWf3GYvz7pHbbwy0Oh0swEP0rom6lSCuoAgAD5ICCALHVfEA==
Date:   Tue, 29 Sep 2020 09:10:03 +0000
Message-ID: <VE1PR04MB66385C2A3ADDB0E36081BA3689320@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <1598892515-30950-1-git-send-email-yibin.gong@nxp.com>
 <20200831105304.6qcpjfmranay6q4y@pengutronix.de>
 <VE1PR04MB6638767DC4367488F9104913892E0@VE1PR04MB6638.eurprd04.prod.outlook.com>
In-Reply-To: <VE1PR04MB6638767DC4367488F9104913892E0@VE1PR04MB6638.eurprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f5aa88ca-696f-484d-a1c5-08d864577356
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Sep 2020 09:10:03.1765
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: gtb/ZZqwR9sNo8a3z6YdkSkj0rZNjE/8k18BSAebZ2G8TQEOZMGda/MnmIjH8oDdf7jEwfDAsVPbTPP5ptmAjg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2862
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/09/01 9:58 Robin Gong <yibin.gong@nxp.com> wrote:
> On 2020/08/31 18:53 Marco Felsch <m.felsch@pengutronix.de> wrote:
> > Hi Robin,
> >
> > On 20-09-01 00:48, Robin Gong wrote:
> > > BuckX enable mode
> > > 00b =3D OFF
> > > 01b =3D ON by PMIC_ON_REQ =3D H
> > > 10b =3D ON by PMIC_ON_REQ =3D H && PMIC_STBY_REQ =3D L 11b =3D Always=
 ON
> > >
> > > For such enable mode, enable_value should be clearly set in
> > > requlator desc,
> > > 00/11 is not enough, correct it now for different bucks. For
> > > example,
> > > buck2 is designed for vddarm which could be off in 'PMIC_STBY_REQ =3D=
 H'
> > > after kernel enter suspend, so should be set '10b' as ON, while other=
s is
> '01b'
> > as ON.
> > > All are the same as the default setting which means bucks no need to
> > > be enabled again during kernel boot even if they have been enabled
> > > already after pmic on.
> >
> > I wouldn't hard-code the regulator behaviour because the behaviour
> > comes from the system design which in most cases are comming from our
> hw-guys.
> > Till now I saw a few intelligent designs don't following the pmic user
> > recommendations to save money. I would love to specify the regulator
> > behaviour/mode within the dt or acpi.
> Well, if so the better way is moving into dts. Will implement it in v2.
Hi Marco,
	Sorry, I'm afraid that big changes have to be made for moving it to dts, b=
ecause
current pca9450 driver is using common regulator_enable_regmap() which mean=
s
rdev->desc is const and 'enable_value' can't be changed runtime with the va=
lue in
dts. Regards to pca9450 is the specific pmic designed for i.mx8m family rat=
her than
generic pmic, the power rails defining should be the same as NXP MEK refere=
nce
board, especially for the BUCKs which match well with SOC's power. So may I=
 leave it
at this time, and pick it up if the 'unbelievable trouble' really come out =
in the future?  =20

>=20
> >
> > > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > > ---
> > >  drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/regulator/pca9450-regulator.c
> > > b/drivers/regulator/pca9450-regulator.c
> > > index eb5822b..79f2a5a 100644
> > > --- a/drivers/regulator/pca9450-regulator.c
> > > +++ b/drivers/regulator/pca9450-regulator.c
> > > @@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc
> > pca9450a_regulators[] =3D {
> > >  			.vsel_mask =3D BUCK1OUT_DVS0_MASK,
> > >  			.enable_reg =3D PCA9450_REG_BUCK1CTRL,
> > >  			.enable_mask =3D BUCK1_ENMODE_MASK,
> > > +			.enable_val =3D BUCK_ENMODE_ONREQ,
> > >  			.owner =3D THIS_MODULE,
> > >  			.of_parse_cb =3D pca9450_set_dvs_levels,
> > >  		},
> > > @@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc
> > pca9450a_regulators[] =3D {
> > >  			.vsel_reg =3D PCA9450_REG_BUCK2OUT_DVS0,
> > >  			.vsel_mask =3D BUCK2OUT_DVS0_MASK,
> > >  			.enable_reg =3D PCA9450_REG_BUCK2CTRL,
> > > -			.enable_mask =3D BUCK1_ENMODE_MASK,
> > > +			.enable_mask =3D BUCK2_ENMODE_MASK,
> >
> > Unrelated change?
> Yes, that's just correct it minor literal since that's BUCK2 although the=
y're the
> same, will split it anyway.