Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
used to keep the driver common which handles multiple regulators.
Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
---
.../bindings/regulator/mps,mpq2286.yaml | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
diff --git a/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml b/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
new file mode 100644
index 000000000000..594b929fe4b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/mps,mpq2286.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Monolithic Power System MPQ2286 PMIC
+
+maintainers:
+ - Saravanan Sekar <saravanan@linumiz.com>
+
+properties:
+ compatible:
+ enum:
+ - mps,mpq2286
+
+ reg:
+ maxItems: 1
+
+ regulators:
+ type: object
+
+ properties:
+ buck0:
+ type: object
+ $ref: regulator.yaml#
+
+ unevaluatedProperties: false
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@3 {
+ compatible = "mps,mpq2286";
+ reg = <0x3>;
+
+ regulators {
+ buck0 {
+ regulator-name = "buck0";
+ regulator-min-microvolt = <1600000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ };
+ };
+ };
+ };
+...
--
2.34.1On Sat, Oct 07, 2023 at 10:28:02PM +0530, Saravanan Sekar wrote:
> Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
> used to keep the driver common which handles multiple regulators.
Sorry for the maybe dumb question, but where can I find the driver
depencency on buck naming ?
Thanks,
Guenter
>
> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
> ---
> .../bindings/regulator/mps,mpq2286.yaml | 59 +++++++++++++++++++
> 1 file changed, 59 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml b/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
> new file mode 100644
> index 000000000000..594b929fe4b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mps,mpq2286.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Monolithic Power System MPQ2286 PMIC
> +
> +maintainers:
> + - Saravanan Sekar <saravanan@linumiz.com>
> +
> +properties:
> + compatible:
> + enum:
> + - mps,mpq2286
> +
> + reg:
> + maxItems: 1
> +
> + regulators:
> + type: object
> +
> + properties:
> + buck0:
> + type: object
> + $ref: regulator.yaml#
> +
> + unevaluatedProperties: false
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - regulators
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@3 {
> + compatible = "mps,mpq2286";
> + reg = <0x3>;
> +
> + regulators {
> + buck0 {
> + regulator-name = "buck0";
> + regulator-min-microvolt = <1600000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-boot-on;
> + };
> + };
> + };
> + };
> +...
> --
> 2.34.1
>
On 08/10/2023 03:20, Guenter Roeck wrote:
> On Sat, Oct 07, 2023 at 10:28:02PM +0530, Saravanan Sekar wrote:
>> Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
>> used to keep the driver common which handles multiple regulators.
>
> Sorry for the maybe dumb question, but where can I find the driver
> depencency on buck naming ?
I guess it is because:
PMBUS_REGULATOR_STEP("buck", 0, MPQ7932_N_VOLTAGES,
creates regulator name as buck+id (so buck0).
However as Rob pointed out, driver can handle buck for this variant. We
want the bindings to be independent from Linux implementation and this
patch here adds such dependency.
Therefore I still believe this should be just "buck", unless the
datasheet says it is "buck0". Which I doubt.
Best regards,
Krzysztof
On Sun, Oct 08, 2023 at 12:40:29PM +0200, Krzysztof Kozlowski wrote:
> On 08/10/2023 03:20, Guenter Roeck wrote:
> > On Sat, Oct 07, 2023 at 10:28:02PM +0530, Saravanan Sekar wrote:
> >> Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
> >> used to keep the driver common which handles multiple regulators.
> >
> > Sorry for the maybe dumb question, but where can I find the driver
> > depencency on buck naming ?
>
> I guess it is because:
> PMBUS_REGULATOR_STEP("buck", 0, MPQ7932_N_VOLTAGES,
> creates regulator name as buck+id (so buck0).
>
Ah, good point. Problem here is that this is already kind of common,
even though the use of "buckX" isn't. Look for "vout0", or
'PMBUS_REGULATOR("vout", 0)'. Apparently so far no one took offence
if a regulator was named "vout0" even if "vout1" didn't exist.
I don't really have a good solution right now, but I guess we'll need
a second set of macros for the single-regulator case, or maybe generate
struct regulator_desc arrays using a function. I'll have to explore
options.
Please let me know how you want the subsystem to handle existing
single-channel regulators with numbered regulator name.
Saravanan - for this driver please just declare a local driver-specific
variant of the PMBUS_REGULATOR_STEP() macro which doesn't use indexing,
use it to initialise a second regulators_desc array, and use that second
array for mpq2286. That is a bit messy, but acceptable for now until
there is a more generic solution (unless of course you have an idea for
one and want to implement it, but that is not a requirement).
Thanks,
Guenter
On 08/10/23 19:22, Guenter Roeck wrote:
> On Sun, Oct 08, 2023 at 12:40:29PM +0200, Krzysztof Kozlowski wrote:
>> On 08/10/2023 03:20, Guenter Roeck wrote:
>>> On Sat, Oct 07, 2023 at 10:28:02PM +0530, Saravanan Sekar wrote:
>>>> Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
>>>> used to keep the driver common which handles multiple regulators.
>>>
>>> Sorry for the maybe dumb question, but where can I find the driver
>>> depencency on buck naming ?
>>
>> I guess it is because:
>> PMBUS_REGULATOR_STEP("buck", 0, MPQ7932_N_VOLTAGES,
>> creates regulator name as buck+id (so buck0).
>>
>
> Ah, good point. Problem here is that this is already kind of common,
> even though the use of "buckX" isn't. Look for "vout0", or
> 'PMBUS_REGULATOR("vout", 0)'. Apparently so far no one took offence
> if a regulator was named "vout0" even if "vout1" didn't exist.
>
> I don't really have a good solution right now, but I guess we'll need
> a second set of macros for the single-regulator case, or maybe generate
> struct regulator_desc arrays using a function. I'll have to explore
> options.
>
> Please let me know how you want the subsystem to handle existing
> single-channel regulators with numbered regulator name.
>
> Saravanan - for this driver please just declare a local driver-specific
> variant of the PMBUS_REGULATOR_STEP() macro which doesn't use indexing,
> use it to initialise a second regulators_desc array, and use that second
> array for mpq2286. That is a bit messy, but acceptable for now until
> there is a more generic solution (unless of course you have an idea for
> one and want to implement it, but that is not a requirement).
Hello Guenter,
Thanks for your proposal as intermediate fix local declaration of macro,
could you please suggest whether below changes is acceptable as workaround?
+++ b/drivers/hwmon/pmbus/mpq7932.c
@@ -118,6 +118,12 @@ static int mpq7932_probe(struct i2c_client *client)
#if IS_ENABLED(CONFIG_SENSORS_MPQ7932_REGULATOR)
info->num_regulators = info->pages;
info->reg_desc = mpq7932_regulators_desc;
+
+ if (info->num_regulators == 1) {
+ mpq7932_regulators_desc->name = "buck";
+ mpq7932_regulators_desc->of_match = of_match_ptr("buck");
+ }
+
>
> Thanks,
> Guenter
Thanks,
Saravanan
On Wed, Oct 11, 2023 at 12:02:44PM +0530, Saravanan Sekar wrote:
> On 08/10/23 19:22, Guenter Roeck wrote:
> > On Sun, Oct 08, 2023 at 12:40:29PM +0200, Krzysztof Kozlowski wrote:
> > > On 08/10/2023 03:20, Guenter Roeck wrote:
> > > > On Sat, Oct 07, 2023 at 10:28:02PM +0530, Saravanan Sekar wrote:
> > > > > Document mpq2286 power-management IC. Instead of simple 'buck', 'buck0' is
> > > > > used to keep the driver common which handles multiple regulators.
> > > >
> > > > Sorry for the maybe dumb question, but where can I find the driver
> > > > depencency on buck naming ?
> > >
> > > I guess it is because:
> > > PMBUS_REGULATOR_STEP("buck", 0, MPQ7932_N_VOLTAGES,
> > > creates regulator name as buck+id (so buck0).
> > >
> >
> > Ah, good point. Problem here is that this is already kind of common,
> > even though the use of "buckX" isn't. Look for "vout0", or
> > 'PMBUS_REGULATOR("vout", 0)'. Apparently so far no one took offence
> > if a regulator was named "vout0" even if "vout1" didn't exist.
> >
> > I don't really have a good solution right now, but I guess we'll need
> > a second set of macros for the single-regulator case, or maybe generate
> > struct regulator_desc arrays using a function. I'll have to explore
> > options.
> >
> > Please let me know how you want the subsystem to handle existing
> > single-channel regulators with numbered regulator name.
> >
> > Saravanan - for this driver please just declare a local driver-specific
> > variant of the PMBUS_REGULATOR_STEP() macro which doesn't use indexing,
> > use it to initialise a second regulators_desc array, and use that second
> > array for mpq2286. That is a bit messy, but acceptable for now until
> > there is a more generic solution (unless of course you have an idea for
> > one and want to implement it, but that is not a requirement).
> Hello Guenter,
>
> Thanks for your proposal as intermediate fix local declaration of macro,
> could you please suggest whether below changes is acceptable as workaround?
>
No, because that would overwrite a data structure which might be needed
by another mpq7932 in the system. mpq7932_regulators_desc should really
be declared const to clarify that it is not supposed to be changed.
Thinking more about it, the solution is actually quite simple. Please add
a second patch adding PMBUS_REGULATOR_STEP_ONE() and PMBUS_REGULATOR_ONE()
macros to drivers/hwmon/pmbus/pmbus.h and use the new macro in this patch.
That would result in code such as
static const struct regulator_desc mpq7932_regulators_desc_one[] = {
PMBUS_REGULATOR_STEP_ONE("buck", MPQ7932_N_VOLTAGES,
MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
};
...
if (info->num_regulators == 1)
info->reg_desc = mpq7932_regulators_desc_one;
else
info->reg_desc = mpq7932_regulators_desc;
We can then use the xxx_ONE macros when adding regulator support to existing
or new drivers, and either keep existing drivers as-is or update them based
on DT maintainer input.
Thanks,
Guenter
© 2016 - 2026 Red Hat, Inc.