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: AQHWf3GYvz7pHbbwy0Oh0swEP0rom6lSCuoAgAD5ICA=
Date:   Tue, 1 Sep 2020 01:58:07 +0000
Message-ID: <VE1PR04MB6638767DC4367488F9104913892E0@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <1598892515-30950-1-git-send-email-yibin.gong@nxp.com>
 <20200831105304.6qcpjfmranay6q4y@pengutronix.de>
In-Reply-To: <20200831105304.6qcpjfmranay6q4y@pengutronix.de>
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: 982f6fbd-5359-4c3e-9b71-08d84e1a78b3
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Sep 2020 01:58:07.2798
 (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: fIYXVZI9siblfWeZVZXY0Z92KfaP1q2SSZaHXpdgyLUZt+qUN6SUjgGWIpC6lGd2GDVGZDS4LkeoDcLEWUPJ6Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB7374
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/08/31 18:53 Marco Felsch <m.felsch@pengutronix.de> wrote:
> Hi Robin,
>=20
> 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 O=
N
> >
> > 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 others =
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.
>=20
> 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.

>=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,
>=20
> Unrelated change?
Yes, that's just correct it minor literal since that's BUCK2 although
they're the same, will split it anyway.