Skip to contentSkip to conversation

Power: pfuze100: support driver model and regulator

PATCH Superseded Peng Fan 4 participants 21 messages Jul 28, 2015 – Aug 4, 2015

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Peng Fan

author
#0 lore raw

cover Power: pfuze100: support driver model and regulator

This patch set is to support driver model for pfuze100 and implement regulator driver for pfuze100. Patches has been tested on i.MX7D validation board.

Here registeres for standby mode are not touched, all operation read/write register are for NORMAL state. For example, to pfuze100, sw1volt is for controlling voltage for normal state, sw1stdby is for controlling voltage for standby state. The driver only read/write sw1volt, but not touch sw1stdby. This will be done later, since I do not come up with a good idea to operate sw1stdby.

This pfuze driver model part and regulator driver use max77686 driver as a reference.

Peng Fan (5):
power: pfuze100 correct SWBST macro definition
power: regulator use node name when no regulator-name
power: pmic: pfuze100 support driver model
power: regulator: add pfuze100 support
fsl: common: pfuze: no use original pfuze code if DM_PMIC

 board/freescale/common/pfuze.c             |   2 +
 drivers/power/pmic/Kconfig                 |   7 +
 drivers/power/pmic/pmic_pfuze100.c         |  89 +++++
 drivers/power/regulator/Kconfig            |   8 +
 drivers/power/regulator/Makefile           |   1 +
 drivers/power/regulator/pfuze100.c         | 596 +++++++++++++++++++++++++++++
 drivers/power/regulator/regulator-uclass.c |   4 +-
 include/power/pfuze100_pmic.h              |  29 +-
 8 files changed, 730 insertions(+), 6 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
Signature · 1 line
1.8.4
author
#1 lore raw

PATCH 1/5 power: pfuze100 correct SWBST macro definition

According to datasheet, SWBST_MODE starts from bit 2 and it occupies 2 bits. So SWBST_MODE_MASK should be 0xC, and SWBST_MODE_xx should be ([mode] << 2).

Cc Przemyslaw Marczak <[email protected]>
Cc Stefano Babic <[email protected]>
---
 include/power/pfuze100_pmic.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
include/power/pfuze100_pmic.h +4 -4
index 57b9ca9..cb10605 100644
@@ -193,11 +193,11 @@ enum {
 #define SWBST_5_15V	3
 
 #define SWBST_VOL_MASK	0x3
-#define SWBST_MODE_MASK	0x6
-#define SWBST_MODE_OFF	(2 << 0)
-#define SWBST_MODE_PFM	(2 << 1)
+#define SWBST_MODE_MASK	0xC
+#define SWBST_MODE_OFF	(0 << 2)
+#define SWBST_MODE_PFM	(1 << 2)
 #define SWBST_MODE_AUTO	(2 << 2)
-#define SWBST_MODE_APS	(2 << 3)
+#define SWBST_MODE_APS	(3 << 2)
 
 /*
  * Regulator Mode Control
Signature · 1 line
1.8.4
#2 lore raw

PATCH 1/5 power: pfuze100 correct SWBST macro definition

Quoted 9 lines · Peng Fan

According to datasheet, SWBST_MODE starts from bit 2 and it occupies 2 bits. So SWBST_MODE_MASK should be 0xC, and SWBST_MODE_xx should be ([mode] << 2).

Cc Przemyslaw Marczak <[email protected]>
Cc Stefano Babic <[email protected]>
---
 include/power/pfuze100_pmic.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
author
#3 lore raw

PATCH 2/5 power: regulator use node name when no regulator-name

In reply to Peng Fan

If there is no property named 'regulator-name' for regulators, choose node name instead, but not directly return failure value.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/regulator/regulator-uclass.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/regulator/regulator-uclass.c +3 -1
index 12e141b..d4f06d5 100644
@@ -256,7 +256,9 @@ static int regulator_post_bind(struct udevice *dev)
 	if (!uc_pdata->name) {
 		debug("%s: dev: %s has no property 'regulator-name'\n",
 		      __func__, dev->name);
-		return -EINVAL;
+		uc_pdata->name = fdt_get_name(blob, offset, NULL);
+		if (!uc_pdata->name)
+			return -EINVAL;
 	}
 
 	if (regulator_name_is_unique(dev, uc_pdata->name))
Signature · 1 line
1.8.4
#4 lore raw

PATCH 2/5 power: regulator use node name when no regulator-name

Hi Peng,

Quoted 26 lines · Peng Fan

If there is no property named 'regulator-name' for regulators, choose node name instead, but not directly return failure value.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/regulator/regulator-uclass.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/regulator/regulator-uclass.c +3 -2
index 12e141b..d4f06d5 100644
@@ -256,7 +256,9 @@ static int regulator_post_bind(struct udevice *dev)
        if (!uc_pdata->name) {
                debug("%s: dev: %s has no property 'regulator-name'\n",
                      __func__, dev->name);
-               return -EINVAL;
+               uc_pdata->name = fdt_get_name(blob, offset, NULL);
+               if (!uc_pdata->name)
+                       return -EINVAL;
        }
 
        if (regulator_name_is_unique(dev, uc_pdata->name))
--
Quoted 3 lines

1.8.4

What is this for please? Can you point me to the device tree binding that needs this? I suspect it is fine, but I would like to understand the purpose.

Regards,
Simon

#5 lore raw

PATCH 2/5 power: regulator use node name when no regulator-name

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Peng,

Quoted 24 lines · Peng Fan

If there is no property named 'regulator-name' for regulators, choose node name instead, but not directly return failure value.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/regulator/regulator-uclass.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/regulator/regulator-uclass.c +3 -1
index 12e141b..d4f06d5 100644
@@ -256,7 +256,9 @@ static int regulator_post_bind(struct udevice *dev)
        if (!uc_pdata->name) {
                debug("%s: dev: %s has no property 'regulator-name'\n",
                      __func__, dev->name);
-               return -EINVAL;
+               uc_pdata->name = fdt_get_name(blob, offset, NULL);
+               if (!uc_pdata->name)
+                       return -EINVAL;
        }
 
Quoted 9 lines
Quoted 5 lines
drivers/power/regulator/regulator-uclass.c +0 -1
        if (regulator_name_is_unique(dev, uc_pdata->name))
--

1.8.4

What is this for please? Can you point me to the device tree binding that needs this? I suspect it is fine, but I would like to understand the purpose.

The regulator device tree is as following:

       regulators {
	   	sw1a_reg: sw1a {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};
	   		
	   		/* use sw1c_reg to align with pfuze100/pfuze200 */
	   	sw1c_reg: sw1b {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};

There is no regulator-name property. From the current regulator framework, if there is no regulator-name property, it will return -EINVAL. So, I did this patch to use node name, if there is no regulator-name property.

Regards,
Peng.

#6 lore raw

PATCH 2/5 power: regulator use node name when no regulator-name

Hello,

Quoted 60 lines · Peng Fan

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Peng,

Quoted 24 lines · Peng Fan

If there is no property named 'regulator-name' for regulators, choose node name instead, but not directly return failure value.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
  drivers/power/regulator/regulator-uclass.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/regulator/regulator-uclass.c +3 -1
index 12e141b..d4f06d5 100644
@@ -256,7 +256,9 @@ static int regulator_post_bind(struct udevice *dev)
         if (!uc_pdata->name) {
                 debug("%s: dev: %s has no property 'regulator-name'\n",
                       __func__, dev->name);
-               return -EINVAL;
+               uc_pdata->name = fdt_get_name(blob, offset, NULL);
+               if (!uc_pdata->name)
+                       return -EINVAL;
         }
 
Quoted 9 lines
Quoted 5 lines
drivers/power/regulator/regulator-uclass.c +0 -1
         if (regulator_name_is_unique(dev, uc_pdata->name))
--

1.8.4

What is this for please? Can you point me to the device tree binding that needs this? I suspect it is fine, but I would like to understand the purpose.

The regulator device tree is as following:

        regulators {
	   	sw1a_reg: sw1a {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};
	   		
	   		/* use sw1c_reg to align with pfuze100/pfuze200 */
	   	sw1c_reg: sw1b {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};
Quoted 7 lines

There is no regulator-name property. From the current regulator framework, if there is no regulator-name property, it will return -EINVAL. So, I did this patch to use node name, if there is no regulator-name property.

Regards,
Peng.

There are few boards in the kernel without the "regulator-name" properties in device tree files, so this is right solution.

Please also update the comments in those files:
- include/power/regulator.h
- doc/device-tree-bindings/regulator/regulator.txt

Best regards,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#7 lore raw

PATCH 2/5 power: regulator use node name when no regulator-name

Hello Przemyslaw,

Quoted 71 lines · Przemyslaw Marczak

Hello,

Quoted 60 lines · Peng Fan

Hi Simon,

Quoted 27 lines · Simon Glass

Hi Peng,

Quoted 24 lines · Peng Fan

If there is no property named 'regulator-name' for regulators, choose node name instead, but not directly return failure value.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/regulator/regulator-uclass.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/power/regulator/regulator-uclass.c +3 -1
index 12e141b..d4f06d5 100644
@@ -256,7 +256,9 @@ static int regulator_post_bind(struct udevice *dev)
        if (!uc_pdata->name) {
                debug("%s: dev: %s has no property 'regulator-name'\n",
                      __func__, dev->name);
-               return -EINVAL;
+               uc_pdata->name = fdt_get_name(blob, offset, NULL);
+               if (!uc_pdata->name)
+                       return -EINVAL;
        }
 
Quoted 9 lines
Quoted 5 lines
drivers/power/regulator/regulator-uclass.c +0 -1
        if (regulator_name_is_unique(dev, uc_pdata->name))
--

1.8.4

What is this for please? Can you point me to the device tree binding that needs this? I suspect it is fine, but I would like to understand the purpose.

The regulator device tree is as following:

       regulators {
	   	sw1a_reg: sw1a {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};
	   		
	   		/* use sw1c_reg to align with pfuze100/pfuze200 */
	   	sw1c_reg: sw1b {
	   		regulator-min-microvolt = <700000>;
	   		regulator-max-microvolt = <1475000>;
	   		regulator-boot-on;
	   		regulator-always-on;
	   		regulator-ramp-delay = <6250>;
	   	};
Quoted 7 lines

There is no regulator-name property. From the current regulator framework, if there is no regulator-name property, it will return -EINVAL. So, I did this patch to use node name, if there is no regulator-name property.

Regards,
Peng.

Quoted 6 lines

There are few boards in the kernel without the "regulator-name" properties in device tree files, so this is right solution.

Please also update the comments in those files:
- include/power/regulator.h
- doc/device-tree-bindings/regulator/regulator.txt

Ok. I'll do this, but I prefer to make the documentation update a single patch.

Regards,
Peng.

Quoted 7 lines

Best regards,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
author
#8 lore raw

PATCH 3/5 power: pmic: pfuze100 support driver model

In reply to Peng Fan

1. Support driver model for pfuze100.
2. Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
3. This driver intends to support PF100, PF200 and PF3000, so add the device id into the udevice_id array.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/pmic/Kconfig         |  7 +++
 drivers/power/pmic/pmic_pfuze100.c | 89 ++++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  3 ++
 3 files changed, 99 insertions(+)
drivers/power/pmic/Kconfig +7 -0
index 164f421..0df91be 100644
@@ -10,6 +10,13 @@ config DM_PMIC
 	- 'drivers/power/pmic/pmic-uclass.c'
 	- 'include/power/pmic.h'
 
+config DM_PMIC_PFUZE100
+	bool "Enable Driver Model for PMIC PFUZE100"
+	depends on DM_PMIC
+	---help---
+	This config enables implementation of driver-model pmic uclass features
+	for PMIC PFUZE100. The driver implements read/write operations.
+
 config DM_PMIC_MAX77686
 	bool "Enable Driver Model for PMIC MAX77686"
 	depends on DM_PMIC
drivers/power/pmic/pmic_pfuze100.c +89 -0

107 lines not on this page · raw message

include/power/pfuze100_pmic.h +3 -0
index cb10605..b13a780 100644
@@ -8,6 +8,9 @@
 #ifndef __PFUZE100_PMIC_H_
 #define __PFUZE100_PMIC_H_
 
+/* Device ID */
+enum {PFUZE100 = 0x10, PFUZE200 = 0x11, PFUZE3000 = 0x30};
+
 /* PFUZE100 registers */
 enum {
 	PFUZE100_DEVICEID	= 0x00,
Signature · 1 line
1.8.4
#9 lore raw

PATCH 3/5 power: pmic: pfuze100 support driver model

Quoted 5 lines · Peng Fan

1. Support driver model for pfuze100.
2. Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
3. This driver intends to support PF100, PF200 and PF3000, so add the device id into the udevice_id array.

Quoted 8 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/pmic/Kconfig         |  7 +++
 drivers/power/pmic/pmic_pfuze100.c | 89 ++++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  3 ++
 3 files changed, 99 insertions(+)
#10 lore raw

PATCH 3/5 power: pmic: pfuze100 support driver model

In reply to Peng Fan

Hello Peng,

I have few comments.

drivers/power/pmic/pmic_pfuze100.c

Quoted 36 lines · Peng Fan

1. Support driver model for pfuze100.
2. Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
3. This driver intends to support PF100, PF200 and PF3000, so add the device id into the udevice_id array.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
  drivers/power/pmic/Kconfig         |  7 +++
  drivers/power/pmic/pmic_pfuze100.c | 89 ++++++++++++++++++++++++++++++++++++++
  include/power/pfuze100_pmic.h      |  3 ++
  3 files changed, 99 insertions(+)
drivers/power/pmic/Kconfig +7 -0
index 164f421..0df91be 100644
@@ -10,6 +10,13 @@ config DM_PMIC
  	- 'drivers/power/pmic/pmic-uclass.c'
  	- 'include/power/pmic.h'
 
+config DM_PMIC_PFUZE100
+	bool "Enable Driver Model for PMIC PFUZE100"
+	depends on DM_PMIC
+	---help---
+	This config enables implementation of driver-model pmic uclass features
+	for PMIC PFUZE100. The driver implements read/write operations.
+
  config DM_PMIC_MAX77686
  	bool "Enable Driver Model for PMIC MAX77686"
  	depends on DM_PMIC
drivers/power/pmic/pmic_pfuze100.c +0 -0
index 22a04c0..be9d05c 100644

Please keep the new convention of file naming and use just pfuze100.c. Then, later we will remove the old files.

Quoted 24 lines
drivers/power/pmic/pmic_pfuze100.c +7 -0
@@ -2,15 +2,22 @@
   * Copyright (C) 2014 Gateworks Corporation
   * Tim Harvey <[email protected]>
   *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc
+ * Peng Fan <[email protected]>
+ *
   * SPDX-License-Identifier:      GPL-2.0+
   */
 
  #include <common.h>
+#include <fdtdec.h>
  #include <errno.h>
+#include <dm.h>
  #include <i2c.h>
  #include <power/pmic.h>
+#include <power/regulator.h>
  #include <power/pfuze100_pmic.h>
 
+#ifndef CONFIG_DM_PMIC
  int power_pfuze100_init(unsigned char bus)
  {
  	static const char name[] = "PFUZE100";

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 44

Quoted 8 lines
drivers/power/pmic/pmic_pfuze100.c +5 -0
@@ -30,3 +37,85 @@ int power_pfuze100_init(unsigned char bus)
 
  	return 0;
  }
+#else
+DECLARE_GLOBAL_DATA_PTR;
+
+static const struct pmic_child_info pmic_children_info[] = {
+	/* sw[x], swbst */

The driver name is used at two places, so could please define it in the header?

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 52

Quoted 8 lines
drivers/power/pmic/pmic_pfuze100.c +8 -0
+	{ .prefix = "s", .driver = "pfuze100_regulator" },
+	/* vgen[x], vsnvs, vcc, v33, vcc_sd */
+	{ .prefix = "v", .driver = "pfuze100_regulator" },
+	{ },
+};
+
+static int pfuze100_reg_count(struct udevice *dev)
+{

This enum name is too general, so please add proper prefix.

Quoted 16 lines
drivers/power/pmic/pmic_pfuze100.c +16 -0
+	return PMIC_NUM_OF_REGS;
+}
+
+static int pfuze100_write(struct udevice *dev, uint reg, const uint8_t *buff,
+			  int len)
+{
+	if (dm_i2c_write(dev, reg, buff, len)) {
+		error("write error to device: %p register: %#x!", dev, reg);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int pfuze100_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
+{

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 78

Quoted 10 lines
drivers/power/pmic/pmic_pfuze100.c +10 -0
+	if (dm_i2c_read(dev, reg, buff, len)) {
+		error("read error from device: %p register: %#x!", dev, reg);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int pfuze100_bind(struct udevice *dev)
+{

Please sort the variables below.

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 82

Quoted 4 lines
drivers/power/pmic/pmic_pfuze100.c +4 -0
+	int regulators_node;
+	const void *blob = gd->fdt_blob;
+	int children;
+

...snip...

Best regards,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#11 lore raw

PATCH 3/5 power: pmic: pfuze100 support driver model

Hello Przemyslaw,

Quoted 36 lines · Przemyslaw Marczak

Hello Peng,

I have few comments.

Quoted 31 lines · Peng Fan

1. Support driver model for pfuze100.
2. Introduce a new Kconfig entry DM_PMIC_PFUZE100 for pfuze100
3. This driver intends to support PF100, PF200 and PF3000, so add the device id into the udevice_id array.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/pmic/Kconfig         |  7 +++
 drivers/power/pmic/pmic_pfuze100.c | 89 ++++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  3 ++
 3 files changed, 99 insertions(+)
drivers/power/pmic/Kconfig +7 -0
index 164f421..0df91be 100644
@@ -10,6 +10,13 @@ config DM_PMIC
 	- 'drivers/power/pmic/pmic-uclass.c'
 	- 'include/power/pmic.h'
 
+config DM_PMIC_PFUZE100
+	bool "Enable Driver Model for PMIC PFUZE100"
+	depends on DM_PMIC
+	---help---
+	This config enables implementation of driver-model pmic uclass features
+	for PMIC PFUZE100. The driver implements read/write operations.
+
 config DM_PMIC_MAX77686
 	bool "Enable Driver Model for PMIC MAX77686"
Quoted 8 lines

drivers/power/pmic/pmic_pfuze100.c

Quoted 5 lines
drivers/power/pmic/Kconfig +0 -0
 	depends on DM_PMIC
drivers/power/pmic/pmic_pfuze100.c +0 -0
index 22a04c0..be9d05c 100644

Please keep the new convention of file naming and use just pfuze100.c. Then, later we will remove the old files.

Ok. Will rename the file to pfuze100.c in V2.

Quoted 26 lines
Quoted 26 lines
drivers/power/pmic/pmic_pfuze100.c +7 -0
@@ -2,15 +2,22 @@
  * Copyright (C) 2014 Gateworks Corporation
  * Tim Harvey <[email protected]>
  *
+ * Copyright (C) 2015 Freescale Semiconductor, Inc
+ * Peng Fan <[email protected]>
+ *
  * SPDX-License-Identifier:      GPL-2.0+
  */
 
 #include <common.h>
+#include <fdtdec.h>
 #include <errno.h>
+#include <dm.h>
 #include <i2c.h>
 #include <power/pmic.h>
+#include <power/regulator.h>
 #include <power/pfuze100_pmic.h>
 
+#ifndef CONFIG_DM_PMIC
 int power_pfuze100_init(unsigned char bus)
 {
 	static const char name[] = "PFUZE100";
@@ -30,3 +37,85 @@ int power_pfuze100_init(unsigned char bus)
 
Quoted 10 lines

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 44

Quoted 7 lines
drivers/power/pmic/pmic_pfuze100.c +5 -0
 	return 0;
 }
+#else
+DECLARE_GLOBAL_DATA_PTR;
+
+static const struct pmic_child_info pmic_children_info[] = {
+	/* sw[x], swbst */

The driver name is used at two places, so could please define it in the header?

Ok. Will fix in V2.

Quoted 11 lines

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 52

Quoted 9 lines
drivers/power/pmic/pmic_pfuze100.c +8 -0
+	{ .prefix = "s", .driver = "pfuze100_regulator" },
+	/* vgen[x], vsnvs, vcc, v33, vcc_sd */
+	{ .prefix = "v", .driver = "pfuze100_regulator" },
+	{ },
+};
+
+static int pfuze100_reg_count(struct udevice *dev)
+{

This enum name is too general, so please add proper prefix.

Will fix this in V2.

Quoted 21 lines
Quoted 21 lines
drivers/power/pmic/pmic_pfuze100.c +20 -0
+	return PMIC_NUM_OF_REGS;
+}
+
+static int pfuze100_write(struct udevice *dev, uint reg, const uint8_t *buff,
+			  int len)
+{
+	if (dm_i2c_write(dev, reg, buff, len)) {
+		error("write error to device: %p register: %#x!", dev, reg);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int pfuze100_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
+{
+	if (dm_i2c_read(dev, reg, buff, len)) {
+		error("read error from device: %p register: %#x!", dev, reg);
+		return -EIO;
+	}
Quoted 8 lines

drivers/power/pmic/pmic_pfuze100.c int power_pfuze100_init(unsigned char bus) · line 78

Quoted 6 lines
drivers/power/pmic/pmic_pfuze100.c +6 -0
+
+	return 0;
+}
+
+static int pfuze100_bind(struct udevice *dev)
+{

Please sort the variables below.

Ok.

[...]

Regards,
Peng.

author
#12 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

In reply to Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
 	for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
 	otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+	bool "Enable Driver Model for REGULATOR PFUZE100"
+	depends on DM_REGULATOR && DM_PMIC_PFUZE100
+	---help---
+	This config enables implementation of driver-model regulator uclass
+	features for REGULATOR PFUZE100. The driver implements get/set api for:
+	value, enable and mode.
+
 config DM_REGULATOR_MAX77686
 	bool "Enable Driver Model for REGULATOR MAX77686"
 	depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +596 -0

596 lines not on this page · raw message

include/power/pfuze100_pmic.h +20 -4
index b13a780..36cc212 100644
@@ -60,6 +60,13 @@ enum {
 	PMIC_NUM_OF_REGS	= 0x7f,
 };
 
+/* Registor offset based on VOLT register */
+#define PFUZE100_VOL_OFFSET	0
+#define PFUZE100_STBY_OFFSET	1
+#define PFUZE100_OFF_OFFSET	2
+#define PFUZE100_MODE_OFFSET	3
+#define PFUZE100_CONF_OFFSET	4
+
 /*
  * Buck Regulators
  */
@@ -136,6 +143,9 @@ enum {
 #define SW1x_STBY_MASK    0x3f
 #define SW1x_OFF_MASK     0x3f
 
+#define SW_MODE_MASK	0xf
+#define SW_MODE_SHIFT	0
+
 #define SW1xCONF_DVSSPEED_MASK 0xc0
 #define SW1xCONF_DVSSPEED_2US  0x00
 #define SW1xCONF_DVSSPEED_4US  0x40
@@ -184,7 +194,12 @@ enum {
 
 #define LDO_VOL_MASK	0xf
 #define LDO_EN		(1 << 4)
+#define LDO_MODE_SHIFT	4
+#define LDO_MODE_MASK	(1 << 4)
+#define LDO_MODE_OFF	0
+#define LDO_MODE_ON	1
 
+#define VREFDDRCON_EN	(1 << 4)
 /*
  * Boost Regulator
  */
@@ -197,10 +212,11 @@ enum {
 
 #define SWBST_VOL_MASK	0x3
 #define SWBST_MODE_MASK	0xC
-#define SWBST_MODE_OFF	(0 << 2)
-#define SWBST_MODE_PFM	(1 << 2)
-#define SWBST_MODE_AUTO	(2 << 2)
-#define SWBST_MODE_APS	(3 << 2)
+#define SWBST_MODE_SHIFT 0x2
+#define SWBST_MODE_OFF	0
+#define SWBST_MODE_PFM	1
+#define SWBST_MODE_AUTO	2
+#define SWBST_MODE_APS	3
 
 /*
  * Regulator Mode Control
Signature · 1 line
1.8.4
#13 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

Hi Peng,

Quoted 9 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Quoted 267 lines
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
        for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
        otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
 config DM_REGULATOR_MAX77686
        bool "Enable Driver Model for REGULATOR MAX77686"
        depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +223 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */

drivers/power/regulator/pfuze100.c line 233

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

drivers/power/regulator/pfuze100.c line 246

Quoted 13 lines
drivers/power/regulator/pfuze100.c new file +13 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

Quoted 43 lines
drivers/power/regulator/pfuze100.c new file +43 -0
+
+       desc = dev_get_platdata(dev);
+
+       switch (dev_get_driver_data(dev_get_parent(dev))) {
+       case PFUZE100:
+               *desc = se_desc(pfuze100_regulators,
+                               ARRAY_SIZE(pfuze100_regulators),
+                               dev->name);
+               break;
+       case PFUZE200:
+               *desc = se_desc(pfuze200_regulators,
+                               ARRAY_SIZE(pfuze200_regulators),
+                               dev->name);
+               break;
+       case PFUZE3000:
+               *desc = se_desc(pfuze3000_regulators,
+                               ARRAY_SIZE(pfuze3000_regulators),
+                               dev->name);
+               break;
+       }
+       if (!*desc) {
+               debug("Do not support regulator %s\n", dev->name);
+               return -EINVAL;
+       }
+
+       uc_pdata = dev_get_uclass_platdata(dev);
+
+       uc_pdata->type = (*desc)->type;
+       if (uc_pdata->type == REGULATOR_TYPE_BUCK) {
+               if (!strcmp(dev->name, "swbst")) {
+                       uc_pdata->mode = pfuze_swbst_modes;
+                       uc_pdata->mode_count = ARRAY_SIZE(pfuze_swbst_modes);
+               } else {
+                       uc_pdata->mode = pfuze_sw_modes;
+                       uc_pdata->mode_count = ARRAY_SIZE(pfuze_sw_modes);
+               }
+       } else if (uc_pdata->type == REGULATOR_TYPE_LDO) {
+               uc_pdata->mode = pfuze_ldo_modes;
+               uc_pdata->mode_count = ARRAY_SIZE(pfuze_ldo_modes);
+       } else {
+               uc_pdata->mode = NULL;
+               uc_pdata->mode_count = 0;
+       }

drivers/power/regulator/pfuze100.c line 299

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+
+       return 0;
+}
+
+static int pfuze100_regulator_mode(struct udevice *dev, int op, int *opmode)
+{
+       unsigned char val;
+       int ret;
+       struct pfuze100_regulator_desc *desc =
+               *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);

You don't actually need the cast.

Quoted 8 lines
drivers/power/regulator/pfuze100.c new file +8 -0
+
+       if (op == PMIC_OP_GET) {
+               if (desc->type == REGULATOR_TYPE_BUCK) {
+                       if (!strcmp(dev->name, "swbst")) {
+                               ret = pmic_read(dev->parent, desc->vsel_reg,
+                                               &val, 1);
+                               if (ret)
+                                       return ret;

drivers/power/regulator/pfuze100.c line 317

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+
+                               val &= SWBST_MODE_MASK;
+                               val >>= SWBST_MODE_SHIFT;
+                               *opmode = val;
+
+                               return 0;
+                       }
+                       ret = pmic_read(dev->parent,
+                                       desc->vsel_reg + PFUZE100_MODE_OFFSET,
+                                       &val, 1);

pmic_reg_read()

Quoted 24 lines
drivers/power/regulator/pfuze100.c new file +24 -0
+                       if (ret)
+                               return ret;
+
+                       val &= SW_MODE_MASK;
+                       val >>= SW_MODE_SHIFT;
+                       *opmode = val;
+
+                       return 0;
+
+               } else if (desc->type == REGULATOR_TYPE_LDO) {
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
+                               return ret;
+
+                       val &= LDO_MODE_MASK;
+                       val >>= LDO_MODE_SHIFT;
+                       *opmode = val;
+
+                       return 0;
+               } else {
+                       return -EINVAL;
+               }
+       }
+

drivers/power/regulator/pfuze100.c line 351

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+       if (desc->type == REGULATOR_TYPE_BUCK) {
+               if (!strcmp(dev->name, "swbst")) {
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
+                               return ret;
+
+                       val &= ~SWBST_MODE_MASK;
+                       val |= *opmode << SWBST_MODE_SHIFT;
+
+                       ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);

pmic_reg_write

Quoted 7 lines
drivers/power/regulator/pfuze100.c new file +7 -0
+                       if (ret)
+                               return ret;
+
+                       return 0;
+               }
+               ret = pmic_read(dev->parent,
+                               desc->vsel_reg + PFUZE100_MODE_OFFSET,

drivers/power/regulator/pfuze100.c line 368

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+                               &val, 1);
+               if (ret)
+                       return ret;
+
+               val &= ~SW_MODE_MASK;
+               val |= *opmode << SW_MODE_SHIFT;
+
+               ret = pmic_write(dev->parent,
+                                desc->vsel_reg + PFUZE100_MODE_OFFSET,
+                                &val, 1);

pmic_clrsetbits() can replace all of this

Quoted 286 lines
drivers/power/regulator/pfuze100.c new file +228 -0
+               if (ret)
+                       return ret;
+
+               return 0;
+       } else if (desc->type == REGULATOR_TYPE_LDO) {
+               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+
+               val &= ~LDO_MODE_MASK;
+               val |= *opmode << LDO_MODE_SHIFT;
+
+               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+
+               return 0;
+       } else {
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
+static int pfuze100_regulator_enable(struct udevice *dev, int op, bool *enable)
+{
+       int ret, on_off;
+       unsigned char val;
+       struct dm_regulator_uclass_platdata *uc_pdata =
+               dev_get_uclass_platdata(dev);
+       if (op == PMIC_OP_GET) {
+               if (!strcmp(dev->name, "vrefddr")) {
+                       ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
+                                       &val, 1);
+                       if (ret)
+                               return ret;
+                       if (val & VREFDDRCON_EN)
+                               *enable = true;
+                       else
+                               *enable = false;
+                       return 0;
+               }
+               ret = pfuze100_regulator_mode(dev, op, &on_off);
+               if (ret)
+                       return ret;
+               switch (on_off) {
+               /* OFF_OFF, SWBST_MODE_OFF, LDO_MODE_OFF have same value */
+               case OFF_OFF:
+                       *enable = false;
+                       break;
+               default:
+                       *enable = true;
+                       break;
+               }
+       } else if (op == PMIC_OP_SET) {
+               if (!strcmp(dev->name, "vrefddr")) {
+                       ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
+                                       &val, 1);
+                       if (ret)
+                               return ret;
+
+                       if (val & VREFDDRCON_EN)
+                               return 0;
+                       val |= VREFDDRCON_EN;
+
+                       return pmic_write(dev->parent, PFUZE100_VREFDDRCON,
+                                         &val, 1);
+               }
+
+               if (uc_pdata->type == REGULATOR_TYPE_LDO) {
+                       on_off = *enable ? LDO_MODE_ON : LDO_MODE_OFF;
+               } else if (uc_pdata->type == REGULATOR_TYPE_BUCK) {
+                       if (!strcmp(dev->name, "swbst"))
+                               on_off = *enable ? SWBST_MODE_AUTO :
+                                       SWBST_MODE_OFF;
+                       else
+                               on_off = *enable ? APS_PFM : OFF_OFF;
+               } else {
+                       return -EINVAL;
+               }
+
+               return pfuze100_regulator_mode(dev, op, &on_off);
+       }
+
+       return 0;
+}
+
+static int pfuze100_regulator_val(struct udevice *dev, int op, int *uV)
+{
+       unsigned char val;
+       int i, ret;
+       struct pfuze100_regulator_desc *desc =
+               *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);
+       struct dm_regulator_uclass_platdata *uc_pdata =
+               dev_get_uclass_platdata(dev);
+
+       if (op == PMIC_OP_GET) {
+               *uV = 0;
+               if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
+                       *uV = desc->voltage;
+               } else if (desc->volt_table) {
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
+                               return ret;
+                       val &= desc->vsel_mask;
+                       *uV = desc->volt_table[val];
+               } else {
+                       if (uc_pdata->min_uV < 0) {
+                               debug("Need to provide min_uV in dts.\n");
+                               return -EINVAL;
+                       }
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
+                               return ret;
+                       val &= desc->vsel_mask;
+                       *uV = uc_pdata->min_uV + (int)val * desc->uV_step;
+               }
+
+               return 0;
+       }
+
+       if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
+               debug("Set voltage for REGULATOR_TYPE_FIXED regulator\n");
+               return -EINVAL;
+       } else if (desc->volt_table) {
+               for (i = 0; i < desc->vsel_mask; i++) {
+                       if (*uV == desc->volt_table[i])
+                               break;
+               }
+               if (i == desc->vsel_mask) {
+                       debug("Unsupported voltage %u\n", *uV);
+                       return -EINVAL;
+               }
+               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+               val &= ~(desc->vsel_mask);
+               val |= i;
+               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+       } else {
+               if (uc_pdata->min_uV < 0) {
+                       debug("Need to provide min_uV in dts.\n");
+                       return -EINVAL;
+               }
+               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+
+               val &= ~(desc->vsel_mask);
+               val |= (*uV - uc_pdata->min_uV) / desc->uV_step;
+
+               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+               if (ret)
+                       return ret;
+       }
+
+       return 0;
+}
+
+static int pfuze100_regulator_get_value(struct udevice *dev)
+{
+       int uV;
+       int ret;
+
+       ret = pfuze100_regulator_val(dev, PMIC_OP_GET, &uV);
+       if (ret)
+               return ret;
+
+       return uV;
+}
+
+static int pfuze100_regulator_set_value(struct udevice *dev, int uV)
+{
+       return pfuze100_regulator_val(dev, PMIC_OP_SET, &uV);
+}
+
+static bool pfuze100_regulator_get_enable(struct udevice *dev)
+{
+       bool enable = false;
+       int ret;
+
+       ret = pfuze100_regulator_enable(dev, PMIC_OP_GET, &enable);
+       if (ret)
+               return ret;
+
+       return enable;
+}
+
+static int pfuze100_regulator_set_enable(struct udevice *dev, bool enable)
+{
+       return pfuze100_regulator_enable(dev, PMIC_OP_SET, &enable);
+}
+
+static int pfuze100_regulator_get_mode(struct udevice *dev)
+{
+       int mode;
+       int ret;
+
+       ret = pfuze100_regulator_mode(dev, PMIC_OP_GET, &mode);
+       if (ret)
+               return ret;
+
+       return mode;
+}
+
+static int pfuze100_regulator_set_mode(struct udevice *dev, int mode)
+{
+       return pfuze100_regulator_mode(dev, PMIC_OP_SET, &mode);
+}
+
+static const struct dm_regulator_ops pfuze100_regulator_ops = {
+       .get_value  = pfuze100_regulator_get_value,
+       .set_value  = pfuze100_regulator_set_value,
+       .get_enable = pfuze100_regulator_get_enable,
+       .set_enable = pfuze100_regulator_set_enable,
+       .get_mode   = pfuze100_regulator_get_mode,
+       .set_mode   = pfuze100_regulator_set_mode,
+};
+
+U_BOOT_DRIVER(pfuze100_regulator) = {
+       .name = "pfuze100_regulator",
+       .id = UCLASS_REGULATOR,
+       .ops = &pfuze100_regulator_ops,
+       .probe = pfuze100_regulator_probe,
+       .platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc **),
+};
include/power/pfuze100_pmic.h +20 -5
index b13a780..36cc212 100644
@@ -60,6 +60,13 @@ enum {
        PMIC_NUM_OF_REGS        = 0x7f,
 };
 
+/* Registor offset based on VOLT register */
+#define PFUZE100_VOL_OFFSET    0
+#define PFUZE100_STBY_OFFSET   1
+#define PFUZE100_OFF_OFFSET    2
+#define PFUZE100_MODE_OFFSET   3
+#define PFUZE100_CONF_OFFSET   4
+
 /*
  * Buck Regulators
  */
@@ -136,6 +143,9 @@ enum {
 #define SW1x_STBY_MASK    0x3f
 #define SW1x_OFF_MASK     0x3f
 
+#define SW_MODE_MASK   0xf
+#define SW_MODE_SHIFT  0
+
 #define SW1xCONF_DVSSPEED_MASK 0xc0
 #define SW1xCONF_DVSSPEED_2US  0x00
 #define SW1xCONF_DVSSPEED_4US  0x40
@@ -184,7 +194,12 @@ enum {
 
 #define LDO_VOL_MASK   0xf
 #define LDO_EN         (1 << 4)
+#define LDO_MODE_SHIFT 4
+#define LDO_MODE_MASK  (1 << 4)
+#define LDO_MODE_OFF   0
+#define LDO_MODE_ON    1
 
+#define VREFDDRCON_EN  (1 << 4)
 /*
  * Boost Regulator
  */
@@ -197,10 +212,11 @@ enum {
 
 #define SWBST_VOL_MASK 0x3
 #define SWBST_MODE_MASK        0xC
-#define SWBST_MODE_OFF (0 << 2)
-#define SWBST_MODE_PFM (1 << 2)
-#define SWBST_MODE_AUTO        (2 << 2)
-#define SWBST_MODE_APS (3 << 2)
+#define SWBST_MODE_SHIFT 0x2
+#define SWBST_MODE_OFF 0
+#define SWBST_MODE_PFM 1
+#define SWBST_MODE_AUTO        2
+#define SWBST_MODE_APS 3
 
 /*
  * Regulator Mode Control
--
Quoted 3 lines

1.8.4

Regards,
Simon

#14 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Peng,

Quoted 6 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Quoted 6 lines
Quoted 3 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Ok. Will fix them in V2.

Quoted 272 lines
Quoted 272 lines
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
        for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
        otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
 config DM_REGULATOR_MAX77686
        bool "Enable Driver Model for REGULATOR MAX77686"
        depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +227 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 233

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

Will fix in V2.

Quoted 16 lines
Quoted 4 lines
drivers/power/regulator/pfuze100.c new file +3 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;

drivers/power/regulator/pfuze100.c line 246

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

hmm. My bad, will fix this.

Quoted 48 lines
Quoted 48 lines
drivers/power/regulator/pfuze100.c new file +47 -0
+
+       desc = dev_get_platdata(dev);
+
+       switch (dev_get_driver_data(dev_get_parent(dev))) {
+       case PFUZE100:
+               *desc = se_desc(pfuze100_regulators,
+                               ARRAY_SIZE(pfuze100_regulators),
+                               dev->name);
+               break;
+       case PFUZE200:
+               *desc = se_desc(pfuze200_regulators,
+                               ARRAY_SIZE(pfuze200_regulators),
+                               dev->name);
+               break;
+       case PFUZE3000:
+               *desc = se_desc(pfuze3000_regulators,
+                               ARRAY_SIZE(pfuze3000_regulators),
+                               dev->name);
+               break;
+       }
+       if (!*desc) {
+               debug("Do not support regulator %s\n", dev->name);
+               return -EINVAL;
+       }
+
+       uc_pdata = dev_get_uclass_platdata(dev);
+
+       uc_pdata->type = (*desc)->type;
+       if (uc_pdata->type == REGULATOR_TYPE_BUCK) {
+               if (!strcmp(dev->name, "swbst")) {
+                       uc_pdata->mode = pfuze_swbst_modes;
+                       uc_pdata->mode_count = ARRAY_SIZE(pfuze_swbst_modes);
+               } else {
+                       uc_pdata->mode = pfuze_sw_modes;
+                       uc_pdata->mode_count = ARRAY_SIZE(pfuze_sw_modes);
+               }
+       } else if (uc_pdata->type == REGULATOR_TYPE_LDO) {
+               uc_pdata->mode = pfuze_ldo_modes;
+               uc_pdata->mode_count = ARRAY_SIZE(pfuze_ldo_modes);
+       } else {
+               uc_pdata->mode = NULL;
+               uc_pdata->mode_count = 0;
+       }
+
+       return 0;
+}
+
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 299

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+static int pfuze100_regulator_mode(struct udevice *dev, int op, int *opmode)
+{
+       unsigned char val;
+       int ret;
+       struct pfuze100_regulator_desc *desc =
+               *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);

You don't actually need the cast.

Will discard the cast.

Quoted 50 lines
Quoted 9 lines
drivers/power/regulator/pfuze100.c new file +8 -0
+
+       if (op == PMIC_OP_GET) {
+               if (desc->type == REGULATOR_TYPE_BUCK) {
+                       if (!strcmp(dev->name, "swbst")) {
+                               ret = pmic_read(dev->parent, desc->vsel_reg,
+                                               &val, 1);
+                               if (ret)
+                                       return ret;

drivers/power/regulator/pfuze100.c line 317

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+
+                               val &= SWBST_MODE_MASK;
+                               val >>= SWBST_MODE_SHIFT;
+                               *opmode = val;
+
+                               return 0;
+                       }
+                       ret = pmic_read(dev->parent,
+                                       desc->vsel_reg + PFUZE100_MODE_OFFSET,
+                                       &val, 1);

pmic_reg_read()

Quoted 28 lines
drivers/power/regulator/pfuze100.c new file +28 -0
+                       if (ret)
+                               return ret;
+
+                       val &= SW_MODE_MASK;
+                       val >>= SW_MODE_SHIFT;
+                       *opmode = val;
+
+                       return 0;
+
+               } else if (desc->type == REGULATOR_TYPE_LDO) {
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
+                               return ret;
+
+                       val &= LDO_MODE_MASK;
+                       val >>= LDO_MODE_SHIFT;
+                       *opmode = val;
+
+                       return 0;
+               } else {
+                       return -EINVAL;
+               }
+       }
+
+       if (desc->type == REGULATOR_TYPE_BUCK) {
+               if (!strcmp(dev->name, "swbst")) {
+                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+                       if (ret)
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 351

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+                               return ret;
+
+                       val &= ~SWBST_MODE_MASK;
+                       val |= *opmode << SWBST_MODE_SHIFT;
+
+                       ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);

pmic_reg_write

Ok.

Quoted 12 lines
Quoted 12 lines
drivers/power/regulator/pfuze100.c new file +11 -0
+                       if (ret)
+                               return ret;
+
+                       return 0;
+               }
+               ret = pmic_read(dev->parent,
+                               desc->vsel_reg + PFUZE100_MODE_OFFSET,
+                               &val, 1);
+               if (ret)
+                       return ret;
+
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 368

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+               val &= ~SW_MODE_MASK;
+               val |= *opmode << SW_MODE_SHIFT;
+
+               ret = pmic_write(dev->parent,
+                                desc->vsel_reg + PFUZE100_MODE_OFFSET,
+                                &val, 1);

pmic_clrsetbits() can replace all of this

ok. Will use this new way.

[...]

Thanks,
Peng.

#15 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

Quoted 301 lines · Peng Fan

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Peng,

Quoted 6 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Quoted 6 lines
Quoted 3 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Ok. Will fix them in V2.

Quoted 272 lines
Quoted 272 lines
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
        for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
        otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
 config DM_REGULATOR_MAX77686
        bool "Enable Driver Model for REGULATOR MAX77686"
        depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +227 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 233

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

Quoted 18 lines

Will fix in V2.

Quoted 16 lines
Quoted 4 lines
drivers/power/regulator/pfuze100.c new file +3 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;

drivers/power/regulator/pfuze100.c line 246

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

After a thought, a single pointer can not make it work.

struct pfuze100_regulator_desc is for per regulator.
If use a single pointer, then "desc = dev_get_platdata(dev);" can not change the platdata for each regulator. I need to use "struct xx **" to reflect the change into platdata for each regulator.

[...]

Regards,
Peng.

#16 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

Hi Peng,

Quoted 321 lines · Peng Fan
Quoted 301 lines · Peng Fan

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Peng,

Quoted 6 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Quoted 6 lines
Quoted 3 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Ok. Will fix them in V2.

Quoted 272 lines
Quoted 272 lines
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
        for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
        otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
 config DM_REGULATOR_MAX77686
        bool "Enable Driver Model for REGULATOR MAX77686"
        depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +227 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 233

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

Quoted 18 lines

Will fix in V2.

Quoted 16 lines
Quoted 4 lines
drivers/power/regulator/pfuze100.c new file +3 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;

drivers/power/regulator/pfuze100.c line 246

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

Quoted 10 lines

After a thought, a single pointer can not make it work.

struct pfuze100_regulator_desc is for per regulator.
If use a single pointer, then "desc = dev_get_platdata(dev);" can not change the platdata for each regulator. I need to use "struct xx **" to reflect the change into platdata for each regulator.

[...]

I see that you a searching your table based the device name. Normally we name the regulators LDO1, LDO2 and use the number (1, 2) to find the right device. Can you please point me to your device tree contents?

You could make the platdata be something like

struct pfuze_platdata {
struct pfuze100_regulator_desc *desc;
};

I think the code would be clearer that way. You can assign to the desc member instead of a double pointer indirect.

Regards,
Simon

#17 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

Hello Simon,

Quoted 346 lines · Simon Glass

Hi Peng,

Quoted 321 lines · Peng Fan
Quoted 301 lines · Peng Fan

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Peng,

Quoted 6 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Quoted 6 lines
Quoted 3 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Ok. Will fix them in V2.

Quoted 272 lines
Quoted 272 lines
---
  drivers/power/regulator/Kconfig    |   8 +
  drivers/power/regulator/Makefile   |   1 +
  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
  include/power/pfuze100_pmic.h      |  24 +-
  4 files changed, 625 insertions(+), 4 deletions(-)

create mode 100644 drivers/power/regulator/pfuze100.c

drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
         for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
         otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
  config DM_REGULATOR_MAX77686
         bool "Enable Driver Model for REGULATOR MAX77686"
         depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
  obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
  obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
  obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
  obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +227 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 233

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

Quoted 18 lines

Will fix in V2.

Quoted 16 lines
Quoted 4 lines
drivers/power/regulator/pfuze100.c new file +3 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;

drivers/power/regulator/pfuze100.c line 246

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

Quoted 10 lines

After a thought, a single pointer can not make it work.

struct pfuze100_regulator_desc is for per regulator.
If use a single pointer, then "desc = dev_get_platdata(dev);" can not change the platdata for each regulator. I need to use "struct xx **" to reflect the change into platdata for each regulator.

[...]

I see that you a searching your table based the device name. Normally we name the regulators LDO1, LDO2 and use the number (1, 2) to find the right device. Can you please point me to your device tree contents?

You could make the platdata be something like

struct pfuze_platdata {
    struct pfuze100_regulator_desc *desc;
};
Quoted 6 lines

I think the code would be clearer that way. You can assign to the desc member instead of a double pointer indirect.

Regards,
Simon

I think, that we can use a single pointer for this, because function se_desc() returns a pointer to the single descriptor, for each regulator device. Please look at my comments to this patch.

Best regards,

Signature · 4 lines
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
#18 lore raw

PATCH 4/5 power: regulator: add pfuze100 support

In reply to Simon Glass
Quoted 331 lines · Simon Glass

Hi Peng,

Quoted 328 lines · Peng Fan
Quoted 301 lines · Peng Fan

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Peng,

Quoted 6 lines · Peng Fan

1. Add new regulator driver pfuze100.
* Introduce struct pfuze100_regulator_desc for mataining info for regulator.
2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
3. This driver intends to support PF100, PF200 and PF3000.
4. Add related macro definition in pfuze header file.

Quoted 6 lines
Quoted 3 lines
Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>

It looks correct but I have code style comments - see below. They all apply globally.

Ok. Will fix them in V2.

Quoted 272 lines
Quoted 272 lines
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c
drivers/power/regulator/Kconfig +8 -0
index 6289b83..b854773 100644
@@ -16,6 +16,14 @@ config DM_REGULATOR
        for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node()
        otherwise. Detailed information can be found in the header file.
 
+config DM_REGULATOR_PFUZE100
+       bool "Enable Driver Model for REGULATOR PFUZE100"
+       depends on DM_REGULATOR && DM_PMIC_PFUZE100
+       ---help---
+       This config enables implementation of driver-model regulator uclass
+       features for REGULATOR PFUZE100. The driver implements get/set api for:
+       value, enable and mode.
+
 config DM_REGULATOR_MAX77686
        bool "Enable Driver Model for REGULATOR MAX77686"
        depends on DM_REGULATOR && DM_PMIC_MAX77686
drivers/power/regulator/Makefile +1 -0
index 96aa624..9f8f17b 100644
@@ -7,5 +7,6 @@
 
 obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
 obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
+obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
 obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
 obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
drivers/power/regulator/pfuze100.c new file +227 -0
new file mode 100644
index 0000000..6c513e9
@@ -0,0 +1,596 @@
+#include <common.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <dm.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <power/regulator.h>
+#include <power/pfuze100_pmic.h>
+
+/**
+ * struct pfuze100_regulator_desc - regulator descriptor
+ *
+ * @name: Identify name for the regulator.
+ * @type: Indicates the regulator type.
+ * @uV_step: Voltage increase for each selector.
+ * @vsel_reg: Register for adjust regulator voltage for normal.
+ * @vsel_mask: Mask bit for setting regulator voltage for normal.
+ * @stby_reg: Register for adjust regulator voltage for standby.
+ * @stby_mask: Mask bit for setting regulator voltage for standby.
+ * @volt_table: Voltage mapping table (if table based mapping).
+ * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
+ */
+struct pfuze100_regulator_desc {
+       char *name;
+       enum regulator_type type;
+       unsigned int uV_step;
+       unsigned int vsel_reg;
+       unsigned int vsel_mask;
+       unsigned int stby_reg;
+       unsigned int stby_mask;
+       unsigned int *volt_table;
+       unsigned int voltage;
+};
+
+#define PFUZE100_FIXED_REG(_name, base, vol)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_FIXED,           \
+               .voltage        =       (vol),                          \
+       }
+
+#define PFUZE100_SW_REG(_name, base, step)                             \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x3F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x3F,                           \
+       }
+
+#define PFUZE100_SWB_REG(_name, base, mask, step, voltages)            \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_SNVS_REG(_name, base, mask, voltages)                 \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_OTHER,           \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       (mask),                         \
+               .volt_table     =       (voltages),                     \
+       }
+
+#define PFUZE100_VGEN_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+       }
+
+#define PFUZE3000_VCC_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_LDO,             \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base),                         \
+               .vsel_mask      =       0x3,                            \
+               .stby_reg       =       (base),                         \
+               .stby_mask      =       0x20,                           \
+}
+
+#define PFUZE3000_SW1_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x1F,                           \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x1F,                           \
+       }
+
+#define PFUZE3000_SW2_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0x7,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0x7,                            \
+       }
+
+#define PFUZE3000_SW3_REG(_name, base, step)                           \
+       {                                                               \
+               .name           =       #_name,                         \
+               .type           =       REGULATOR_TYPE_BUCK,            \
+               .uV_step        =       (step),                         \
+               .vsel_reg       =       (base) + PFUZE100_VOL_OFFSET,   \
+               .vsel_mask      =       0xF,                            \
+               .stby_reg       =       (base) + PFUZE100_STBY_OFFSET,  \
+               .stby_mask      =       0xF,                            \
+       }
+
+static unsigned int pfuze100_swbst[] = {
+       5000000, 5050000, 5100000, 5150000
+};
+
+static unsigned int pfuze100_vsnvs[] = {
+       1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
+};
+
+static unsigned int pfuze3000_vsnvs[] = {
+       -1, -1, -1, -1, -1, -1, 3000000, -1
+};
+
+static unsigned int pfuze3000_sw2lo[] = {
+       1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000, 1850000
+};
+
+/* PFUZE100 */
+static struct pfuze100_regulator_desc pfuze100_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE200 */
+static struct pfuze100_regulator_desc pfuze200_regulators[] = {
+       PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
+       PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
+       PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
+       PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
+       PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
+       PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
+       PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
+};
+
+/* PFUZE3000 */
+static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
+       PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
+       PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
+       PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000, pfuze3000_sw2lo),
+       PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
+       PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000, pfuze100_swbst),
+       PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
+       PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
+       PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
+       PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
+       PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
+       PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
+       PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
+       PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
+};
+
+#define MODE(_id, _val, _name) { \
+       .id = _id, \
+       .register_value = _val, \
+       .name = _name, \
+}
+
+/* SWx Buck regulator mode */
+static struct dm_regulator_mode pfuze_sw_modes[] = {
+       MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
+       MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
+       MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
+       MODE(APS_OFF, APS_OFF, "APS_OFF"),
+       MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
+       MODE(PWM_APS, PWM_APS, "PWM_APS"),
+       MODE(APS_APS, APS_APS, "APS_APS"),
+       MODE(APS_PFM, APS_PFM, "APS_PFM"),
+       MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
+};
+
+/* Boost Buck regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_swbst_modes[] = {
+       MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
+       MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
+       MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
+       MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
+};
+
+/* VGENx LDO regulator mode for normal operation */
+static struct dm_regulator_mode pfuze_ldo_modes[] = {
+       MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
+       MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
+};
Quoted 8 lines

drivers/power/regulator/pfuze100.c line 233

Quoted 6 lines
drivers/power/regulator/pfuze100.c new file +6 -0
+
+static struct pfuze100_regulator_desc *se_desc(struct pfuze100_regulator_desc *desc,
+                                              int size,
+                                              const char *name)
+{
+       int i;

blank line between declarations and rest - please fix global

Quoted 18 lines

Will fix in V2.

Quoted 16 lines
Quoted 4 lines
drivers/power/regulator/pfuze100.c new file +3 -0
+       for (i = 0; i < size; desc++) {
+               if (!strcmp(desc->name, name))
+                       return desc;

drivers/power/regulator/pfuze100.c line 246

Quoted 10 lines
drivers/power/regulator/pfuze100.c new file +10 -0
+               continue;
+       }
+
+       return NULL;
+}
+
+static int pfuze100_regulator_probe(struct udevice *dev)
+{
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       struct pfuze100_regulator_desc **desc = NULL;

Can't this just be a single pointer?

After a thought, a single pointer can not make it work.

struct pfuze100_regulator_desc is for per regulator.
If use a single pointer, then "desc = dev_get_platdata(dev);" can not change the platdata for each regulator. I need to use "struct xx **" to reflect the change into platdata for each regulator.

Quoted 8 lines
Quoted 3 lines

[...]

I see that you a searching your table based the device name. Normally we name the regulators LDO1, LDO2 and use the number (1, 2) to find the right device. Can you please point me to your device tree contents?

I replied in v2 patch set. Still paste url here,
http://lxr.free-electrons.com/source/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
line from 208 to 306.

Quoted 9 lines

You could make the platdata be something like

struct pfuze_platdata {
struct pfuze100_regulator_desc *desc;
};

I think the code would be clearer that way. You can assign to the desc member instead of a double pointer indirect.

Thanks. Will consider to use this way.

Regards,
Peng.

author
#19 lore raw

PATCH 5/5 fsl: common: pfuze: no use original pfuze code if DM_PMIC

In reply to Peng Fan

If enable DM PMIC and REGULATOR, we should not use original power framework. So need to comment out the pfuze code for original power framework, when CONFIG_DM_PMIC_PFUZE100 defined.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
Cc Stefano Babic <[email protected]>
---
 board/freescale/common/pfuze.c | 2 ++
 1 file changed, 2 insertions(+)
board/freescale/common/pfuze.c +2 -0
index d6a209e..783c46d 100644
@@ -9,6 +9,7 @@
 #include <power/pmic.h>
 #include <power/pfuze100_pmic.h>
 
+#ifndef CONFIG_DM_PMIC_PFUZE100
 int pfuze_mode_init(struct pmic *p, u32 mode)
 {
 	unsigned char offset, i, switch_num;
@@ -90,3 +91,4 @@ struct pmic *pfuze_common_init(unsigned char i2cbus)
 
 	return p;
 }
+#endif
Signature · 1 line
1.8.4
#20 lore raw

PATCH 5/5 fsl: common: pfuze: no use original pfuze code if DM_PMIC

Quoted 11 lines · Peng Fan

If enable DM PMIC and REGULATOR, we should not use original power framework. So need to comment out the pfuze code for original power framework, when CONFIG_DM_PMIC_PFUZE100 defined.

Cc Przemyslaw Marczak <[email protected]>
Cc Simon Glass <[email protected]>
Cc Stefano Babic <[email protected]>
---
 board/freescale/common/pfuze.c | 2 ++
 1 file changed, 2 insertions(+)