| Message ID | 20191206140520.10457-2-kieran.bingham@ideasonboard.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | [1/3] media: i2c: max9286: Remove redundant max9286_i2c_mux_state | expand |
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. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > 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) >
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@bigcity.dyn.berto.se/T/#t -- Regards Kieran >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> 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) >> >
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"); + + 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)
Provide a GPIO chip to control the two output lines available on the MAX9286. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- drivers/media/i2c/max9286.c | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)