lkml.org 
[lkml]   [2014]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v7 08/24] mfd: max77686: Add Dynamic Voltage Scaling (DVS) support
From
On Fri, Jul 4, 2014 at 10:24 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> Some regulators on the MAX77686 PMIC have Dynamic Voltage Scaling
> (DVS) support that allows output voltage to change dynamically.
>
> For MAX77686, these regulators are Buck regulators 2, 3 and 4.
>
> Each Buck output voltage is selected using a set of external
> inputs: DVS1-3 and SELB2-4.
>
> DVS registers can be used to configure the output voltages for each
> Buck regulator and which one is active is controled by DVSx lines.
>
> SELBx lines are used to control if individual Buck lines are ON or OFF.
>
> This patch adds support to configure the DVSx and SELBx lines
> from DT and to setup and read the GPIO lines connected to them.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

(...)
> +#include <linux/gpio/consumer.h>

THANKS for using modern interfaces!

> +static void max77686_dt_parse_dvs_gpio(struct device *dev)
> +{
> + struct max77686_platform_data *pd = dev_get_platdata(dev);
> + int i;
> +
> + /*
> + * NOTE: we don't consider GPIO errors fatal; board may have some lines
> + * directly pulled high or low and thus doesn't specify them.
> + */
> + for (i = 0; i < ARRAY_SIZE(pd->buck_gpio_dvs); i++)
> + pd->buck_gpio_dvs[i] =
> + devm_gpiod_get_index(dev, "max77686,pmic-buck-dvs", i);
> +
> + for (i = 0; i < ARRAY_SIZE(pd->buck_gpio_selb); i++)
> + pd->buck_gpio_selb[i] =
> + devm_gpiod_get_index(dev, "max77686,pmic-buck-selb", i);
> +}

Rob Jones has a patch cooking that adds gpio_get_array() so this thing
merits also adding devm_gpiod_get_array() I think?

> +/**
> + * max77686_setup_gpios() - init DVS-related GPIOs
> + * @dev: device whose platform data contains the dvs GPIOs information
> + *
> + * This function claims / initalizations GPIOs related to DVS if they are
> + * defined. This may have the effect of switching voltages if the
> + * pdata->buck_default_idx does not match the boot time state of pins.
> + */
> +int max77686_setup_gpios(struct device *dev)
> +{
> + struct max77686_platform_data *pd = dev_get_platdata(dev);
> + int buck_default_idx = pd->buck_default_idx;
> + int ret;
> + int i;
> +
> + /* Set all SELB high to avoid glitching while DVS is changing */
> + for (i = 0; i < ARRAY_SIZE(pd->buck_gpio_selb); i++) {
> + struct gpio_desc *gpio = pd->buck_gpio_selb[i];
> +
> + /* OK if some GPIOs aren't defined */
> + if (IS_ERR(gpio))
> + continue;
> +
> + ret = gpiod_direction_output_raw(gpio, 1);

Why does this have to be raw? Usually that is not to be used.

Apart from this it looks OK.

Yours,
Linus Walleij


\
 
 \ /
  Last update: 2014-07-10 12:21    [W:0.296 / U:7.056 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site