Hi Linus, On 30 July 2018 at 00:59, Linus Walleij <linus.wall...@linaro.org> wrote:
> On Thu, Jul 26, 2018 at 2:01 AM Tomer Maimon <tmaimo...@gmail.com> wrote: > > > I initialize bgpio as follow: > > > > ret = bgpio_init(&pctrl->gpio_bank[id].gc, > > pctrl->dev, 4, > > pctrl->gpio_bank[id].base + > > NPCM7XX_GP_N_DIN, > > pctrl->gpio_bank[id].base + > > NPCM7XX_GP_N_DOUT, > > NULL, > > NULL, > > pctrl->gpio_bank[id].base + > > NPCM7XX_GP_N_IEM, > > BGPIOF_READ_OUTPUT_REG_SET); > > (...) > > The problem occur when reading the GPIO value from bgpio_get_set > function, > > because the directions value are inverse it reading the wrong I/O > registers > > > > For direction out it reading dat register (instead of set register) > > > > For direction in it calling set register (instead of dat register) > > Hm I don't quite get it... sorry. Maybe if you show your fix and what > you expect to happen I can understand better? > Of course, in the last patch sent three days a go (V3 - https://patchwork.ozlabs.org/patch/949942/) I did as follow to workaround the issue: int (*get)(struct gpio_chip *chip, unsigned offset); int (*get_multiple)(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits); ...... static int npcmgpio_get_value(struct gpio_chip *chip, unsigned int offset) { struct npcm7xx_gpio *bank = gpiochip_get_data(chip); unsigned long tmp_bgpio_dir = bank->gc.bgpio_dir; int val; /* * sets bgpio_dir parameter value to the opposite value * for calling the right registers in bgpio_get_set * function */ * bank->gc.bgpio_dir = ~bank->gc.bgpio_dir; val = bank->get(chip, offset); bank->gc.bgpio_dir = tmp_bgpio_dir;* return val; } static int npcmgpio_get_multiple_value(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct npcm7xx_gpio *bank = gpiochip_get_data(chip); unsigned long tmp_bgpio_dir = bank->gc.bgpio_dir; int val; /* * sets bgpio_dir parameter value to the opposite value * for calling the right registers in bgpio_get_set_multiple * function */ * bank->gc.bgpio_dir = ~bank->gc.bgpio_dir; val = bank->get_multiple(chip, mask, bits); bank->gc.bgpio_dir = tmp_bgpio_dir;* return val; } ...... pctrl->gpio_bank[id].get = pctrl->gpio_bank[id].gc.get; pctrl->gpio_bank[id].gc.get = npcmgpio_get_value; pctrl->gpio_bank[id].get_multiple = ptrl->gpio_bank[id].gc.get_mul tiple; pctrl->gpio_bank[id].gc.get_multiple = npcmgpio_get_multiple_value; but it is not that good solution, because the bold commands are not atomic (locked) operations. > > Do you mean that because you write the inverse value to > IEM this happens, and the BGPIO code assumes that > you always write 1 to set a line as input and 0 to set it > as output? > yes, because of it the bgpio_get_set and bgpio_get_set_multiple setting the opposite data registers . > > I would say if this causes the problem we should just add > a new BGPIOF_INVERTED_REG_DIR with comment in > include/linux/gpio/driver.h and make the necessary fix to > respect this flag in the gpio-mmio.c core so it works right. > > If you do this as a separate patch I would be grateful :) > Sure, I will send a separate patch later on to overcome it. > > Yours, > Linus Walleij > Thanks, Tomer