Re: [PATCH] gpio: rtd1625: minor cleanups and log improvements
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
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