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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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