lkml.org 
[lkml]   [2023]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2023-08-23 14:30    [W:0.118 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site