Re: [PATCH] gpio: rtd1625: minor cleanups and log improvements

From: Andy Shevchenko

Date: Wed Aug 12 2026 - 03:22:24 EST


On Wed, Aug 12, 2026 at 11:19:29AM +0800, Yu-Chun Lin wrote:
> Add minor cleanups based on Andy's feedback:
> - Store 'dev' in driver data to use dev_err_ratelimited().
> - Drop redundant 'ret' initialization and the line break.
> - Narrow the scope of local variables 'i' and 'hwirq'.
> - Use IRQ_TYPE_DEFAULT.

...

> struct rtd1625_gpio {
> + struct device *dev;

Can't this be derived from below regmap?

> struct gpio_regmap *gpio_reg;

Either this...

> const struct rtd1625_gpio_info *info;
> struct regmap *regmap;

...or this?

...

> {
> /* Each GPIO has its own dedicated 32-bit register */

It's obvious that the comment is placed wrongly...

> struct rtd1625_gpio *data = gpio_regmap_get_drvdata(gpio);
> - int val = 0, ret = 0;
> + int val = 0, ret;

..and while at it you can make it reversed xmas tree order.

> *reg = base + offset * 4;

Putting all together, this should be

struct rtd1625_gpio *data = gpio_regmap_get_drvdata(gpio);
/* Each GPIO has its own dedicated 32-bit register */
*reg = base + offset * 4;
int val = 0, ret;

...

> for_each_set_bit(j, &status, 32) {
> - hwirq = i + j;
> + irq_hw_number_t hwirq = i + j;

Now it needs a blank line here.

> irq_type = irq_get_trigger_type(irq_find_mapping(domain, hwirq));

...

Please, split this patch to a few based on the nature of changes (something
like 4 patches in a series).

--
With Best Regards,
Andy Shevchenko