Re: [PATCH v4 2/2] gpio: loongson: add more gpio chip support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2023/8/23 下午8:29, Linus Walleij 写道:
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


okay, I will add it.


         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.



Andy's suggestion seems to look better, We have bgpio_init() handle
this switch case. I will remove this switch case.
https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/commit/?h=gpio/for-next&id=55b2395e4e92adc492c6b30ac109eb78250dcd9d

Thanks,
Yinbo




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux