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@xxxxxxxxxxx> wrote:
> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
> driver support.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx>
(...)
> 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
