> > +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> > + int val)
> > +{
> > + struct wx *wx = gpiochip_get_data(chip);
> > + u32 mask;
> > +
> > + mask = BIT(offset) | BIT(offset - 1);
> > + if (val)
> > + wr32m(wx, WX_GPIO_DR, mask, mask);
> > + else
> > + wr32m(wx, WX_GPIO_DR, mask, 0);
> > +
> > + wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
>
> Can you explain, what prevents to have this flow to be interleaved by other API
> calls, like ->direction_in()? Didn't you missed proper locking schema?
It's true, I should add spinlock for writing GPIO registers.
>
> > + return 0;
> > +}
>
> ...
>
> > + switch (type) {
> > + case IRQ_TYPE_EDGE_BOTH:
> > + level |= BIT(hwirq);
> > + break;
> > + case IRQ_TYPE_EDGE_RISING:
> > + level |= BIT(hwirq);
> > + polarity |= BIT(hwirq);
> > + break;
> > + case IRQ_TYPE_EDGE_FALLING:
> > + level |= BIT(hwirq);
>
> > + polarity &= ~BIT(hwirq);
>
> This...
>
> > + break;
> > + case IRQ_TYPE_LEVEL_HIGH:
> > + level &= ~BIT(hwirq);
>
> ...and this can be done outside of the switch-case. Then you simply set certain
> bits where it's needed.
>
> > + polarity |= BIT(hwirq);
> > + break;
> > + case IRQ_TYPE_LEVEL_LOW:
> > + level &= ~BIT(hwirq);
> > + polarity &= ~BIT(hwirq);
> > + break;
>
> default?
Do you mean that treat IRQ_TYPE_LEVEL_LOW as default case, clear level and
polarity firstly, then set the bits in other needed case?
