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@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > --- > > 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 > > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature