Hi All, On 06/12/2019 14:08, Kieran Bingham wrote: > Hello me, > > On 06/12/2019 14:05, Kieran Bingham wrote: >> Provide a GPIO chip to control the two output lines available on the >> MAX9286. A separate mail thread to Mark Brown regarding the regulator topic [0] has suggested that this should be implemented as an MFD. Does anyone have any comments on this approach? I know we've discussed the use of the MFD framework on this driver before ... thus perhaps it might be a bit contentious ... Due to this, I am not yet going to fold this patch into the max9286 driver. [0] Regulator probe on demand (or circular dependencies) https://lore.kernel.org/linux-renesas-soc/20191209171639.GA27340@xxxxxxxxxxxxxxxxxxxx/T/#t -- Regards Kieran >> Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> >> --- >> drivers/media/i2c/max9286.c | 68 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 68 insertions(+) >> >> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c >> index 6ea08fd87811..c34e7b5c7447 100644 >> --- a/drivers/media/i2c/max9286.c >> +++ b/drivers/media/i2c/max9286.c >> @@ -13,6 +13,7 @@ >> #include <linux/delay.h> >> #include <linux/device.h> >> #include <linux/fwnode.h> >> +#include <linux/gpio/driver.h> >> #include <linux/i2c.h> >> #include <linux/i2c-mux.h> >> #include <linux/module.h> >> @@ -58,6 +59,8 @@ >> #define MAX9286_HVSRC_D0 (2 << 0) >> #define MAX9286_HVSRC_D14 (1 << 0) >> #define MAX9286_HVSRC_D18 (0 << 0) >> +/* Register 0x0f */ >> +#define MAX9286_0X0F_RESERVED BIT(3) >> /* Register 0x12 */ >> #define MAX9286_CSILANECNT(n) (((n) - 1) << 6) >> #define MAX9286_CSIDBL BIT(5) >> @@ -145,6 +148,9 @@ struct max9286_priv { >> struct regulator *regulator; >> bool poc_enabled; >> >> + struct gpio_chip gpio; >> + u8 gpio_state; >> + >> struct i2c_mux_core *mux; >> unsigned int mux_channel; >> bool mux_open; >> @@ -712,6 +718,60 @@ static const struct of_device_id max9286_dt_ids[] = { >> }; >> MODULE_DEVICE_TABLE(of, max9286_dt_ids); >> >> +static void max9286_gpio_set(struct gpio_chip *chip, >> + unsigned int offset, int value) >> +{ >> + struct max9286_priv *priv = gpiochip_get_data(chip); >> + >> + if (value) >> + priv->gpio_state |= BIT(offset); >> + else >> + priv->gpio_state &= ~BIT(offset); >> + >> + max9286_write(priv, 0x0f, MAX9286_0X0F_RESERVED | priv->gpio_state); >> +} >> + >> +static int max9286_gpio_get(struct gpio_chip *chip, unsigned int offset) >> +{ >> + struct max9286_priv *priv = gpiochip_get_data(chip); >> + >> + return priv->gpio_state & BIT(offset); >> +} >> + >> +static int max9286_gpio(struct max9286_priv *priv) >> +{ >> + struct device *dev = &priv->client->dev; >> + struct gpio_chip *gpio = &priv->gpio; >> + int ret; >> + >> + static const char * const names[] = { >> + "GPIO0OUT", >> + "GPIO1OUT", >> + }; >> + >> + /* Configure the GPIO */ >> + gpio->label = dev_name(dev); >> + gpio->parent = dev; >> + gpio->owner = THIS_MODULE; >> + gpio->of_node = dev->of_node; >> + gpio->ngpio = 2; >> + gpio->set = max9286_gpio_set; >> + gpio->get = max9286_gpio_get; >> + gpio->can_sleep = true; >> + gpio->names = names; >> + >> + /* GPIO values default to high */ >> + priv->gpio_state = BIT(0) | BIT(1); >> + >> + ret = devm_gpiochip_add_data(dev, gpio, priv); >> + if (ret) >> + dev_err(dev, "Unable to create gpio_chip\n"); >> + >> + dev_err(dev, "Created gpio_chip for MAX9286\n"); > > This debug line should be removed of course. > > Now removed. > >> + >> + return ret; >> +} >> + >> static int max9286_init(struct device *dev) >> { >> struct max9286_priv *priv; >> @@ -984,6 +1044,14 @@ static int max9286_probe(struct i2c_client *client) >> if (ret) >> return ret; >> >> + /* >> + * It is possible to set up the power regulator from the GPIO lines, >> + * so it needs to be set up early. >> + */ >> + ret = max9286_gpio(priv); >> + if (ret) >> + return ret; >> + >> priv->regulator = regulator_get(&client->dev, "poc"); >> if (IS_ERR(priv->regulator)) { >> if (PTR_ERR(priv->regulator) != -EPROBE_DEFER) >> > -- Regards -- Kieran
