power: pmic: Add support for Palmas family of PMICs

The ctrl reg contains bit fields to enable and disable regulators, and volt_reg has the bit fields to configure the voltage values. The registers are frequently accessed hence make them part of dm_regulator_uclass_platdata structure.

Signed-off-by Keerthy <[email protected]>

---
  include/power/regulator.h | 2 ++
  1 file changed, 2 insertions(+)

include/power/regulator.h +0 -0

index 9bcd728..57b14a3 100644

Hello Keerthy,

Quoted 15 lines · Keerthy

The ctrl reg contains bit fields to enable and disable regulators, and volt_reg has the bit fields to configure the voltage values. The registers are frequently accessed hence make them part of dm_regulator_uclass_platdata structure.

Signed-off-by Keerthy <[email protected]>

---
  include/power/regulator.h | 2 ++
  1 file changed, 2 insertions(+)

include/power/regulator.h +0 -0

index 9bcd728..57b14a3 100644

include/power/regulator.h struct dm_regulator_uclass_platdata { · line 177

Quoted 7 lines

include/power/regulator.h +2 -0

@@ -171,6 +171,8 @@ struct dm_regulator_uclass_platdata {
      bool boot_on;
      const char *name;
      int flags;
+    u8 ctrl_reg;
+    u8 volt_reg;
  };
    /* Regulator device operations */

This structure above is used for some common "high-level" data, which can be used by regulator uclass driver.

Even if most of PMICs has some ctrl/volt/etc regs, the regulator uclass driver doesn't know, how to use it, so from this point of view it is useless.

But, you can keep device/driver data in a proper fields. Please look at those files:

drivers/power/regulator/fixed.c:119
drivers/power/regulator/pfuze100.c:567

To store some device internal data, you can use:
.platdata_auto_alloc_size -> with access by dev_get_platdata() .priv_auto_alloc_size -> with access by dev_get_priv()

Best regards,

Hi Marczak,

Quoted 36 lines · Przemyslaw Marczak

Hello Keerthy,

Quoted 15 lines · Keerthy

The ctrl reg contains bit fields to enable and disable regulators, and volt_reg has the bit fields to configure the voltage values. The registers are frequently accessed hence make them part of dm_regulator_uclass_platdata structure.

Signed-off-by Keerthy <[email protected]>

---
  include/power/regulator.h | 2 ++
  1 file changed, 2 insertions(+)

include/power/regulator.h +0 -0

index 9bcd728..57b14a3 100644

include/power/regulator.h struct dm_regulator_uclass_platdata { · line 177

Quoted 7 lines

include/power/regulator.h +2 -0

@@ -171,6 +171,8 @@ struct dm_regulator_uclass_platdata {
      bool boot_on;
      const char *name;
      int flags;
+    u8 ctrl_reg;
+    u8 volt_reg;
  };
    /* Regulator device operations */

This structure above is used for some common "high-level" data, which can be used by regulator uclass driver.

Even if most of PMICs has some ctrl/volt/etc regs, the regulator uclass driver doesn't know, how to use it, so from this point of view it is useless.

But, you can keep device/driver data in a proper fields. Please look at those files:

Quoted 6 lines

drivers/power/regulator/fixed.c:119
drivers/power/regulator/pfuze100.c:567

To store some device internal data, you can use:
.platdata_auto_alloc_size -> with access by dev_get_platdata() .priv_auto_alloc_size -> with access by dev_get_priv()

Thanks for a quick review. I did look at some of those options before introducing volt and ctrl here.

Many PMICs will have ctrl/volt registers we might end up having lot of private strutures with the same ctrl/volt fields. I agree uclass driver will not know how to use it.

If i have to draw parallels from the kernel world regulator_desc is a common structure which hosts vsel_reg/enable_reg fields.

Isn't it better to have a common structure instead of having a some platform specific structure that might have the same fields?

Let me know your thoughts on this.

Quoted 2 lines

Best regards,

Hello Keerthy,

Quoted 53 lines · Keerthy

Hi Marczak,

Quoted 36 lines · Przemyslaw Marczak

Hello Keerthy,

Quoted 15 lines · Keerthy

The ctrl reg contains bit fields to enable and disable regulators, and volt_reg has the bit fields to configure the voltage values. The registers are frequently accessed hence make them part of dm_regulator_uclass_platdata structure.

Signed-off-by Keerthy <[email protected]>

---
  include/power/regulator.h | 2 ++
  1 file changed, 2 insertions(+)

include/power/regulator.h +0 -0

index 9bcd728..57b14a3 100644

include/power/regulator.h struct dm_regulator_uclass_platdata { · line 177

Quoted 7 lines

include/power/regulator.h +2 -0

@@ -171,6 +171,8 @@ struct dm_regulator_uclass_platdata {
      bool boot_on;
      const char *name;
      int flags;
+    u8 ctrl_reg;
+    u8 volt_reg;
  };
    /* Regulator device operations */

This structure above is used for some common "high-level" data, which can be used by regulator uclass driver.

Even if most of PMICs has some ctrl/volt/etc regs, the regulator uclass driver doesn't know, how to use it, so from this point of view it is useless.

But, you can keep device/driver data in a proper fields. Please look at those files:

Quoted 6 lines

drivers/power/regulator/fixed.c:119
drivers/power/regulator/pfuze100.c:567

To store some device internal data, you can use:
.platdata_auto_alloc_size -> with access by dev_get_platdata() .priv_auto_alloc_size -> with access by dev_get_priv()

Thanks for a quick review. I did look at some of those options before introducing volt and ctrl here.

Many PMICs will have ctrl/volt registers we might end up having lot of private strutures with the same ctrl/volt fields. I agree uclass driver will not know how to use it.

Quoted 13 lines

If i have to draw parallels from the kernel world regulator_desc is a common structure which hosts vsel_reg/enable_reg fields.

Isn't it better to have a common structure instead of having a some platform specific structure that might have the same fields?

Let me know your thoughts on this.

Quoted 2 lines

Best regards,

You are right and I agree with you that make things common is a good approach.

At the begin of introducing this framework, I just wanted to provide a simple user interface for regulators, so I didn't tried to put all common things into a single structure, because not always it could be useful.

The present structure layout seems to be a good representation of regulator's node in the device-tree.

In the other hand, each driver can provide a static arrays with proper data like reg/mask/etc,
the driver:
drivers/power/regulator/s5m8767.c- is a good example.

I'm not going to break your solution, but let's wait for Simon's opinion.

Best regards,

Add support to bind the regulators/child nodes with the pmic.

Signed-off-by Keerthy <[email protected]>

---
 drivers/power/pmic/Kconfig  |  7 +++++
 drivers/power/pmic/Makefile |  1 +
 drivers/power/pmic/palmas.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
 include/power/palmas.h      | 25 +++++++++++++++
 4 files changed, 110 insertions(+)
 create mode 100644 drivers/power/pmic/palmas.c
 create mode 100644 include/power/palmas.h

The driver provides regulator set/get voltage enable/disable functions for palmas family of PMICs.

Signed-off-by Keerthy <[email protected]>

---
 drivers/power/regulator/Kconfig            |   8 +
 drivers/power/regulator/Makefile           |   1 +
 drivers/power/regulator/palmas_regulator.c | 460 +++++++++++++++++++++++++++++
 3 files changed, 469 insertions(+)
 create mode 100644 drivers/power/regulator/palmas_regulator.c

Quoted 10 lines · Keerthy

The driver provides regulator set/get voltage enable/disable functions for palmas family of PMICs.

Signed-off-by Keerthy <[email protected]>

---
 drivers/power/regulator/Kconfig            |   8 +
 drivers/power/regulator/Makefile           |   1 +
 drivers/power/regulator/palmas_regulator.c | 460 +++++++++++++++++++++++++++++
 3 files changed, 469 insertions(+)
 create mode 100644 drivers/power/regulator/palmas_regulator.c

Reviewed-by Simon Glass <[email protected]>

But please consider using dm_i2c_reg_read()

Also you have two blank lines at one point.