| Message ID | 20190927100407.1863293-6-paul.kocialkowski@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | LogiCVC mfd and GPIO support | expand |
Hi Paul, just two nits: pt., 27 wrz 2019 o 12:04 Paul Kocialkowski <paul.kocialkowski@bootlin.com> napisał(a): > > The LogiCVC display hardware block comes with GPIO capabilities > that must be exposed separately from the main driver (as GPIOs) for > use with regulators and panels. A syscon is used to share the same > regmap across the two drivers. > > Since the GPIO capabilities are pretty simple, add them to the syscon > GPIO driver. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 68 insertions(+) > > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c > index 05c537ed73f1..cdcb913b8f0c 100644 > --- a/drivers/gpio/gpio-syscon.c > +++ b/drivers/gpio/gpio-syscon.c > @@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = { > .set = keystone_gpio_set, > }; > > +#define LOGICVC_CTRL_REG 0x40 > +#define LOGICVC_CTRL_GPIO_SHIFT 11 > +#define LOGICVC_CTRL_GPIO_BITS 5 > + > +#define LOGICVC_POWER_CTRL_REG 0x78 > +#define LOGICVC_POWER_CTRL_GPIO_SHIFT 0 > +#define LOGICVC_POWER_CTRL_GPIO_BITS 4 > + > +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv, > + unsigned offset, unsigned int *reg, > + unsigned int *bit) > +{ > + if (offset >= LOGICVC_CTRL_GPIO_BITS) { > + *reg = LOGICVC_POWER_CTRL_REG; > + > + /* To the (virtual) power ctrl offset. */ > + offset -= LOGICVC_CTRL_GPIO_BITS; > + /* To the actual bit offset in reg. */ > + offset += LOGICVC_POWER_CTRL_GPIO_SHIFT; > + } else { > + *reg = LOGICVC_CTRL_REG; > + > + /* To the actual bit offset in reg. */ > + offset += LOGICVC_CTRL_GPIO_SHIFT; > + } > + > + *bit = BIT(offset); > +} > + > +static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset) > +{ > + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); > + unsigned int reg; > + unsigned int bit; > + unsigned int value; Can you put these on a single line? > + int ret; > + > + logicvc_gpio_offset(priv, offset, ®, &bit); > + > + ret = regmap_read(priv->syscon, reg, &value); > + if (ret) > + return ret; > + > + return !!(value & bit); > +} > + > +static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > +{ > + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); > + unsigned int reg; > + unsigned int bit; Same here. Bart > + > + logicvc_gpio_offset(priv, offset, ®, &bit); > + > + regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0); > +} > + > +static const struct syscon_gpio_data logicvc_gpio = { > + .flags = GPIO_SYSCON_FEAT_OUT, > + .bit_count = LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS, > + .get = logicvc_gpio_get, > + .set = logicvc_gpio_set, > +}; > + > static const struct of_device_id syscon_gpio_ids[] = { > { > .compatible = "cirrus,ep7209-mctrl-gpio", > @@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = { > .compatible = "rockchip,rk3328-grf-gpio", > .data = &rockchip_rk3328_gpio_mute, > }, > + { > + .compatible = "xylon,logicvc-3.02.a-gpio", > + .data = &logicvc_gpio, > + }, > { } > }; > MODULE_DEVICE_TABLE(of, syscon_gpio_ids); > -- > 2.23.0 >
Hi and thanks for the review! On Thu 03 Oct 19, 10:26, Bartosz Golaszewski wrote: > Hi Paul, > > just two nits: > > pt., 27 wrz 2019 o 12:04 Paul Kocialkowski > <paul.kocialkowski@bootlin.com> napisał(a): > > > > The LogiCVC display hardware block comes with GPIO capabilities > > that must be exposed separately from the main driver (as GPIOs) for > > use with regulators and panels. A syscon is used to share the same > > regmap across the two drivers. > > > > Since the GPIO capabilities are pretty simple, add them to the syscon > > GPIO driver. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > > --- > > drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 68 insertions(+) > > > > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c > > index 05c537ed73f1..cdcb913b8f0c 100644 > > --- a/drivers/gpio/gpio-syscon.c > > +++ b/drivers/gpio/gpio-syscon.c > > @@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = { > > .set = keystone_gpio_set, > > }; > > > > +#define LOGICVC_CTRL_REG 0x40 > > +#define LOGICVC_CTRL_GPIO_SHIFT 11 > > +#define LOGICVC_CTRL_GPIO_BITS 5 > > + > > +#define LOGICVC_POWER_CTRL_REG 0x78 > > +#define LOGICVC_POWER_CTRL_GPIO_SHIFT 0 > > +#define LOGICVC_POWER_CTRL_GPIO_BITS 4 > > + > > +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv, > > + unsigned offset, unsigned int *reg, > > + unsigned int *bit) > > +{ > > + if (offset >= LOGICVC_CTRL_GPIO_BITS) { > > + *reg = LOGICVC_POWER_CTRL_REG; > > + > > + /* To the (virtual) power ctrl offset. */ > > + offset -= LOGICVC_CTRL_GPIO_BITS; > > + /* To the actual bit offset in reg. */ > > + offset += LOGICVC_POWER_CTRL_GPIO_SHIFT; > > + } else { > > + *reg = LOGICVC_CTRL_REG; > > + > > + /* To the actual bit offset in reg. */ > > + offset += LOGICVC_CTRL_GPIO_SHIFT; > > + } > > + > > + *bit = BIT(offset); > > +} > > + > > +static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset) > > +{ > > + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); > > + unsigned int reg; > > + unsigned int bit; > > + unsigned int value; > > Can you put these on a single line? Sure thing. > > + int ret; > > + > > + logicvc_gpio_offset(priv, offset, ®, &bit); > > + > > + ret = regmap_read(priv->syscon, reg, &value); > > + if (ret) > > + return ret; > > + > > + return !!(value & bit); > > +} > > + > > +static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > > +{ > > + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); > > + unsigned int reg; > > + unsigned int bit; > > Same here. Will do! Cheers, Paul > > + > > + logicvc_gpio_offset(priv, offset, ®, &bit); > > + > > + regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0); > > +} > > + > > +static const struct syscon_gpio_data logicvc_gpio = { > > + .flags = GPIO_SYSCON_FEAT_OUT, > > + .bit_count = LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS, > > + .get = logicvc_gpio_get, > > + .set = logicvc_gpio_set, > > +}; > > + > > static const struct of_device_id syscon_gpio_ids[] = { > > { > > .compatible = "cirrus,ep7209-mctrl-gpio", > > @@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = { > > .compatible = "rockchip,rk3328-grf-gpio", > > .data = &rockchip_rk3328_gpio_mute, > > }, > > + { > > + .compatible = "xylon,logicvc-3.02.a-gpio", > > + .data = &logicvc_gpio, > > + }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, syscon_gpio_ids); > > -- > > 2.23.0 > >
diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c index 05c537ed73f1..cdcb913b8f0c 100644 --- a/drivers/gpio/gpio-syscon.c +++ b/drivers/gpio/gpio-syscon.c @@ -190,6 +190,70 @@ static const struct syscon_gpio_data keystone_dsp_gpio = { .set = keystone_gpio_set, }; +#define LOGICVC_CTRL_REG 0x40 +#define LOGICVC_CTRL_GPIO_SHIFT 11 +#define LOGICVC_CTRL_GPIO_BITS 5 + +#define LOGICVC_POWER_CTRL_REG 0x78 +#define LOGICVC_POWER_CTRL_GPIO_SHIFT 0 +#define LOGICVC_POWER_CTRL_GPIO_BITS 4 + +static void logicvc_gpio_offset(struct syscon_gpio_priv *priv, + unsigned offset, unsigned int *reg, + unsigned int *bit) +{ + if (offset >= LOGICVC_CTRL_GPIO_BITS) { + *reg = LOGICVC_POWER_CTRL_REG; + + /* To the (virtual) power ctrl offset. */ + offset -= LOGICVC_CTRL_GPIO_BITS; + /* To the actual bit offset in reg. */ + offset += LOGICVC_POWER_CTRL_GPIO_SHIFT; + } else { + *reg = LOGICVC_CTRL_REG; + + /* To the actual bit offset in reg. */ + offset += LOGICVC_CTRL_GPIO_SHIFT; + } + + *bit = BIT(offset); +} + +static int logicvc_gpio_get(struct gpio_chip *chip, unsigned offset) +{ + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); + unsigned int reg; + unsigned int bit; + unsigned int value; + int ret; + + logicvc_gpio_offset(priv, offset, ®, &bit); + + ret = regmap_read(priv->syscon, reg, &value); + if (ret) + return ret; + + return !!(value & bit); +} + +static void logicvc_gpio_set(struct gpio_chip *chip, unsigned offset, int val) +{ + struct syscon_gpio_priv *priv = gpiochip_get_data(chip); + unsigned int reg; + unsigned int bit; + + logicvc_gpio_offset(priv, offset, ®, &bit); + + regmap_update_bits(priv->syscon, reg, bit, val ? bit : 0); +} + +static const struct syscon_gpio_data logicvc_gpio = { + .flags = GPIO_SYSCON_FEAT_OUT, + .bit_count = LOGICVC_CTRL_GPIO_BITS + LOGICVC_POWER_CTRL_GPIO_BITS, + .get = logicvc_gpio_get, + .set = logicvc_gpio_set, +}; + static const struct of_device_id syscon_gpio_ids[] = { { .compatible = "cirrus,ep7209-mctrl-gpio", @@ -203,6 +267,10 @@ static const struct of_device_id syscon_gpio_ids[] = { .compatible = "rockchip,rk3328-grf-gpio", .data = &rockchip_rk3328_gpio_mute, }, + { + .compatible = "xylon,logicvc-3.02.a-gpio", + .data = &logicvc_gpio, + }, { } }; MODULE_DEVICE_TABLE(of, syscon_gpio_ids);
The LogiCVC display hardware block comes with GPIO capabilities that must be exposed separately from the main driver (as GPIOs) for use with regulators and panels. A syscon is used to share the same regmap across the two drivers. Since the GPIO capabilities are pretty simple, add them to the syscon GPIO driver. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> --- drivers/gpio/gpio-syscon.c | 68 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)