Messages in this thread | | | From | Linus Walleij <> | Date | Wed, 23 Aug 2023 14:29:25 +0200 | Subject | Re: [PATCH v4 2/2] gpio: loongson: add more gpio chip support |
| |
Hi Yinbo,
thanks for the new patch, it's starting to look really good! The main point with offsets in the match data is very nice.
On Wed, Aug 23, 2023 at 5:34 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip > driver support. > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> (...)
> static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset) > { > + unsigned int u; > struct platform_device *pdev = to_platform_device(chip->parent); > + struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip); > + > + if (lgpio->chip_data->mode == BIT_CTRL_MODE) { > + u = readl(lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4); > + u |= BIT(offset % 32); > + writel(u, lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
This offset / 32 * 4 is really hard to read. What about
/* Get the register index from offset then multiply by bytes per register */ (offset / 32) * 4
> lgpio->reg_base = reg_base; > + if (device_property_read_u32(dev, "ngpios", &ngpios) || !ngpios) > + return -EINVAL; > + > + ret = DIV_ROUND_UP(ngpios, 8); > + switch (ret) { > + case 1 ... 2: > + io_width = ret; > + break; > + case 3 ... 4: > + io_width = 0x4; > + break; > + case 5 ... 8: > + io_width = 0x8; > + break; > + default: > + dev_err(dev, "unsupported io width\n"); > + return -EINVAL; > + }
Is it really a good idea to infer the register width from ngpios?
What about just putting this into the struct loongson_gpio_chip_data as well? Certainly it will be fixed for a certain device.
Yours, Linus Walleij
| |