linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Fix issues on pf8x00 driver
@ 2021-01-08 22:50 Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 1/6] regulator: pf8x00: add a doc for the module Adrien Grassein
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

Hello,

this patch set aims is to fix some issues contains in the pf8x00 driver.
Some issues are documentation ones, some others are in code.

These issues where found while developing another version of the same
driver. I prefer to share with you the patch for it.

I do these patches on the master branch of the linux-next repository.

1: there is a missing documentation when we build the driver as a
module.

2: nxp,phase-shift documentation was not optimal.

3: nxp,ilim-ma is useless since another property does (nearly) the same.

4: Same than 4, in driver code.

5: It's better to use a linear_range to describe the voltage of bucks.

6: Fix several mistakes in the handle of nxp,phase-shift property.

Changes introduced in v2:
Some patches were mixed (and did not compile separately);
Add path 7 for the handle of nxp,phase-shift property.

Changes introduced in v3:
Remove useless patch 0;
Do requested fixes in patches.

Thanks,

Adrien Grassein (6):
  regulator: pf8x00: add a doc for the module
  regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc
  regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property
  regulator: pf8x00: remove nxp,ilim-ma property
  regulator: pf8x00: use linear range for buck 1-6
  regulator: pf8x00: fix nxp,phase-shift

 .../regulator/nxp,pf8x00-regulator.yaml       |  27 +--
 drivers/regulator/Kconfig                     |   4 +
 drivers/regulator/pf8x00-regulator.c          | 174 ++++++++----------
 3 files changed, 79 insertions(+), 126 deletions(-)

-- 
2.25.1


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

* [PATCH v3 1/6] regulator: pf8x00: add a doc for the module
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc Adrien Grassein
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

pf8x00 module build was not documented.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/regulator/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index eafa13be6e3e..ba8c09d90701 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -846,6 +846,10 @@ config REGULATOR_PF8X00
 	  Say y here to support the regulators found on the NXP
 	  PF8100/PF8121A/PF8200 PMIC.
 
+	  Say M here if you want to support for the regulators found
+	  on the NXP PF8100/PF8121A/PF8200 PMIC. The module will be named
+	  "pf8x00-regulator".
+
 config REGULATOR_PFUZE100
 	tristate "Freescale PFUZE100/200/3000/3001 regulator driver"
 	depends on I2C && OF
-- 
2.25.1


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

* [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 1/6] regulator: pf8x00: add a doc for the module Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  2021-01-13 15:28   ` Rob Herring
  2021-01-08 22:50 ` [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property Adrien Grassein
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

nxp,phase-shift is an enum so use enum format to describe it.
Minimum and maximum values are also wrong.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 .../bindings/regulator/nxp,pf8x00-regulator.yaml   | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
index 956156fe52a3..095cfdae7b67 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -73,21 +73,11 @@ properties:
 
           nxp,phase-shift:
             $ref: "/schemas/types.yaml#/definitions/uint32"
-            minimum: 45
-            maximum: 0
+            default: 0
+            enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
             description:
               BUCK regulators phase shift control in degrees.
 
-              Listed phase shift control values in degrees are,
-              45
-              90
-              135
-              180
-              225
-              270
-              315
-              0 (default)
-
         unevaluatedProperties: false
 
       "^vsnvs$":
-- 
2.25.1


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

* [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 1/6] regulator: pf8x00: add a doc for the module Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  2021-01-11 17:20   ` Mark Brown
  2021-01-08 22:50 ` [PATCH v3 4/6] regulator: " Adrien Grassein
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein, Rob Herring

This property seems useless because we can use the
regulator-max-microamp generic property to do the same
and using generic code.

The only things it changes is the default value. The original
code was using "2100" (mA) as default one, but I think we should
keep the value in the OTP memory as the default one. This value
is automatically loaded in the register by the chip.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/regulator/nxp,pf8x00-regulator.yaml    | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
index 095cfdae7b67..05d5e9a2219c 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -58,19 +58,6 @@ properties:
             description:
               should be "buck1", ..., "buck7"
 
-          nxp,ilim-ma:
-            $ref: "/schemas/types.yaml#/definitions/uint32"
-            minimum: 2100
-            maximum: 4500
-            description:
-              BUCK regulators current limit in mA.
-
-              Listed current limits in mA are,
-              2100 (default)
-              2600
-              3000
-              4500
-
           nxp,phase-shift:
             $ref: "/schemas/types.yaml#/definitions/uint32"
             default: 0
-- 
2.25.1


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

* [PATCH v3 4/6] regulator: pf8x00: remove nxp,ilim-ma property
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
                   ` (2 preceding siblings ...)
  2021-01-08 22:50 ` [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 5/6] regulator: pf8x00: use linear range for buck 1-6 Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 6/6] regulator: pf8x00: fix nxp,phase-shift Adrien Grassein
  5 siblings, 0 replies; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

This property seems useless because we can use the
regulator-max-microamp generic property to do the same
and using generic code.

The only things it changes is the default value. The original
code was using "2100" (mA) as default one, but I think we should
keep the value in the OTP memory as the default one. This value
is automatically loaded in the register by the chip.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 drivers/regulator/pf8x00-regulator.c | 95 ++++++++++++++--------------
 1 file changed, 47 insertions(+), 48 deletions(-)

diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
index af9918cd27aa..90383e394f85 100644
--- a/drivers/regulator/pf8x00-regulator.c
+++ b/drivers/regulator/pf8x00-regulator.c
@@ -105,13 +105,6 @@ enum pf8x00_ldo_states {
 };
 #define PF8X00_LDO_BASE(i)		(6 * (i - PF8X00_LDO1) + PF8X00_LDO1_CONFIG1)
 
-enum swxilim_bits {
-	SWXILIM_2100_MA,
-	SWXILIM_2600_MA,
-	SWXILIM_3000_MA,
-	SWXILIM_4500_MA,
-};
-#define PF8X00_SWXILIM_SHIFT		3
 #define PF8X00_SWXILIM_MASK		GENMASK(4, 3)
 #define PF8X00_SWXPHASE_MASK		GENMASK(2, 0)
 #define PF8X00_SWXPHASE_DEFAULT		0
@@ -128,7 +121,6 @@ enum pf8x00_devid {
 
 struct pf8x00_regulator {
 	struct regulator_desc desc;
-	u8 ilim;
 	u8 phase_shift;
 };
 
@@ -150,6 +142,11 @@ static const int pf8x00_ldo_voltages[] = {
 	3100000, 3150000, 3200000, 3300000, 3350000, 1650000, 1700000, 5000000,
 };
 
+/* Output: 2.1A to 4.5A */
+static const unsigned int pf8x00_sw_current_table[] = {
+	2100000, 2600000, 3000000, 4500000,
+};
+
 #define SWV(i)		(6250 * i + 400000)
 #define SWV_LINE(i)	SWV(i*8+0), SWV(i*8+1), SWV(i*8+2), SWV(i*8+3), \
 			SWV(i*8+4), SWV(i*8+5), SWV(i*8+6), SWV(i*8+7)
@@ -199,32 +196,6 @@ static struct pf8x00_regulator *desc_to_regulator(const struct regulator_desc *d
 	return container_of(desc, struct pf8x00_regulator, desc);
 }
 
-static void swxilim_select(const struct regulator_desc *desc, int ilim)
-{
-	struct pf8x00_regulator *data = desc_to_regulator(desc);
-	u8 ilim_sel;
-
-	switch (ilim) {
-	case 2100:
-		ilim_sel = SWXILIM_2100_MA;
-		break;
-	case 2600:
-		ilim_sel = SWXILIM_2600_MA;
-		break;
-	case 3000:
-		ilim_sel = SWXILIM_3000_MA;
-		break;
-	case 4500:
-		ilim_sel = SWXILIM_4500_MA;
-		break;
-	default:
-		ilim_sel = SWXILIM_2100_MA;
-		break;
-	}
-
-	data->ilim = ilim_sel;
-}
-
 static int pf8x00_of_parse_cb(struct device_node *np,
 			      const struct regulator_desc *desc,
 			      struct regulator_config *config)
@@ -235,13 +206,6 @@ static int pf8x00_of_parse_cb(struct device_node *np,
 	int val;
 	int ret;
 
-	ret = of_property_read_u32(np, "nxp,ilim-ma", &val);
-	if (ret)
-		dev_dbg(chip->dev, "unspecified ilim for BUCK%d, use 2100 mA\n",
-			desc->id - PF8X00_LDO4);
-
-	swxilim_select(desc, val);
-
 	ret = of_property_read_u32(np, "nxp,phase-shift", &val);
 	if (ret) {
 		dev_dbg(chip->dev,
@@ -272,7 +236,18 @@ static const struct regulator_ops pf8x00_ldo_ops = {
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 };
 
-static const struct regulator_ops pf8x00_buck_ops = {
+static const struct regulator_ops pf8x00_buck1_6_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.get_current_limit = regulator_get_current_limit_regmap,
+	.set_current_limit = regulator_set_current_limit_regmap,
+};
+
+static const struct regulator_ops pf8x00_buck7_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
@@ -320,13 +295,41 @@ static const struct regulator_ops pf8x00_vsnvs_ops = {
 			.regulators_node = "regulators",	\
 			.of_parse_cb = pf8x00_of_parse_cb,	\
 			.n_voltages = ARRAY_SIZE(voltages),	\
-			.ops = &pf8x00_buck_ops,		\
+			.ops = &pf8x00_buck1_6_ops,		\
 			.type = REGULATOR_VOLTAGE,		\
 			.id = PF8X00_BUCK ## _id,		\
 			.owner = THIS_MODULE,			\
 			.volt_table = voltages,			\
 			.vsel_reg = (base) + SW_RUN_VOLT,	\
 			.vsel_mask = 0xff,			\
+			.curr_table = pf8x00_sw_current_table, \
+			.n_current_limits = \
+				ARRAY_SIZE(pf8x00_sw_current_table), \
+			.csel_reg = (base) + SW_CONFIG2,	\
+			.csel_mask = PF8X00_SWXILIM_MASK,	\
+			.enable_reg = (base) + SW_MODE1,	\
+			.enable_val = 0x3,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 0x3,			\
+			.enable_time = 500,			\
+		},						\
+	}
+
+#define PF8X00BUCK7(_name, base, voltages)			\
+	[PF8X00_BUCK7] = {				\
+		.desc = {					\
+			.name = _name,				\
+			.of_match = _name,			\
+			.regulators_node = "regulators",	\
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(voltages),	\
+			.ops = &pf8x00_buck7_ops,		\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = PF8X00_BUCK7,		\
+			.owner = THIS_MODULE,			\
+			.volt_table = voltages,			\
+			.vsel_reg = (base) + SW_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
 			.enable_reg = (base) + SW_MODE1,	\
 			.enable_val = 0x3,			\
 			.disable_val = 0x0,			\
@@ -363,7 +366,7 @@ static struct pf8x00_regulator pf8x00_regulators_data[PF8X00_MAX_REGULATORS] = {
 	PF8X00BUCK(4, "buck4", PF8X00_SW_BASE(PF8X00_BUCK4), pf8x00_sw1_to_6_voltages),
 	PF8X00BUCK(5, "buck5", PF8X00_SW_BASE(PF8X00_BUCK5), pf8x00_sw1_to_6_voltages),
 	PF8X00BUCK(6, "buck6", PF8X00_SW_BASE(PF8X00_BUCK6), pf8x00_sw1_to_6_voltages),
-	PF8X00BUCK(7, "buck7", PF8X00_SW_BASE(PF8X00_BUCK7), pf8x00_sw7_voltages),
+	PF8X00BUCK7("buck7", PF8X00_SW_BASE(PF8X00_BUCK7), pf8x00_sw7_voltages),
 	PF8X00VSNVS("vsnvs", PF8X00_VSNVS_CONFIG1, pf8x00_vsnvs_voltages),
 };
 
@@ -458,10 +461,6 @@ static int pf8x00_i2c_probe(struct i2c_client *client)
 			regmap_update_bits(chip->regmap, reg,
 					   PF8X00_SWXPHASE_MASK,
 					   data->phase_shift);
-
-			regmap_update_bits(chip->regmap, reg,
-					   PF8X00_SWXILIM_MASK,
-					   data->ilim << PF8X00_SWXILIM_SHIFT);
 		}
 	}
 
-- 
2.25.1


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

* [PATCH v3 5/6] regulator: pf8x00: use linear range for buck 1-6
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
                   ` (3 preceding siblings ...)
  2021-01-08 22:50 ` [PATCH v3 4/6] regulator: " Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  2021-01-08 22:50 ` [PATCH v3 6/6] regulator: pf8x00: fix nxp,phase-shift Adrien Grassein
  5 siblings, 0 replies; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

Use a linear range to describe the voltages of the
bucks 1-6 instead of listing it one by one (via a macro)

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 drivers/regulator/pf8x00-regulator.c | 40 +++++++---------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
index 90383e394f85..4b57f74e7326 100644
--- a/drivers/regulator/pf8x00-regulator.c
+++ b/drivers/regulator/pf8x00-regulator.c
@@ -147,35 +147,11 @@ static const unsigned int pf8x00_sw_current_table[] = {
 	2100000, 2600000, 3000000, 4500000,
 };
 
-#define SWV(i)		(6250 * i + 400000)
-#define SWV_LINE(i)	SWV(i*8+0), SWV(i*8+1), SWV(i*8+2), SWV(i*8+3), \
-			SWV(i*8+4), SWV(i*8+5), SWV(i*8+6), SWV(i*8+7)
-
 /* Output: 0.4V to 1.8V */
-static const int pf8x00_sw1_to_6_voltages[] = {
-	SWV_LINE(0),
-	SWV_LINE(1),
-	SWV_LINE(2),
-	SWV_LINE(3),
-	SWV_LINE(4),
-	SWV_LINE(5),
-	SWV_LINE(6),
-	SWV_LINE(7),
-	SWV_LINE(8),
-	SWV_LINE(9),
-	SWV_LINE(10),
-	SWV_LINE(11),
-	SWV_LINE(12),
-	SWV_LINE(13),
-	SWV_LINE(14),
-	SWV_LINE(15),
-	SWV_LINE(16),
-	SWV_LINE(17),
-	SWV_LINE(18),
-	SWV_LINE(19),
-	SWV_LINE(20),
-	SWV_LINE(21),
-	1500000, 1800000,
+#define PF8XOO_SW1_6_VOLTAGE_NUM 0xB2
+static const struct linear_range pf8x00_sw1_to_6_voltages[] = {
+	REGULATOR_LINEAR_RANGE(400000, 0x00, 0xB0, 6250),
+	REGULATOR_LINEAR_RANGE(1800000, 0xB1, 0xB1, 0),
 };
 
 /* Output: 1.0V to 4.1V */
@@ -240,7 +216,7 @@ static const struct regulator_ops pf8x00_buck1_6_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
-	.list_voltage = regulator_list_voltage_table,
+	.list_voltage = regulator_list_voltage_linear_range,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.get_current_limit = regulator_get_current_limit_regmap,
@@ -294,12 +270,14 @@ static const struct regulator_ops pf8x00_vsnvs_ops = {
 			.of_match = _name,			\
 			.regulators_node = "regulators",	\
 			.of_parse_cb = pf8x00_of_parse_cb,	\
-			.n_voltages = ARRAY_SIZE(voltages),	\
+			.n_voltages = PF8XOO_SW1_6_VOLTAGE_NUM,	\
 			.ops = &pf8x00_buck1_6_ops,		\
 			.type = REGULATOR_VOLTAGE,		\
 			.id = PF8X00_BUCK ## _id,		\
 			.owner = THIS_MODULE,			\
-			.volt_table = voltages,			\
+			.linear_ranges = pf8x00_sw1_to_6_voltages, \
+			.n_linear_ranges = \
+				ARRAY_SIZE(pf8x00_sw1_to_6_voltages), \
 			.vsel_reg = (base) + SW_RUN_VOLT,	\
 			.vsel_mask = 0xff,			\
 			.curr_table = pf8x00_sw_current_table, \
-- 
2.25.1


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

* [PATCH v3 6/6] regulator: pf8x00: fix nxp,phase-shift
  2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
                   ` (4 preceding siblings ...)
  2021-01-08 22:50 ` [PATCH v3 5/6] regulator: pf8x00: use linear range for buck 1-6 Adrien Grassein
@ 2021-01-08 22:50 ` Adrien Grassein
  5 siblings, 0 replies; 11+ messages in thread
From: Adrien Grassein @ 2021-01-08 22:50 UTC (permalink / raw)
  Cc: broonie, jagan, lgirdwood, robh+dt, linux-kernel, devicetree,
	Adrien Grassein

Fix the ternary condition which is a bad coding style
in the kernel

I also remove the defering configuration of the nxp,phase-shift.
The configuration is now done at parsing time. It save some memory
and it's better for comprehension.

I also use the OTP default configuration when the parameter is wrong
or not specified.
I think that it's better to use the default configuration from the chip
than an arbitrary value.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 drivers/regulator/pf8x00-regulator.c | 45 +++++++++++++---------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
index 4b57f74e7326..9932a580619c 100644
--- a/drivers/regulator/pf8x00-regulator.c
+++ b/drivers/regulator/pf8x00-regulator.c
@@ -107,7 +107,6 @@ enum pf8x00_ldo_states {
 
 #define PF8X00_SWXILIM_MASK		GENMASK(4, 3)
 #define PF8X00_SWXPHASE_MASK		GENMASK(2, 0)
-#define PF8X00_SWXPHASE_DEFAULT		0
 #define PF8X00_SWXPHASE_SHIFT		7
 
 enum pf8x00_devid {
@@ -121,7 +120,6 @@ enum pf8x00_devid {
 
 struct pf8x00_regulator {
 	struct regulator_desc desc;
-	u8 phase_shift;
 };
 
 struct pf8x00_chip {
@@ -167,17 +165,13 @@ static const int pf8x00_vsnvs_voltages[] = {
 	0, 1800000, 3000000, 3300000,
 };
 
-static struct pf8x00_regulator *desc_to_regulator(const struct regulator_desc *desc)
-{
-	return container_of(desc, struct pf8x00_regulator, desc);
-}
-
 static int pf8x00_of_parse_cb(struct device_node *np,
 			      const struct regulator_desc *desc,
 			      struct regulator_config *config)
 {
-	struct pf8x00_regulator *data = desc_to_regulator(desc);
 	struct pf8x00_chip *chip = config->driver_data;
+	unsigned char id = desc->id - PF8X00_LDO4;
+	unsigned char reg = PF8X00_SW_BASE(id) + SW_CONFIG2;
 	int phase;
 	int val;
 	int ret;
@@ -185,21 +179,30 @@ static int pf8x00_of_parse_cb(struct device_node *np,
 	ret = of_property_read_u32(np, "nxp,phase-shift", &val);
 	if (ret) {
 		dev_dbg(chip->dev,
-			"unspecified phase-shift for BUCK%d, use 0 degrees\n",
-			desc->id - PF8X00_LDO4);
-		val = PF8X00_SWXPHASE_DEFAULT;
+			"unspecified phase-shift for BUCK%d, using OTP configuration\n",
+			id);
+		goto end;
 	}
 
-	phase = val / 45;
-	if ((phase * 45) != val) {
+	if (val < 0 || val > 315 || val % 45 != 0) {
 		dev_warn(config->dev,
-			 "invalid phase_shift %d for BUCK%d, use 0 degrees\n",
-			 (phase * 45), desc->id - PF8X00_LDO4);
-		phase = PF8X00_SWXPHASE_SHIFT;
+			 "invalid phase_shift %d for BUCK%d, using OTP configuration\n",
+			 val, id);
+		goto end;
 	}
 
-	data->phase_shift = (phase >= 1) ? phase - 1 : PF8X00_SWXPHASE_SHIFT;
+	phase = val / 45;
+
+	if (phase >= 1)
+		phase -= 1;
+	else
+		phase = PF8X00_SWXPHASE_SHIFT;
+
+	regmap_update_bits(chip->regmap, reg,
+			PF8X00_SWXPHASE_MASK,
+			phase);
 
+end:
 	return 0;
 }
 
@@ -432,14 +435,6 @@ static int pf8x00_i2c_probe(struct i2c_client *client)
 				"failed to register %s regulator\n", data->desc.name);
 			return PTR_ERR(rdev);
 		}
-
-		if ((id >= PF8X00_BUCK1) && (id <= PF8X00_BUCK7)) {
-			u8 reg = PF8X00_SW_BASE(id) + SW_CONFIG2;
-
-			regmap_update_bits(chip->regmap, reg,
-					   PF8X00_SWXPHASE_MASK,
-					   data->phase_shift);
-		}
 	}
 
 	return 0;
-- 
2.25.1


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

* Re: [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property
  2021-01-08 22:50 ` [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property Adrien Grassein
@ 2021-01-11 17:20   ` Mark Brown
  2021-01-11 17:27     ` Adrien Grassein
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2021-01-11 17:20 UTC (permalink / raw)
  To: Adrien Grassein
  Cc: jagan, lgirdwood, robh+dt, linux-kernel, devicetree, Rob Herring

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Fri, Jan 08, 2021 at 11:50:03PM +0100, Adrien Grassein wrote:
> This property seems useless because we can use the
> regulator-max-microamp generic property to do the same
> and using generic code.

This made it into a kernel release so we should continue to honour it,
it's fine to deprecate it though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property
  2021-01-11 17:20   ` Mark Brown
@ 2021-01-11 17:27     ` Adrien Grassein
  2021-01-11 17:32       ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Adrien Grassein @ 2021-01-11 17:27 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jagan Teki, Liam Girdwood, Rob Herring, linux-kernel, DTML, Rob Herring

Hello,

Is there a way to declare it deprecated in the DTB ?
I never saw a deprecated value.

Thanks,

Le lun. 11 janv. 2021 à 18:20, Mark Brown <broonie@kernel.org> a écrit :
>
> On Fri, Jan 08, 2021 at 11:50:03PM +0100, Adrien Grassein wrote:
> > This property seems useless because we can use the
> > regulator-max-microamp generic property to do the same
> > and using generic code.
>
> This made it into a kernel release so we should continue to honour it,
> it's fine to deprecate it though.

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

* Re: [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property
  2021-01-11 17:27     ` Adrien Grassein
@ 2021-01-11 17:32       ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2021-01-11 17:32 UTC (permalink / raw)
  To: Adrien Grassein
  Cc: Jagan Teki, Liam Girdwood, Rob Herring, linux-kernel, DTML, Rob Herring

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

On Mon, Jan 11, 2021 at 06:27:52PM +0100, Adrien Grassein wrote:
> Hello,

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

> Is there a way to declare it deprecated in the DTB ?
> I never saw a deprecated value.

If nothing else you can put a note in the text description, I don't know
about the YAML format stuff.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc
  2021-01-08 22:50 ` [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc Adrien Grassein
@ 2021-01-13 15:28   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-01-13 15:28 UTC (permalink / raw)
  To: Adrien Grassein
  Cc: robh+dt, jagan, broonie, lgirdwood, linux-kernel, devicetree

On Fri, 08 Jan 2021 23:50:02 +0100, Adrien Grassein wrote:
> nxp,phase-shift is an enum so use enum format to describe it.
> Minimum and maximum values are also wrong.
> 
> Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
> ---
>  .../bindings/regulator/nxp,pf8x00-regulator.yaml   | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

end of thread, other threads:[~2021-01-13 15:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08 22:50 [PATCH v3 0/6] Fix issues on pf8x00 driver Adrien Grassein
2021-01-08 22:50 ` [PATCH v3 1/6] regulator: pf8x00: add a doc for the module Adrien Grassein
2021-01-08 22:50 ` [PATCH v3 2/6] regulator: dt-bindings: pf8x00: fix nxp,phase-shift doc Adrien Grassein
2021-01-13 15:28   ` Rob Herring
2021-01-08 22:50 ` [PATCH v3 3/6] regulator: dt-bindings: pf8x00: remove nxp,ilim-ma property Adrien Grassein
2021-01-11 17:20   ` Mark Brown
2021-01-11 17:27     ` Adrien Grassein
2021-01-11 17:32       ` Mark Brown
2021-01-08 22:50 ` [PATCH v3 4/6] regulator: " Adrien Grassein
2021-01-08 22:50 ` [PATCH v3 5/6] regulator: pf8x00: use linear range for buck 1-6 Adrien Grassein
2021-01-08 22:50 ` [PATCH v3 6/6] regulator: pf8x00: fix nxp,phase-shift Adrien Grassein

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).