[PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant

devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant
       [not found] <20230407141813.89-1-wiagn233@outlook.com>
@ 2023-04-07 14:18 ` Shengyu Qu
  2023-04-10 17:27   ` Krzysztof Kozlowski
  2023-04-07 14:18 ` [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC Shengyu Qu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Shengyu Qu @ 2023-04-07 14:18 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel
  Cc: Shengyu Qu

The X-Powers AXP15060 is a PMIC seen on Starfive Visionfive 2 board. Add
relative compatible item and CPUSLDO support for it.

Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
---
 Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
index b7a8747d5fa0..e150a8bf040a 100644
--- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
+++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
@@ -92,6 +92,7 @@ properties:
           - x-powers,axp806
           - x-powers,axp809
           - x-powers,axp813
+          - x-powers,axp15060
       - items:
           - const: x-powers,axp228
           - const: x-powers,axp221
@@ -260,7 +261,7 @@ properties:
           Defines the work frequency of DC-DC in kHz.
 
     patternProperties:
-      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":
+      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|cpusldo|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":
         $ref: /schemas/regulator/regulator.yaml#
         type: object
         unevaluatedProperties: false
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant
  2023-04-07 14:18 ` [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant Shengyu Qu
@ 2023-04-10 17:27   ` Krzysztof Kozlowski
  2023-04-10 17:43     ` Shengyu Qu
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-04-10 17:27 UTC (permalink / raw)
  To: Shengyu Qu, lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood,
	broonie, devicetree, linux-kernel

On 07/04/2023 16:18, Shengyu Qu wrote:
> The X-Powers AXP15060 is a PMIC seen on Starfive Visionfive 2 board. Add
> relative compatible item and CPUSLDO support for it.
> 
> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> ---
>  Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 3 ++-

I received cover letter disattached from rest of patchset and everything
duplicate. Please check your process of sending patches.

>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> index b7a8747d5fa0..e150a8bf040a 100644
> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> @@ -92,6 +92,7 @@ properties:
>            - x-powers,axp806
>            - x-powers,axp809
>            - x-powers,axp813
> +          - x-powers,axp15060

So you have chosen order by absolute number, not alphabetically. That's
fine, but keep it for the future entries as well. :)



>        - items:
>            - const: x-powers,axp228
>            - const: x-powers,axp221
> @@ -260,7 +261,7 @@ properties:
>            Defines the work frequency of DC-DC in kHz.
>  
>      patternProperties:
> -      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":
> +      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|cpusldo|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":

It seems fixed strings are at the end of entire pattern, so "cpusldo"
should be before "drivevbus".



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant
  2023-04-10 17:27   ` Krzysztof Kozlowski
@ 2023-04-10 17:43     ` Shengyu Qu
  0 siblings, 0 replies; 14+ messages in thread
From: Shengyu Qu @ 2023-04-10 17:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski, lee, robh+dt, krzysztof.kozlowski+dt, wens,
	lgirdwood, broonie, devicetree, linux-kernel

Hello,

> On 07/04/2023 16:18, Shengyu Qu wrote:
>> The X-Powers AXP15060 is a PMIC seen on Starfive Visionfive 2 board. Add
>> relative compatible item and CPUSLDO support for it.
>>
>> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 3 ++-
> 
> I received cover letter disattached from rest of patchset and everything
> duplicate. Please check your process of sending patches.
Duplicate is because first batch is disattached and a small typo. I thought
disattach was my fault, but the second batch with re-generated patch file also 
has this problem... git send-email reports everything correctly so I don't know
what really happens to my series..
> 
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> index b7a8747d5fa0..e150a8bf040a 100644
>> --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
>> @@ -92,6 +92,7 @@ properties:
>>            - x-powers,axp806
>>            - x-powers,axp809
>>            - x-powers,axp813
>> +          - x-powers,axp15060
> 
> So you have chosen order by absolute number, not alphabetically. That's
> fine, but keep it for the future entries as well. :)
OK :)
> 
> 
> 
>>        - items:
>>            - const: x-powers,axp228
>>            - const: x-powers,axp221
>> @@ -260,7 +261,7 @@ properties:
>>            Defines the work frequency of DC-DC in kHz.
>>  
>>      patternProperties:
>> -      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":
>> +      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|cpusldo|(dc1)?sw|rtc(_|-)ldo|drivevbus|dc5ldo)$":
> 
> It seems fixed strings are at the end of entire pattern, so "cpusldo"
> should be before "drivevbus".
Thanks, would fix in next version.
> 
> 
> 
> Best regards,
> Krzysztof
> 
Best regards,
Shengyu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC
       [not found] <20230407141813.89-1-wiagn233@outlook.com>
  2023-04-07 14:18 ` [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant Shengyu Qu
@ 2023-04-07 14:18 ` Shengyu Qu
  2023-04-17 14:05   ` Andre Przywara
  2023-04-07 14:18 ` [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support Shengyu Qu
  2023-04-07 14:18 ` [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists Shengyu Qu
  3 siblings, 1 reply; 14+ messages in thread
From: Shengyu Qu @ 2023-04-07 14:18 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel
  Cc: Shengyu Qu

The AXP15060 is a PMIC chip produced by X-Powers, and could be connected
via an I2C bus.

Describe the regmap and the MFD bits, along with the registers exposed
via I2C. Eventually advertise the device using a new compatible string
and add support for power off the system.

The driver would disable PEK function if IRQ is not configured in device
tree, since some boards (For example, Starfive Visionfive 2) didn't
connect IRQ line of PMIC to SOC.

GPIO function isn't enabled in this commit, since its configuration
operation is different from any existing AXP PMICs and needs
logic modification on existing driver. GPIO support might come in later
patches.

Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
---
 drivers/mfd/axp20x-i2c.c   |  2 +
 drivers/mfd/axp20x.c       | 90 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/axp20x.h | 85 +++++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+)

diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
index f49fbd307958..b4f5cb457117 100644
--- a/drivers/mfd/axp20x-i2c.c
+++ b/drivers/mfd/axp20x-i2c.c
@@ -65,6 +65,7 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
 	{ .compatible = "x-powers,axp223", .data = (void *)AXP223_ID },
 	{ .compatible = "x-powers,axp803", .data = (void *)AXP803_ID },
 	{ .compatible = "x-powers,axp806", .data = (void *)AXP806_ID },
+	{ .compatible = "x-powers,axp15060", .data = (void *)AXP15060_ID },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
@@ -78,6 +79,7 @@ static const struct i2c_device_id axp20x_i2c_id[] = {
 	{ "axp223", 0 },
 	{ "axp803", 0 },
 	{ "axp806", 0 },
+	{ "axp15060", 0 },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 01a6bbb6d266..42ec27a4dfc4 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -43,6 +43,7 @@ static const char * const axp20x_model_names[] = {
 	"AXP806",
 	"AXP809",
 	"AXP813",
+	"AXP15060",
 };
 
 static const struct regmap_range axp152_writeable_ranges[] = {
@@ -168,6 +169,31 @@ static const struct regmap_access_table axp806_volatile_table = {
 	.n_yes_ranges	= ARRAY_SIZE(axp806_volatile_ranges),
 };
 
+static const struct regmap_range axp15060_writeable_ranges[] = {
+	regmap_reg_range(AXP15060_PWR_OUT_CTRL1, AXP15060_DCDC_MODE_CTRL2),
+	regmap_reg_range(AXP15060_OUTPUT_MONITOR_DISCHARGE, AXP15060_CPUSLDO_V_CTRL),
+	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
+	regmap_reg_range(AXP15060_PEK_KEY, AXP15060_PEK_KEY),
+	regmap_reg_range(AXP15060_IRQ1_EN, AXP15060_IRQ2_EN),
+	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
+};
+
+static const struct regmap_range axp15060_volatile_ranges[] = {
+	regmap_reg_range(AXP15060_STARTUP_SRC, AXP15060_STARTUP_SRC),
+	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
+	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
+};
+
+static const struct regmap_access_table axp15060_writeable_table = {
+	.yes_ranges	= axp15060_writeable_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp15060_writeable_ranges),
+};
+
+static const struct regmap_access_table axp15060_volatile_table = {
+	.yes_ranges	= axp15060_volatile_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp15060_volatile_ranges),
+};
+
 static const struct resource axp152_pek_resources[] = {
 	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
 	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
@@ -236,6 +262,11 @@ static const struct resource axp809_pek_resources[] = {
 	DEFINE_RES_IRQ_NAMED(AXP809_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
 };
 
+static const struct resource axp15060_pek_resources[] = {
+	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
+	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
+};
+
 static const struct regmap_config axp152_regmap_config = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
@@ -281,6 +312,15 @@ static const struct regmap_config axp806_regmap_config = {
 	.cache_type	= REGCACHE_RBTREE,
 };
 
+static const struct regmap_config axp15060_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.wr_table	= &axp15060_writeable_table,
+	.volatile_table	= &axp15060_volatile_table,
+	.max_register	= AXP15060_IRQ2_STATE,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
 #define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask)			\
 	[_variant##_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
 
@@ -502,6 +542,23 @@ static const struct regmap_irq axp809_regmap_irqs[] = {
 	INIT_REGMAP_IRQ(AXP809, GPIO0_INPUT,		4, 0),
 };
 
+static const struct regmap_irq axp15060_regmap_irqs[] = {
+	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV1,	0, 0),
+	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV2,	0, 1),
+	INIT_REGMAP_IRQ(AXP15060, DCDC1_V_LOW,		0, 2),
+	INIT_REGMAP_IRQ(AXP15060, DCDC2_V_LOW,		0, 3),
+	INIT_REGMAP_IRQ(AXP15060, DCDC3_V_LOW,		0, 4),
+	INIT_REGMAP_IRQ(AXP15060, DCDC4_V_LOW,		0, 5),
+	INIT_REGMAP_IRQ(AXP15060, DCDC5_V_LOW,		0, 6),
+	INIT_REGMAP_IRQ(AXP15060, DCDC6_V_LOW,		0, 7),
+	INIT_REGMAP_IRQ(AXP15060, PEK_LONG,			1, 0),
+	INIT_REGMAP_IRQ(AXP15060, PEK_SHORT,			1, 1),
+	INIT_REGMAP_IRQ(AXP15060, GPIO1_INPUT,		1, 2),
+	INIT_REGMAP_IRQ(AXP15060, PEK_FAL_EDGE,			1, 3),
+	INIT_REGMAP_IRQ(AXP15060, PEK_RIS_EDGE,			1, 4),
+	INIT_REGMAP_IRQ(AXP15060, GPIO2_INPUT,		1, 5),
+};
+
 static const struct regmap_irq_chip axp152_regmap_irq_chip = {
 	.name			= "axp152_irq_chip",
 	.status_base		= AXP152_IRQ1_STATE,
@@ -581,6 +638,17 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
 	.num_regs		= 5,
 };
 
+static const struct regmap_irq_chip axp15060_regmap_irq_chip = {
+	.name			= "axp15060",
+	.status_base		= AXP15060_IRQ1_STATE,
+	.ack_base		= AXP15060_IRQ1_STATE,
+	.unmask_base		= AXP15060_IRQ1_EN,
+	.init_ack_masked	= true,
+	.irqs			= axp15060_regmap_irqs,
+	.num_irqs		= ARRAY_SIZE(axp15060_regmap_irqs),
+	.num_regs		= 2,
+};
+
 static const struct mfd_cell axp20x_cells[] = {
 	{
 		.name		= "axp20x-gpio",
@@ -825,6 +893,16 @@ static const struct mfd_cell axp813_cells[] = {
 	},
 };
 
+static const struct mfd_cell axp15060_cells[] = {
+	{
+		.name		= "axp221-pek",
+		.num_resources	= ARRAY_SIZE(axp15060_pek_resources),
+		.resources	= axp15060_pek_resources,
+	}, {
+		.name		= "axp20x-regulator",
+	},
+};
+
 static int axp20x_power_off(struct sys_off_data *data)
 {
 	struct axp20x_dev *axp20x = data->cb_data;
@@ -934,6 +1012,18 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
 		 */
 		axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
 		break;
+	case AXP15060_ID:
+		/* Don't register the power key part if there is no interrupt line. */
+		if (axp20x->irq > 0) {
+			axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
+			axp20x->cells = axp15060_cells;
+		} else {
+			axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
+			axp20x->cells = axp806_cells;
+		}
+		axp20x->regmap_cfg = &axp15060_regmap_config;
+		axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
+		break;
 	default:
 		dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x->variant);
 		return -EINVAL;
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 2058194807bd..abc2bdc54bf5 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -21,6 +21,7 @@ enum axp20x_variants {
 	AXP806_ID,
 	AXP809_ID,
 	AXP813_ID,
+	AXP15060_ID,
 	NR_AXP20X_VARIANTS,
 };
 
@@ -131,6 +132,39 @@ enum axp20x_variants {
 /* Other DCDC regulator control registers are the same as AXP803 */
 #define AXP813_DCDC7_V_OUT		0x26
 
+#define AXP15060_STARTUP_SRC		0x00
+#define AXP15060_PWR_OUT_CTRL1		0x10
+#define AXP15060_PWR_OUT_CTRL2		0x11
+#define AXP15060_PWR_OUT_CTRL3		0x12
+#define AXP15060_DCDC1_V_CTRL		0x13
+#define AXP15060_DCDC2_V_CTRL		0x14
+#define AXP15060_DCDC3_V_CTRL		0x15
+#define AXP15060_DCDC4_V_CTRL		0x16
+#define AXP15060_DCDC5_V_CTRL		0x17
+#define AXP15060_DCDC6_V_CTRL		0x18
+#define AXP15060_ALDO1_V_CTRL		0x19
+#define AXP15060_DCDC_MODE_CTRL1		0x1a
+#define AXP15060_DCDC_MODE_CTRL2		0x1b
+#define AXP15060_OUTPUT_MONITOR_DISCHARGE		0x1e
+#define AXP15060_IRQ_PWROK_VOFF		0x1f
+#define AXP15060_ALDO2_V_CTRL		0x20
+#define AXP15060_ALDO3_V_CTRL		0x21
+#define AXP15060_ALDO4_V_CTRL		0x22
+#define AXP15060_ALDO5_V_CTRL		0x23
+#define AXP15060_BLDO1_V_CTRL		0x24
+#define AXP15060_BLDO2_V_CTRL		0x25
+#define AXP15060_BLDO3_V_CTRL		0x26
+#define AXP15060_BLDO4_V_CTRL		0x27
+#define AXP15060_BLDO5_V_CTRL		0x28
+#define AXP15060_CLDO1_V_CTRL		0x29
+#define AXP15060_CLDO2_V_CTRL		0x2a
+#define AXP15060_CLDO3_V_CTRL		0x2b
+#define AXP15060_CLDO4_V_CTRL		0x2d
+#define AXP15060_CPUSLDO_V_CTRL		0x2e
+#define AXP15060_PWR_WAKEUP_CTRL		0x31
+#define AXP15060_PWR_DISABLE_DOWN_SEQ		0x32
+#define AXP15060_PEK_KEY		0x36
+
 /* Interrupt */
 #define AXP152_IRQ1_EN			0x40
 #define AXP152_IRQ2_EN			0x41
@@ -139,6 +173,11 @@ enum axp20x_variants {
 #define AXP152_IRQ2_STATE		0x49
 #define AXP152_IRQ3_STATE		0x4a
 
+#define AXP15060_IRQ1_EN		0x40
+#define AXP15060_IRQ2_EN		0x41
+#define AXP15060_IRQ1_STATE		0x48
+#define AXP15060_IRQ2_STATE		0x49
+
 #define AXP20X_IRQ1_EN			0x40
 #define AXP20X_IRQ2_EN			0x41
 #define AXP20X_IRQ3_EN			0x42
@@ -222,6 +261,8 @@ enum axp20x_variants {
 #define AXP22X_GPIO_STATE		0x94
 #define AXP22X_GPIO_PULL_DOWN		0x95
 
+#define AXP15060_CLDO4_GPIO2_MODESET		0x2c
+
 /* Battery */
 #define AXP20X_CHRG_CC_31_24		0xb0
 #define AXP20X_CHRG_CC_23_16		0xb1
@@ -419,6 +460,33 @@ enum {
 	AXP813_REG_ID_MAX,
 };
 
+enum {
+	AXP15060_DCDC1 = 0,
+	AXP15060_DCDC2,
+	AXP15060_DCDC3,
+	AXP15060_DCDC4,
+	AXP15060_DCDC5,
+	AXP15060_DCDC6,
+	AXP15060_ALDO1,
+	AXP15060_ALDO2,
+	AXP15060_ALDO3,
+	AXP15060_ALDO4,
+	AXP15060_ALDO5,
+	AXP15060_BLDO1,
+	AXP15060_BLDO2,
+	AXP15060_BLDO3,
+	AXP15060_BLDO4,
+	AXP15060_BLDO5,
+	AXP15060_CLDO1,
+	AXP15060_CLDO2,
+	AXP15060_CLDO3,
+	AXP15060_CLDO4,
+	AXP15060_CPUSLDO,
+	AXP15060_SW,
+	AXP15060_RTC_LDO,
+	AXP15060_REG_ID_MAX,
+};
+
 /* IRQs */
 enum {
 	AXP152_IRQ_LDO0IN_CONNECT = 1,
@@ -637,6 +705,23 @@ enum axp809_irqs {
 	AXP809_IRQ_GPIO0_INPUT,
 };
 
+enum axp15060_irqs {
+	AXP15060_IRQ_DIE_TEMP_HIGH_LV1 = 1,
+	AXP15060_IRQ_DIE_TEMP_HIGH_LV2,
+	AXP15060_IRQ_DCDC1_V_LOW,
+	AXP15060_IRQ_DCDC2_V_LOW,
+	AXP15060_IRQ_DCDC3_V_LOW,
+	AXP15060_IRQ_DCDC4_V_LOW,
+	AXP15060_IRQ_DCDC5_V_LOW,
+	AXP15060_IRQ_DCDC6_V_LOW,
+	AXP15060_IRQ_PEK_LONG,
+	AXP15060_IRQ_PEK_SHORT,
+	AXP15060_IRQ_GPIO1_INPUT,
+	AXP15060_IRQ_PEK_FAL_EDGE,
+	AXP15060_IRQ_PEK_RIS_EDGE,
+	AXP15060_IRQ_GPIO2_INPUT,
+};
+
 struct axp20x_dev {
 	struct device			*dev;
 	int				irq;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC
  2023-04-07 14:18 ` [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC Shengyu Qu
@ 2023-04-17 14:05   ` Andre Przywara
  2023-04-17 16:21     ` Shengyu Qu
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2023-04-17 14:05 UTC (permalink / raw)
  To: Shengyu Qu
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka, martin.botka1

On Fri,  7 Apr 2023 22:18:11 +0800
Shengyu Qu <wiagn233@outlook.com> wrote:

Hi Shengyu,

thanks for doing the tedious work and sending this!

> The AXP15060 is a PMIC chip produced by X-Powers, and could be connected
> via an I2C bus.
> 
> Describe the regmap and the MFD bits, along with the registers exposed
> via I2C. Eventually advertise the device using a new compatible string
> and add support for power off the system.
> 
> The driver would disable PEK function if IRQ is not configured in device
> tree, since some boards (For example, Starfive Visionfive 2) didn't
> connect IRQ line of PMIC to SOC.
> 
> GPIO function isn't enabled in this commit, since its configuration
> operation is different from any existing AXP PMICs and needs
> logic modification on existing driver. GPIO support might come in later
> patches.
> 
> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> ---
>  drivers/mfd/axp20x-i2c.c   |  2 +
>  drivers/mfd/axp20x.c       | 90 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/axp20x.h | 85 +++++++++++++++++++++++++++++++++++
>  3 files changed, 177 insertions(+)
> 
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index f49fbd307958..b4f5cb457117 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -65,6 +65,7 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
>  	{ .compatible = "x-powers,axp223", .data = (void *)AXP223_ID },
>  	{ .compatible = "x-powers,axp803", .data = (void *)AXP803_ID },
>  	{ .compatible = "x-powers,axp806", .data = (void *)AXP806_ID },
> +	{ .compatible = "x-powers,axp15060", .data = (void *)AXP15060_ID },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
> @@ -78,6 +79,7 @@ static const struct i2c_device_id axp20x_i2c_id[] = {
>  	{ "axp223", 0 },
>  	{ "axp803", 0 },
>  	{ "axp806", 0 },
> +	{ "axp15060", 0 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 01a6bbb6d266..42ec27a4dfc4 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -43,6 +43,7 @@ static const char * const axp20x_model_names[] = {
>  	"AXP806",
>  	"AXP809",
>  	"AXP813",
> +	"AXP15060",
>  };
>  
>  static const struct regmap_range axp152_writeable_ranges[] = {
> @@ -168,6 +169,31 @@ static const struct regmap_access_table axp806_volatile_table = {
>  	.n_yes_ranges	= ARRAY_SIZE(axp806_volatile_ranges),
>  };
>  
> +static const struct regmap_range axp15060_writeable_ranges[] = {
> +	regmap_reg_range(AXP15060_PWR_OUT_CTRL1, AXP15060_DCDC_MODE_CTRL2),
> +	regmap_reg_range(AXP15060_OUTPUT_MONITOR_DISCHARGE, AXP15060_CPUSLDO_V_CTRL),
> +	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
> +	regmap_reg_range(AXP15060_PEK_KEY, AXP15060_PEK_KEY),
> +	regmap_reg_range(AXP15060_IRQ1_EN, AXP15060_IRQ2_EN),
> +	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
> +};
> +
> +static const struct regmap_range axp15060_volatile_ranges[] = {
> +	regmap_reg_range(AXP15060_STARTUP_SRC, AXP15060_STARTUP_SRC),
> +	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
> +	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
> +};
> +
> +static const struct regmap_access_table axp15060_writeable_table = {
> +	.yes_ranges	= axp15060_writeable_ranges,
> +	.n_yes_ranges	= ARRAY_SIZE(axp15060_writeable_ranges),
> +};
> +
> +static const struct regmap_access_table axp15060_volatile_table = {
> +	.yes_ranges	= axp15060_volatile_ranges,
> +	.n_yes_ranges	= ARRAY_SIZE(axp15060_volatile_ranges),
> +};
> +
>  static const struct resource axp152_pek_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
>  	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> @@ -236,6 +262,11 @@ static const struct resource axp809_pek_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(AXP809_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
>  };
>  
> +static const struct resource axp15060_pek_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
> +	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> +};
> +
>  static const struct regmap_config axp152_regmap_config = {
>  	.reg_bits	= 8,
>  	.val_bits	= 8,
> @@ -281,6 +312,15 @@ static const struct regmap_config axp806_regmap_config = {
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> +static const struct regmap_config axp15060_regmap_config = {
> +	.reg_bits	= 8,
> +	.val_bits	= 8,
> +	.wr_table	= &axp15060_writeable_table,
> +	.volatile_table	= &axp15060_volatile_table,
> +	.max_register	= AXP15060_IRQ2_STATE,
> +	.cache_type	= REGCACHE_RBTREE,
> +};
> +
>  #define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask)			\
>  	[_variant##_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
>  
> @@ -502,6 +542,23 @@ static const struct regmap_irq axp809_regmap_irqs[] = {
>  	INIT_REGMAP_IRQ(AXP809, GPIO0_INPUT,		4, 0),
>  };
>  
> +static const struct regmap_irq axp15060_regmap_irqs[] = {
> +	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV1,	0, 0),
> +	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV2,	0, 1),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC1_V_LOW,		0, 2),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC2_V_LOW,		0, 3),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC3_V_LOW,		0, 4),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC4_V_LOW,		0, 5),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC5_V_LOW,		0, 6),
> +	INIT_REGMAP_IRQ(AXP15060, DCDC6_V_LOW,		0, 7),
> +	INIT_REGMAP_IRQ(AXP15060, PEK_LONG,			1, 0),
> +	INIT_REGMAP_IRQ(AXP15060, PEK_SHORT,			1, 1),
> +	INIT_REGMAP_IRQ(AXP15060, GPIO1_INPUT,		1, 2),
> +	INIT_REGMAP_IRQ(AXP15060, PEK_FAL_EDGE,			1, 3),
> +	INIT_REGMAP_IRQ(AXP15060, PEK_RIS_EDGE,			1, 4),
> +	INIT_REGMAP_IRQ(AXP15060, GPIO2_INPUT,		1, 5),
> +};
> +
>  static const struct regmap_irq_chip axp152_regmap_irq_chip = {
>  	.name			= "axp152_irq_chip",
>  	.status_base		= AXP152_IRQ1_STATE,
> @@ -581,6 +638,17 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
>  	.num_regs		= 5,
>  };
>  
> +static const struct regmap_irq_chip axp15060_regmap_irq_chip = {
> +	.name			= "axp15060",
> +	.status_base		= AXP15060_IRQ1_STATE,
> +	.ack_base		= AXP15060_IRQ1_STATE,
> +	.unmask_base		= AXP15060_IRQ1_EN,
> +	.init_ack_masked	= true,
> +	.irqs			= axp15060_regmap_irqs,
> +	.num_irqs		= ARRAY_SIZE(axp15060_regmap_irqs),
> +	.num_regs		= 2,
> +};
> +
>  static const struct mfd_cell axp20x_cells[] = {
>  	{
>  		.name		= "axp20x-gpio",
> @@ -825,6 +893,16 @@ static const struct mfd_cell axp813_cells[] = {
>  	},
>  };
>  
> +static const struct mfd_cell axp15060_cells[] = {
> +	{
> +		.name		= "axp221-pek",
> +		.num_resources	= ARRAY_SIZE(axp15060_pek_resources),
> +		.resources	= axp15060_pek_resources,
> +	}, {
> +		.name		= "axp20x-regulator",
> +	},

As Lee mentioned the other day, there are the MFD_CELL_NAME and
MFD_CELL_RES macro to wrap those now.

I compared all the register offsets and bit names against the (AXP853T)
manual, they match.

> +};
> +
>  static int axp20x_power_off(struct sys_off_data *data)
>  {
>  	struct axp20x_dev *axp20x = data->cb_data;
> @@ -934,6 +1012,18 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
>  		 */
>  		axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
>  		break;
> +	case AXP15060_ID:
> +		/* Don't register the power key part if there is no interrupt line. */

Mmh, I don't know if this is really needed. If the IRQ line is not
connected, it isn't really critical, it just wouldn't work, would it? I
wonder if the same problem applies to all the other PMICs?
I see that we handle it like this for the AXP806, but I wonder if this is
more a by-product of the slave mode requirement there.

> +		if (axp20x->irq > 0) {
> +			axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
> +			axp20x->cells = axp15060_cells;
> +		} else {
> +			axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
> +			axp20x->cells = axp806_cells;

That looks a bit odd. Either we rename this alternative axp806_cells
definition somewhere above to something generic (axp_regulator_cells?), or
there should be a comment here explaining why all of a sudden an AXP15060
without an IRQ line is the same as the AXP806.

> +		}
> +		axp20x->regmap_cfg = &axp15060_regmap_config;
> +		axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
> +		break;
>  	default:
>  		dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x->variant);
>  		return -EINVAL;
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 2058194807bd..abc2bdc54bf5 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -21,6 +21,7 @@ enum axp20x_variants {
>  	AXP806_ID,
>  	AXP809_ID,
>  	AXP813_ID,
> +	AXP15060_ID,
>  	NR_AXP20X_VARIANTS,
>  };
>  
> @@ -131,6 +132,39 @@ enum axp20x_variants {
>  /* Other DCDC regulator control registers are the same as AXP803 */
>  #define AXP813_DCDC7_V_OUT		0x26
>  
> +#define AXP15060_STARTUP_SRC		0x00
> +#define AXP15060_PWR_OUT_CTRL1		0x10
> +#define AXP15060_PWR_OUT_CTRL2		0x11
> +#define AXP15060_PWR_OUT_CTRL3		0x12
> +#define AXP15060_DCDC1_V_CTRL		0x13
> +#define AXP15060_DCDC2_V_CTRL		0x14
> +#define AXP15060_DCDC3_V_CTRL		0x15
> +#define AXP15060_DCDC4_V_CTRL		0x16
> +#define AXP15060_DCDC5_V_CTRL		0x17
> +#define AXP15060_DCDC6_V_CTRL		0x18
> +#define AXP15060_ALDO1_V_CTRL		0x19
> +#define AXP15060_DCDC_MODE_CTRL1		0x1a
> +#define AXP15060_DCDC_MODE_CTRL2		0x1b
> +#define AXP15060_OUTPUT_MONITOR_DISCHARGE		0x1e
> +#define AXP15060_IRQ_PWROK_VOFF		0x1f
> +#define AXP15060_ALDO2_V_CTRL		0x20
> +#define AXP15060_ALDO3_V_CTRL		0x21
> +#define AXP15060_ALDO4_V_CTRL		0x22
> +#define AXP15060_ALDO5_V_CTRL		0x23
> +#define AXP15060_BLDO1_V_CTRL		0x24
> +#define AXP15060_BLDO2_V_CTRL		0x25
> +#define AXP15060_BLDO3_V_CTRL		0x26
> +#define AXP15060_BLDO4_V_CTRL		0x27
> +#define AXP15060_BLDO5_V_CTRL		0x28
> +#define AXP15060_CLDO1_V_CTRL		0x29
> +#define AXP15060_CLDO2_V_CTRL		0x2a
> +#define AXP15060_CLDO3_V_CTRL		0x2b
> +#define AXP15060_CLDO4_V_CTRL		0x2d
> +#define AXP15060_CPUSLDO_V_CTRL		0x2e
> +#define AXP15060_PWR_WAKEUP_CTRL		0x31
> +#define AXP15060_PWR_DISABLE_DOWN_SEQ		0x32
> +#define AXP15060_PEK_KEY		0x36
> +
>  /* Interrupt */
>  #define AXP152_IRQ1_EN			0x40
>  #define AXP152_IRQ2_EN			0x41
> @@ -139,6 +173,11 @@ enum axp20x_variants {
>  #define AXP152_IRQ2_STATE		0x49
>  #define AXP152_IRQ3_STATE		0x4a
>  
> +#define AXP15060_IRQ1_EN		0x40
> +#define AXP15060_IRQ2_EN		0x41
> +#define AXP15060_IRQ1_STATE		0x48
> +#define AXP15060_IRQ2_STATE		0x49
> +
>  #define AXP20X_IRQ1_EN			0x40
>  #define AXP20X_IRQ2_EN			0x41
>  #define AXP20X_IRQ3_EN			0x42
> @@ -222,6 +261,8 @@ enum axp20x_variants {
>  #define AXP22X_GPIO_STATE		0x94
>  #define AXP22X_GPIO_PULL_DOWN		0x95
>  
> +#define AXP15060_CLDO4_GPIO2_MODESET		0x2c
> +

Compared the register offsets against the manual, they match.

Cheers,
Andre

>  /* Battery */
>  #define AXP20X_CHRG_CC_31_24		0xb0
>  #define AXP20X_CHRG_CC_23_16		0xb1
> @@ -419,6 +460,33 @@ enum {
>  	AXP813_REG_ID_MAX,
>  };
>  
> +enum {
> +	AXP15060_DCDC1 = 0,
> +	AXP15060_DCDC2,
> +	AXP15060_DCDC3,
> +	AXP15060_DCDC4,
> +	AXP15060_DCDC5,
> +	AXP15060_DCDC6,
> +	AXP15060_ALDO1,
> +	AXP15060_ALDO2,
> +	AXP15060_ALDO3,
> +	AXP15060_ALDO4,
> +	AXP15060_ALDO5,
> +	AXP15060_BLDO1,
> +	AXP15060_BLDO2,
> +	AXP15060_BLDO3,
> +	AXP15060_BLDO4,
> +	AXP15060_BLDO5,
> +	AXP15060_CLDO1,
> +	AXP15060_CLDO2,
> +	AXP15060_CLDO3,
> +	AXP15060_CLDO4,
> +	AXP15060_CPUSLDO,
> +	AXP15060_SW,
> +	AXP15060_RTC_LDO,
> +	AXP15060_REG_ID_MAX,
> +};
> +
>  /* IRQs */
>  enum {
>  	AXP152_IRQ_LDO0IN_CONNECT = 1,
> @@ -637,6 +705,23 @@ enum axp809_irqs {
>  	AXP809_IRQ_GPIO0_INPUT,
>  };
>  
> +enum axp15060_irqs {
> +	AXP15060_IRQ_DIE_TEMP_HIGH_LV1 = 1,
> +	AXP15060_IRQ_DIE_TEMP_HIGH_LV2,
> +	AXP15060_IRQ_DCDC1_V_LOW,
> +	AXP15060_IRQ_DCDC2_V_LOW,
> +	AXP15060_IRQ_DCDC3_V_LOW,
> +	AXP15060_IRQ_DCDC4_V_LOW,
> +	AXP15060_IRQ_DCDC5_V_LOW,
> +	AXP15060_IRQ_DCDC6_V_LOW,
> +	AXP15060_IRQ_PEK_LONG,
> +	AXP15060_IRQ_PEK_SHORT,
> +	AXP15060_IRQ_GPIO1_INPUT,
> +	AXP15060_IRQ_PEK_FAL_EDGE,
> +	AXP15060_IRQ_PEK_RIS_EDGE,
> +	AXP15060_IRQ_GPIO2_INPUT,
> +};
> +
>  struct axp20x_dev {
>  	struct device			*dev;
>  	int				irq;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC
  2023-04-17 14:05   ` Andre Przywara
@ 2023-04-17 16:21     ` Shengyu Qu
  0 siblings, 0 replies; 14+ messages in thread
From: Shengyu Qu @ 2023-04-17 16:21 UTC (permalink / raw)
  To: Andre Przywara
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka, martin.botka1


[-- Attachment #1.1.1: Type: text/plain, Size: 13521 bytes --]

Hi Andre,

> On Fri,  7 Apr 2023 22:18:11 +0800
> Shengyu Qu <wiagn233@outlook.com> wrote:
>
> Hi Shengyu,
>
> thanks for doing the tedious work and sending this!
>
>> The AXP15060 is a PMIC chip produced by X-Powers, and could be connected
>> via an I2C bus.
>>
>> Describe the regmap and the MFD bits, along with the registers exposed
>> via I2C. Eventually advertise the device using a new compatible string
>> and add support for power off the system.
>>
>> The driver would disable PEK function if IRQ is not configured in device
>> tree, since some boards (For example, Starfive Visionfive 2) didn't
>> connect IRQ line of PMIC to SOC.
>>
>> GPIO function isn't enabled in this commit, since its configuration
>> operation is different from any existing AXP PMICs and needs
>> logic modification on existing driver. GPIO support might come in later
>> patches.
>>
>> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
>> ---
>>   drivers/mfd/axp20x-i2c.c   |  2 +
>>   drivers/mfd/axp20x.c       | 90 ++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/axp20x.h | 85 +++++++++++++++++++++++++++++++++++
>>   3 files changed, 177 insertions(+)
>>
>> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
>> index f49fbd307958..b4f5cb457117 100644
>> --- a/drivers/mfd/axp20x-i2c.c
>> +++ b/drivers/mfd/axp20x-i2c.c
>> @@ -65,6 +65,7 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
>>   	{ .compatible = "x-powers,axp223", .data = (void *)AXP223_ID },
>>   	{ .compatible = "x-powers,axp803", .data = (void *)AXP803_ID },
>>   	{ .compatible = "x-powers,axp806", .data = (void *)AXP806_ID },
>> +	{ .compatible = "x-powers,axp15060", .data = (void *)AXP15060_ID },
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
>> @@ -78,6 +79,7 @@ static const struct i2c_device_id axp20x_i2c_id[] = {
>>   	{ "axp223", 0 },
>>   	{ "axp803", 0 },
>>   	{ "axp806", 0 },
>> +	{ "axp15060", 0 },
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 01a6bbb6d266..42ec27a4dfc4 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -43,6 +43,7 @@ static const char * const axp20x_model_names[] = {
>>   	"AXP806",
>>   	"AXP809",
>>   	"AXP813",
>> +	"AXP15060",
>>   };
>>   
>>   static const struct regmap_range axp152_writeable_ranges[] = {
>> @@ -168,6 +169,31 @@ static const struct regmap_access_table axp806_volatile_table = {
>>   	.n_yes_ranges	= ARRAY_SIZE(axp806_volatile_ranges),
>>   };
>>   
>> +static const struct regmap_range axp15060_writeable_ranges[] = {
>> +	regmap_reg_range(AXP15060_PWR_OUT_CTRL1, AXP15060_DCDC_MODE_CTRL2),
>> +	regmap_reg_range(AXP15060_OUTPUT_MONITOR_DISCHARGE, AXP15060_CPUSLDO_V_CTRL),
>> +	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
>> +	regmap_reg_range(AXP15060_PEK_KEY, AXP15060_PEK_KEY),
>> +	regmap_reg_range(AXP15060_IRQ1_EN, AXP15060_IRQ2_EN),
>> +	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
>> +};
>> +
>> +static const struct regmap_range axp15060_volatile_ranges[] = {
>> +	regmap_reg_range(AXP15060_STARTUP_SRC, AXP15060_STARTUP_SRC),
>> +	regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
>> +	regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
>> +};
>> +
>> +static const struct regmap_access_table axp15060_writeable_table = {
>> +	.yes_ranges	= axp15060_writeable_ranges,
>> +	.n_yes_ranges	= ARRAY_SIZE(axp15060_writeable_ranges),
>> +};
>> +
>> +static const struct regmap_access_table axp15060_volatile_table = {
>> +	.yes_ranges	= axp15060_volatile_ranges,
>> +	.n_yes_ranges	= ARRAY_SIZE(axp15060_volatile_ranges),
>> +};
>> +
>>   static const struct resource axp152_pek_resources[] = {
>>   	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
>>   	DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
>> @@ -236,6 +262,11 @@ static const struct resource axp809_pek_resources[] = {
>>   	DEFINE_RES_IRQ_NAMED(AXP809_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
>>   };
>>   
>> +static const struct resource axp15060_pek_resources[] = {
>> +	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
>> +	DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
>> +};
>> +
>>   static const struct regmap_config axp152_regmap_config = {
>>   	.reg_bits	= 8,
>>   	.val_bits	= 8,
>> @@ -281,6 +312,15 @@ static const struct regmap_config axp806_regmap_config = {
>>   	.cache_type	= REGCACHE_RBTREE,
>>   };
>>   
>> +static const struct regmap_config axp15060_regmap_config = {
>> +	.reg_bits	= 8,
>> +	.val_bits	= 8,
>> +	.wr_table	= &axp15060_writeable_table,
>> +	.volatile_table	= &axp15060_volatile_table,
>> +	.max_register	= AXP15060_IRQ2_STATE,
>> +	.cache_type	= REGCACHE_RBTREE,
>> +};
>> +
>>   #define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask)			\
>>   	[_variant##_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
>>   
>> @@ -502,6 +542,23 @@ static const struct regmap_irq axp809_regmap_irqs[] = {
>>   	INIT_REGMAP_IRQ(AXP809, GPIO0_INPUT,		4, 0),
>>   };
>>   
>> +static const struct regmap_irq axp15060_regmap_irqs[] = {
>> +	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV1,	0, 0),
>> +	INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV2,	0, 1),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC1_V_LOW,		0, 2),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC2_V_LOW,		0, 3),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC3_V_LOW,		0, 4),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC4_V_LOW,		0, 5),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC5_V_LOW,		0, 6),
>> +	INIT_REGMAP_IRQ(AXP15060, DCDC6_V_LOW,		0, 7),
>> +	INIT_REGMAP_IRQ(AXP15060, PEK_LONG,			1, 0),
>> +	INIT_REGMAP_IRQ(AXP15060, PEK_SHORT,			1, 1),
>> +	INIT_REGMAP_IRQ(AXP15060, GPIO1_INPUT,		1, 2),
>> +	INIT_REGMAP_IRQ(AXP15060, PEK_FAL_EDGE,			1, 3),
>> +	INIT_REGMAP_IRQ(AXP15060, PEK_RIS_EDGE,			1, 4),
>> +	INIT_REGMAP_IRQ(AXP15060, GPIO2_INPUT,		1, 5),
>> +};
>> +
>>   static const struct regmap_irq_chip axp152_regmap_irq_chip = {
>>   	.name			= "axp152_irq_chip",
>>   	.status_base		= AXP152_IRQ1_STATE,
>> @@ -581,6 +638,17 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
>>   	.num_regs		= 5,
>>   };
>>   
>> +static const struct regmap_irq_chip axp15060_regmap_irq_chip = {
>> +	.name			= "axp15060",
>> +	.status_base		= AXP15060_IRQ1_STATE,
>> +	.ack_base		= AXP15060_IRQ1_STATE,
>> +	.unmask_base		= AXP15060_IRQ1_EN,
>> +	.init_ack_masked	= true,
>> +	.irqs			= axp15060_regmap_irqs,
>> +	.num_irqs		= ARRAY_SIZE(axp15060_regmap_irqs),
>> +	.num_regs		= 2,
>> +};
>> +
>>   static const struct mfd_cell axp20x_cells[] = {
>>   	{
>>   		.name		= "axp20x-gpio",
>> @@ -825,6 +893,16 @@ static const struct mfd_cell axp813_cells[] = {
>>   	},
>>   };
>>   
>> +static const struct mfd_cell axp15060_cells[] = {
>> +	{
>> +		.name		= "axp221-pek",
>> +		.num_resources	= ARRAY_SIZE(axp15060_pek_resources),
>> +		.resources	= axp15060_pek_resources,
>> +	}, {
>> +		.name		= "axp20x-regulator",
>> +	},
> As Lee mentioned the other day, there are the MFD_CELL_NAME and
> MFD_CELL_RES macro to wrap those now.

I think a whole conversion commit would be much better to keep code

readable rather than multiple description method.

> I compared all the register offsets and bit names against the (AXP853T)
> manual, they match.
Thanks.
>
>> +};
>> +
>>   static int axp20x_power_off(struct sys_off_data *data)
>>   {
>>   	struct axp20x_dev *axp20x = data->cb_data;
>> @@ -934,6 +1012,18 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
>>   		 */
>>   		axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
>>   		break;
>> +	case AXP15060_ID:
>> +		/* Don't register the power key part if there is no interrupt line. */
> Mmh, I don't know if this is really needed. If the IRQ line is not
> connected, it isn't really critical, it just wouldn't work, would it? I
> wonder if the same problem applies to all the other PMICs?
> I see that we handle it like this for the AXP806, but I wonder if this is
> more a by-product of the slave mode requirement there.

There would be a OOPS about pek driver in dmesg output if this

part does not exist. Since most use cases of AXP PMICs are Allwinner

SOCs and board designers would follow AW's reference design, this

shouldn't be a problem for other variants.

>> +		if (axp20x->irq > 0) {
>> +			axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
>> +			axp20x->cells = axp15060_cells;
>> +		} else {
>> +			axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
>> +			axp20x->cells = axp806_cells;
> That looks a bit odd. Either we rename this alternative axp806_cells
> definition somewhere above to something generic (axp_regulator_cells?), or
> there should be a comment here explaining why all of a sudden an AXP15060
> without an IRQ line is the same as the AXP806.

Agree, I'm thinking about creating a new cell named axp_regulator_only_cells

or something with no .id configured, and add comment for it.

>
>> +		}
>> +		axp20x->regmap_cfg = &axp15060_regmap_config;
>> +		axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
>> +		break;
>>   	default:
>>   		dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x->variant);
>>   		return -EINVAL;
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> index 2058194807bd..abc2bdc54bf5 100644
>> --- a/include/linux/mfd/axp20x.h
>> +++ b/include/linux/mfd/axp20x.h
>> @@ -21,6 +21,7 @@ enum axp20x_variants {
>>   	AXP806_ID,
>>   	AXP809_ID,
>>   	AXP813_ID,
>> +	AXP15060_ID,
>>   	NR_AXP20X_VARIANTS,
>>   };
>>   
>> @@ -131,6 +132,39 @@ enum axp20x_variants {
>>   /* Other DCDC regulator control registers are the same as AXP803 */
>>   #define AXP813_DCDC7_V_OUT		0x26
>>   
>> +#define AXP15060_STARTUP_SRC		0x00
>> +#define AXP15060_PWR_OUT_CTRL1		0x10
>> +#define AXP15060_PWR_OUT_CTRL2		0x11
>> +#define AXP15060_PWR_OUT_CTRL3		0x12
>> +#define AXP15060_DCDC1_V_CTRL		0x13
>> +#define AXP15060_DCDC2_V_CTRL		0x14
>> +#define AXP15060_DCDC3_V_CTRL		0x15
>> +#define AXP15060_DCDC4_V_CTRL		0x16
>> +#define AXP15060_DCDC5_V_CTRL		0x17
>> +#define AXP15060_DCDC6_V_CTRL		0x18
>> +#define AXP15060_ALDO1_V_CTRL		0x19
>> +#define AXP15060_DCDC_MODE_CTRL1		0x1a
>> +#define AXP15060_DCDC_MODE_CTRL2		0x1b
>> +#define AXP15060_OUTPUT_MONITOR_DISCHARGE		0x1e
>> +#define AXP15060_IRQ_PWROK_VOFF		0x1f
>> +#define AXP15060_ALDO2_V_CTRL		0x20
>> +#define AXP15060_ALDO3_V_CTRL		0x21
>> +#define AXP15060_ALDO4_V_CTRL		0x22
>> +#define AXP15060_ALDO5_V_CTRL		0x23
>> +#define AXP15060_BLDO1_V_CTRL		0x24
>> +#define AXP15060_BLDO2_V_CTRL		0x25
>> +#define AXP15060_BLDO3_V_CTRL		0x26
>> +#define AXP15060_BLDO4_V_CTRL		0x27
>> +#define AXP15060_BLDO5_V_CTRL		0x28
>> +#define AXP15060_CLDO1_V_CTRL		0x29
>> +#define AXP15060_CLDO2_V_CTRL		0x2a
>> +#define AXP15060_CLDO3_V_CTRL		0x2b
>> +#define AXP15060_CLDO4_V_CTRL		0x2d
>> +#define AXP15060_CPUSLDO_V_CTRL		0x2e
>> +#define AXP15060_PWR_WAKEUP_CTRL		0x31
>> +#define AXP15060_PWR_DISABLE_DOWN_SEQ		0x32
>> +#define AXP15060_PEK_KEY		0x36
>> +
>>   /* Interrupt */
>>   #define AXP152_IRQ1_EN			0x40
>>   #define AXP152_IRQ2_EN			0x41
>> @@ -139,6 +173,11 @@ enum axp20x_variants {
>>   #define AXP152_IRQ2_STATE		0x49
>>   #define AXP152_IRQ3_STATE		0x4a
>>   
>> +#define AXP15060_IRQ1_EN		0x40
>> +#define AXP15060_IRQ2_EN		0x41
>> +#define AXP15060_IRQ1_STATE		0x48
>> +#define AXP15060_IRQ2_STATE		0x49
>> +
>>   #define AXP20X_IRQ1_EN			0x40
>>   #define AXP20X_IRQ2_EN			0x41
>>   #define AXP20X_IRQ3_EN			0x42
>> @@ -222,6 +261,8 @@ enum axp20x_variants {
>>   #define AXP22X_GPIO_STATE		0x94
>>   #define AXP22X_GPIO_PULL_DOWN		0x95
>>   
>> +#define AXP15060_CLDO4_GPIO2_MODESET		0x2c
>> +
> Compared the register offsets against the manual, they match.
>
> Cheers,
> Andre

Thanks,

Shengyu

>>   /* Battery */
>>   #define AXP20X_CHRG_CC_31_24		0xb0
>>   #define AXP20X_CHRG_CC_23_16		0xb1
>> @@ -419,6 +460,33 @@ enum {
>>   	AXP813_REG_ID_MAX,
>>   };
>>   
>> +enum {
>> +	AXP15060_DCDC1 = 0,
>> +	AXP15060_DCDC2,
>> +	AXP15060_DCDC3,
>> +	AXP15060_DCDC4,
>> +	AXP15060_DCDC5,
>> +	AXP15060_DCDC6,
>> +	AXP15060_ALDO1,
>> +	AXP15060_ALDO2,
>> +	AXP15060_ALDO3,
>> +	AXP15060_ALDO4,
>> +	AXP15060_ALDO5,
>> +	AXP15060_BLDO1,
>> +	AXP15060_BLDO2,
>> +	AXP15060_BLDO3,
>> +	AXP15060_BLDO4,
>> +	AXP15060_BLDO5,
>> +	AXP15060_CLDO1,
>> +	AXP15060_CLDO2,
>> +	AXP15060_CLDO3,
>> +	AXP15060_CLDO4,
>> +	AXP15060_CPUSLDO,
>> +	AXP15060_SW,
>> +	AXP15060_RTC_LDO,
>> +	AXP15060_REG_ID_MAX,
>> +};
>> +
>>   /* IRQs */
>>   enum {
>>   	AXP152_IRQ_LDO0IN_CONNECT = 1,
>> @@ -637,6 +705,23 @@ enum axp809_irqs {
>>   	AXP809_IRQ_GPIO0_INPUT,
>>   };
>>   
>> +enum axp15060_irqs {
>> +	AXP15060_IRQ_DIE_TEMP_HIGH_LV1 = 1,
>> +	AXP15060_IRQ_DIE_TEMP_HIGH_LV2,
>> +	AXP15060_IRQ_DCDC1_V_LOW,
>> +	AXP15060_IRQ_DCDC2_V_LOW,
>> +	AXP15060_IRQ_DCDC3_V_LOW,
>> +	AXP15060_IRQ_DCDC4_V_LOW,
>> +	AXP15060_IRQ_DCDC5_V_LOW,
>> +	AXP15060_IRQ_DCDC6_V_LOW,
>> +	AXP15060_IRQ_PEK_LONG,
>> +	AXP15060_IRQ_PEK_SHORT,
>> +	AXP15060_IRQ_GPIO1_INPUT,
>> +	AXP15060_IRQ_PEK_FAL_EDGE,
>> +	AXP15060_IRQ_PEK_RIS_EDGE,
>> +	AXP15060_IRQ_GPIO2_INPUT,
>> +};
>> +
>>   struct axp20x_dev {
>>   	struct device			*dev;
>>   	int				irq;

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support
       [not found] <20230407141813.89-1-wiagn233@outlook.com>
  2023-04-07 14:18 ` [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant Shengyu Qu
  2023-04-07 14:18 ` [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC Shengyu Qu
@ 2023-04-07 14:18 ` Shengyu Qu
  2023-04-17 14:18   ` Andre Przywara
  2023-04-07 14:18 ` [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists Shengyu Qu
  3 siblings, 1 reply; 14+ messages in thread
From: Shengyu Qu @ 2023-04-07 14:18 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel
  Cc: Shengyu Qu

The AXP15060 is a typical I2C-controlled PMIC, seen on multiple boards
with different default register value. Current driver is tested on
Starfive Visionfive 2.

The RTCLDO is fixed, and cannot even be turned on or off. On top of
that, its voltage is customisable (either 1.8V or 3.3V). We pretend it's
a fixed 1.8V regulator since other AXP driver also do like this. Also,
BSP code ignores this regulator and it's not used according to VF2
schematic.

Describe the AXP15060's voltage settings and switch registers, how the
voltages are encoded, and connect this to the MFD device via its
regulator ID.

Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
---
 drivers/regulator/axp20x-regulator.c | 229 ++++++++++++++++++++++++++-
 1 file changed, 221 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index d260c442b788..ece4af93df7b 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -270,6 +270,74 @@
 
 #define AXP813_PWR_OUT_DCDC7_MASK	BIT_MASK(6)
 
+#define AXP15060_DCDC1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_DCDC2_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC3_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC4_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC5_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC6_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO4_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO5_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO4_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO5_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO4_V_CTRL_MASK		GENMASK(5, 0)
+#define AXP15060_CPUSLDO_V_CTRL_MASK		GENMASK(3, 0)
+
+#define AXP15060_PWR_OUT_DCDC1_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_DCDC3_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_DCDC4_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_DCDC5_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_DCDC6_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_ALDO1_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_ALDO2_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_ALDO3_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_ALDO4_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_ALDO5_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_BLDO1_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_BLDO2_MASK	BIT_MASK(6)
+#define AXP15060_PWR_OUT_BLDO3_MASK	BIT_MASK(7)
+#define AXP15060_PWR_OUT_BLDO4_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_BLDO5_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_CLDO1_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_CLDO2_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_CLDO3_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_CLDO4_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_CPUSLDO_MASK	BIT_MASK(6)
+#define AXP15060_PWR_OUT_SW_MASK		BIT_MASK(7)
+
+#define AXP15060_DCDC23_POLYPHASE_DUAL_MASK		BIT_MASK(6)
+#define AXP15060_DCDC46_POLYPHASE_DUAL_MASK		BIT_MASK(7)
+
+#define AXP15060_DCDC234_500mV_START	0x00
+#define AXP15060_DCDC234_500mV_STEPS	70
+#define AXP15060_DCDC234_500mV_END		\
+	(AXP15060_DCDC234_500mV_START + AXP15060_DCDC234_500mV_STEPS)
+#define AXP15060_DCDC234_1220mV_START	0x47
+#define AXP15060_DCDC234_1220mV_STEPS	16
+#define AXP15060_DCDC234_1220mV_END		\
+	(AXP15060_DCDC234_1220mV_START + AXP15060_DCDC234_1220mV_STEPS)
+#define AXP15060_DCDC234_NUM_VOLTAGES	88
+
+#define AXP15060_DCDC5_800mV_START	0x00
+#define AXP15060_DCDC5_800mV_STEPS	32
+#define AXP15060_DCDC5_800mV_END		\
+	(AXP15060_DCDC5_800mV_START + AXP15060_DCDC5_800mV_STEPS)
+#define AXP15060_DCDC5_1140mV_START	0x21
+#define AXP15060_DCDC5_1140mV_STEPS	35
+#define AXP15060_DCDC5_1140mV_END		\
+	(AXP15060_DCDC5_1140mV_START + AXP15060_DCDC5_1140mV_STEPS)
+#define AXP15060_DCDC5_NUM_VOLTAGES	69
+
 #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
 		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
 	[_family##_##_id] = {							\
@@ -1001,6 +1069,104 @@ static const struct regulator_desc axp813_regulators[] = {
 		    AXP22X_PWR_OUT_CTRL2, AXP22X_PWR_OUT_DC1SW_MASK),
 };
 
+static const struct linear_range axp15060_dcdc234_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000,
+			       AXP15060_DCDC234_500mV_START,
+			       AXP15060_DCDC234_500mV_END,
+			       10000),
+	REGULATOR_LINEAR_RANGE(1220000,
+			       AXP15060_DCDC234_1220mV_START,
+			       AXP15060_DCDC234_1220mV_END,
+			       20000),
+};
+
+static const struct linear_range axp15060_dcdc5_ranges[] = {
+	REGULATOR_LINEAR_RANGE(800000,
+			       AXP15060_DCDC5_800mV_START,
+			       AXP15060_DCDC5_800mV_END,
+			       10000),
+	REGULATOR_LINEAR_RANGE(1140000,
+			       AXP15060_DCDC5_1140mV_START,
+			       AXP15060_DCDC5_1140mV_END,
+			       20000),
+};
+
+static const struct regulator_desc axp15060_regulators[] = {
+	AXP_DESC(AXP15060, DCDC1, "dcdc1", "vin1", 1500, 3400, 100,
+		 AXP15060_DCDC1_V_CTRL, AXP15060_DCDC1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC1_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC2, "dcdc2", "vin2",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC2_V_CTRL, AXP15060_DCDC2_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC2_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC3, "dcdc3", "vin3",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC3_V_CTRL, AXP15060_DCDC3_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC3_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC4, "dcdc4", "vin4",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC4_V_CTRL, AXP15060_DCDC4_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC4_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC5, "dcdc5", "vin5",
+			axp15060_dcdc5_ranges, AXP15060_DCDC5_NUM_VOLTAGES,
+			AXP15060_DCDC5_V_CTRL, AXP15060_DCDC5_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC5_MASK),
+	AXP_DESC(AXP15060, DCDC6, "dcdc6", "vin6", 500, 3400, 100,
+		 AXP15060_DCDC6_V_CTRL, AXP15060_DCDC6_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC6_MASK),
+	AXP_DESC(AXP15060, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO1_V_CTRL, AXP15060_ALDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO1_MASK),
+	AXP_DESC(AXP15060, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO2_V_CTRL, AXP15060_ALDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO2_MASK),
+	AXP_DESC(AXP15060, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO3_V_CTRL, AXP15060_ALDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO3_MASK),
+	AXP_DESC(AXP15060, ALDO4, "aldo4", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO4_V_CTRL, AXP15060_ALDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO4_MASK),
+	AXP_DESC(AXP15060, ALDO5, "aldo5", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO5_V_CTRL, AXP15060_ALDO5_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO5_MASK),
+	AXP_DESC(AXP15060, BLDO1, "bldo1", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO1_V_CTRL, AXP15060_BLDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO1_MASK),
+	AXP_DESC(AXP15060, BLDO2, "bldo2", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO2_V_CTRL, AXP15060_BLDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO2_MASK),
+	AXP_DESC(AXP15060, BLDO3, "bldo3", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO3_V_CTRL, AXP15060_BLDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO3_MASK),
+	AXP_DESC(AXP15060, BLDO4, "bldo4", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO4_V_CTRL, AXP15060_BLDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO4_MASK),
+	AXP_DESC(AXP15060, BLDO5, "bldo5", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO5_V_CTRL, AXP15060_BLDO5_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO5_MASK),
+	AXP_DESC(AXP15060, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO1_V_CTRL, AXP15060_CLDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO1_MASK),
+	AXP_DESC(AXP15060, CLDO2, "cldo2", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO2_V_CTRL, AXP15060_CLDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO2_MASK),
+	AXP_DESC(AXP15060, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO3_V_CTRL, AXP15060_CLDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO3_MASK),
+	AXP_DESC(AXP15060, CLDO4, "cldo4", "cldoin", 700, 4200, 100,
+		 AXP15060_CLDO4_V_CTRL, AXP15060_CLDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO4_MASK),
+	AXP_DESC(AXP15060, CPUSLDO, "cpusldo", NULL, 700, 1400, 50,
+		 AXP15060_CPUSLDO_V_CTRL, AXP15060_CPUSLDO_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CPUSLDO_MASK),
+	/* Supply comes from DCDC5 */
+	AXP_DESC_SW(AXP15060, SW, "swout", NULL,
+		    AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_SW_MASK),
+	/* Supply comes from DCDC1 */
+	AXP_DESC_FIXED(AXP15060, RTC_LDO, "rtc-ldo", NULL, 1800),
+	/* Supply comes from ALDO1 */
+};
+
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -1145,6 +1311,15 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 		workmode <<= id - AXP813_DCDC1;
 		break;
 
+	case AXP15060_ID:
+		reg = AXP15060_DCDC_MODE_CTRL2;
+		if (id < AXP15060_DCDC1 || id > AXP15060_DCDC6)
+			return -EINVAL;
+
+		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP15060_DCDC1);
+		workmode <<= id - AXP15060_DCDC1;
+		break;
+
 	default:
 		/* should not happen */
 		WARN_ON(1);
@@ -1164,7 +1339,7 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 
 	/*
 	 * Currently in our supported AXP variants, only AXP803, AXP806,
-	 * and AXP813 have polyphase regulators.
+	 * AXP813 and AXP15060 have polyphase regulators.
 	 */
 	switch (axp20x->variant) {
 	case AXP803_ID:
@@ -1196,6 +1371,17 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 		}
 		break;
 
+	case AXP15060_ID:
+		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
+
+		switch (id) {
+		case AXP15060_DCDC3:
+			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
+		case AXP15060_DCDC6:
+			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
+		}
+		break;
+
 	default:
 		return false;
 	}
@@ -1217,6 +1403,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	const char *aldo1_name = axp15060_regulators[AXP15060_ALDO1].name;
 	bool drivevbus = false;
 
 	switch (axp20x->variant) {
@@ -1252,6 +1439,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
+	case AXP15060_ID:
+		regulators = axp15060_regulators;
+		nregulators = AXP15060_REG_ID_MAX;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
 			axp20x->variant);
@@ -1278,8 +1469,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			continue;
 
 		/*
-		 * Regulators DC1SW and DC5LDO are connected internally,
-		 * so we have to handle their supply names separately.
+		 * Regulators DC1SW, DC5LDO and RTCLDO on AXP15060 are
+		 * connected internally, so we have to handle their supply
+		 * names separately.
 		 *
 		 * We always register the regulators in proper sequence,
 		 * so the supply names are correctly read. See the last
@@ -1288,7 +1480,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		 */
 		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
 		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
-		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
+			(regulators == axp809_regulators && i == AXP809_DC1SW) ||
+		    (regulators == axp15060_regulators && i == AXP15060_SW)) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			if (!new_desc)
@@ -1300,7 +1493,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 
 		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
-		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
+			(regulators == axp809_regulators && i == AXP809_DC5LDO) ||
+		    (regulators == axp15060_regulators && i == AXP15060_CPUSLDO)) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			if (!new_desc)
@@ -1311,6 +1505,18 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			desc = new_desc;
 		}
 
+
+		if (regulators == axp15060_regulators && i == AXP15060_RTC_LDO) {
+			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
+						GFP_KERNEL);
+			if (!new_desc)
+				return -ENOMEM;
+
+			*new_desc = regulators[i];
+			new_desc->supply_name = aldo1_name;
+			desc = new_desc;
+		}
+
 		rdev = devm_regulator_register(&pdev->dev, desc, &config);
 		if (IS_ERR(rdev)) {
 			dev_err(&pdev->dev, "Failed to register %s\n",
@@ -1329,19 +1535,26 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 
 		/*
-		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
+		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
 		 */
 		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC1))
+		    (regulators == axp809_regulators && i == AXP809_DCDC1) ||
+			(regulators == axp15060_regulators && i == AXP15060_DCDC1))
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc1_name);
 
 		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC5))
+		    (regulators == axp809_regulators && i == AXP809_DCDC5) ||
+			(regulators == axp15060_regulators && i == AXP15060_DCDC5))
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc5_name);
+
+		if (regulators == axp15060_regulators && i == AXP15060_ALDO1)
+			of_property_read_string(rdev->dev.of_node,
+						"regulator-name",
+						&aldo1_name);
 	}
 
 	if (drivevbus) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support
  2023-04-07 14:18 ` [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support Shengyu Qu
@ 2023-04-17 14:18   ` Andre Przywara
  2023-04-17 16:01     ` Shengyu Qu
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2023-04-17 14:18 UTC (permalink / raw)
  To: Shengyu Qu
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka

On Fri,  7 Apr 2023 22:18:12 +0800
Shengyu Qu <wiagn233@outlook.com> wrote:

Hi,

> The AXP15060 is a typical I2C-controlled PMIC, seen on multiple boards
> with different default register value. Current driver is tested on
> Starfive Visionfive 2.
> 
> The RTCLDO is fixed, and cannot even be turned on or off. On top of
> that, its voltage is customisable (either 1.8V or 3.3V). We pretend it's
> a fixed 1.8V regulator since other AXP driver also do like this. Also,
> BSP code ignores this regulator and it's not used according to VF2
> schematic.
> 
> Describe the AXP15060's voltage settings and switch registers, how the
> voltages are encoded, and connect this to the MFD device via its
> regulator ID.
> 
> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> ---
>  drivers/regulator/axp20x-regulator.c | 229 ++++++++++++++++++++++++++-
>  1 file changed, 221 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index d260c442b788..ece4af93df7b 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -270,6 +270,74 @@
>  
>  #define AXP813_PWR_OUT_DCDC7_MASK	BIT_MASK(6)
>  
> +#define AXP15060_DCDC1_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_DCDC2_V_CTRL_MASK		GENMASK(6, 0)
> +#define AXP15060_DCDC3_V_CTRL_MASK		GENMASK(6, 0)
> +#define AXP15060_DCDC4_V_CTRL_MASK		GENMASK(6, 0)
> +#define AXP15060_DCDC5_V_CTRL_MASK		GENMASK(6, 0)
> +#define AXP15060_DCDC6_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_ALDO1_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_ALDO2_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_ALDO3_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_ALDO4_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_ALDO5_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_BLDO1_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_BLDO2_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_BLDO3_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_BLDO4_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_BLDO5_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_CLDO1_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_CLDO2_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_CLDO3_V_CTRL_MASK		GENMASK(4, 0)
> +#define AXP15060_CLDO4_V_CTRL_MASK		GENMASK(5, 0)
> +#define AXP15060_CPUSLDO_V_CTRL_MASK		GENMASK(3, 0)
> +
> +#define AXP15060_PWR_OUT_DCDC1_MASK	BIT_MASK(0)

I'd say "BIT(0)" is the canonical way of denoting single bit masks,
outside of Linux' bitmask framework.

> +#define AXP15060_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
> +#define AXP15060_PWR_OUT_DCDC3_MASK	BIT_MASK(2)
> +#define AXP15060_PWR_OUT_DCDC4_MASK	BIT_MASK(3)
> +#define AXP15060_PWR_OUT_DCDC5_MASK	BIT_MASK(4)
> +#define AXP15060_PWR_OUT_DCDC6_MASK	BIT_MASK(5)
> +#define AXP15060_PWR_OUT_ALDO1_MASK	BIT_MASK(0)
> +#define AXP15060_PWR_OUT_ALDO2_MASK	BIT_MASK(1)
> +#define AXP15060_PWR_OUT_ALDO3_MASK	BIT_MASK(2)
> +#define AXP15060_PWR_OUT_ALDO4_MASK	BIT_MASK(3)
> +#define AXP15060_PWR_OUT_ALDO5_MASK	BIT_MASK(4)
> +#define AXP15060_PWR_OUT_BLDO1_MASK	BIT_MASK(5)
> +#define AXP15060_PWR_OUT_BLDO2_MASK	BIT_MASK(6)
> +#define AXP15060_PWR_OUT_BLDO3_MASK	BIT_MASK(7)
> +#define AXP15060_PWR_OUT_BLDO4_MASK	BIT_MASK(0)
> +#define AXP15060_PWR_OUT_BLDO5_MASK	BIT_MASK(1)
> +#define AXP15060_PWR_OUT_CLDO1_MASK	BIT_MASK(2)
> +#define AXP15060_PWR_OUT_CLDO2_MASK	BIT_MASK(3)
> +#define AXP15060_PWR_OUT_CLDO3_MASK	BIT_MASK(4)
> +#define AXP15060_PWR_OUT_CLDO4_MASK	BIT_MASK(5)
> +#define AXP15060_PWR_OUT_CPUSLDO_MASK	BIT_MASK(6)
> +#define AXP15060_PWR_OUT_SW_MASK		BIT_MASK(7)
> +
> +#define AXP15060_DCDC23_POLYPHASE_DUAL_MASK		BIT_MASK(6)
> +#define AXP15060_DCDC46_POLYPHASE_DUAL_MASK		BIT_MASK(7)
> +
> +#define AXP15060_DCDC234_500mV_START	0x00

Can you make those values consistently decimal? It's a bit confusing to
see the 70 steps in decimal, but the start (71) in hex.

> +#define AXP15060_DCDC234_500mV_STEPS	70
> +#define AXP15060_DCDC234_500mV_END		\
> +	(AXP15060_DCDC234_500mV_START + AXP15060_DCDC234_500mV_STEPS)
> +#define AXP15060_DCDC234_1220mV_START	0x47
> +#define AXP15060_DCDC234_1220mV_STEPS	16
> +#define AXP15060_DCDC234_1220mV_END		\
> +	(AXP15060_DCDC234_1220mV_START + AXP15060_DCDC234_1220mV_STEPS)
> +#define AXP15060_DCDC234_NUM_VOLTAGES	88
> +
> +#define AXP15060_DCDC5_800mV_START	0x00
> +#define AXP15060_DCDC5_800mV_STEPS	32
> +#define AXP15060_DCDC5_800mV_END		\
> +	(AXP15060_DCDC5_800mV_START + AXP15060_DCDC5_800mV_STEPS)
> +#define AXP15060_DCDC5_1140mV_START	0x21
> +#define AXP15060_DCDC5_1140mV_STEPS	35
> +#define AXP15060_DCDC5_1140mV_END		\
> +	(AXP15060_DCDC5_1140mV_START + AXP15060_DCDC5_1140mV_STEPS)
> +#define AXP15060_DCDC5_NUM_VOLTAGES	69
> +
>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
>  		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
>  	[_family##_##_id] = {							\
> @@ -1001,6 +1069,104 @@ static const struct regulator_desc axp813_regulators[] = {
>  		    AXP22X_PWR_OUT_CTRL2, AXP22X_PWR_OUT_DC1SW_MASK),
>  };
>  
> +static const struct linear_range axp15060_dcdc234_ranges[] = {
> +	REGULATOR_LINEAR_RANGE(500000,
> +			       AXP15060_DCDC234_500mV_START,
> +			       AXP15060_DCDC234_500mV_END,
> +			       10000),
> +	REGULATOR_LINEAR_RANGE(1220000,
> +			       AXP15060_DCDC234_1220mV_START,
> +			       AXP15060_DCDC234_1220mV_END,
> +			       20000),
> +};
> +
> +static const struct linear_range axp15060_dcdc5_ranges[] = {
> +	REGULATOR_LINEAR_RANGE(800000,
> +			       AXP15060_DCDC5_800mV_START,
> +			       AXP15060_DCDC5_800mV_END,
> +			       10000),
> +	REGULATOR_LINEAR_RANGE(1140000,
> +			       AXP15060_DCDC5_1140mV_START,
> +			       AXP15060_DCDC5_1140mV_END,
> +			       20000),
> +};
> +
> +static const struct regulator_desc axp15060_regulators[] = {
> +	AXP_DESC(AXP15060, DCDC1, "dcdc1", "vin1", 1500, 3400, 100,
> +		 AXP15060_DCDC1_V_CTRL, AXP15060_DCDC1_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC1_MASK),
> +	AXP_DESC_RANGES(AXP15060, DCDC2, "dcdc2", "vin2",
> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> +			AXP15060_DCDC2_V_CTRL, AXP15060_DCDC2_V_CTRL_MASK,
> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC2_MASK),
> +	AXP_DESC_RANGES(AXP15060, DCDC3, "dcdc3", "vin3",
> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> +			AXP15060_DCDC3_V_CTRL, AXP15060_DCDC3_V_CTRL_MASK,
> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC3_MASK),
> +	AXP_DESC_RANGES(AXP15060, DCDC4, "dcdc4", "vin4",
> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> +			AXP15060_DCDC4_V_CTRL, AXP15060_DCDC4_V_CTRL_MASK,
> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC4_MASK),
> +	AXP_DESC_RANGES(AXP15060, DCDC5, "dcdc5", "vin5",
> +			axp15060_dcdc5_ranges, AXP15060_DCDC5_NUM_VOLTAGES,
> +			AXP15060_DCDC5_V_CTRL, AXP15060_DCDC5_V_CTRL_MASK,
> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC5_MASK),
> +	AXP_DESC(AXP15060, DCDC6, "dcdc6", "vin6", 500, 3400, 100,
> +		 AXP15060_DCDC6_V_CTRL, AXP15060_DCDC6_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC6_MASK),
> +	AXP_DESC(AXP15060, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
> +		 AXP15060_ALDO1_V_CTRL, AXP15060_ALDO1_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO1_MASK),
> +	AXP_DESC(AXP15060, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
> +		 AXP15060_ALDO2_V_CTRL, AXP15060_ALDO2_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO2_MASK),
> +	AXP_DESC(AXP15060, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
> +		 AXP15060_ALDO3_V_CTRL, AXP15060_ALDO3_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO3_MASK),
> +	AXP_DESC(AXP15060, ALDO4, "aldo4", "aldoin", 700, 3300, 100,
> +		 AXP15060_ALDO4_V_CTRL, AXP15060_ALDO4_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO4_MASK),
> +	AXP_DESC(AXP15060, ALDO5, "aldo5", "aldoin", 700, 3300, 100,
> +		 AXP15060_ALDO5_V_CTRL, AXP15060_ALDO5_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO5_MASK),
> +	AXP_DESC(AXP15060, BLDO1, "bldo1", "bldoin", 700, 3300, 100,
> +		 AXP15060_BLDO1_V_CTRL, AXP15060_BLDO1_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO1_MASK),
> +	AXP_DESC(AXP15060, BLDO2, "bldo2", "bldoin", 700, 3300, 100,
> +		 AXP15060_BLDO2_V_CTRL, AXP15060_BLDO2_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO2_MASK),
> +	AXP_DESC(AXP15060, BLDO3, "bldo3", "bldoin", 700, 3300, 100,
> +		 AXP15060_BLDO3_V_CTRL, AXP15060_BLDO3_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO3_MASK),
> +	AXP_DESC(AXP15060, BLDO4, "bldo4", "bldoin", 700, 3300, 100,
> +		 AXP15060_BLDO4_V_CTRL, AXP15060_BLDO4_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO4_MASK),
> +	AXP_DESC(AXP15060, BLDO5, "bldo5", "bldoin", 700, 3300, 100,
> +		 AXP15060_BLDO5_V_CTRL, AXP15060_BLDO5_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO5_MASK),
> +	AXP_DESC(AXP15060, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
> +		 AXP15060_CLDO1_V_CTRL, AXP15060_CLDO1_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO1_MASK),
> +	AXP_DESC(AXP15060, CLDO2, "cldo2", "cldoin", 700, 3300, 100,
> +		 AXP15060_CLDO2_V_CTRL, AXP15060_CLDO2_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO2_MASK),
> +	AXP_DESC(AXP15060, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
> +		 AXP15060_CLDO3_V_CTRL, AXP15060_CLDO3_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO3_MASK),
> +	AXP_DESC(AXP15060, CLDO4, "cldo4", "cldoin", 700, 4200, 100,
> +		 AXP15060_CLDO4_V_CTRL, AXP15060_CLDO4_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO4_MASK),
> +	AXP_DESC(AXP15060, CPUSLDO, "cpusldo", NULL, 700, 1400, 50,
> +		 AXP15060_CPUSLDO_V_CTRL, AXP15060_CPUSLDO_V_CTRL_MASK,
> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CPUSLDO_MASK),
> +	/* Supply comes from DCDC5 */

This comment (and the two below) should be *before* the respective
description.

> +	AXP_DESC_SW(AXP15060, SW, "swout", NULL,

Shouldn't the name here be "dc1sw", to match the other regulators? Also
"swout" wouldn't be covered by the regulator binding regexp, IIUC.

> +		    AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_SW_MASK),
> +	/* Supply comes from DCDC1 */
> +	AXP_DESC_FIXED(AXP15060, RTC_LDO, "rtc-ldo", NULL, 1800),
> +	/* Supply comes from ALDO1 */
> +};

So I compared all the bits and register offset above against the
(AXP853T) manual: they match.

> +
>  static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>  {
>  	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -1145,6 +1311,15 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
>  		workmode <<= id - AXP813_DCDC1;
>  		break;
>  
> +	case AXP15060_ID:
> +		reg = AXP15060_DCDC_MODE_CTRL2;
> +		if (id < AXP15060_DCDC1 || id > AXP15060_DCDC6)
> +			return -EINVAL;
> +
> +		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP15060_DCDC1);
> +		workmode <<= id - AXP15060_DCDC1;
> +		break;
> +
>  	default:
>  		/* should not happen */
>  		WARN_ON(1);
> @@ -1164,7 +1339,7 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>  
>  	/*
>  	 * Currently in our supported AXP variants, only AXP803, AXP806,
> -	 * and AXP813 have polyphase regulators.
> +	 * AXP813 and AXP15060 have polyphase regulators.
>  	 */
>  	switch (axp20x->variant) {
>  	case AXP803_ID:
> @@ -1196,6 +1371,17 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>  		}
>  		break;
>  
> +	case AXP15060_ID:
> +		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
> +
> +		switch (id) {
> +		case AXP15060_DCDC3:
> +			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
> +		case AXP15060_DCDC6:
> +			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
> +		}
> +		break;
> +
>  	default:
>  		return false;
>  	}
> @@ -1217,6 +1403,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  	u32 workmode;
>  	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
>  	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> +	const char *aldo1_name = axp15060_regulators[AXP15060_ALDO1].name;
>  	bool drivevbus = false;
>  
>  	switch (axp20x->variant) {
> @@ -1252,6 +1439,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
>  						  "x-powers,drive-vbus-en");
>  		break;
> +	case AXP15060_ID:
> +		regulators = axp15060_regulators;
> +		nregulators = AXP15060_REG_ID_MAX;
> +		break;
>  	default:
>  		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
>  			axp20x->variant);
> @@ -1278,8 +1469,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  			continue;
>  
>  		/*
> -		 * Regulators DC1SW and DC5LDO are connected internally,
> -		 * so we have to handle their supply names separately.
> +		 * Regulators DC1SW, DC5LDO and RTCLDO on AXP15060 are
> +		 * connected internally, so we have to handle their supply
> +		 * names separately.
>  		 *
>  		 * We always register the regulators in proper sequence,
>  		 * so the supply names are correctly read. See the last
> @@ -1288,7 +1480,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  		 */
>  		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
>  		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
> -		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
> +			(regulators == axp809_regulators && i == AXP809_DC1SW) ||

looks like whitespace is off here

> +		    (regulators == axp15060_regulators && i == AXP15060_SW)) {

Can you name that AXP15060_DC1SW?

>  			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>  						GFP_KERNEL);
>  			if (!new_desc)
> @@ -1300,7 +1493,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  		}
>  
>  		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
> -		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
> +			(regulators == axp809_regulators && i == AXP809_DC5LDO) ||

whitespace, and again some more times below.

The rest looks good to me.

Cheers,
Andre

> +		    (regulators == axp15060_regulators && i == AXP15060_CPUSLDO)) {
>  			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>  						GFP_KERNEL);
>  			if (!new_desc)
> @@ -1311,6 +1505,18 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  			desc = new_desc;
>  		}
>  
> +
> +		if (regulators == axp15060_regulators && i == AXP15060_RTC_LDO) {
> +			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
> +						GFP_KERNEL);
> +			if (!new_desc)
> +				return -ENOMEM;
> +
> +			*new_desc = regulators[i];
> +			new_desc->supply_name = aldo1_name;
> +			desc = new_desc;
> +		}
> +
>  		rdev = devm_regulator_register(&pdev->dev, desc, &config);
>  		if (IS_ERR(rdev)) {
>  			dev_err(&pdev->dev, "Failed to register %s\n",
> @@ -1329,19 +1535,26 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>  		}
>  
>  		/*
> -		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
> +		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
>  		 */
>  		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
> -		    (regulators == axp809_regulators && i == AXP809_DCDC1))
> +		    (regulators == axp809_regulators && i == AXP809_DCDC1) ||
> +			(regulators == axp15060_regulators && i == AXP15060_DCDC1))
>  			of_property_read_string(rdev->dev.of_node,
>  						"regulator-name",
>  						&dcdc1_name);
>  
>  		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
> -		    (regulators == axp809_regulators && i == AXP809_DCDC5))
> +		    (regulators == axp809_regulators && i == AXP809_DCDC5) ||
> +			(regulators == axp15060_regulators && i == AXP15060_DCDC5))
>  			of_property_read_string(rdev->dev.of_node,
>  						"regulator-name",
>  						&dcdc5_name);
> +
> +		if (regulators == axp15060_regulators && i == AXP15060_ALDO1)
> +			of_property_read_string(rdev->dev.of_node,
> +						"regulator-name",
> +						&aldo1_name);
>  	}
>  
>  	if (drivevbus) {


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support
  2023-04-17 14:18   ` Andre Przywara
@ 2023-04-17 16:01     ` Shengyu Qu
  2023-04-18 21:57       ` Andre Przywara
  0 siblings, 1 reply; 14+ messages in thread
From: Shengyu Qu @ 2023-04-17 16:01 UTC (permalink / raw)
  To: Andre Przywara
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka


[-- Attachment #1.1.1: Type: text/plain, Size: 17286 bytes --]

Hi,
> On Fri,  7 Apr 2023 22:18:12 +0800
> Shengyu Qu <wiagn233@outlook.com> wrote:
>
> Hi,
>
>> The AXP15060 is a typical I2C-controlled PMIC, seen on multiple boards
>> with different default register value. Current driver is tested on
>> Starfive Visionfive 2.
>>
>> The RTCLDO is fixed, and cannot even be turned on or off. On top of
>> that, its voltage is customisable (either 1.8V or 3.3V). We pretend it's
>> a fixed 1.8V regulator since other AXP driver also do like this. Also,
>> BSP code ignores this regulator and it's not used according to VF2
>> schematic.
>>
>> Describe the AXP15060's voltage settings and switch registers, how the
>> voltages are encoded, and connect this to the MFD device via its
>> regulator ID.
>>
>> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
>> ---
>>   drivers/regulator/axp20x-regulator.c | 229 ++++++++++++++++++++++++++-
>>   1 file changed, 221 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
>> index d260c442b788..ece4af93df7b 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -270,6 +270,74 @@
>>   
>>   #define AXP813_PWR_OUT_DCDC7_MASK	BIT_MASK(6)
>>   
>> +#define AXP15060_DCDC1_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_DCDC2_V_CTRL_MASK		GENMASK(6, 0)
>> +#define AXP15060_DCDC3_V_CTRL_MASK		GENMASK(6, 0)
>> +#define AXP15060_DCDC4_V_CTRL_MASK		GENMASK(6, 0)
>> +#define AXP15060_DCDC5_V_CTRL_MASK		GENMASK(6, 0)
>> +#define AXP15060_DCDC6_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_ALDO1_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_ALDO2_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_ALDO3_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_ALDO4_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_ALDO5_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_BLDO1_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_BLDO2_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_BLDO3_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_BLDO4_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_BLDO5_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_CLDO1_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_CLDO2_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_CLDO3_V_CTRL_MASK		GENMASK(4, 0)
>> +#define AXP15060_CLDO4_V_CTRL_MASK		GENMASK(5, 0)
>> +#define AXP15060_CPUSLDO_V_CTRL_MASK		GENMASK(3, 0)
>> +
>> +#define AXP15060_PWR_OUT_DCDC1_MASK	BIT_MASK(0)
> I'd say "BIT(0)" is the canonical way of denoting single bit masks,
> outside of Linux' bitmask framework.

I'm using BIT(0) to keep consistent with other existing definitions.

>
>> +#define AXP15060_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
>> +#define AXP15060_PWR_OUT_DCDC3_MASK	BIT_MASK(2)
>> +#define AXP15060_PWR_OUT_DCDC4_MASK	BIT_MASK(3)
>> +#define AXP15060_PWR_OUT_DCDC5_MASK	BIT_MASK(4)
>> +#define AXP15060_PWR_OUT_DCDC6_MASK	BIT_MASK(5)
>> +#define AXP15060_PWR_OUT_ALDO1_MASK	BIT_MASK(0)
>> +#define AXP15060_PWR_OUT_ALDO2_MASK	BIT_MASK(1)
>> +#define AXP15060_PWR_OUT_ALDO3_MASK	BIT_MASK(2)
>> +#define AXP15060_PWR_OUT_ALDO4_MASK	BIT_MASK(3)
>> +#define AXP15060_PWR_OUT_ALDO5_MASK	BIT_MASK(4)
>> +#define AXP15060_PWR_OUT_BLDO1_MASK	BIT_MASK(5)
>> +#define AXP15060_PWR_OUT_BLDO2_MASK	BIT_MASK(6)
>> +#define AXP15060_PWR_OUT_BLDO3_MASK	BIT_MASK(7)
>> +#define AXP15060_PWR_OUT_BLDO4_MASK	BIT_MASK(0)
>> +#define AXP15060_PWR_OUT_BLDO5_MASK	BIT_MASK(1)
>> +#define AXP15060_PWR_OUT_CLDO1_MASK	BIT_MASK(2)
>> +#define AXP15060_PWR_OUT_CLDO2_MASK	BIT_MASK(3)
>> +#define AXP15060_PWR_OUT_CLDO3_MASK	BIT_MASK(4)
>> +#define AXP15060_PWR_OUT_CLDO4_MASK	BIT_MASK(5)
>> +#define AXP15060_PWR_OUT_CPUSLDO_MASK	BIT_MASK(6)
>> +#define AXP15060_PWR_OUT_SW_MASK		BIT_MASK(7)
>> +
>> +#define AXP15060_DCDC23_POLYPHASE_DUAL_MASK		BIT_MASK(6)
>> +#define AXP15060_DCDC46_POLYPHASE_DUAL_MASK		BIT_MASK(7)
>> +
>> +#define AXP15060_DCDC234_500mV_START	0x00
> Can you make those values consistently decimal? It's a bit confusing to
> see the 70 steps in decimal, but the start (71) in hex.
Also consistence problem here.
>
>> +#define AXP15060_DCDC234_500mV_STEPS	70
>> +#define AXP15060_DCDC234_500mV_END		\
>> +	(AXP15060_DCDC234_500mV_START + AXP15060_DCDC234_500mV_STEPS)
>> +#define AXP15060_DCDC234_1220mV_START	0x47
>> +#define AXP15060_DCDC234_1220mV_STEPS	16
>> +#define AXP15060_DCDC234_1220mV_END		\
>> +	(AXP15060_DCDC234_1220mV_START + AXP15060_DCDC234_1220mV_STEPS)
>> +#define AXP15060_DCDC234_NUM_VOLTAGES	88
>> +
>> +#define AXP15060_DCDC5_800mV_START	0x00
>> +#define AXP15060_DCDC5_800mV_STEPS	32
>> +#define AXP15060_DCDC5_800mV_END		\
>> +	(AXP15060_DCDC5_800mV_START + AXP15060_DCDC5_800mV_STEPS)
>> +#define AXP15060_DCDC5_1140mV_START	0x21
>> +#define AXP15060_DCDC5_1140mV_STEPS	35
>> +#define AXP15060_DCDC5_1140mV_END		\
>> +	(AXP15060_DCDC5_1140mV_START + AXP15060_DCDC5_1140mV_STEPS)
>> +#define AXP15060_DCDC5_NUM_VOLTAGES	69
>> +
>>   #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
>>   		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
>>   	[_family##_##_id] = {							\
>> @@ -1001,6 +1069,104 @@ static const struct regulator_desc axp813_regulators[] = {
>>   		    AXP22X_PWR_OUT_CTRL2, AXP22X_PWR_OUT_DC1SW_MASK),
>>   };
>>   
>> +static const struct linear_range axp15060_dcdc234_ranges[] = {
>> +	REGULATOR_LINEAR_RANGE(500000,
>> +			       AXP15060_DCDC234_500mV_START,
>> +			       AXP15060_DCDC234_500mV_END,
>> +			       10000),
>> +	REGULATOR_LINEAR_RANGE(1220000,
>> +			       AXP15060_DCDC234_1220mV_START,
>> +			       AXP15060_DCDC234_1220mV_END,
>> +			       20000),
>> +};
>> +
>> +static const struct linear_range axp15060_dcdc5_ranges[] = {
>> +	REGULATOR_LINEAR_RANGE(800000,
>> +			       AXP15060_DCDC5_800mV_START,
>> +			       AXP15060_DCDC5_800mV_END,
>> +			       10000),
>> +	REGULATOR_LINEAR_RANGE(1140000,
>> +			       AXP15060_DCDC5_1140mV_START,
>> +			       AXP15060_DCDC5_1140mV_END,
>> +			       20000),
>> +};
>> +
>> +static const struct regulator_desc axp15060_regulators[] = {
>> +	AXP_DESC(AXP15060, DCDC1, "dcdc1", "vin1", 1500, 3400, 100,
>> +		 AXP15060_DCDC1_V_CTRL, AXP15060_DCDC1_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC1_MASK),
>> +	AXP_DESC_RANGES(AXP15060, DCDC2, "dcdc2", "vin2",
>> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
>> +			AXP15060_DCDC2_V_CTRL, AXP15060_DCDC2_V_CTRL_MASK,
>> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC2_MASK),
>> +	AXP_DESC_RANGES(AXP15060, DCDC3, "dcdc3", "vin3",
>> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
>> +			AXP15060_DCDC3_V_CTRL, AXP15060_DCDC3_V_CTRL_MASK,
>> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC3_MASK),
>> +	AXP_DESC_RANGES(AXP15060, DCDC4, "dcdc4", "vin4",
>> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
>> +			AXP15060_DCDC4_V_CTRL, AXP15060_DCDC4_V_CTRL_MASK,
>> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC4_MASK),
>> +	AXP_DESC_RANGES(AXP15060, DCDC5, "dcdc5", "vin5",
>> +			axp15060_dcdc5_ranges, AXP15060_DCDC5_NUM_VOLTAGES,
>> +			AXP15060_DCDC5_V_CTRL, AXP15060_DCDC5_V_CTRL_MASK,
>> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC5_MASK),
>> +	AXP_DESC(AXP15060, DCDC6, "dcdc6", "vin6", 500, 3400, 100,
>> +		 AXP15060_DCDC6_V_CTRL, AXP15060_DCDC6_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC6_MASK),
>> +	AXP_DESC(AXP15060, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
>> +		 AXP15060_ALDO1_V_CTRL, AXP15060_ALDO1_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO1_MASK),
>> +	AXP_DESC(AXP15060, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
>> +		 AXP15060_ALDO2_V_CTRL, AXP15060_ALDO2_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO2_MASK),
>> +	AXP_DESC(AXP15060, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
>> +		 AXP15060_ALDO3_V_CTRL, AXP15060_ALDO3_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO3_MASK),
>> +	AXP_DESC(AXP15060, ALDO4, "aldo4", "aldoin", 700, 3300, 100,
>> +		 AXP15060_ALDO4_V_CTRL, AXP15060_ALDO4_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO4_MASK),
>> +	AXP_DESC(AXP15060, ALDO5, "aldo5", "aldoin", 700, 3300, 100,
>> +		 AXP15060_ALDO5_V_CTRL, AXP15060_ALDO5_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO5_MASK),
>> +	AXP_DESC(AXP15060, BLDO1, "bldo1", "bldoin", 700, 3300, 100,
>> +		 AXP15060_BLDO1_V_CTRL, AXP15060_BLDO1_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO1_MASK),
>> +	AXP_DESC(AXP15060, BLDO2, "bldo2", "bldoin", 700, 3300, 100,
>> +		 AXP15060_BLDO2_V_CTRL, AXP15060_BLDO2_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO2_MASK),
>> +	AXP_DESC(AXP15060, BLDO3, "bldo3", "bldoin", 700, 3300, 100,
>> +		 AXP15060_BLDO3_V_CTRL, AXP15060_BLDO3_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO3_MASK),
>> +	AXP_DESC(AXP15060, BLDO4, "bldo4", "bldoin", 700, 3300, 100,
>> +		 AXP15060_BLDO4_V_CTRL, AXP15060_BLDO4_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO4_MASK),
>> +	AXP_DESC(AXP15060, BLDO5, "bldo5", "bldoin", 700, 3300, 100,
>> +		 AXP15060_BLDO5_V_CTRL, AXP15060_BLDO5_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO5_MASK),
>> +	AXP_DESC(AXP15060, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
>> +		 AXP15060_CLDO1_V_CTRL, AXP15060_CLDO1_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO1_MASK),
>> +	AXP_DESC(AXP15060, CLDO2, "cldo2", "cldoin", 700, 3300, 100,
>> +		 AXP15060_CLDO2_V_CTRL, AXP15060_CLDO2_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO2_MASK),
>> +	AXP_DESC(AXP15060, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
>> +		 AXP15060_CLDO3_V_CTRL, AXP15060_CLDO3_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO3_MASK),
>> +	AXP_DESC(AXP15060, CLDO4, "cldo4", "cldoin", 700, 4200, 100,
>> +		 AXP15060_CLDO4_V_CTRL, AXP15060_CLDO4_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO4_MASK),
>> +	AXP_DESC(AXP15060, CPUSLDO, "cpusldo", NULL, 700, 1400, 50,
>> +		 AXP15060_CPUSLDO_V_CTRL, AXP15060_CPUSLDO_V_CTRL_MASK,
>> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CPUSLDO_MASK),
>> +	/* Supply comes from DCDC5 */
> This comment (and the two below) should be *before* the respective
> description.
Thanks, would fix.
>> +	AXP_DESC_SW(AXP15060, SW, "swout", NULL,
> Shouldn't the name here be "dc1sw", to match the other regulators? Also
> "swout" wouldn't be covered by the regulator binding regexp, IIUC.

In my version of datasheet(AXP15060 v1.0), it's named SW, not dc1sw in

other variants' datasheet. So I think keeping "sw" would be better?

"swout" would be fixed, thanks.

>> +		    AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_SW_MASK),
>> +	/* Supply comes from DCDC1 */
>> +	AXP_DESC_FIXED(AXP15060, RTC_LDO, "rtc-ldo", NULL, 1800),
>> +	/* Supply comes from ALDO1 */
>> +};
> So I compared all the bits and register offset above against the
> (AXP853T) manual: they match.
>
>> +
>>   static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>>   {
>>   	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
>> @@ -1145,6 +1311,15 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
>>   		workmode <<= id - AXP813_DCDC1;
>>   		break;
>>   
>> +	case AXP15060_ID:
>> +		reg = AXP15060_DCDC_MODE_CTRL2;
>> +		if (id < AXP15060_DCDC1 || id > AXP15060_DCDC6)
>> +			return -EINVAL;
>> +
>> +		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP15060_DCDC1);
>> +		workmode <<= id - AXP15060_DCDC1;
>> +		break;
>> +
>>   	default:
>>   		/* should not happen */
>>   		WARN_ON(1);
>> @@ -1164,7 +1339,7 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>>   
>>   	/*
>>   	 * Currently in our supported AXP variants, only AXP803, AXP806,
>> -	 * and AXP813 have polyphase regulators.
>> +	 * AXP813 and AXP15060 have polyphase regulators.
>>   	 */
>>   	switch (axp20x->variant) {
>>   	case AXP803_ID:
>> @@ -1196,6 +1371,17 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>>   		}
>>   		break;
>>   
>> +	case AXP15060_ID:
>> +		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
>> +
>> +		switch (id) {
>> +		case AXP15060_DCDC3:
>> +			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
>> +		case AXP15060_DCDC6:
>> +			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
>> +		}
>> +		break;
>> +
>>   	default:
>>   		return false;
>>   	}
>> @@ -1217,6 +1403,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   	u32 workmode;
>>   	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
>>   	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
>> +	const char *aldo1_name = axp15060_regulators[AXP15060_ALDO1].name;
>>   	bool drivevbus = false;
>>   
>>   	switch (axp20x->variant) {
>> @@ -1252,6 +1439,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
>>   						  "x-powers,drive-vbus-en");
>>   		break;
>> +	case AXP15060_ID:
>> +		regulators = axp15060_regulators;
>> +		nregulators = AXP15060_REG_ID_MAX;
>> +		break;
>>   	default:
>>   		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
>>   			axp20x->variant);
>> @@ -1278,8 +1469,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   			continue;
>>   
>>   		/*
>> -		 * Regulators DC1SW and DC5LDO are connected internally,
>> -		 * so we have to handle their supply names separately.
>> +		 * Regulators DC1SW, DC5LDO and RTCLDO on AXP15060 are
>> +		 * connected internally, so we have to handle their supply
>> +		 * names separately.
>>   		 *
>>   		 * We always register the regulators in proper sequence,
>>   		 * so the supply names are correctly read. See the last
>> @@ -1288,7 +1480,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   		 */
>>   		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
>>   		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
>> -		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
>> +			(regulators == axp809_regulators && i == AXP809_DC1SW) ||
> looks like whitespace is off here
Would fix.
>
>> +		    (regulators == axp15060_regulators && i == AXP15060_SW)) {
> Can you name that AXP15060_DC1SW?
>
>>   			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>>   						GFP_KERNEL);
>>   			if (!new_desc)
>> @@ -1300,7 +1493,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   		}
>>   
>>   		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
>> -		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
>> +			(regulators == axp809_regulators && i == AXP809_DC5LDO) ||
> whitespace, and again some more times below.
OK.
>
> The rest looks good to me.

Thanks for reviewing.

Best regards,

Shengyu

>
> Cheers,
> Andre
>
>> +		    (regulators == axp15060_regulators && i == AXP15060_CPUSLDO)) {
>>   			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>>   						GFP_KERNEL);
>>   			if (!new_desc)
>> @@ -1311,6 +1505,18 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   			desc = new_desc;
>>   		}
>>   
>> +
>> +		if (regulators == axp15060_regulators && i == AXP15060_RTC_LDO) {
>> +			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>> +						GFP_KERNEL);
>> +			if (!new_desc)
>> +				return -ENOMEM;
>> +
>> +			*new_desc = regulators[i];
>> +			new_desc->supply_name = aldo1_name;
>> +			desc = new_desc;
>> +		}
>> +
>>   		rdev = devm_regulator_register(&pdev->dev, desc, &config);
>>   		if (IS_ERR(rdev)) {
>>   			dev_err(&pdev->dev, "Failed to register %s\n",
>> @@ -1329,19 +1535,26 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>>   		}
>>   
>>   		/*
>> -		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
>> +		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
>>   		 */
>>   		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
>> -		    (regulators == axp809_regulators && i == AXP809_DCDC1))
>> +		    (regulators == axp809_regulators && i == AXP809_DCDC1) ||
>> +			(regulators == axp15060_regulators && i == AXP15060_DCDC1))
>>   			of_property_read_string(rdev->dev.of_node,
>>   						"regulator-name",
>>   						&dcdc1_name);
>>   
>>   		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
>> -		    (regulators == axp809_regulators && i == AXP809_DCDC5))
>> +		    (regulators == axp809_regulators && i == AXP809_DCDC5) ||
>> +			(regulators == axp15060_regulators && i == AXP15060_DCDC5))
>>   			of_property_read_string(rdev->dev.of_node,
>>   						"regulator-name",
>>   						&dcdc5_name);
>> +
>> +		if (regulators == axp15060_regulators && i == AXP15060_ALDO1)
>> +			of_property_read_string(rdev->dev.of_node,
>> +						"regulator-name",
>> +						&aldo1_name);
>>   	}
>>   
>>   	if (drivevbus) {

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support
  2023-04-17 16:01     ` Shengyu Qu
@ 2023-04-18 21:57       ` Andre Przywara
  0 siblings, 0 replies; 14+ messages in thread
From: Andre Przywara @ 2023-04-18 21:57 UTC (permalink / raw)
  To: Shengyu Qu
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka

On Tue, 18 Apr 2023 00:01:27 +0800
Shengyu Qu <wiagn233@outlook.com> wrote:

Hi Shengyu,

I don't insist on the changes below (that's more up to the maintainers
to decide), but just wanted to add some more rationale to my former
suggestions.

> Hi,
> > On Fri,  7 Apr 2023 22:18:12 +0800
> > Shengyu Qu <wiagn233@outlook.com> wrote:
> >
> > Hi,
> >  
> >> The AXP15060 is a typical I2C-controlled PMIC, seen on multiple boards
> >> with different default register value. Current driver is tested on
> >> Starfive Visionfive 2.
> >>
> >> The RTCLDO is fixed, and cannot even be turned on or off. On top of
> >> that, its voltage is customisable (either 1.8V or 3.3V). We pretend it's
> >> a fixed 1.8V regulator since other AXP driver also do like this. Also,
> >> BSP code ignores this regulator and it's not used according to VF2
> >> schematic.
> >>
> >> Describe the AXP15060's voltage settings and switch registers, how the
> >> voltages are encoded, and connect this to the MFD device via its
> >> regulator ID.
> >>
> >> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> >> ---
> >>   drivers/regulator/axp20x-regulator.c | 229 ++++++++++++++++++++++++++-
> >>   1 file changed, 221 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> >> index d260c442b788..ece4af93df7b 100644
> >> --- a/drivers/regulator/axp20x-regulator.c
> >> +++ b/drivers/regulator/axp20x-regulator.c
> >> @@ -270,6 +270,74 @@
> >>   
> >>   #define AXP813_PWR_OUT_DCDC7_MASK	BIT_MASK(6)
> >>   
> >> +#define AXP15060_DCDC1_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_DCDC2_V_CTRL_MASK		GENMASK(6, 0)
> >> +#define AXP15060_DCDC3_V_CTRL_MASK		GENMASK(6, 0)
> >> +#define AXP15060_DCDC4_V_CTRL_MASK		GENMASK(6, 0)
> >> +#define AXP15060_DCDC5_V_CTRL_MASK		GENMASK(6, 0)
> >> +#define AXP15060_DCDC6_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_ALDO1_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_ALDO2_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_ALDO3_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_ALDO4_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_ALDO5_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_BLDO1_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_BLDO2_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_BLDO3_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_BLDO4_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_BLDO5_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_CLDO1_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_CLDO2_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_CLDO3_V_CTRL_MASK		GENMASK(4, 0)
> >> +#define AXP15060_CLDO4_V_CTRL_MASK		GENMASK(5, 0)
> >> +#define AXP15060_CPUSLDO_V_CTRL_MASK		GENMASK(3, 0)
> >> +
> >> +#define AXP15060_PWR_OUT_DCDC1_MASK	BIT_MASK(0)  
> > I'd say "BIT(0)" is the canonical way of denoting single bit masks,
> > outside of Linux' bitmask framework.  
> 
> I'm using BIT(0) to keep consistent with other existing definitions.

I don't know if it's a good idea to copy from code which is suboptimal.
To just denote a bit in a register, I see BIT() everywhere, BIT_MASK
does the modulo sizeof(long), which doesn't make much sense here. It
just looks like it's meant to be used with Linux' bitmasks, which can
span several longs, so you use a combination of BIT_WORD/BIT_MASK.

> >> +#define AXP15060_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
> >> +#define AXP15060_PWR_OUT_DCDC3_MASK	BIT_MASK(2)
> >> +#define AXP15060_PWR_OUT_DCDC4_MASK	BIT_MASK(3)
> >> +#define AXP15060_PWR_OUT_DCDC5_MASK	BIT_MASK(4)
> >> +#define AXP15060_PWR_OUT_DCDC6_MASK	BIT_MASK(5)
> >> +#define AXP15060_PWR_OUT_ALDO1_MASK	BIT_MASK(0)
> >> +#define AXP15060_PWR_OUT_ALDO2_MASK	BIT_MASK(1)
> >> +#define AXP15060_PWR_OUT_ALDO3_MASK	BIT_MASK(2)
> >> +#define AXP15060_PWR_OUT_ALDO4_MASK	BIT_MASK(3)
> >> +#define AXP15060_PWR_OUT_ALDO5_MASK	BIT_MASK(4)
> >> +#define AXP15060_PWR_OUT_BLDO1_MASK	BIT_MASK(5)
> >> +#define AXP15060_PWR_OUT_BLDO2_MASK	BIT_MASK(6)
> >> +#define AXP15060_PWR_OUT_BLDO3_MASK	BIT_MASK(7)
> >> +#define AXP15060_PWR_OUT_BLDO4_MASK	BIT_MASK(0)
> >> +#define AXP15060_PWR_OUT_BLDO5_MASK	BIT_MASK(1)
> >> +#define AXP15060_PWR_OUT_CLDO1_MASK	BIT_MASK(2)
> >> +#define AXP15060_PWR_OUT_CLDO2_MASK	BIT_MASK(3)
> >> +#define AXP15060_PWR_OUT_CLDO3_MASK	BIT_MASK(4)
> >> +#define AXP15060_PWR_OUT_CLDO4_MASK	BIT_MASK(5)
> >> +#define AXP15060_PWR_OUT_CPUSLDO_MASK	BIT_MASK(6)
> >> +#define AXP15060_PWR_OUT_SW_MASK		BIT_MASK(7)
> >> +
> >> +#define AXP15060_DCDC23_POLYPHASE_DUAL_MASK		BIT_MASK(6)
> >> +#define AXP15060_DCDC46_POLYPHASE_DUAL_MASK		BIT_MASK(7)
> >> +
> >> +#define AXP15060_DCDC234_500mV_START	0x00  
> > Can you make those values consistently decimal? It's a bit confusing to
> > see the 70 steps in decimal, but the start (71) in hex.  
> Also consistence problem here.

I would actually prefer consistency within one definition. I don't
think we should copy all old code just because it is there already. I
honestly don't find those definitions easy to read, compare, understand
or review.
For the AXP313a we went with something more compressed, by just putting
the actual values into the definition, skipping the longish macros:

	REGULATOR_LINEAR_RANGE(500000,   0, 70, 10000),
	REGULATOR_LINEAR_RANGE(1220000, 71, 87, 20000),

> >> +#define AXP15060_DCDC234_500mV_STEPS	70
> >> +#define AXP15060_DCDC234_500mV_END		\
> >> +	(AXP15060_DCDC234_500mV_START + AXP15060_DCDC234_500mV_STEPS)
> >> +#define AXP15060_DCDC234_1220mV_START	0x47
> >> +#define AXP15060_DCDC234_1220mV_STEPS	16
> >> +#define AXP15060_DCDC234_1220mV_END		\
> >> +	(AXP15060_DCDC234_1220mV_START + AXP15060_DCDC234_1220mV_STEPS)
> >> +#define AXP15060_DCDC234_NUM_VOLTAGES	88
> >> +
> >> +#define AXP15060_DCDC5_800mV_START	0x00
> >> +#define AXP15060_DCDC5_800mV_STEPS	32
> >> +#define AXP15060_DCDC5_800mV_END		\
> >> +	(AXP15060_DCDC5_800mV_START + AXP15060_DCDC5_800mV_STEPS)
> >> +#define AXP15060_DCDC5_1140mV_START	0x21
> >> +#define AXP15060_DCDC5_1140mV_STEPS	35
> >> +#define AXP15060_DCDC5_1140mV_END		\
> >> +	(AXP15060_DCDC5_1140mV_START + AXP15060_DCDC5_1140mV_STEPS)
> >> +#define AXP15060_DCDC5_NUM_VOLTAGES	69
> >> +
> >>   #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
> >>   		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
> >>   	[_family##_##_id] = {							\
> >> @@ -1001,6 +1069,104 @@ static const struct regulator_desc axp813_regulators[] = {
> >>   		    AXP22X_PWR_OUT_CTRL2, AXP22X_PWR_OUT_DC1SW_MASK),
> >>   };
> >>   
> >> +static const struct linear_range axp15060_dcdc234_ranges[] = {
> >> +	REGULATOR_LINEAR_RANGE(500000,
> >> +			       AXP15060_DCDC234_500mV_START,
> >> +			       AXP15060_DCDC234_500mV_END,
> >> +			       10000),
> >> +	REGULATOR_LINEAR_RANGE(1220000,
> >> +			       AXP15060_DCDC234_1220mV_START,
> >> +			       AXP15060_DCDC234_1220mV_END,
> >> +			       20000),
> >> +};
> >> +
> >> +static const struct linear_range axp15060_dcdc5_ranges[] = {
> >> +	REGULATOR_LINEAR_RANGE(800000,
> >> +			       AXP15060_DCDC5_800mV_START,
> >> +			       AXP15060_DCDC5_800mV_END,
> >> +			       10000),
> >> +	REGULATOR_LINEAR_RANGE(1140000,
> >> +			       AXP15060_DCDC5_1140mV_START,
> >> +			       AXP15060_DCDC5_1140mV_END,
> >> +			       20000),
> >> +};
> >> +
> >> +static const struct regulator_desc axp15060_regulators[] = {
> >> +	AXP_DESC(AXP15060, DCDC1, "dcdc1", "vin1", 1500, 3400, 100,
> >> +		 AXP15060_DCDC1_V_CTRL, AXP15060_DCDC1_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC1_MASK),
> >> +	AXP_DESC_RANGES(AXP15060, DCDC2, "dcdc2", "vin2",
> >> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> >> +			AXP15060_DCDC2_V_CTRL, AXP15060_DCDC2_V_CTRL_MASK,
> >> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC2_MASK),
> >> +	AXP_DESC_RANGES(AXP15060, DCDC3, "dcdc3", "vin3",
> >> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> >> +			AXP15060_DCDC3_V_CTRL, AXP15060_DCDC3_V_CTRL_MASK,
> >> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC3_MASK),
> >> +	AXP_DESC_RANGES(AXP15060, DCDC4, "dcdc4", "vin4",
> >> +			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
> >> +			AXP15060_DCDC4_V_CTRL, AXP15060_DCDC4_V_CTRL_MASK,
> >> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC4_MASK),
> >> +	AXP_DESC_RANGES(AXP15060, DCDC5, "dcdc5", "vin5",
> >> +			axp15060_dcdc5_ranges, AXP15060_DCDC5_NUM_VOLTAGES,
> >> +			AXP15060_DCDC5_V_CTRL, AXP15060_DCDC5_V_CTRL_MASK,
> >> +			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC5_MASK),
> >> +	AXP_DESC(AXP15060, DCDC6, "dcdc6", "vin6", 500, 3400, 100,
> >> +		 AXP15060_DCDC6_V_CTRL, AXP15060_DCDC6_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC6_MASK),
> >> +	AXP_DESC(AXP15060, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
> >> +		 AXP15060_ALDO1_V_CTRL, AXP15060_ALDO1_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO1_MASK),
> >> +	AXP_DESC(AXP15060, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
> >> +		 AXP15060_ALDO2_V_CTRL, AXP15060_ALDO2_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO2_MASK),
> >> +	AXP_DESC(AXP15060, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
> >> +		 AXP15060_ALDO3_V_CTRL, AXP15060_ALDO3_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO3_MASK),
> >> +	AXP_DESC(AXP15060, ALDO4, "aldo4", "aldoin", 700, 3300, 100,
> >> +		 AXP15060_ALDO4_V_CTRL, AXP15060_ALDO4_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO4_MASK),
> >> +	AXP_DESC(AXP15060, ALDO5, "aldo5", "aldoin", 700, 3300, 100,
> >> +		 AXP15060_ALDO5_V_CTRL, AXP15060_ALDO5_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO5_MASK),
> >> +	AXP_DESC(AXP15060, BLDO1, "bldo1", "bldoin", 700, 3300, 100,
> >> +		 AXP15060_BLDO1_V_CTRL, AXP15060_BLDO1_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO1_MASK),
> >> +	AXP_DESC(AXP15060, BLDO2, "bldo2", "bldoin", 700, 3300, 100,
> >> +		 AXP15060_BLDO2_V_CTRL, AXP15060_BLDO2_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO2_MASK),
> >> +	AXP_DESC(AXP15060, BLDO3, "bldo3", "bldoin", 700, 3300, 100,
> >> +		 AXP15060_BLDO3_V_CTRL, AXP15060_BLDO3_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO3_MASK),
> >> +	AXP_DESC(AXP15060, BLDO4, "bldo4", "bldoin", 700, 3300, 100,
> >> +		 AXP15060_BLDO4_V_CTRL, AXP15060_BLDO4_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO4_MASK),
> >> +	AXP_DESC(AXP15060, BLDO5, "bldo5", "bldoin", 700, 3300, 100,
> >> +		 AXP15060_BLDO5_V_CTRL, AXP15060_BLDO5_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO5_MASK),
> >> +	AXP_DESC(AXP15060, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
> >> +		 AXP15060_CLDO1_V_CTRL, AXP15060_CLDO1_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO1_MASK),
> >> +	AXP_DESC(AXP15060, CLDO2, "cldo2", "cldoin", 700, 3300, 100,
> >> +		 AXP15060_CLDO2_V_CTRL, AXP15060_CLDO2_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO2_MASK),
> >> +	AXP_DESC(AXP15060, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
> >> +		 AXP15060_CLDO3_V_CTRL, AXP15060_CLDO3_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO3_MASK),
> >> +	AXP_DESC(AXP15060, CLDO4, "cldo4", "cldoin", 700, 4200, 100,
> >> +		 AXP15060_CLDO4_V_CTRL, AXP15060_CLDO4_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO4_MASK),
> >> +	AXP_DESC(AXP15060, CPUSLDO, "cpusldo", NULL, 700, 1400, 50,
> >> +		 AXP15060_CPUSLDO_V_CTRL, AXP15060_CPUSLDO_V_CTRL_MASK,
> >> +		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CPUSLDO_MASK),
> >> +	/* Supply comes from DCDC5 */  
> > This comment (and the two below) should be *before* the respective
> > description.  
> Thanks, would fix.
> >> +	AXP_DESC_SW(AXP15060, SW, "swout", NULL,  
> > Shouldn't the name here be "dc1sw", to match the other regulators? Also
> > "swout" wouldn't be covered by the regulator binding regexp, IIUC.  
> 
> In my version of datasheet(AXP15060 v1.0), it's named SW, not dc1sw in
> other variants' datasheet. So I think keeping "sw" would be better?

I don't know, just "sw" doesn't tell much, and other PMICs already use
DC1SW, so it's immediately clear what regulator this switch feeds
from.

> "swout" would be fixed, thanks.

You wouldn't need to add to this regexp there if you just use dc1sw.

Cheers,
Andre

> >> +		    AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_SW_MASK),
> >> +	/* Supply comes from DCDC1 */
> >> +	AXP_DESC_FIXED(AXP15060, RTC_LDO, "rtc-ldo", NULL, 1800),
> >> +	/* Supply comes from ALDO1 */
> >> +};  
> > So I compared all the bits and register offset above against the
> > (AXP853T) manual: they match.
> >  
> >> +
> >>   static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
> >>   {
> >>   	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> >> @@ -1145,6 +1311,15 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
> >>   		workmode <<= id - AXP813_DCDC1;
> >>   		break;
> >>   
> >> +	case AXP15060_ID:
> >> +		reg = AXP15060_DCDC_MODE_CTRL2;
> >> +		if (id < AXP15060_DCDC1 || id > AXP15060_DCDC6)
> >> +			return -EINVAL;
> >> +
> >> +		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP15060_DCDC1);
> >> +		workmode <<= id - AXP15060_DCDC1;
> >> +		break;
> >> +
> >>   	default:
> >>   		/* should not happen */
> >>   		WARN_ON(1);
> >> @@ -1164,7 +1339,7 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
> >>   
> >>   	/*
> >>   	 * Currently in our supported AXP variants, only AXP803, AXP806,
> >> -	 * and AXP813 have polyphase regulators.
> >> +	 * AXP813 and AXP15060 have polyphase regulators.
> >>   	 */
> >>   	switch (axp20x->variant) {
> >>   	case AXP803_ID:
> >> @@ -1196,6 +1371,17 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
> >>   		}
> >>   		break;
> >>   
> >> +	case AXP15060_ID:
> >> +		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
> >> +
> >> +		switch (id) {
> >> +		case AXP15060_DCDC3:
> >> +			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
> >> +		case AXP15060_DCDC6:
> >> +			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
> >> +		}
> >> +		break;
> >> +
> >>   	default:
> >>   		return false;
> >>   	}
> >> @@ -1217,6 +1403,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   	u32 workmode;
> >>   	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
> >>   	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> >> +	const char *aldo1_name = axp15060_regulators[AXP15060_ALDO1].name;
> >>   	bool drivevbus = false;
> >>   
> >>   	switch (axp20x->variant) {
> >> @@ -1252,6 +1439,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
> >>   						  "x-powers,drive-vbus-en");
> >>   		break;
> >> +	case AXP15060_ID:
> >> +		regulators = axp15060_regulators;
> >> +		nregulators = AXP15060_REG_ID_MAX;
> >> +		break;
> >>   	default:
> >>   		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> >>   			axp20x->variant);
> >> @@ -1278,8 +1469,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   			continue;
> >>   
> >>   		/*
> >> -		 * Regulators DC1SW and DC5LDO are connected internally,
> >> -		 * so we have to handle their supply names separately.
> >> +		 * Regulators DC1SW, DC5LDO and RTCLDO on AXP15060 are
> >> +		 * connected internally, so we have to handle their supply
> >> +		 * names separately.
> >>   		 *
> >>   		 * We always register the regulators in proper sequence,
> >>   		 * so the supply names are correctly read. See the last
> >> @@ -1288,7 +1480,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   		 */
> >>   		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
> >>   		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
> >> -		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
> >> +			(regulators == axp809_regulators && i == AXP809_DC1SW) ||  
> > looks like whitespace is off here  
> Would fix.
> >  
> >> +		    (regulators == axp15060_regulators && i == AXP15060_SW)) {  
> > Can you name that AXP15060_DC1SW?
> >  
> >>   			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
> >>   						GFP_KERNEL);
> >>   			if (!new_desc)
> >> @@ -1300,7 +1493,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   		}
> >>   
> >>   		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
> >> -		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
> >> +			(regulators == axp809_regulators && i == AXP809_DC5LDO) ||  
> > whitespace, and again some more times below.  
> OK.
> >
> > The rest looks good to me.  
> 
> Thanks for reviewing.
> 
> Best regards,
> 
> Shengyu
> 
> >
> > Cheers,
> > Andre
> >  
> >> +		    (regulators == axp15060_regulators && i == AXP15060_CPUSLDO)) {
> >>   			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
> >>   						GFP_KERNEL);
> >>   			if (!new_desc)
> >> @@ -1311,6 +1505,18 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   			desc = new_desc;
> >>   		}
> >>   
> >> +
> >> +		if (regulators == axp15060_regulators && i == AXP15060_RTC_LDO) {
> >> +			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
> >> +						GFP_KERNEL);
> >> +			if (!new_desc)
> >> +				return -ENOMEM;
> >> +
> >> +			*new_desc = regulators[i];
> >> +			new_desc->supply_name = aldo1_name;
> >> +			desc = new_desc;
> >> +		}
> >> +
> >>   		rdev = devm_regulator_register(&pdev->dev, desc, &config);
> >>   		if (IS_ERR(rdev)) {
> >>   			dev_err(&pdev->dev, "Failed to register %s\n",
> >> @@ -1329,19 +1535,26 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
> >>   		}
> >>   
> >>   		/*
> >> -		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
> >> +		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
> >>   		 */
> >>   		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
> >> -		    (regulators == axp809_regulators && i == AXP809_DCDC1))
> >> +		    (regulators == axp809_regulators && i == AXP809_DCDC1) ||
> >> +			(regulators == axp15060_regulators && i == AXP15060_DCDC1))
> >>   			of_property_read_string(rdev->dev.of_node,
> >>   						"regulator-name",
> >>   						&dcdc1_name);
> >>   
> >>   		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
> >> -		    (regulators == axp809_regulators && i == AXP809_DCDC5))
> >> +		    (regulators == axp809_regulators && i == AXP809_DCDC5) ||
> >> +			(regulators == axp15060_regulators && i == AXP15060_DCDC5))
> >>   			of_property_read_string(rdev->dev.of_node,
> >>   						"regulator-name",
> >>   						&dcdc5_name);
> >> +
> >> +		if (regulators == axp15060_regulators && i == AXP15060_ALDO1)
> >> +			of_property_read_string(rdev->dev.of_node,
> >> +						"regulator-name",
> >> +						&aldo1_name);
> >>   	}
> >>   
> >>   	if (drivevbus) {  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists
       [not found] <20230407141813.89-1-wiagn233@outlook.com>
                   ` (2 preceding siblings ...)
  2023-04-07 14:18 ` [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support Shengyu Qu
@ 2023-04-07 14:18 ` Shengyu Qu
  2023-04-17 14:20   ` Andre Przywara
  3 siblings, 1 reply; 14+ messages in thread
From: Shengyu Qu @ 2023-04-07 14:18 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel
  Cc: Shengyu Qu

Current axp20x regulator driver would always set DCDC frequency even if
there is no x-powers,dcdc-freq in device tree data. Causing meaningless
warning info output on variants that do not support DCDC frequency
modification. So only try to set DCDC frequency when there is frequency
property.

Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
---
 drivers/regulator/axp20x-regulator.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index ece4af93df7b..12a12923bc7b 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -1247,10 +1247,10 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
 	if (!regulators) {
 		dev_warn(&pdev->dev, "regulators node not found\n");
 	} else {
-		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
-		ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
+		if (of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq) != -EINVAL) {
+			ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
+			if (ret < 0)
+				dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
 		}
 		of_node_put(regulators);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists
  2023-04-07 14:18 ` [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists Shengyu Qu
@ 2023-04-17 14:20   ` Andre Przywara
  2023-04-17 16:31     ` Shengyu Qu
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2023-04-17 14:20 UTC (permalink / raw)
  To: Shengyu Qu
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka

On Fri,  7 Apr 2023 22:18:13 +0800
Shengyu Qu <wiagn233@outlook.com> wrote:

> Current axp20x regulator driver would always set DCDC frequency even if
> there is no x-powers,dcdc-freq in device tree data. Causing meaningless
> warning info output on variants that do not support DCDC frequency
> modification. So only try to set DCDC frequency when there is frequency
> property.

This patch should not be needed. You should disallow the
x-powers,dcdc-freq property in the binding (see [1]), then handle that
like the AXP313a driver does: by explicitly checking that the property
does not exist, then returning, see [2].

In general you might want to rebase your series on top of the AXP313a v10
series, as this most likely goes in before.

Cheers,
Andre

[1]
https://lore.kernel.org/linux-sunxi/20230401001850.4988-2-andre.przywara@arm.com/
[2]
https://lore.kernel.org/linux-sunxi/20230401001850.4988-4-andre.przywara@arm.com/

> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> ---
>  drivers/regulator/axp20x-regulator.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index ece4af93df7b..12a12923bc7b 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -1247,10 +1247,10 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
>  	if (!regulators) {
>  		dev_warn(&pdev->dev, "regulators node not found\n");
>  	} else {
> -		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
> -		ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
> +		if (of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq) != -EINVAL) {
> +			ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
> +			if (ret < 0)
> +				dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
>  		}
>  		of_node_put(regulators);
>  	}


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists
  2023-04-17 14:20   ` Andre Przywara
@ 2023-04-17 16:31     ` Shengyu Qu
  0 siblings, 0 replies; 14+ messages in thread
From: Shengyu Qu @ 2023-04-17 16:31 UTC (permalink / raw)
  To: Andre Przywara
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel, Martin Botka


[-- Attachment #1.1.1: Type: text/plain, Size: 2215 bytes --]

Hi Andre,

> On Fri,  7 Apr 2023 22:18:13 +0800
> Shengyu Qu <wiagn233@outlook.com> wrote:
>
>> Current axp20x regulator driver would always set DCDC frequency even if
>> there is no x-powers,dcdc-freq in device tree data. Causing meaningless
>> warning info output on variants that do not support DCDC frequency
>> modification. So only try to set DCDC frequency when there is frequency
>> property.
> This patch should not be needed. You should disallow the
> x-powers,dcdc-freq property in the binding (see [1]), then handle that
> like the AXP313a driver does: by explicitly checking that the property
> does not exist, then returning, see [2].
>
> In general you might want to rebase your series on top of the AXP313a v10
> series, as this most likely goes in before.

Thanks for pointing out this :) Would fix and rebase in next version.

Best regards,

Shengyu

> Cheers,
> Andre
>
> [1]
> https://lore.kernel.org/linux-sunxi/20230401001850.4988-2-andre.przywara@arm.com/
> [2]
> https://lore.kernel.org/linux-sunxi/20230401001850.4988-4-andre.przywara@arm.com/
>
>> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
>> ---
>>   drivers/regulator/axp20x-regulator.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
>> index ece4af93df7b..12a12923bc7b 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -1247,10 +1247,10 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
>>   	if (!regulators) {
>>   		dev_warn(&pdev->dev, "regulators node not found\n");
>>   	} else {
>> -		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
>> -		ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
>> -		if (ret < 0) {
>> -			dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
>> +		if (of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq) != -EINVAL) {
>> +			ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
>> +			if (ret < 0)
>> +				dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
>>   		}
>>   		of_node_put(regulators);
>>   	}

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

[parent not found: <20230407135717.17381-1-wiagn233@outlook.com>]

* [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support
       [not found] <20230407135717.17381-1-wiagn233@outlook.com>
@ 2023-04-07 13:57 ` Shengyu Qu
  0 siblings, 0 replies; 14+ messages in thread
From: Shengyu Qu @ 2023-04-07 13:57 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, wens, lgirdwood, broonie,
	devicetree, linux-kernel
  Cc: Shengyu Qu

The AXP15060 is a typical I2C-controlled PMIC, seen on multiple boards
with different default register value. Current driver is tested on
Starfive Visionfive 2.

The RTCLDO is fixed, and cannot even be turned on or off,. On top of
that, its voltage is customisable (either 1.8V or 3.3V). We pretend it's
a fixed 1.8V regulator since other AXP driver also do like this. Also,
BSP code ignores this regulator and it's not used according to VF2
schematic.

Describe the AXP15060's voltage settings and switch registers, how the
voltages are encoded, and connect this to the MFD device via its
regulator ID.

Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
---
 drivers/regulator/axp20x-regulator.c | 229 ++++++++++++++++++++++++++-
 1 file changed, 221 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index d260c442b788..ece4af93df7b 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -270,6 +270,74 @@
 
 #define AXP813_PWR_OUT_DCDC7_MASK	BIT_MASK(6)
 
+#define AXP15060_DCDC1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_DCDC2_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC3_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC4_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC5_V_CTRL_MASK		GENMASK(6, 0)
+#define AXP15060_DCDC6_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO4_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_ALDO5_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO4_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_BLDO5_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO1_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO2_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO3_V_CTRL_MASK		GENMASK(4, 0)
+#define AXP15060_CLDO4_V_CTRL_MASK		GENMASK(5, 0)
+#define AXP15060_CPUSLDO_V_CTRL_MASK		GENMASK(3, 0)
+
+#define AXP15060_PWR_OUT_DCDC1_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_DCDC2_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_DCDC3_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_DCDC4_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_DCDC5_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_DCDC6_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_ALDO1_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_ALDO2_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_ALDO3_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_ALDO4_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_ALDO5_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_BLDO1_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_BLDO2_MASK	BIT_MASK(6)
+#define AXP15060_PWR_OUT_BLDO3_MASK	BIT_MASK(7)
+#define AXP15060_PWR_OUT_BLDO4_MASK	BIT_MASK(0)
+#define AXP15060_PWR_OUT_BLDO5_MASK	BIT_MASK(1)
+#define AXP15060_PWR_OUT_CLDO1_MASK	BIT_MASK(2)
+#define AXP15060_PWR_OUT_CLDO2_MASK	BIT_MASK(3)
+#define AXP15060_PWR_OUT_CLDO3_MASK	BIT_MASK(4)
+#define AXP15060_PWR_OUT_CLDO4_MASK	BIT_MASK(5)
+#define AXP15060_PWR_OUT_CPUSLDO_MASK	BIT_MASK(6)
+#define AXP15060_PWR_OUT_SW_MASK		BIT_MASK(7)
+
+#define AXP15060_DCDC23_POLYPHASE_DUAL_MASK		BIT_MASK(6)
+#define AXP15060_DCDC46_POLYPHASE_DUAL_MASK		BIT_MASK(7)
+
+#define AXP15060_DCDC234_500mV_START	0x00
+#define AXP15060_DCDC234_500mV_STEPS	70
+#define AXP15060_DCDC234_500mV_END		\
+	(AXP15060_DCDC234_500mV_START + AXP15060_DCDC234_500mV_STEPS)
+#define AXP15060_DCDC234_1220mV_START	0x47
+#define AXP15060_DCDC234_1220mV_STEPS	16
+#define AXP15060_DCDC234_1220mV_END		\
+	(AXP15060_DCDC234_1220mV_START + AXP15060_DCDC234_1220mV_STEPS)
+#define AXP15060_DCDC234_NUM_VOLTAGES	88
+
+#define AXP15060_DCDC5_800mV_START	0x00
+#define AXP15060_DCDC5_800mV_STEPS	32
+#define AXP15060_DCDC5_800mV_END		\
+	(AXP15060_DCDC5_800mV_START + AXP15060_DCDC5_800mV_STEPS)
+#define AXP15060_DCDC5_1140mV_START	0x21
+#define AXP15060_DCDC5_1140mV_STEPS	35
+#define AXP15060_DCDC5_1140mV_END		\
+	(AXP15060_DCDC5_1140mV_START + AXP15060_DCDC5_1140mV_STEPS)
+#define AXP15060_DCDC5_NUM_VOLTAGES	69
+
 #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
 		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
 	[_family##_##_id] = {							\
@@ -1001,6 +1069,104 @@ static const struct regulator_desc axp813_regulators[] = {
 		    AXP22X_PWR_OUT_CTRL2, AXP22X_PWR_OUT_DC1SW_MASK),
 };
 
+static const struct linear_range axp15060_dcdc234_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000,
+			       AXP15060_DCDC234_500mV_START,
+			       AXP15060_DCDC234_500mV_END,
+			       10000),
+	REGULATOR_LINEAR_RANGE(1220000,
+			       AXP15060_DCDC234_1220mV_START,
+			       AXP15060_DCDC234_1220mV_END,
+			       20000),
+};
+
+static const struct linear_range axp15060_dcdc5_ranges[] = {
+	REGULATOR_LINEAR_RANGE(800000,
+			       AXP15060_DCDC5_800mV_START,
+			       AXP15060_DCDC5_800mV_END,
+			       10000),
+	REGULATOR_LINEAR_RANGE(1140000,
+			       AXP15060_DCDC5_1140mV_START,
+			       AXP15060_DCDC5_1140mV_END,
+			       20000),
+};
+
+static const struct regulator_desc axp15060_regulators[] = {
+	AXP_DESC(AXP15060, DCDC1, "dcdc1", "vin1", 1500, 3400, 100,
+		 AXP15060_DCDC1_V_CTRL, AXP15060_DCDC1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC1_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC2, "dcdc2", "vin2",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC2_V_CTRL, AXP15060_DCDC2_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC2_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC3, "dcdc3", "vin3",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC3_V_CTRL, AXP15060_DCDC3_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC3_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC4, "dcdc4", "vin4",
+			axp15060_dcdc234_ranges, AXP15060_DCDC234_NUM_VOLTAGES,
+			AXP15060_DCDC4_V_CTRL, AXP15060_DCDC4_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC4_MASK),
+	AXP_DESC_RANGES(AXP15060, DCDC5, "dcdc5", "vin5",
+			axp15060_dcdc5_ranges, AXP15060_DCDC5_NUM_VOLTAGES,
+			AXP15060_DCDC5_V_CTRL, AXP15060_DCDC5_V_CTRL_MASK,
+			AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC5_MASK),
+	AXP_DESC(AXP15060, DCDC6, "dcdc6", "vin6", 500, 3400, 100,
+		 AXP15060_DCDC6_V_CTRL, AXP15060_DCDC6_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL1, AXP15060_PWR_OUT_DCDC6_MASK),
+	AXP_DESC(AXP15060, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO1_V_CTRL, AXP15060_ALDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO1_MASK),
+	AXP_DESC(AXP15060, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO2_V_CTRL, AXP15060_ALDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO2_MASK),
+	AXP_DESC(AXP15060, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO3_V_CTRL, AXP15060_ALDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO3_MASK),
+	AXP_DESC(AXP15060, ALDO4, "aldo4", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO4_V_CTRL, AXP15060_ALDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO4_MASK),
+	AXP_DESC(AXP15060, ALDO5, "aldo5", "aldoin", 700, 3300, 100,
+		 AXP15060_ALDO5_V_CTRL, AXP15060_ALDO5_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_ALDO5_MASK),
+	AXP_DESC(AXP15060, BLDO1, "bldo1", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO1_V_CTRL, AXP15060_BLDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO1_MASK),
+	AXP_DESC(AXP15060, BLDO2, "bldo2", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO2_V_CTRL, AXP15060_BLDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO2_MASK),
+	AXP_DESC(AXP15060, BLDO3, "bldo3", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO3_V_CTRL, AXP15060_BLDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL2, AXP15060_PWR_OUT_BLDO3_MASK),
+	AXP_DESC(AXP15060, BLDO4, "bldo4", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO4_V_CTRL, AXP15060_BLDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO4_MASK),
+	AXP_DESC(AXP15060, BLDO5, "bldo5", "bldoin", 700, 3300, 100,
+		 AXP15060_BLDO5_V_CTRL, AXP15060_BLDO5_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_BLDO5_MASK),
+	AXP_DESC(AXP15060, CLDO1, "cldo1", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO1_V_CTRL, AXP15060_CLDO1_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO1_MASK),
+	AXP_DESC(AXP15060, CLDO2, "cldo2", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO2_V_CTRL, AXP15060_CLDO2_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO2_MASK),
+	AXP_DESC(AXP15060, CLDO3, "cldo3", "cldoin", 700, 3300, 100,
+		 AXP15060_CLDO3_V_CTRL, AXP15060_CLDO3_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO3_MASK),
+	AXP_DESC(AXP15060, CLDO4, "cldo4", "cldoin", 700, 4200, 100,
+		 AXP15060_CLDO4_V_CTRL, AXP15060_CLDO4_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CLDO4_MASK),
+	AXP_DESC(AXP15060, CPUSLDO, "cpusldo", NULL, 700, 1400, 50,
+		 AXP15060_CPUSLDO_V_CTRL, AXP15060_CPUSLDO_V_CTRL_MASK,
+		 AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_CPUSLDO_MASK),
+	/* Supply comes from DCDC5 */
+	AXP_DESC_SW(AXP15060, SW, "swout", NULL,
+		    AXP15060_PWR_OUT_CTRL3, AXP15060_PWR_OUT_SW_MASK),
+	/* Supply comes from DCDC1 */
+	AXP_DESC_FIXED(AXP15060, RTC_LDO, "rtc-ldo", NULL, 1800),
+	/* Supply comes from ALDO1 */
+};
+
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -1145,6 +1311,15 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 		workmode <<= id - AXP813_DCDC1;
 		break;
 
+	case AXP15060_ID:
+		reg = AXP15060_DCDC_MODE_CTRL2;
+		if (id < AXP15060_DCDC1 || id > AXP15060_DCDC6)
+			return -EINVAL;
+
+		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP15060_DCDC1);
+		workmode <<= id - AXP15060_DCDC1;
+		break;
+
 	default:
 		/* should not happen */
 		WARN_ON(1);
@@ -1164,7 +1339,7 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 
 	/*
 	 * Currently in our supported AXP variants, only AXP803, AXP806,
-	 * and AXP813 have polyphase regulators.
+	 * AXP813 and AXP15060 have polyphase regulators.
 	 */
 	switch (axp20x->variant) {
 	case AXP803_ID:
@@ -1196,6 +1371,17 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
 		}
 		break;
 
+	case AXP15060_ID:
+		regmap_read(axp20x->regmap, AXP15060_DCDC_MODE_CTRL1, &reg);
+
+		switch (id) {
+		case AXP15060_DCDC3:
+			return !!(reg & AXP15060_DCDC23_POLYPHASE_DUAL_MASK);
+		case AXP15060_DCDC6:
+			return !!(reg & AXP15060_DCDC46_POLYPHASE_DUAL_MASK);
+		}
+		break;
+
 	default:
 		return false;
 	}
@@ -1217,6 +1403,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	const char *aldo1_name = axp15060_regulators[AXP15060_ALDO1].name;
 	bool drivevbus = false;
 
 	switch (axp20x->variant) {
@@ -1252,6 +1439,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		drivevbus = of_property_read_bool(pdev->dev.parent->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
+	case AXP15060_ID:
+		regulators = axp15060_regulators;
+		nregulators = AXP15060_REG_ID_MAX;
+		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
 			axp20x->variant);
@@ -1278,8 +1469,9 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			continue;
 
 		/*
-		 * Regulators DC1SW and DC5LDO are connected internally,
-		 * so we have to handle their supply names separately.
+		 * Regulators DC1SW, DC5LDO and RTCLDO on AXP15060 are
+		 * connected internally, so we have to handle their supply
+		 * names separately.
 		 *
 		 * We always register the regulators in proper sequence,
 		 * so the supply names are correctly read. See the last
@@ -1288,7 +1480,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		 */
 		if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
 		    (regulators == axp803_regulators && i == AXP803_DC1SW) ||
-		    (regulators == axp809_regulators && i == AXP809_DC1SW)) {
+			(regulators == axp809_regulators && i == AXP809_DC1SW) ||
+		    (regulators == axp15060_regulators && i == AXP15060_SW)) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			if (!new_desc)
@@ -1300,7 +1493,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 
 		if ((regulators == axp22x_regulators && i == AXP22X_DC5LDO) ||
-		    (regulators == axp809_regulators && i == AXP809_DC5LDO)) {
+			(regulators == axp809_regulators && i == AXP809_DC5LDO) ||
+		    (regulators == axp15060_regulators && i == AXP15060_CPUSLDO)) {
 			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
 						GFP_KERNEL);
 			if (!new_desc)
@@ -1311,6 +1505,18 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			desc = new_desc;
 		}
 
+
+		if (regulators == axp15060_regulators && i == AXP15060_RTC_LDO) {
+			new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
+						GFP_KERNEL);
+			if (!new_desc)
+				return -ENOMEM;
+
+			*new_desc = regulators[i];
+			new_desc->supply_name = aldo1_name;
+			desc = new_desc;
+		}
+
 		rdev = devm_regulator_register(&pdev->dev, desc, &config);
 		if (IS_ERR(rdev)) {
 			dev_err(&pdev->dev, "Failed to register %s\n",
@@ -1329,19 +1535,26 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 
 		/*
-		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
+		 * Save AXP22X DCDC1 / DCDC5 / AXP15060 ALDO1 regulator names for later.
 		 */
 		if ((regulators == axp22x_regulators && i == AXP22X_DCDC1) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC1))
+		    (regulators == axp809_regulators && i == AXP809_DCDC1) ||
+			(regulators == axp15060_regulators && i == AXP15060_DCDC1))
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc1_name);
 
 		if ((regulators == axp22x_regulators && i == AXP22X_DCDC5) ||
-		    (regulators == axp809_regulators && i == AXP809_DCDC5))
+		    (regulators == axp809_regulators && i == AXP809_DCDC5) ||
+			(regulators == axp15060_regulators && i == AXP15060_DCDC5))
 			of_property_read_string(rdev->dev.of_node,
 						"regulator-name",
 						&dcdc5_name);
+
+		if (regulators == axp15060_regulators && i == AXP15060_ALDO1)
+			of_property_read_string(rdev->dev.of_node,
+						"regulator-name",
+						&aldo1_name);
 	}
 
 	if (drivevbus) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-04-18 21:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20230407141813.89-1-wiagn233@outlook.com>
2023-04-07 14:18 ` [PATCH v1 1/4] dt-bindings: mfd: x-powers,axp152: Document the AXP15060 variant Shengyu Qu
2023-04-10 17:27   ` Krzysztof Kozlowski
2023-04-10 17:43     ` Shengyu Qu
2023-04-07 14:18 ` [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC Shengyu Qu
2023-04-17 14:05   ` Andre Przywara
2023-04-17 16:21     ` Shengyu Qu
2023-04-07 14:18 ` [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support Shengyu Qu
2023-04-17 14:18   ` Andre Przywara
2023-04-17 16:01     ` Shengyu Qu
2023-04-18 21:57       ` Andre Przywara
2023-04-07 14:18 ` [PATCH v1 4/4] regulator: axp20x: Set DCDC frequency only when property exists Shengyu Qu
2023-04-17 14:20   ` Andre Przywara
2023-04-17 16:31     ` Shengyu Qu
     [not found] <20230407135717.17381-1-wiagn233@outlook.com>
2023-04-07 13:57 ` [PATCH v1 3/4] regulator: axp20x: Add AXP15060 support Shengyu Qu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).