2024-02-20 08:36:34

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 0/5] Convert some regulator drivers to GPIO descriptors

Despite the work to convert the regulator core to GPIO
descriptors, there are some outliers that use legacy GPIO
numbers in various ways. Convert them all over.

Signed-off-by: Linus Walleij <[email protected]>
---
Linus Walleij (5):
regulator: max8973: Finalize switch to GPIO descriptors
regulator: da9055: Fully convert to GPIO descriptors
regulator: lp8788-buck: Fully convert to GPIO descriptors
regulator: max8997: Convert to GPIO descriptors
regulator: max8998: Convert to GPIO descriptors

drivers/regulator/da9055-regulator.c | 48 ++++-----
drivers/regulator/lp8788-buck.c | 64 +++++-------
drivers/regulator/max8973-regulator.c | 36 +++----
drivers/regulator/max8997-regulator.c | 85 +++++-----------
drivers/regulator/max8998.c | 150 ++++++++++------------------
include/linux/mfd/da9055/pdata.h | 13 ---
include/linux/mfd/lp8788.h | 9 +-
include/linux/mfd/max8997.h | 1 -
include/linux/mfd/max8998.h | 6 --
include/linux/regulator/max8973-regulator.h | 6 --
10 files changed, 143 insertions(+), 275 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240212-descriptors-regulators-f455ccf0119c

Best regards,
--
Linus Walleij <[email protected]>



2024-02-20 08:36:41

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 1/5] regulator: max8973: Finalize switch to GPIO descriptors

The dvs gpio was still using a legacy number passed from the
platform data. There are no in-tree users of the platform data
so just switch it to a gpio descriptor and obtain it in probe(),
the device tree users will work just as fine with this.

Drop the entirely unused enable_gpio from the platform data
as well. The device tree bindings mentions this but the driver
does not look for it and makes no use of it: it should probably
be implemented properly in a separate patch.

Signed-off-by: Linus Walleij <[email protected]>
---
drivers/regulator/max8973-regulator.c | 36 ++++++++++-------------------
include/linux/regulator/max8973-regulator.h | 6 -----
2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/max8973-regulator.c b/drivers/regulator/max8973-regulator.c
index 8d5193207552..f8bb6828feef 100644
--- a/drivers/regulator/max8973-regulator.c
+++ b/drivers/regulator/max8973-regulator.c
@@ -20,9 +20,7 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/max8973-regulator.h>
#include <linux/regulator/of_regulator.h>
-#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
-#include <linux/of_gpio.h>
#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/regmap.h>
@@ -102,7 +100,7 @@ struct max8973_chip {
struct regulator_desc desc;
struct regmap *regmap;
bool enable_external_control;
- int dvs_gpio;
+ struct gpio_desc *dvs_gpiod;
int lru_index[MAX8973_MAX_VOUT_REG];
int curr_vout_val[MAX8973_MAX_VOUT_REG];
int curr_vout_reg;
@@ -184,7 +182,7 @@ static int max8973_dcdc_set_voltage_sel(struct regulator_dev *rdev,
* If gpios are available to select the VOUT register then least
* recently used register for new configuration.
*/
- if (gpio_is_valid(max->dvs_gpio))
+ if (max->dvs_gpiod)
found = find_voltage_set_register(max, vsel,
&vout_reg, &gpio_val);

@@ -201,8 +199,8 @@ static int max8973_dcdc_set_voltage_sel(struct regulator_dev *rdev,
}

/* Select proper VOUT register vio gpios */
- if (gpio_is_valid(max->dvs_gpio)) {
- gpio_set_value_cansleep(max->dvs_gpio, gpio_val & 0x1);
+ if (max->dvs_gpiod) {
+ gpiod_set_value_cansleep(max->dvs_gpiod, gpio_val & 0x1);
max->curr_gpio_val = gpio_val;
}
return 0;
@@ -531,7 +529,6 @@ static struct max8973_regulator_platform_data *max8973_parse_dt(

pdata->enable_ext_control = of_property_read_bool(np,
"maxim,externally-enable");
- pdata->dvs_gpio = of_get_named_gpio(np, "maxim,dvs-gpio", 0);

ret = of_property_read_u32(np, "maxim,dvs-default-state", &pval);
if (!ret)
@@ -612,13 +609,17 @@ static int max8973_probe(struct i2c_client *client)
return -EIO;
}

- if (pdata->dvs_gpio == -EPROBE_DEFER)
- return -EPROBE_DEFER;
-
max = devm_kzalloc(&client->dev, sizeof(*max), GFP_KERNEL);
if (!max)
return -ENOMEM;

+ max->dvs_gpiod = devm_gpiod_get_optional(&client->dev, "maxim,dvs",
+ (pdata->dvs_def_state) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
+ if (IS_ERR(max->dvs_gpiod))
+ return dev_err_probe(&client->dev, PTR_ERR(max->dvs_gpiod),
+ "failed to obtain dvs gpio\n");
+ gpiod_set_consumer_name(max->dvs_gpiod, "max8973-dvs");
+
max->regmap = devm_regmap_init_i2c(client, &max8973_regmap_config);
if (IS_ERR(max->regmap)) {
ret = PTR_ERR(max->regmap);
@@ -663,7 +664,6 @@ static int max8973_probe(struct i2c_client *client)
max->desc.ramp_delay_table = max8973_buck_ramp_table;
max->desc.n_ramp_values = ARRAY_SIZE(max8973_buck_ramp_table);

- max->dvs_gpio = (pdata->dvs_gpio) ? pdata->dvs_gpio : -EINVAL;
max->enable_external_control = pdata->enable_ext_control;
max->curr_gpio_val = pdata->dvs_def_state;
max->curr_vout_reg = MAX8973_VOUT + pdata->dvs_def_state;
@@ -671,21 +671,9 @@ static int max8973_probe(struct i2c_client *client)

max->lru_index[0] = max->curr_vout_reg;

- if (gpio_is_valid(max->dvs_gpio)) {
- int gpio_flags;
+ if (max->dvs_gpiod) {
int i;

- gpio_flags = (pdata->dvs_def_state) ?
- GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
- ret = devm_gpio_request_one(&client->dev, max->dvs_gpio,
- gpio_flags, "max8973-dvs");
- if (ret) {
- dev_err(&client->dev,
- "gpio_request for gpio %d failed, err = %d\n",
- max->dvs_gpio, ret);
- return ret;
- }
-
/*
* Initialize the lru index with vout_reg id
* The index 0 will be most recently used and
diff --git a/include/linux/regulator/max8973-regulator.h b/include/linux/regulator/max8973-regulator.h
index 8313e7ed6aec..a225e9eeb30d 100644
--- a/include/linux/regulator/max8973-regulator.h
+++ b/include/linux/regulator/max8973-regulator.h
@@ -48,10 +48,6 @@
* control signal from EN input pin. If it is false then
* voltage output will be enabled/disabled through EN bit of
* device register.
- * @enable_gpio: Enable GPIO. If EN pin is controlled through GPIO from host
- * then GPIO number can be provided. If no GPIO controlled then
- * it should be -1.
- * @dvs_gpio: GPIO for dvs. It should be -1 if this is tied with fixed logic.
* @dvs_def_state: Default state of dvs. 1 if it is high else 0.
*/
struct max8973_regulator_platform_data {
@@ -59,8 +55,6 @@ struct max8973_regulator_platform_data {
unsigned long control_flags;
unsigned long junction_temp_warning;
bool enable_ext_control;
- int enable_gpio;
- int dvs_gpio;
unsigned dvs_def_state:1;
};


--
2.34.1


2024-02-20 08:36:52

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 2/5] regulator: da9055: Fully convert to GPIO descriptors

The DA9055 regulator was touched before, requireing enable GPIOs
to be passed from pdata.

As we have a device for each regulator, obtain the three gpios
ren ("regulator enable"), rsel ("regulator select") and the
ena ("enable") GPIO associated with the regulator enable
directly from the device and cut down on the amount of
GPIO numbers passed as platform data.

The ren and rsel are just requested as inputs: these are
actually handled by hardware. The ena gpios are driven
actively by the regulator core.

There are no in-tree users, but the regulators are instantiated
from the (undocumed) device tree nodes with "dlg,da9055-regulator"
as compatible, and by simply adding regulator-enable-gpios,
regulator-select-gpios and enable-gpios to this DT node, all
will work as before.

Signed-off-by: Linus Walleij <[email protected]>
---
drivers/regulator/da9055-regulator.c | 48 ++++++++++++++++++------------------
include/linux/mfd/da9055/pdata.h | 13 ----------
2 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/regulator/da9055-regulator.c b/drivers/regulator/da9055-regulator.c
index 8fd9ac787588..352547c375bd 100644
--- a/drivers/regulator/da9055-regulator.c
+++ b/drivers/regulator/da9055-regulator.c
@@ -9,7 +9,6 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/err.h>
-#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/platform_device.h>
#include <linux/regulator/driver.h>
@@ -413,31 +412,35 @@ static struct da9055_regulator_info da9055_regulator_info[] = {
* GPIO can control regulator state and/or select the regulator register
* set A/B for voltage ramping.
*/
-static int da9055_gpio_init(struct da9055_regulator *regulator,
+static int da9055_gpio_init(struct device *dev,
+ struct da9055_regulator *regulator,
struct regulator_config *config,
struct da9055_pdata *pdata, int id)
{
struct da9055_regulator_info *info = regulator->info;
+ struct gpio_desc *ren;
+ struct gpio_desc *ena;
+ struct gpio_desc *rsel;
int ret = 0;

- if (!pdata)
- return 0;
+ /* Look for "regulator-enable-gpios" GPIOs in the regulator node */
+ ren = devm_gpiod_get_optional(dev, "regulator-enable", GPIOD_IN);
+ if (IS_ERR(ren))
+ return PTR_ERR(ren);

- if (pdata->gpio_ren && pdata->gpio_ren[id]) {
- char name[18];
- int gpio_mux = pdata->gpio_ren[id];
+ if (ren) {
+ /* This GPIO is not optional at this point */
+ ena = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
+ if (IS_ERR(ena))
+ return PTR_ERR(ena);

- config->ena_gpiod = pdata->ena_gpiods[id];
+ config->ena_gpiod = ena;

/*
* GPI pin is muxed with regulator to control the
* regulator state.
*/
- sprintf(name, "DA9055 GPI %d", gpio_mux);
- ret = devm_gpio_request_one(config->dev, gpio_mux, GPIOF_DIR_IN,
- name);
- if (ret < 0)
- goto err;
+ gpiod_set_consumer_name(ren, "DA9055 ren GPI");

/*
* Let the regulator know that its state is controlled
@@ -448,24 +451,22 @@ static int da9055_gpio_init(struct da9055_regulator *regulator,
pdata->reg_ren[id]
<< DA9055_E_GPI_SHIFT);
if (ret < 0)
- goto err;
+ return ret;
}

- if (pdata->gpio_rsel && pdata->gpio_rsel[id]) {
- char name[18];
- int gpio_mux = pdata->gpio_rsel[id];
+ /* Look for "regulator-select-gpios" GPIOs in the regulator node */
+ rsel = devm_gpiod_get_optional(dev, "regulator-select", GPIOD_IN);
+ if (IS_ERR(rsel))
+ return PTR_ERR(rsel);

+ if (rsel) {
regulator->reg_rselect = pdata->reg_rsel[id];

/*
* GPI pin is muxed with regulator to select the
* regulator register set A/B for voltage ramping.
*/
- sprintf(name, "DA9055 GPI %d", gpio_mux);
- ret = devm_gpio_request_one(config->dev, gpio_mux, GPIOF_DIR_IN,
- name);
- if (ret < 0)
- goto err;
+ gpiod_set_consumer_name(rsel, "DA9055 rsel GPI");

/*
* Let the regulator know that its register set A/B
@@ -477,7 +478,6 @@ static int da9055_gpio_init(struct da9055_regulator *regulator,
<< DA9055_V_GPI_SHIFT);
}

-err:
return ret;
}

@@ -532,7 +532,7 @@ static int da9055_regulator_probe(struct platform_device *pdev)
if (pdata)
config.init_data = pdata->regulators[pdev->id];

- ret = da9055_gpio_init(regulator, &config, pdata, pdev->id);
+ ret = da9055_gpio_init(&pdev->dev, regulator, &config, pdata, pdev->id);
if (ret < 0)
return ret;

diff --git a/include/linux/mfd/da9055/pdata.h b/include/linux/mfd/da9055/pdata.h
index d3f126990ad0..137a2b067512 100644
--- a/include/linux/mfd/da9055/pdata.h
+++ b/include/linux/mfd/da9055/pdata.h
@@ -7,7 +7,6 @@
#define DA9055_MAX_REGULATORS 8

struct da9055;
-struct gpio_desc;

enum gpio_select {
NO_GPIO = 0,
@@ -23,16 +22,6 @@ struct da9055_pdata {
struct regulator_init_data *regulators[DA9055_MAX_REGULATORS];
/* Enable RTC in RESET Mode */
bool reset_enable;
- /*
- * GPI muxed pin to control
- * regulator state A/B, 0 if not available.
- */
- int *gpio_ren;
- /*
- * GPI muxed pin to control
- * regulator set, 0 if not available.
- */
- int *gpio_rsel;
/*
* Regulator mode control bits value (GPI offset) that
* controls the regulator state, 0 if not available.
@@ -43,7 +32,5 @@ struct da9055_pdata {
* controls the regulator set A/B, 0 if not available.
*/
enum gpio_select *reg_rsel;
- /* GPIO descriptors to enable regulator, NULL if not available */
- struct gpio_desc **ena_gpiods;
};
#endif /* __DA9055_PDATA_H */

--
2.34.1


2024-02-20 08:36:58

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 3/5] regulator: lp8788-buck: Fully convert to GPIO descriptors

This converts the LP8788 BUCK regulator driver to use GPIO
descriptors.

BUCK1 can use one DVS GPIO and BUCK2 can use two DVS GPIOS,
and no more so just hardcode two GPIO descriptors into
the per-DVS state containers.

Obtain the descriptors from each regulators subdevice.

As there are no in-tree users, board files need to populate
descriptor tables for the buck regulator devices when
they want to use this driver. BUCK1 need a GPIO descriptor
at index 0 and BUCK2 needs two GPIO descriptors at
indices 0 and 1.

Signed-off-by: Linus Walleij <[email protected]>
---
drivers/regulator/lp8788-buck.c | 64 +++++++++++++++++------------------------
include/linux/mfd/lp8788.h | 9 ++----
2 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/drivers/regulator/lp8788-buck.c b/drivers/regulator/lp8788-buck.c
index e97ade09dede..712eaa6ff8ab 100644
--- a/drivers/regulator/lp8788-buck.c
+++ b/drivers/regulator/lp8788-buck.c
@@ -13,7 +13,7 @@
#include <linux/platform_device.h>
#include <linux/regulator/driver.h>
#include <linux/mfd/lp8788.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>

/* register address */
#define LP8788_EN_BUCK 0x0C
@@ -69,8 +69,8 @@
#define BUCK_FPWM_SHIFT(x) (x)

enum lp8788_dvs_state {
- DVS_LOW = GPIOF_OUT_INIT_LOW,
- DVS_HIGH = GPIOF_OUT_INIT_HIGH,
+ DVS_LOW = 0,
+ DVS_HIGH = 1,
};

enum lp8788_dvs_mode {
@@ -89,6 +89,8 @@ struct lp8788_buck {
struct lp8788 *lp;
struct regulator_dev *regulator;
void *dvs;
+ struct gpio_desc *gpio1;
+ struct gpio_desc *gpio2; /* Only used on BUCK2 */
};

/* BUCK 1 ~ 4 voltage ranges */
@@ -106,8 +108,7 @@ static void lp8788_buck1_set_dvs(struct lp8788_buck *buck)
return;

pinstate = dvs->vsel == DVS_SEL_V0 ? DVS_LOW : DVS_HIGH;
- if (gpio_is_valid(dvs->gpio))
- gpio_set_value(dvs->gpio, pinstate);
+ gpiod_set_value(buck->gpio1, pinstate);
}

static void lp8788_buck2_set_dvs(struct lp8788_buck *buck)
@@ -139,11 +140,8 @@ static void lp8788_buck2_set_dvs(struct lp8788_buck *buck)
return;
}

- if (gpio_is_valid(dvs->gpio[0]))
- gpio_set_value(dvs->gpio[0], pin1);
-
- if (gpio_is_valid(dvs->gpio[1]))
- gpio_set_value(dvs->gpio[1], pin2);
+ gpiod_set_value(buck->gpio1, pin1);
+ gpiod_set_value(buck->gpio2, pin2);
}

static void lp8788_set_dvs(struct lp8788_buck *buck, enum lp8788_buck_id id)
@@ -202,19 +200,13 @@ static u8 lp8788_select_buck_vout_addr(struct lp8788_buck *buck,
enum lp8788_buck_id id)
{
enum lp8788_dvs_mode mode = lp8788_get_buck_dvs_ctrl_mode(buck, id);
- struct lp8788_buck1_dvs *b1_dvs;
- struct lp8788_buck2_dvs *b2_dvs;
u8 val, idx, addr;
int pin1, pin2;

switch (id) {
case BUCK1:
if (mode == EXTPIN) {
- b1_dvs = (struct lp8788_buck1_dvs *)buck->dvs;
- if (!b1_dvs)
- goto err;
-
- idx = gpio_get_value(b1_dvs->gpio) ? 1 : 0;
+ idx = gpiod_get_value(buck->gpio1);
} else {
lp8788_read_byte(buck->lp, LP8788_BUCK_DVS_SEL, &val);
idx = (val & LP8788_BUCK1_DVS_M) >> LP8788_BUCK1_DVS_S;
@@ -223,12 +215,8 @@ static u8 lp8788_select_buck_vout_addr(struct lp8788_buck *buck,
break;
case BUCK2:
if (mode == EXTPIN) {
- b2_dvs = (struct lp8788_buck2_dvs *)buck->dvs;
- if (!b2_dvs)
- goto err;
-
- pin1 = gpio_get_value(b2_dvs->gpio[0]);
- pin2 = gpio_get_value(b2_dvs->gpio[1]);
+ pin1 = gpiod_get_value(buck->gpio1);
+ pin2 = gpiod_get_value(buck->gpio2);

if (pin1 == PIN_LOW && pin2 == PIN_LOW)
idx = 0;
@@ -424,28 +412,28 @@ static int lp8788_dvs_gpio_request(struct platform_device *pdev,
enum lp8788_buck_id id)
{
struct lp8788_platform_data *pdata = buck->lp->pdata;
- char *b1_name = "LP8788_B1_DVS";
- char *b2_name[] = { "LP8788_B2_DVS1", "LP8788_B2_DVS2" };
- int i, gpio, ret;
+ struct device *dev = &pdev->dev;

switch (id) {
case BUCK1:
- gpio = pdata->buck1_dvs->gpio;
- ret = devm_gpio_request_one(&pdev->dev, gpio, DVS_LOW,
- b1_name);
- if (ret)
- return ret;
+ buck->gpio1 = devm_gpiod_get(dev, "dvs", GPIOD_OUT_LOW);
+ if (IS_ERR(buck->gpio1))
+ return PTR_ERR(buck->gpio1);
+ gpiod_set_consumer_name(buck->gpio1, "LP8788_B1_DVS");

buck->dvs = pdata->buck1_dvs;
break;
case BUCK2:
- for (i = 0; i < LP8788_NUM_BUCK2_DVS; i++) {
- gpio = pdata->buck2_dvs->gpio[i];
- ret = devm_gpio_request_one(&pdev->dev, gpio,
- DVS_LOW, b2_name[i]);
- if (ret)
- return ret;
- }
+ buck->gpio1 = devm_gpiod_get_index(dev, "dvs", 0, GPIOD_OUT_LOW);
+ if (IS_ERR(buck->gpio1))
+ return PTR_ERR(buck->gpio1);
+ gpiod_set_consumer_name(buck->gpio1, "LP8788_B2_DVS1");
+
+ buck->gpio2 = devm_gpiod_get_index(dev, "dvs", 1, GPIOD_OUT_LOW);
+ if (IS_ERR(buck->gpio1))
+ return PTR_ERR(buck->gpio1);
+ gpiod_set_consumer_name(buck->gpio1, "LP8788_B2_DVS2");
+
buck->dvs = pdata->buck2_dvs;
break;
default:
diff --git a/include/linux/mfd/lp8788.h b/include/linux/mfd/lp8788.h
index 3d5c480d58ea..51b47966a04d 100644
--- a/include/linux/mfd/lp8788.h
+++ b/include/linux/mfd/lp8788.h
@@ -10,7 +10,6 @@
#ifndef __MFD_LP8788_H__
#define __MFD_LP8788_H__

-#include <linux/gpio.h>
#include <linux/irqdomain.h>
#include <linux/pwm.h>
#include <linux/regmap.h>
@@ -159,21 +158,17 @@ struct lp8788;

/*
* lp8788_buck1_dvs
- * @gpio : gpio pin number for dvs control
* @vsel : dvs selector for buck v1 register
*/
struct lp8788_buck1_dvs {
- int gpio;
enum lp8788_dvs_sel vsel;
};

/*
* lp8788_buck2_dvs
- * @gpio : two gpio pin numbers are used for dvs
* @vsel : dvs selector for buck v2 register
*/
struct lp8788_buck2_dvs {
- int gpio[LP8788_NUM_BUCK2_DVS];
enum lp8788_dvs_sel vsel;
};

@@ -268,8 +263,8 @@ struct lp8788_vib_platform_data {
* @buck_data : regulator initial data for buck
* @dldo_data : regulator initial data for digital ldo
* @aldo_data : regulator initial data for analog ldo
- * @buck1_dvs : gpio configurations for buck1 dvs
- * @buck2_dvs : gpio configurations for buck2 dvs
+ * @buck1_dvs : configurations for buck1 dvs
+ * @buck2_dvs : configurations for buck2 dvs
* @chg_pdata : platform data for charger driver
* @alarm_sel : rtc alarm selection (1 or 2)
* @bl_pdata : configurable data for backlight driver

--
2.34.1


2024-02-20 08:37:09

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 4/5] regulator: max8997: Convert to GPIO descriptors

This rewrites the max8997 regulator driver to fetch the dvs
regulators as descriptors. This will likely mostly come from
the device tree since there are no in-tree users of the platform
data, but supplying GPIO descriptor tables from board files is
also possible if needed.

Signed-off-by: Linus Walleij <[email protected]>
---
drivers/regulator/max8997-regulator.c | 85 +++++++++++------------------------
include/linux/mfd/max8997.h | 1 -
2 files changed, 25 insertions(+), 61 deletions(-)

diff --git a/drivers/regulator/max8997-regulator.c b/drivers/regulator/max8997-regulator.c
index 0b38eaa73597..5f201ee9a5b8 100644
--- a/drivers/regulator/max8997-regulator.c
+++ b/drivers/regulator/max8997-regulator.c
@@ -9,8 +9,7 @@

#include <linux/bug.h>
#include <linux/err.h>
-#include <linux/gpio.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -32,7 +31,7 @@ struct max8997_data {
u8 buck1_vol[8];
u8 buck2_vol[8];
u8 buck5_vol[8];
- int buck125_gpios[3];
+ struct gpio_desc *buck125_gpiods[3];
int buck125_gpioindex;
bool ignore_gpiodvs_side_effect;

@@ -52,9 +51,9 @@ static inline void max8997_set_gpio(struct max8997_data *max8997)
int set2 = ((max8997->buck125_gpioindex) >> 1) & 0x1;
int set1 = ((max8997->buck125_gpioindex) >> 2) & 0x1;

- gpio_set_value(max8997->buck125_gpios[0], set1);
- gpio_set_value(max8997->buck125_gpios[1], set2);
- gpio_set_value(max8997->buck125_gpios[2], set3);
+ gpiod_set_value(max8997->buck125_gpiods[0], set1);
+ gpiod_set_value(max8997->buck125_gpiods[1], set2);
+ gpiod_set_value(max8997->buck125_gpiods[2], set3);
}

struct voltage_map_desc {
@@ -873,31 +872,13 @@ static struct regulator_desc regulators[] = {
};

#ifdef CONFIG_OF
-static int max8997_pmic_dt_parse_dvs_gpio(struct platform_device *pdev,
- struct max8997_platform_data *pdata,
- struct device_node *pmic_np)
-{
- int i, gpio;
-
- for (i = 0; i < 3; i++) {
- gpio = of_get_named_gpio(pmic_np,
- "max8997,pmic-buck125-dvs-gpios", i);
- if (!gpio_is_valid(gpio)) {
- dev_err(&pdev->dev, "invalid gpio[%d]: %d\n", i, gpio);
- return -EINVAL;
- }
- pdata->buck125_gpios[i] = gpio;
- }
- return 0;
-}
-
static int max8997_pmic_dt_parse_pdata(struct platform_device *pdev,
struct max8997_platform_data *pdata)
{
struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
struct device_node *pmic_np, *regulators_np, *reg_np;
struct max8997_regulator_data *rdata;
- unsigned int i, dvs_voltage_nr = 1, ret;
+ unsigned int i, dvs_voltage_nr = 1;

pmic_np = iodev->dev->of_node;
if (!pmic_np) {
@@ -949,10 +930,6 @@ static int max8997_pmic_dt_parse_pdata(struct platform_device *pdev,

if (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
pdata->buck5_gpiodvs) {
- ret = max8997_pmic_dt_parse_dvs_gpio(pdev, pdata, pmic_np);
- if (ret)
- return -EINVAL;
-
if (of_property_read_u32(pmic_np,
"max8997,pmic-buck125-default-dvs-idx",
&pdata->buck125_default_idx)) {
@@ -1039,7 +1016,6 @@ static int max8997_pmic_probe(struct platform_device *pdev)
max8997->buck1_gpiodvs = pdata->buck1_gpiodvs;
max8997->buck2_gpiodvs = pdata->buck2_gpiodvs;
max8997->buck5_gpiodvs = pdata->buck5_gpiodvs;
- memcpy(max8997->buck125_gpios, pdata->buck125_gpios, sizeof(int) * 3);
max8997->ignore_gpiodvs_side_effect = pdata->ignore_gpiodvs_side_effect;

nr_dvs = (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
@@ -1110,38 +1086,27 @@ static int max8997_pmic_probe(struct platform_device *pdev)
*/
if (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
pdata->buck5_gpiodvs) {
+ const char *gpio_names[3] = {"MAX8997 SET1", "MAX8997 SET2", "MAX8997 SET3"};

- if (!gpio_is_valid(pdata->buck125_gpios[0]) ||
- !gpio_is_valid(pdata->buck125_gpios[1]) ||
- !gpio_is_valid(pdata->buck125_gpios[2])) {
- dev_err(&pdev->dev, "GPIO NOT VALID\n");
- return -EINVAL;
- }
+ for (i = 0; i < 3; i++) {
+ enum gpiod_flags flags;

- ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[0],
- "MAX8997 SET1");
- if (ret)
- return ret;
-
- ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[1],
- "MAX8997 SET2");
- if (ret)
- return ret;
-
- ret = devm_gpio_request(&pdev->dev, pdata->buck125_gpios[2],
- "MAX8997 SET3");
- if (ret)
- return ret;
-
- gpio_direction_output(pdata->buck125_gpios[0],
- (max8997->buck125_gpioindex >> 2)
- & 0x1); /* SET1 */
- gpio_direction_output(pdata->buck125_gpios[1],
- (max8997->buck125_gpioindex >> 1)
- & 0x1); /* SET2 */
- gpio_direction_output(pdata->buck125_gpios[2],
- (max8997->buck125_gpioindex >> 0)
- & 0x1); /* SET3 */
+ if (max8997->buck125_gpioindex & BIT(2 - i))
+ flags = GPIOD_OUT_HIGH;
+ else
+ flags = GPIOD_OUT_LOW;
+
+ max8997->buck125_gpiods[i] = devm_gpiod_get_index(iodev->dev,
+ "max8997,pmic-buck125-dvs",
+ i,
+ flags);
+ if (IS_ERR(max8997->buck125_gpiods[i])) {
+ ret = PTR_ERR(max8997->buck125_gpiods[i]);
+ return dev_err_probe(iodev->dev, ret, "cant get GPIO %d (%d)\n",
+ i, ret);
+ }
+ gpiod_set_consumer_name(max8997->buck125_gpiods[i], gpio_names[i]);
+ }
}

/* DVS-GPIO disabled */
diff --git a/include/linux/mfd/max8997.h b/include/linux/mfd/max8997.h
index 6193905abbb5..5c2cc1103437 100644
--- a/include/linux/mfd/max8997.h
+++ b/include/linux/mfd/max8997.h
@@ -178,7 +178,6 @@ struct max8997_platform_data {
*
*/
bool ignore_gpiodvs_side_effect;
- int buck125_gpios[3]; /* GPIO of [0]SET1, [1]SET2, [2]SET3 */
int buck125_default_idx; /* Default value of SET1, 2, 3 */
unsigned int buck1_voltage[8]; /* buckx_voltage in uV */
bool buck1_gpiodvs;

--
2.34.1


2024-02-20 08:37:26

by Linus Walleij

[permalink] [raw]
Subject: [PATCH 5/5] regulator: max8998: Convert to GPIO descriptors

This rewrites the max8998 regulator driver to fetch the dvs
regulators as descriptors. This will likely mostly come from
the device tree since there are no in-tree users of the platform
data, but supplying GPIO descriptor tables from board files is
also possible if needed.

Signed-off-by: Linus Walleij <[email protected]>
---
drivers/regulator/max8998.c | 150 ++++++++++++++++----------------------------
include/linux/mfd/max8998.h | 6 --
2 files changed, 54 insertions(+), 102 deletions(-)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index fadb4717384a..254a77887f66 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -10,12 +10,12 @@
#include <linux/init.h>
#include <linux/i2c.h>
#include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/bits.h>
+#include <linux/gpio/consumer.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/mutex.h>
#include <linux/of.h>
-#include <linux/of_gpio.h>
#include <linux/platform_device.h>
#include <linux/regulator/driver.h>
#include <linux/regulator/of_regulator.h>
@@ -31,6 +31,9 @@ struct max8998_data {
unsigned int buck1_idx; /* index to last changed voltage */
/* value in a set */
unsigned int buck2_idx;
+ struct gpio_desc *buck1_gpio1;
+ struct gpio_desc *buck1_gpio2;
+ struct gpio_desc *buck2_gpio;
};

static const unsigned int charger_current_table[] = {
@@ -227,15 +230,15 @@ static int max8998_set_voltage_ldo_sel(struct regulator_dev *rdev,
return ret;
}

-static inline void buck1_gpio_set(int gpio1, int gpio2, int v)
+static inline void buck1_gpio_set(struct gpio_desc *gpio1, struct gpio_desc *gpio2, int v)
{
- gpio_set_value(gpio1, v & 0x1);
- gpio_set_value(gpio2, (v >> 1) & 0x1);
+ gpiod_set_value(gpio1, v & 0x1);
+ gpiod_set_value(gpio2, (v >> 1) & 0x1);
}

-static inline void buck2_gpio_set(int gpio, int v)
+static inline void buck2_gpio_set(struct gpio_desc *gpio, int v)
{
- gpio_set_value(gpio, v & 0x1);
+ gpiod_set_value(gpio, v & 0x1);
}

static int max8998_set_voltage_buck_sel(struct regulator_dev *rdev,
@@ -260,16 +263,15 @@ static int max8998_set_voltage_buck_sel(struct regulator_dev *rdev,
selector, max8998->buck1_vol[0], max8998->buck1_vol[1],
max8998->buck1_vol[2], max8998->buck1_vol[3]);

- if (gpio_is_valid(pdata->buck1_set1) &&
- gpio_is_valid(pdata->buck1_set2)) {
+ if (max8998->buck1_gpio1 && max8998->buck1_gpio2) {

/* check if requested voltage */
/* value is already defined */
for (j = 0; j < ARRAY_SIZE(max8998->buck1_vol); j++) {
if (max8998->buck1_vol[j] == selector) {
max8998->buck1_idx = j;
- buck1_gpio_set(pdata->buck1_set1,
- pdata->buck1_set2, j);
+ buck1_gpio_set(max8998->buck1_gpio1,
+ max8998->buck1_gpio2, j);
goto buck1_exit;
}
}
@@ -286,13 +288,13 @@ static int max8998_set_voltage_buck_sel(struct regulator_dev *rdev,
&shift,
&mask);
ret = max8998_write_reg(i2c, reg, selector);
- buck1_gpio_set(pdata->buck1_set1,
- pdata->buck1_set2, max8998->buck1_idx);
+ buck1_gpio_set(max8998->buck1_gpio1,
+ max8998->buck1_gpio2, max8998->buck1_idx);
buck1_last_val++;
buck1_exit:
dev_dbg(max8998->dev, "%s: SET1:%d, SET2:%d\n",
- i2c->name, gpio_get_value(pdata->buck1_set1),
- gpio_get_value(pdata->buck1_set2));
+ i2c->name, gpiod_get_value(max8998->buck1_gpio1),
+ gpiod_get_value(max8998->buck1_gpio2));
break;
} else {
ret = max8998_write_reg(i2c, reg, selector);
@@ -303,14 +305,13 @@ static int max8998_set_voltage_buck_sel(struct regulator_dev *rdev,
dev_dbg(max8998->dev,
"BUCK2, selector:%d buck2_vol1:%d, buck2_vol2:%d\n",
selector, max8998->buck2_vol[0], max8998->buck2_vol[1]);
- if (gpio_is_valid(pdata->buck2_set3)) {
-
+ if (max8998->buck2_gpio) {
/* check if requested voltage */
/* value is already defined */
for (j = 0; j < ARRAY_SIZE(max8998->buck2_vol); j++) {
if (max8998->buck2_vol[j] == selector) {
max8998->buck2_idx = j;
- buck2_gpio_set(pdata->buck2_set3, j);
+ buck2_gpio_set(max8998->buck2_gpio, j);
goto buck2_exit;
}
}
@@ -322,10 +323,10 @@ static int max8998_set_voltage_buck_sel(struct regulator_dev *rdev,
&reg, &shift, &mask);
ret = max8998_write_reg(i2c, reg, selector);
max8998->buck2_vol[max8998->buck2_idx] = selector;
- buck2_gpio_set(pdata->buck2_set3, max8998->buck2_idx);
+ buck2_gpio_set(max8998->buck2_gpio, max8998->buck2_idx);
buck2_exit:
dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
- gpio_get_value(pdata->buck2_set3));
+ gpiod_get_value(max8998->buck2_gpio));
} else {
ret = max8998_write_reg(i2c, reg, selector);
}
@@ -539,36 +540,6 @@ static const struct regulator_desc regulators[] = {
charger_current_table, MAX8998_REG_CHGR1, 0x7),
};

-static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
- struct max8998_platform_data *pdata,
- struct device_node *pmic_np)
-{
- int gpio;
-
- gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
- if (!gpio_is_valid(gpio)) {
- dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
- return -EINVAL;
- }
- pdata->buck1_set1 = gpio;
-
- gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
- if (!gpio_is_valid(gpio)) {
- dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
- return -EINVAL;
- }
- pdata->buck1_set2 = gpio;
-
- gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
- if (!gpio_is_valid(gpio)) {
- dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
- return -EINVAL;
- }
- pdata->buck2_set3 = gpio;
-
- return 0;
-}
-
static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
struct max8998_platform_data *pdata)
{
@@ -614,10 +585,6 @@ static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
of_node_put(reg_np);
of_node_put(regulators_np);

- ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
- if (ret)
- return -EINVAL;
-
pdata->buck_voltage_lock = of_property_read_bool(pmic_np, "max8998,pmic-buck-voltage-lock");

ret = of_property_read_u32(pmic_np,
@@ -665,6 +632,7 @@ static int max8998_pmic_probe(struct platform_device *pdev)
struct regulator_dev *rdev;
struct max8998_data *max8998;
struct i2c_client *i2c;
+ enum gpiod_flags flags;
int i, ret;
unsigned int v;

@@ -693,37 +661,38 @@ static int max8998_pmic_probe(struct platform_device *pdev)
max8998->buck1_idx = pdata->buck1_default_idx;
max8998->buck2_idx = pdata->buck2_default_idx;

- /* NOTE: */
- /* For unused GPIO NOT marked as -1 (thereof equal to 0) WARN_ON */
- /* will be displayed */
-
/* Check if MAX8998 voltage selection GPIOs are defined */
- if (gpio_is_valid(pdata->buck1_set1) &&
- gpio_is_valid(pdata->buck1_set2)) {
- /* Check if SET1 is not equal to 0 */
- if (!pdata->buck1_set1) {
- dev_err(&pdev->dev,
- "MAX8998 SET1 GPIO defined as 0 !\n");
- WARN_ON(!pdata->buck1_set1);
- return -EIO;
- }
- /* Check if SET2 is not equal to 0 */
- if (!pdata->buck1_set2) {
- dev_err(&pdev->dev,
- "MAX8998 SET2 GPIO defined as 0 !\n");
- WARN_ON(!pdata->buck1_set2);
- return -EIO;
- }
-
- gpio_request(pdata->buck1_set1, "MAX8998 BUCK1_SET1");
- gpio_direction_output(pdata->buck1_set1,
- max8998->buck1_idx & 0x1);
-
-
- gpio_request(pdata->buck1_set2, "MAX8998 BUCK1_SET2");
- gpio_direction_output(pdata->buck1_set2,
- (max8998->buck1_idx >> 1) & 0x1);
-
+ flags = (max8998->buck1_idx & BIT(0)) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ max8998->buck1_gpio1 = devm_gpiod_get_index_optional(iodev->dev,
+ "max8998,pmic-buck1-dvs",
+ 0,
+ flags);
+ if (IS_ERR(max8998->buck1_gpio1))
+ return dev_err_probe(&pdev->dev, PTR_ERR(max8998->buck1_gpio1),
+ "could not get BUCK1 GPIO1\n");
+ gpiod_set_consumer_name(max8998->buck1_gpio1, "MAX8998 BUCK1_SET1");
+
+ flags = (max8998->buck1_idx & BIT(1)) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ max8998->buck1_gpio2 = devm_gpiod_get_index_optional(iodev->dev,
+ "max8998,pmic-buck1-dvs",
+ 1,
+ flags);
+ if (IS_ERR(max8998->buck1_gpio2))
+ return dev_err_probe(&pdev->dev, PTR_ERR(max8998->buck1_gpio2),
+ "could not get BUCK1 GPIO2\n");
+ gpiod_set_consumer_name(max8998->buck1_gpio1, "MAX8998 BUCK1_SET2");
+
+ flags = (max8998->buck2_idx & BIT(0)) ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+ max8998->buck2_gpio = devm_gpiod_get_index_optional(iodev->dev,
+ "max8998,pmic-buck2-dvs",
+ 0,
+ flags);
+ if (IS_ERR(max8998->buck2_gpio))
+ return dev_err_probe(&pdev->dev, PTR_ERR(max8998->buck2_gpio),
+ "could not get BUCK2 GPIO\n");
+ gpiod_set_consumer_name(max8998->buck1_gpio1, "MAX8998 BUCK2_SET3");
+
+ if (max8998->buck1_gpio1 && max8998->buck1_gpio2) {
/* Set predefined values for BUCK1 registers */
for (v = 0; v < ARRAY_SIZE(pdata->buck1_voltage); ++v) {
int index = MAX8998_BUCK1 - MAX8998_LDO2;
@@ -742,18 +711,7 @@ static int max8998_pmic_probe(struct platform_device *pdev)
}
}

- if (gpio_is_valid(pdata->buck2_set3)) {
- /* Check if SET3 is not equal to 0 */
- if (!pdata->buck2_set3) {
- dev_err(&pdev->dev,
- "MAX8998 SET3 GPIO defined as 0 !\n");
- WARN_ON(!pdata->buck2_set3);
- return -EIO;
- }
- gpio_request(pdata->buck2_set3, "MAX8998 BUCK2_SET3");
- gpio_direction_output(pdata->buck2_set3,
- max8998->buck2_idx & 0x1);
-
+ if (max8998->buck2_gpio) {
/* Set predefined values for BUCK2 registers */
for (v = 0; v < ARRAY_SIZE(pdata->buck2_voltage); ++v) {
int index = MAX8998_BUCK2 - MAX8998_LDO2;
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 79c020bd0c70..a054e55c8646 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -65,10 +65,7 @@ struct max8998_regulator_data {
* be other than the preset values.
* @buck1_voltage: BUCK1 DVS mode 1 voltage registers
* @buck2_voltage: BUCK2 DVS mode 2 voltage registers
- * @buck1_set1: BUCK1 gpio pin 1 to set output voltage
- * @buck1_set2: BUCK1 gpio pin 2 to set output voltage
* @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
- * @buck2_set3: BUCK2 gpio pin to set output voltage
* @buck2_default_idx: Default for BUCK2 gpio pin.
* @wakeup: Allow to wake up from suspend
* @rtc_delay: LP3974 RTC chip bug that requires delay after a register
@@ -91,10 +88,7 @@ struct max8998_platform_data {
bool buck_voltage_lock;
int buck1_voltage[4];
int buck2_voltage[2];
- int buck1_set1;
- int buck1_set2;
int buck1_default_idx;
- int buck2_set3;
int buck2_default_idx;
bool wakeup;
bool rtc_delay;

--
2.34.1


2024-02-23 16:26:36

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 0/5] Convert some regulator drivers to GPIO descriptors

On Tue, 20 Feb 2024, Linus Walleij wrote:

> Despite the work to convert the regulator core to GPIO
> descriptors, there are some outliers that use legacy GPIO
> numbers in various ways. Convert them all over.
>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> Linus Walleij (5):
> regulator: max8973: Finalize switch to GPIO descriptors
> regulator: da9055: Fully convert to GPIO descriptors
> regulator: lp8788-buck: Fully convert to GPIO descriptors
> regulator: max8997: Convert to GPIO descriptors
> regulator: max8998: Convert to GPIO descriptors
>
> drivers/regulator/da9055-regulator.c | 48 ++++-----
> drivers/regulator/lp8788-buck.c | 64 +++++-------
> drivers/regulator/max8973-regulator.c | 36 +++----
> drivers/regulator/max8997-regulator.c | 85 +++++-----------
> drivers/regulator/max8998.c | 150 ++++++++++------------------

> include/linux/mfd/da9055/pdata.h | 13 ---
> include/linux/mfd/lp8788.h | 9 +-
> include/linux/mfd/max8997.h | 1 -
> include/linux/mfd/max8998.h | 6 --

Acked-by: Lee Jones <[email protected]>

> include/linux/regulator/max8973-regulator.h | 6 --
> 10 files changed, 143 insertions(+), 275 deletions(-)
> ---
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> change-id: 20240212-descriptors-regulators-f455ccf0119c
>
> Best regards,
> --
> Linus Walleij <[email protected]>
>

--
Lee Jones [李琼斯]

2024-02-27 11:45:55

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/5] Convert some regulator drivers to GPIO descriptors

On Tue, 20 Feb 2024 09:36:23 +0100, Linus Walleij wrote:
> Despite the work to convert the regulator core to GPIO
> descriptors, there are some outliers that use legacy GPIO
> numbers in various ways. Convert them all over.
>
>

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/5] regulator: max8973: Finalize switch to GPIO descriptors
commit: 4d52f575e258c6f93f4180c21afda8634b0d2af5
[2/5] regulator: da9055: Fully convert to GPIO descriptors
commit: e450a2b3a335332d4a51fe10c9fff8150c6e2364
[3/5] regulator: lp8788-buck: Fully convert to GPIO descriptors
commit: 95daa868f22b509ad641bf003d9d441d6a2fa505
[4/5] regulator: max8997: Convert to GPIO descriptors
commit: 84618d5e31cfd01fc3f53a8c2ebb68bc43d8b760
[5/5] regulator: max8998: Convert to GPIO descriptors
commit: f25828a1eae1ee1a9257e2818b237b8208bd383e

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark