On Tue, Oct 30, 2018 at 02:04:20PM +0100, Linus Walleij wrote:
> On Fri, Oct 19, 2018 at 11:50 AM Charles Keepax
> <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> > Lochnagar is an evaluation and development board for Cirrus
> > Logic Smart CODEC and Amp devices. It allows the connection of
> > most Cirrus Logic devices on mini-cards, as well as allowing
> > connection of various application processor systems to provide a
> > full evaluation platform. This driver supports the board
> > controller chip on the Lochnagar board.
> >
> > Lochnagar provides many pins which can generally be used for an
> > audio function such as an AIF or a PDM interface, but also as
> > GPIOs.
> >
> > Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > +static void lochnagar_gpio_set(struct gpio_chip *chip,
> > + unsigned int offset, int value)
> > +{
> > + struct lochnagar_pin_priv *priv = gpiochip_get_data(chip);
> > + struct lochnagar *lochnagar = priv->lochnagar;
> > + const struct lochnagar_pin *pin = priv->pins[offset].drv_data;
> > + int ret;
> > +
> > + value = !!value;
>
> value = value ? BIT(pin->shift) : 0;
>
I think this will end up more complex because...
> > + dev_dbg(priv->dev, "Set GPIO %s to %s\n",
> > + pin->name, value ? "high" : "low");
> > +
> > + switch (pin->type) {
> > + case LN_PTYPE_MUX:
> > + value |= LN2_OP_GPIO;
I want the value to be in the lowest bit for this path.
> > +
> > + ret = lochnagar_pin_set_mux(priv, pin, value);
> > + break;
> > + case LN_PTYPE_GPIO:
> > + if (pin->invert)
> > + value = !value;
And then I need to invert the value here.
> > +
> > + ret = regmap_update_bits(lochnagar->regmap, pin->reg,
> > + 0x1 << pin->shift,
>
> BIT(pin->shift)
>
> > + value << pin->shift);
>
> Just value provided you used the construction above
> to construct it.
>
All the other comments look good will fixup for the next spin.
Thanks,
Charles
