Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Thu, May 17, 2018 at 01:48:41PM -0700, David Collins wrote: > The RPMh hardware is configured by the boot loader. The configuration > does reflect reality; however, it cannot handle all configurations at > initialization time. Specific headroom management typically comes up in > modem

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Thu, May 17, 2018 at 01:48:41PM -0700, David Collins wrote: > The RPMh hardware is configured by the boot loader. The configuration > does reflect reality; however, it cannot handle all configurations at > initialization time. Specific headroom management typically comes up in > modem

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 05/16/2018 11:09 PM, Mark Brown wrote: > On Tue, Apr 24, 2018 at 01:46:21PM -0700, David Collins wrote: >> The RPMh hardware is aware of the parent-child connections between >> regulators as well as minimum headroom to ensure stable LDO voltage output >> for subregulated LDOs. The intention of

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 05/16/2018 11:09 PM, Mark Brown wrote: > On Tue, Apr 24, 2018 at 01:46:21PM -0700, David Collins wrote: >> The RPMh hardware is aware of the parent-child connections between >> regulators as well as minimum headroom to ensure stable LDO voltage output >> for subregulated LDOs. The intention of

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Apr 24, 2018 at 01:46:21PM -0700, David Collins wrote: > On 04/24/2018 10:41 AM, Mark Brown wrote: > > If the hardware has full knowledge of all these constraints and enforces > > them transparently then why does the kernel care that it's doing that? > > Doesn't it defeat the point of it

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Apr 24, 2018 at 01:46:21PM -0700, David Collins wrote: > On 04/24/2018 10:41 AM, Mark Brown wrote: > > If the hardware has full knowledge of all these constraints and enforces > > them transparently then why does the kernel care that it's doing that? > > Doesn't it defeat the point of it

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/24/2018 10:41 AM, Mark Brown wrote: > On Fri, Apr 20, 2018 at 12:28:21PM -0700, David Collins wrote: >> RPMh hardware enforces the requested minimum headroom voltage for all >> regulators with a parent. It has full knowledge of the parent-child >> connections of regulators on the board (as

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/24/2018 10:41 AM, Mark Brown wrote: > On Fri, Apr 20, 2018 at 12:28:21PM -0700, David Collins wrote: >> RPMh hardware enforces the requested minimum headroom voltage for all >> regulators with a parent. It has full knowledge of the parent-child >> connections of regulators on the board (as

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

>> + ret = cmd_db_ready(); >> + if (ret < 0) { >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Command DB not available, >> ret=%d\n", ret); >> + return ret; >> + } > > We should just make

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

>> + ret = cmd_db_ready(); >> + if (ret < 0) { >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Command DB not available, >> ret=%d\n", ret); >> + return ret; >> + } > > We should just make

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Apr 20, 2018 at 12:28:21PM -0700, David Collins wrote: > On 04/19/2018 05:08 AM, Mark Brown wrote: > > This doesn't sound like what the min_dropout_uV constraint is intended > > to handle - that's there for the regulator driver (not constraints) to > > indicate how much headroom the

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Apr 20, 2018 at 12:28:21PM -0700, David Collins wrote: > On 04/19/2018 05:08 AM, Mark Brown wrote: > > This doesn't sound like what the min_dropout_uV constraint is intended > > to handle - that's there for the regulator driver (not constraints) to > > indicate how much headroom the

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Apr 20 2018 at 13:07 -0600, David Collins wrote: On 04/18/2018 10:55 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-22 18:30:06) On 03/21/2018 12:07 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-16 18:09:10) diff --git a/drivers/regulator/Kconfig

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Apr 20 2018 at 13:07 -0600, David Collins wrote: On 04/18/2018 10:55 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-22 18:30:06) On 03/21/2018 12:07 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-16 18:09:10) diff --git a/drivers/regulator/Kconfig

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/19/2018 05:08 AM, Mark Brown wrote: > On Wed, Apr 18, 2018 at 10:55:49PM -0700, Stephen Boyd wrote: Is this regulator-microvolt-offset? Ah I guess it's a thing in the RPMh registers. This probably needs to be pushed into the framework and come down through a 'set_headroom' op

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/19/2018 05:08 AM, Mark Brown wrote: > On Wed, Apr 18, 2018 at 10:55:49PM -0700, Stephen Boyd wrote: Is this regulator-microvolt-offset? Ah I guess it's a thing in the RPMh registers. This probably needs to be pushed into the framework and come down through a 'set_headroom' op

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/18/2018 10:55 PM, Stephen Boyd wrote: > Quoting David Collins (2018-03-22 18:30:06) >> On 03/21/2018 12:07 PM, Stephen Boyd wrote: >>> Quoting David Collins (2018-03-16 18:09:10) diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 097f617..e0ecd0a 100644

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 04/18/2018 10:55 PM, Stephen Boyd wrote: > Quoting David Collins (2018-03-22 18:30:06) >> On 03/21/2018 12:07 PM, Stephen Boyd wrote: >>> Quoting David Collins (2018-03-16 18:09:10) diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 097f617..e0ecd0a 100644

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Wed, Apr 18, 2018 at 10:55:49PM -0700, Stephen Boyd wrote: > I lost the code context, but my general comment was that the modes that > the hardware supports should come from the driver. If there's some sort > of constraint on what those modes end up being because some board has an > issue with

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Wed, Apr 18, 2018 at 10:55:49PM -0700, Stephen Boyd wrote: > I lost the code context, but my general comment was that the modes that > the hardware supports should come from the driver. If there's some sort > of constraint on what those modes end up being because some board has an > issue with

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Quoting David Collins (2018-03-22 18:30:06) > On 03/21/2018 12:07 PM, Stephen Boyd wrote: > > Quoting David Collins (2018-03-16 18:09:10) > >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > >> index 097f617..e0ecd0a 100644 > >> --- a/drivers/regulator/Kconfig > >> +++

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Quoting David Collins (2018-03-22 18:30:06) > On 03/21/2018 12:07 PM, Stephen Boyd wrote: > > Quoting David Collins (2018-03-16 18:09:10) > >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > >> index 097f617..e0ecd0a 100644 > >> --- a/drivers/regulator/Kconfig > >> +++

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Wed, Mar 21, 2018 at 12:07 PM, Stephen Boyd wrote: >> +static int rpmh_regulator_remove(struct platform_device *pdev) >> +{ >> + struct rpmh_pmic *pmic = platform_get_drvdata(pdev); >> + >> + rpmh_release(pmic->rpmh_client); > > I'm still lost on what

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Wed, Mar 21, 2018 at 12:07 PM, Stephen Boyd wrote: >> +static int rpmh_regulator_remove(struct platform_device *pdev) >> +{ >> + struct rpmh_pmic *pmic = platform_get_drvdata(pdev); >> + >> + rpmh_release(pmic->rpmh_client); > > I'm still lost on what rpmh_client is giving us

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Mar 27, 2018 at 01:51:56PM -0700, Doug Anderson wrote: > Assuming I didn't mess up my analysis, the entire job of of_map_mode() > is to convert from one integer to another. It should take the number > that was specified in the device tree and convert it to a > REGULATOR_MODE_XXX. That

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Mar 27, 2018 at 01:51:56PM -0700, Doug Anderson wrote: > Assuming I didn't mess up my analysis, the entire job of of_map_mode() > is to convert from one integer to another. It should take the number > that was specified in the device tree and convert it to a > REGULATOR_MODE_XXX. That

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Mar 27, 2018 at 04:38:07PM -0700, David Collins wrote: > On 03/27/2018 04:56 AM, Mark Brown wrote: > > I didn't spot this in the code but something called "device tree mode" > > sounds like it's going to be awfully confusing... > As I explained in the earlier email, it makes the device

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Tue, Mar 27, 2018 at 04:38:07PM -0700, David Collins wrote: > On 03/27/2018 04:56 AM, Mark Brown wrote: > > I didn't spot this in the code but something called "device tree mode" > > sounds like it's going to be awfully confusing... > As I explained in the earlier email, it makes the device

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 03/27/2018 04:56 AM, Mark Brown wrote: > On Fri, Mar 23, 2018 at 01:00:47PM -0700, Doug Anderson wrote: >> On Thu, Mar 22, 2018 at 3:31 PM, David Collins >> wrote: > >>> There are two cases that I can think of: 1. Having a set_voltage() >>> callback allows for

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 03/27/2018 04:56 AM, Mark Brown wrote: > On Fri, Mar 23, 2018 at 01:00:47PM -0700, Doug Anderson wrote: >> On Thu, Mar 22, 2018 at 3:31 PM, David Collins >> wrote: > >>> There are two cases that I can think of: 1. Having a set_voltage() >>> callback allows for delaying for an RPMh request

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 03/23/2018 01:00 PM, Doug Anderson wrote: > On Thu, Mar 22, 2018 at 3:31 PM, David Collins > wrote: +static int rpmh_regulator_is_enabled(struct regulator_dev *rdev) +{ + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev); + + return

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On 03/23/2018 01:00 PM, Doug Anderson wrote: > On Thu, Mar 22, 2018 at 3:31 PM, David Collins > wrote: +static int rpmh_regulator_is_enabled(struct regulator_dev *rdev) +{ + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev); + + return vreg->enabled; >>> >>>

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Tue, Mar 27, 2018 at 4:56 AM, Mark Brown wrote: >> > Here is an explanation for why the "device tree mode" abstraction is >> > present in the first place. Between different Qualcomm Technologies, Inc. >> > PMICs, regulators support a subset of the same modes (HPM/PWM,

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Tue, Mar 27, 2018 at 4:56 AM, Mark Brown wrote: >> > Here is an explanation for why the "device tree mode" abstraction is >> > present in the first place. Between different Qualcomm Technologies, Inc. >> > PMICs, regulators support a subset of the same modes (HPM/PWM, AUTO, >> > LPM/PFM,

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Mar 23, 2018 at 01:00:47PM -0700, Doug Anderson wrote: > On Thu, Mar 22, 2018 at 3:31 PM, David Collins > wrote: > > There are two cases that I can think of: 1. Having a set_voltage() > > callback allows for delaying for an RPMh request ACK during certain > >

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Fri, Mar 23, 2018 at 01:00:47PM -0700, Doug Anderson wrote: > On Thu, Mar 22, 2018 at 3:31 PM, David Collins > wrote: > > There are two cases that I can think of: 1. Having a set_voltage() > > callback allows for delaying for an RPMh request ACK during certain > > voltage set point

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Thu, Mar 22 2018 at 19:30 -0600, David Collins wrote: Hello Stephen, Thank you for the very detailed review feedback. On 03/21/2018 12:07 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-16 18:09:10) +static int rpmh_regulator_remove(struct platform_device *pdev) +{ + struct

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

On Thu, Mar 22 2018 at 19:30 -0600, David Collins wrote: Hello Stephen, Thank you for the very detailed review feedback. On 03/21/2018 12:07 PM, Stephen Boyd wrote: Quoting David Collins (2018-03-16 18:09:10) +static int rpmh_regulator_remove(struct platform_device *pdev) +{ + struct

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Thu, Mar 22, 2018 at 3:31 PM, David Collins wrote: >>> +static int rpmh_regulator_is_enabled(struct regulator_dev *rdev) >>> +{ >>> + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev); >>> + >>> + return vreg->enabled; >> >> You can't read hardware? The

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Thu, Mar 22, 2018 at 3:31 PM, David Collins wrote: >>> +static int rpmh_regulator_is_enabled(struct regulator_dev *rdev) >>> +{ >>> + struct rpmh_vreg *vreg = rdev_get_drvdata(rdev); >>> + >>> + return vreg->enabled; >> >> You can't read hardware? The regulator framework

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hello Stephen, Thank you for the very detailed review feedback. On 03/21/2018 12:07 PM, Stephen Boyd wrote: > Quoting David Collins (2018-03-16 18:09:10) >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig >> index 097f617..e0ecd0a 100644 >> --- a/drivers/regulator/Kconfig >>

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hello Stephen, Thank you for the very detailed review feedback. On 03/21/2018 12:07 PM, Stephen Boyd wrote: > Quoting David Collins (2018-03-16 18:09:10) >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig >> index 097f617..e0ecd0a 100644 >> --- a/drivers/regulator/Kconfig >>

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hello Doug, Thank you for the very detailed review feedback. On 03/20/2018 07:16 PM, Doug Anderson wrote: > On Fri, Mar 16, 2018 at 6:09 PM, David Collins > wrote: >> +/** >> + * struct rpmh_regulator_mode - RPMh VRM mode attributes >> + * @pmic_mode:

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hello Doug, Thank you for the very detailed review feedback. On 03/20/2018 07:16 PM, Doug Anderson wrote: > On Fri, Mar 16, 2018 at 6:09 PM, David Collins > wrote: >> +/** >> + * struct rpmh_regulator_mode - RPMh VRM mode attributes >> + * @pmic_mode: Raw PMIC mode value

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Quoting David Collins (2018-03-16 18:09:10) > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > index 097f617..e0ecd0a 100644 > --- a/drivers/regulator/Kconfig > +++ b/drivers/regulator/Kconfig > @@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM > Qualcomm RPM as a

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Quoting David Collins (2018-03-16 18:09:10) > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > index 097f617..e0ecd0a 100644 > --- a/drivers/regulator/Kconfig > +++ b/drivers/regulator/Kconfig > @@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM > Qualcomm RPM as a

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Fri, Mar 16, 2018 at 6:09 PM, David Collins wrote: > +/** > + * struct rpmh_regulator_mode - RPMh VRM mode attributes > + * @pmic_mode: Raw PMIC mode value written into VRM mode > voting > + * register (i.e.

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi, On Fri, Mar 16, 2018 at 6:09 PM, David Collins wrote: > +/** > + * struct rpmh_regulator_mode - RPMh VRM mode attributes > + * @pmic_mode: Raw PMIC mode value written into VRM mode > voting > + * register (i.e. RPMH_REGULATOR_MODE_*) > + *

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi David, Thank you for the patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url:

Re: [PATCH 1/2] regulator: add QCOM RPMh regulator driver

Hi David, Thank you for the patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url:

[PATCH 1/2] regulator: add QCOM RPMh regulator driver

Add the QCOM RPMh regulator driver to manage PMIC regulators which are controlled via RPMh on some Qualcomm Technologies, Inc. SoCs. RPMh is a hardware block which contains several accelerators which are used to manage various hardware resources that are shared between the processors of the SoC.

[PATCH 1/2] regulator: add QCOM RPMh regulator driver

Add the QCOM RPMh regulator driver to manage PMIC regulators which are controlled via RPMh on some Qualcomm Technologies, Inc. SoCs. RPMh is a hardware block which contains several accelerators which are used to manage various hardware resources that are shared between the processors of the SoC.