[PATCH 0/3] Add support for mpq2286 PMIC IC

[PATCH 0/3] Add support for mpq2286 PMIC IC

[Saravanan Sekar]

The MPQ2286 is a programmable, high frequency synchronous buck regulator with
integrated internal high side and low side power MOSFET. Application in
Automotive compenents such as ADAS, Infotainment, SOC System core, DDR memory.

Saravanan Sekar (3):
  hwmon: (pmbus/mpq7932) Get page count based on chip info
  dt-bindings: regulator: Add mps,mpq2286 power-management IC
  hwmon: (pmbus/mpq2286) Add a support for mpq2286 Power Management IC

 .../bindings/regulator/mps,mpq2286.yaml       | 59 +++++++++++++++++++
 drivers/hwmon/pmbus/mpq7932.c                 |  9 ++-
 2 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/mps,mpq2286.yaml

-- 
2.34.1

[PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info

[Saravanan Sekar]

Get page count using compatible match to support the series of chipsets
which differs in number of regualator/page.

Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
---
 drivers/hwmon/pmbus/mpq7932.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c
index 865d42edda1a..af3e5e9590c8 100644
--- a/drivers/hwmon/pmbus/mpq7932.c
+++ b/drivers/hwmon/pmbus/mpq7932.c
@@ -105,7 +105,7 @@ static int mpq7932_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	info = &data->info;
-	info->pages = MPQ7932_NUM_PAGES;
+	info->pages = (int)device_get_match_data(&client->dev);
 	info->format[PSC_VOLTAGE_OUT] = direct;
 	info->m[PSC_VOLTAGE_OUT] = 160;
 	info->b[PSC_VOLTAGE_OUT] = -33;
@@ -115,7 +115,7 @@ static int mpq7932_probe(struct i2c_client *client)
 	}
 
 #if IS_ENABLED(CONFIG_SENSORS_MPQ7932_REGULATOR)
-	info->num_regulators = ARRAY_SIZE(mpq7932_regulators_desc);
+	info->num_regulators = info->pages;
 	info->reg_desc = mpq7932_regulators_desc;
 #endif
 
@@ -129,7 +129,7 @@ static int mpq7932_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id mpq7932_of_match[] = {
-	{ .compatible = "mps,mpq7932"},
+	{ .compatible = "mps,mpq7932", .data = (void *)MPQ7932_NUM_PAGES },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mpq7932_of_match);
-- 
2.34.1

[PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Saravanan Sekar]

Document mpq2286 power-management IC

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..d00d887870a9
--- /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 {
+                buck {
+                    regulator-name = "buck0";
+                    regulator-min-microvolt = <1600000>;
+                    regulator-max-microvolt = <1800000>;
+                    regulator-boot-on;
+                };
+            };
+        };
+    };
+...
-- 
2.34.1

[PATCH 3/3] hwmon: (pmbus/mpq2286) Add a support for mpq2286 Power Management IC

[Saravanan Sekar]

The MPQ2286 is a programmable, high frequency synchronous buck regulator
designed to power a variety of Automotive system peripherals. Single buck
converters with hardware monitoring capability is configurable over PMBus
interface.

Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
---
 drivers/hwmon/pmbus/mpq7932.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c
index af3e5e9590c8..3ffeece28e2d 100644
--- a/drivers/hwmon/pmbus/mpq7932.c
+++ b/drivers/hwmon/pmbus/mpq7932.c
@@ -21,6 +21,7 @@
 #define MPQ7932_N_VOLTAGES		256
 #define MPQ7932_VOUT_MAX		0xFF
 #define MPQ7932_NUM_PAGES		6
+#define MPQ2286_NUM_PAGES		1
 
 #define MPQ7932_TON_DELAY		0x60
 #define MPQ7932_VOUT_STARTUP_SLEW	0xA3
@@ -130,12 +131,14 @@ static int mpq7932_probe(struct i2c_client *client)
 
 static const struct of_device_id mpq7932_of_match[] = {
 	{ .compatible = "mps,mpq7932", .data = (void *)MPQ7932_NUM_PAGES },
+	{ .compatible = "mps,mpq2286", .data = (void *)MPQ2286_NUM_PAGES },
 	{},
 };
 MODULE_DEVICE_TABLE(of, mpq7932_of_match);
 
 static const struct i2c_device_id mpq7932_id[] = {
 	{ "mpq7932", },
+	{ "mpq2286", },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, mpq7932_id);
-- 
2.34.1

Re: [PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Rob Herring]

On Mon, 11 Sep 2023 09:11:49 +0530, Saravanan Sekar wrote:
> Document mpq2286 power-management IC
> 
> 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
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/mps,mpq2286.example.dtb: pmic@3: regulators: 'buck' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/regulator/mps,mpq2286.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230911034150.181880-3-saravanan@linumiz.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/3] hwmon: (pmbus/mpq2286) Add a support for mpq2286 Power Management IC

[Guenter Roeck]

On 9/10/23 20:41, Saravanan Sekar wrote:
> The MPQ2286 is a programmable, high frequency synchronous buck regulator
> designed to power a variety of Automotive system peripherals. Single buck
> converters with hardware monitoring capability is configurable over PMBus
> interface.
> 
> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
> ---
>   drivers/hwmon/pmbus/mpq7932.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c
> index af3e5e9590c8..3ffeece28e2d 100644
> --- a/drivers/hwmon/pmbus/mpq7932.c
> +++ b/drivers/hwmon/pmbus/mpq7932.c
> @@ -21,6 +21,7 @@
>   #define MPQ7932_N_VOLTAGES		256
>   #define MPQ7932_VOUT_MAX		0xFF
>   #define MPQ7932_NUM_PAGES		6
> +#define MPQ2286_NUM_PAGES		1
>   
>   #define MPQ7932_TON_DELAY		0x60
>   #define MPQ7932_VOUT_STARTUP_SLEW	0xA3
> @@ -130,12 +131,14 @@ static int mpq7932_probe(struct i2c_client *client)
>   
>   static const struct of_device_id mpq7932_of_match[] = {
>   	{ .compatible = "mps,mpq7932", .data = (void *)MPQ7932_NUM_PAGES },
> +	{ .compatible = "mps,mpq2286", .data = (void *)MPQ2286_NUM_PAGES },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, mpq7932_of_match);
>   
>   static const struct i2c_device_id mpq7932_id[] = {
>   	{ "mpq7932", },
> +	{ "mpq2286", },

Please keep alphabetic order.

>   	{ },
>   };
>   MODULE_DEVICE_TABLE(i2c, mpq7932_id);

This is one of those super-secret automotive chips where almost no information
is available to the public. I'll need authoritative confirmation that all the
various parameters (b. m, output voltage format, minimum and maximum output
voltage, step size) match mpq7932 exactly. That is rarely the case, so consider
me skeptic.

Guenter

Re: [PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info

[Guenter Roeck]

On 9/10/23 20:41, Saravanan Sekar wrote:
> Get page count using compatible match to support the series of chipsets
> which differs in number of regualator/page.
> 
> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
> ---
>   drivers/hwmon/pmbus/mpq7932.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/mpq7932.c b/drivers/hwmon/pmbus/mpq7932.c
> index 865d42edda1a..af3e5e9590c8 100644
> --- a/drivers/hwmon/pmbus/mpq7932.c
> +++ b/drivers/hwmon/pmbus/mpq7932.c
> @@ -105,7 +105,7 @@ static int mpq7932_probe(struct i2c_client *client)
>   		return -ENOMEM;
>   
>   	info = &data->info;
> -	info->pages = MPQ7932_NUM_PAGES;
> +	info->pages = (int)device_get_match_data(&client->dev);

I kind of doubt that this works without generating a compiler warning.

Also, does this work with CONFIG_OF=n ?

Guenter

>   	info->format[PSC_VOLTAGE_OUT] = direct;
>   	info->m[PSC_VOLTAGE_OUT] = 160;
>   	info->b[PSC_VOLTAGE_OUT] = -33;
> @@ -115,7 +115,7 @@ static int mpq7932_probe(struct i2c_client *client)
>   	}
>   
>   #if IS_ENABLED(CONFIG_SENSORS_MPQ7932_REGULATOR)
> -	info->num_regulators = ARRAY_SIZE(mpq7932_regulators_desc);
> +	info->num_regulators = info->pages;
>   	info->reg_desc = mpq7932_regulators_desc;
>   #endif
>   
> @@ -129,7 +129,7 @@ static int mpq7932_probe(struct i2c_client *client)
>   }
>   
>   static const struct of_device_id mpq7932_of_match[] = {
> -	{ .compatible = "mps,mpq7932"},
> +	{ .compatible = "mps,mpq7932", .data = (void *)MPQ7932_NUM_PAGES },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, mpq7932_of_match);

Re: [PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Guenter Roeck]

On 9/10/23 20:41, Saravanan Sekar wrote:
> Document mpq2286 power-management IC
> 
> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>

I don't know how this kind of thing is typically handled in .yaml
files, but given that the chip is supposedly almost the same as MPQ7932
(except for the number of regulators), would it possibly make sense
to describe both in a single devicetree file ?

Guenter

> ---
>   .../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..d00d887870a9
> --- /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 {
> +                buck {
> +                    regulator-name = "buck0";
> +                    regulator-min-microvolt = <1600000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-boot-on;
> +                };
> +            };
> +        };
> +    };
> +...

Re: [PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info

[kernel test robot]

Hi Saravanan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on broonie-regulator/for-next linus/master v6.6-rc1 next-20230911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Saravanan-Sekar/hwmon-pmbus-mpq7932-Get-page-count-based-on-chip-info/20230911-114451
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230911034150.181880-2-saravanan%40linumiz.com
patch subject: [PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230911/202309111345.BXkwu3pD-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230911/202309111345.BXkwu3pD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309111345.BXkwu3pD-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/hwmon/pmbus/mpq7932.c: In function 'mpq7932_probe':
>> drivers/hwmon/pmbus/mpq7932.c:108:23: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     108 |         info->pages = (int)device_get_match_data(&client->dev);
         |                       ^


vim +108 drivers/hwmon/pmbus/mpq7932.c

    95	
    96	static int mpq7932_probe(struct i2c_client *client)
    97	{
    98		struct mpq7932_data *data;
    99		struct pmbus_driver_info *info;
   100		struct device *dev = &client->dev;
   101		int i;
   102	
   103		data = devm_kzalloc(dev, sizeof(struct mpq7932_data), GFP_KERNEL);
   104		if (!data)
   105			return -ENOMEM;
   106	
   107		info = &data->info;
 > 108		info->pages = (int)device_get_match_data(&client->dev);
   109		info->format[PSC_VOLTAGE_OUT] = direct;
   110		info->m[PSC_VOLTAGE_OUT] = 160;
   111		info->b[PSC_VOLTAGE_OUT] = -33;
   112		for (i = 0; i < info->pages; i++) {
   113			info->func[i] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
   114					| PMBUS_HAVE_STATUS_TEMP;
   115		}
   116	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Krzysztof Kozlowski]

On 11/09/2023 05:41, Saravanan Sekar wrote:
> Document mpq2286 power-management IC
> 
> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. It's: regulator: dt-bindings:

> ---
>  .../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..d00d887870a9
> --- /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":

You did not test it... Sigh. Anyway, there is no need for entire
regulators node for one regulator. Can the device do anything else than
being a regulator?

Best regards,
Krzysztof

Re: [PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info

[kernel test robot]

Hi Saravanan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on broonie-regulator/for-next linus/master v6.6-rc1 next-20230911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Saravanan-Sekar/hwmon-pmbus-mpq7932-Get-page-count-based-on-chip-info/20230911-114451
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230911034150.181880-2-saravanan%40linumiz.com
patch subject: [PATCH 1/3] hwmon: (pmbus/mpq7932) Get page count based on chip info
config: x86_64-randconfig-005-20230911 (https://download.01.org/0day-ci/archive/20230911/202309111406.BSKHGe9U-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230911/202309111406.BSKHGe9U-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309111406.BSKHGe9U-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/mpq7932.c:108:16: warning: cast to smaller integer type 'int' from 'const void *' [-Wvoid-pointer-to-int-cast]
           info->pages = (int)device_get_match_data(&client->dev);
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +108 drivers/hwmon/pmbus/mpq7932.c

    95	
    96	static int mpq7932_probe(struct i2c_client *client)
    97	{
    98		struct mpq7932_data *data;
    99		struct pmbus_driver_info *info;
   100		struct device *dev = &client->dev;
   101		int i;
   102	
   103		data = devm_kzalloc(dev, sizeof(struct mpq7932_data), GFP_KERNEL);
   104		if (!data)
   105			return -ENOMEM;
   106	
   107		info = &data->info;
 > 108		info->pages = (int)device_get_match_data(&client->dev);
   109		info->format[PSC_VOLTAGE_OUT] = direct;
   110		info->m[PSC_VOLTAGE_OUT] = 160;
   111		info->b[PSC_VOLTAGE_OUT] = -33;
   112		for (i = 0; i < info->pages; i++) {
   113			info->func[i] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
   114					| PMBUS_HAVE_STATUS_TEMP;
   115		}
   116	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Saravanan Sekar]

On 11/09/23 11:56, Krzysztof Kozlowski wrote:
> On 11/09/2023 05:41, Saravanan Sekar wrote:
>> Document mpq2286 power-management IC
>>
>> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. It's: regulator: dt-bindings:

Thanks for your time to review. git log has mix of "regulator: 
dt-bindings" and "dt-bindings: regualtor". I had referred my own 
accepted driver regulator/mps,mpq7932.yaml

> 
>> ---
>>   .../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..d00d887870a9
>> --- /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":
> 
> You did not test it... Sigh. Anyway, there is no need for entire
> regulators node for one regulator. Can the device do anything else than
> being a regulator?
> 

I tested it, but documentation is not updated with test findings (buck0).

Other chipset has multiple regulator so dts has regulators node to keep 
the driver common

Primarily device is a regulator with pmbus capability like fault status 
(OV,OC, VIN, VOUT, Power good status), temperature status.

> Best regards,
> Krzysztof
>
Thanks,
Saravanan

Re: [PATCH 2/3] dt-bindings: regulator: Add mps,mpq2286 power-management IC

[Krzysztof Kozlowski]

On 11/09/2023 09:12, Saravanan Sekar wrote:
> On 11/09/23 11:56, Krzysztof Kozlowski wrote:
>> On 11/09/2023 05:41, Saravanan Sekar wrote:
>>> Document mpq2286 power-management IC
>>>
>>> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching. It's: regulator: dt-bindings:
> 
> Thanks for your time to review. git log has mix of "regulator: 
> dt-bindings" and "dt-bindings: regualtor".

Just take a look how many times each appear... they are just few cases
of the latter.


> I had referred my own 
> accepted driver regulator/mps,mpq7932.yaml
> 
>>
>>> ---
>>>   .../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..d00d887870a9
>>> --- /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":
>>
>> You did not test it... Sigh. Anyway, there is no need for entire
>> regulators node for one regulator. Can the device do anything else than
>> being a regulator?
>>
> 
> I tested it, but documentation is not updated with test findings (buck0).

I mean, testing bindings. You did not test this patch. Testing this
patch produces warnings, so this is considered a failed test.

> 
> Other chipset has multiple regulator so dts has regulators node to keep 
> the driver common
> 
> Primarily device is a regulator with pmbus capability like fault status 
> (OV,OC, VIN, VOUT, Power good status), temperature status.

OK, then regulators node seems fine. Anyway, drop quotes and test this
patch (which means: test the bindings).

Best regards,
Krzysztof

Re: [PATCH 3/3] hwmon: (pmbus/mpq2286) Add a support for mpq2286 Power Management IC

[Saravanan Sekar]

On 11/09/23 09:53, Guenter Roeck wrote:
> On 9/10/23 20:41, Saravanan Sekar wrote:
>> The MPQ2286 is a programmable, high frequency synchronous buck regulator
>> designed to power a variety of Automotive system peripherals. Single buck
>> converters with hardware monitoring capability is configurable over PMBus
>> interface.
>>
>> Signed-off-by: Saravanan Sekar <saravanan@linumiz.com>
>> ---
>>   drivers/hwmon/pmbus/mpq7932.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/hwmon/pmbus/mpq7932.c 
>> b/drivers/hwmon/pmbus/mpq7932.c
>> index af3e5e9590c8..3ffeece28e2d 100644
>> --- a/drivers/hwmon/pmbus/mpq7932.c
>> +++ b/drivers/hwmon/pmbus/mpq7932.c
>> @@ -21,6 +21,7 @@
>>   #define MPQ7932_N_VOLTAGES        256
>>   #define MPQ7932_VOUT_MAX        0xFF
>>   #define MPQ7932_NUM_PAGES        6
>> +#define MPQ2286_NUM_PAGES        1
>>   #define MPQ7932_TON_DELAY        0x60
>>   #define MPQ7932_VOUT_STARTUP_SLEW    0xA3
>> @@ -130,12 +131,14 @@ static int mpq7932_probe(struct i2c_client *client)
>>   static const struct of_device_id mpq7932_of_match[] = {
>>       { .compatible = "mps,mpq7932", .data = (void *)MPQ7932_NUM_PAGES },
>> +    { .compatible = "mps,mpq2286", .data = (void *)MPQ2286_NUM_PAGES },
>>       {},
>>   };
>>   MODULE_DEVICE_TABLE(of, mpq7932_of_match);
>>   static const struct i2c_device_id mpq7932_id[] = {
>>       { "mpq7932", },
>> +    { "mpq2286", },
> 
> Please keep alphabetic order.

agree, will change in v2

> 
>>       { },
>>   };
>>   MODULE_DEVICE_TABLE(i2c, mpq7932_id);
> 
> This is one of those super-secret automotive chips where almost no 
> information
> is available to the public. I'll need authoritative confirmation that 
> all the
> various parameters (b. m, output voltage format, minimum and maximum output
> voltage, step size) match mpq7932 exactly. That is rarely the case, so 
> consider
> me skeptic.

Thanks for your time for review. yes, datasheet is not available in 
public but Monolithic shall share on request. I confirm all the 
parameters are match with mpq7932 datasheet and I have tested the device.

> 
> Guenter
> 

Thanks,
Saravanan