This converts the Wolfson Micro WM831x DCDC converter to use
a GPIO descriptor for the GPIO driving the DVS pin.
There is just one (non-DT) machine in the kernel using this, and
that is the Wolfson Micro (now Cirrus) Cragganmore 6410 so we
patch this board to pass a descriptor table and fix up the driver
accordingly.
Cc: Charles Keepax <[email protected]>
Cc: Richard Fitzgerald <[email protected]>
Cc: [email protected]
Signed-off-by: Linus Walleij <[email protected]>
---
arch/arm/mach-s3c64xx/mach-crag6410.c | 20 +++++++++++++++++-
drivers/regulator/wm831x-dcdc.c | 29 ++++++++++++---------------
include/linux/mfd/wm831x/pdata.h | 1 -
3 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
index 379424d72ae7..b90ded91916c 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -15,6 +15,7 @@
#include <linux/io.h>
#include <linux/init.h>
#include <linux/gpio.h>
+#include <linux/gpio/machine.h>
#include <linux/leds.h>
#include <linux/delay.h>
#include <linux/mmc/host.h>
@@ -398,7 +399,6 @@ static struct pca953x_platform_data crag6410_pca_data = {
/* VDDARM is controlled by DVS1 connected to GPK(0) */
static struct wm831x_buckv_pdata vddarm_pdata = {
.dvs_control_src = 1,
- .dvs_gpio = S3C64XX_GPK(0),
};
static struct regulator_consumer_supply vddarm_consumers[] = {
@@ -596,6 +596,23 @@ static struct wm831x_pdata crag_pmic_pdata = {
.touch = &touch_pdata,
};
+/*
+ * VDDARM is eventually ending up as a regulator hanging on the MFD cell device
+ * "wm831x-buckv.1" spawn from drivers/mfd/wm831x-core.c.
+ *
+ * From the note on the platform data we can see that this is clearly DVS1
+ * and assigned as dcdc1 resource to the MFD core which sets .id of the cell
+ * spawning the DVS1 platform device to 1, resulting in the device name
+ * "wm831x-buckv.1".
+ */
+static struct gpiod_lookup_table crag_pmic_gpiod_table = {
+ .dev_id = "wm831x-buckv.1",
+ .table = {
+ GPIO_LOOKUP("GPIOK", 0, "dvs", GPIO_ACTIVE_HIGH),
+ { },
+ },
+};
+
static struct i2c_board_info i2c_devs0[] = {
{ I2C_BOARD_INFO("24c08", 0x50), },
{ I2C_BOARD_INFO("tca6408", 0x20),
@@ -836,6 +853,7 @@ static void __init crag6410_machine_init(void)
s3c_fb_set_platdata(&crag6410_lcd_pdata);
dwc2_hsotg_set_platdata(&crag6410_hsotg_pdata);
+ gpiod_add_lookup_table(&crag_pmic_gpiod_table);
i2c_register_board_info(0, i2c_devs0, ARRAY_SIZE(i2c_devs0));
i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index b422eef97b77..319a743e242a 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -15,7 +15,7 @@
#include <linux/platform_device.h>
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/slab.h>
#include <linux/mfd/wm831x/core.h>
@@ -50,7 +50,7 @@ struct wm831x_dcdc {
int base;
struct wm831x *wm831x;
struct regulator_dev *regulator;
- int dvs_gpio;
+ struct gpio_desc *dvs_gpiod;
int dvs_gpio_state;
int on_vsel;
int dvs_vsel;
@@ -217,7 +217,7 @@ static int wm831x_buckv_set_dvs(struct regulator_dev *rdev, int state)
return 0;
dcdc->dvs_gpio_state = state;
- gpio_set_value(dcdc->dvs_gpio, state);
+ gpiod_set_value(dcdc->dvs_gpiod, state);
/* Should wait for DVS state change to be asserted if we have
* a GPIO for it, for now assume the device is configured
@@ -237,10 +237,10 @@ static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
int ret;
/* If this value is already set then do a GPIO update if we can */
- if (dcdc->dvs_gpio && dcdc->on_vsel == vsel)
+ if (dcdc->dvs_gpiod && dcdc->on_vsel == vsel)
return wm831x_buckv_set_dvs(rdev, 0);
- if (dcdc->dvs_gpio && dcdc->dvs_vsel == vsel)
+ if (dcdc->dvs_gpiod && dcdc->dvs_vsel == vsel)
return wm831x_buckv_set_dvs(rdev, 1);
/* Always set the ON status to the minimum voltage */
@@ -249,7 +249,7 @@ static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
return ret;
dcdc->on_vsel = vsel;
- if (!dcdc->dvs_gpio)
+ if (!dcdc->dvs_gpiod)
return ret;
/* Kick the voltage transition now */
@@ -296,7 +296,7 @@ static int wm831x_buckv_get_voltage_sel(struct regulator_dev *rdev)
{
struct wm831x_dcdc *dcdc = rdev_get_drvdata(rdev);
- if (dcdc->dvs_gpio && dcdc->dvs_gpio_state)
+ if (dcdc->dvs_gpiod && dcdc->dvs_gpio_state)
return dcdc->dvs_vsel;
else
return dcdc->on_vsel;
@@ -337,7 +337,7 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
int ret;
u16 ctrl;
- if (!pdata || !pdata->dvs_gpio)
+ if (!pdata)
return;
/* gpiolib won't let us read the GPIO status so pick the higher
@@ -345,17 +345,14 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
*/
dcdc->dvs_gpio_state = pdata->dvs_init_state;
- ret = devm_gpio_request_one(&pdev->dev, pdata->dvs_gpio,
- dcdc->dvs_gpio_state ? GPIOF_INIT_HIGH : 0,
- "DCDC DVS");
- if (ret < 0) {
- dev_err(wm831x->dev, "Failed to get %s DVS GPIO: %d\n",
- dcdc->name, ret);
+ dcdc->dvs_gpiod = devm_gpiod_get_optional(&pdev->dev, "dvs",
+ dcdc->dvs_gpio_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
+ if (IS_ERR(dcdc->dvs_gpiod)) {
+ dev_err(wm831x->dev, "Failed to get %s DVS GPIO: %ld\n",
+ dcdc->name, PTR_ERR(dcdc->dvs_gpiod));
return;
}
- dcdc->dvs_gpio = pdata->dvs_gpio;
-
switch (pdata->dvs_control_src) {
case 1:
ctrl = 2 << WM831X_DC1_DVS_SRC_SHIFT;
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631b3052..1b8bb36e13b8 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -52,7 +52,6 @@ struct wm831x_battery_pdata {
* I2C or SPI buses.
*/
struct wm831x_buckv_pdata {
- int dvs_gpio; /** CPU GPIO to use for DVS switching */
int dvs_control_src; /** Hardware DVS source to use (1 or 2) */
int dvs_init_state; /** DVS state to expect on startup */
int dvs_state_gpio; /** CPU GPIO to use for monitoring status */
--
2.20.1
On Sun, Jun 09, 2019 at 11:00:25PM +0200, Linus Walleij wrote:
> This converts the Wolfson Micro WM831x DCDC converter to use
> a GPIO descriptor for the GPIO driving the DVS pin.
>
> There is just one (non-DT) machine in the kernel using this, and
> that is the Wolfson Micro (now Cirrus) Cragganmore 6410 so we
> patch this board to pass a descriptor table and fix up the driver
> accordingly.
>
> Cc: Charles Keepax <[email protected]>
> Cc: Richard Fitzgerald <[email protected]>
> Cc: [email protected]
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> +/*
> + * VDDARM is eventually ending up as a regulator hanging on the MFD cell device
> + * "wm831x-buckv.1" spawn from drivers/mfd/wm831x-core.c.
> + *
> + * From the note on the platform data we can see that this is clearly DVS1
> + * and assigned as dcdc1 resource to the MFD core which sets .id of the cell
> + * spawning the DVS1 platform device to 1, resulting in the device name
> + * "wm831x-buckv.1".
> + */
> +static struct gpiod_lookup_table crag_pmic_gpiod_table = {
> + .dev_id = "wm831x-buckv.1",
This is not correct, the mfd_add_devices in wm831x-core passes an
ID to the MFD core. Which is calculated from wm831x_num * 10,
taken from the pdata. So this should be "wm831x-buckv.11".
> + .table = {
> + GPIO_LOOKUP("GPIOK", 0, "dvs", GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
<snip>
> - ret = devm_gpio_request_one(&pdev->dev, pdata->dvs_gpio,
> - dcdc->dvs_gpio_state ? GPIOF_INIT_HIGH : 0,
> - "DCDC DVS");
> - if (ret < 0) {
> - dev_err(wm831x->dev, "Failed to get %s DVS GPIO: %d\n",
> - dcdc->name, ret);
> + dcdc->dvs_gpiod = devm_gpiod_get_optional(&pdev->dev, "dvs",
> + dcdc->dvs_gpio_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> + if (IS_ERR(dcdc->dvs_gpiod)) {
> + dev_err(wm831x->dev, "Failed to get %s DVS GPIO: %ld\n",
> + dcdc->name, PTR_ERR(dcdc->dvs_gpiod));
> return;
> }
>
Whats the thinking behind using get_optional here? A plain
devm_gpiod_get looks like it would be closer to the original
code. Previously if there was no GPIO we would log an error and
not execute the rest of the function, this is I think no longer
the case after this change.
Thanks,
Charles