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

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

[Yu-Chun Lin]

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.

Link: https://lore.kernel.org/lkml/anUcSPcJjJkLh0Z-@ashevche-desk.local/
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
---
 drivers/gpio/gpio-rtd1625.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-rtd1625.c b/drivers/gpio/gpio-rtd1625.c
index 483e44cf5abc..c9ff33245ea2 100644
--- a/drivers/gpio/gpio-rtd1625.c
+++ b/drivers/gpio/gpio-rtd1625.c
@@ -79,6 +79,7 @@ struct rtd1625_gpio_info {
 };
 
 struct rtd1625_gpio {
+	struct device *dev;
 	struct gpio_regmap *gpio_reg;
 	const struct rtd1625_gpio_info *info;
 	struct regmap *regmap;
@@ -109,7 +110,7 @@ static int rtd1625_reg_mask_xlate(struct gpio_regmap *gpio, enum gpio_regmap_ope
 {
 	/* Each GPIO has its own dedicated 32-bit register */
 	struct rtd1625_gpio *data = gpio_regmap_get_drvdata(gpio);
-	int val = 0, ret = 0;
+	int val = 0, ret;
 	*reg = base + offset * 4;
 
 	switch (op) {
@@ -223,8 +224,8 @@ static void rtd1625_gpio_irq_handle(struct irq_desc *desc)
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	unsigned int irq = irq_desc_get_irq(desc);
 	struct irq_domain *domain = data->domain;
-	unsigned int reg_offset, i, j, val;
-	irq_hw_number_t hwirq;
+	unsigned int reg_offset, j, val;
+	struct device *dev = data->dev;
 	unsigned long status;
 	u32 irq_type;
 	int ret;
@@ -240,11 +241,12 @@ static void rtd1625_gpio_irq_handle(struct irq_desc *desc)
 
 	chained_irq_enter(chip, desc);
 
-	for (i = 0; i < data->info->num_gpios; i += 32) {
+	for (unsigned int i = 0; i < data->info->num_gpios; i += 32) {
 		reg_offset = get_reg_offset(data, i);
 		ret = regmap_read(data->regmap, reg_offset, &val);
 		if (ret) {
-			pr_err_ratelimited("Failed to read IRQ status for GPIO %u: %d\n", i, ret);
+			dev_err_ratelimited(dev, "Failed to read IRQ status for GPIO %u: %d\n",
+					    i, ret);
 			continue;
 		}
 
@@ -261,12 +263,13 @@ static void rtd1625_gpio_irq_handle(struct irq_desc *desc)
 		if (irq != data->irqs[RTD1625_IRQ_LEVEL]) {
 			ret = regmap_write(data->regmap, reg_offset, status);
 			if (ret)
-				pr_err_ratelimited("Failed to clear edge IRQ for GPIO %u: %d\n",
-						   i, ret);
+				dev_err_ratelimited(dev,
+						    "Failed to clear edge IRQ for GPIO %u: %d\n",
+						    i, ret);
 		}
 
 		for_each_set_bit(j, &status, 32) {
-			hwirq = i + j;
+			irq_hw_number_t hwirq = i + j;
 			irq_type = irq_get_trigger_type(irq_find_mapping(domain, hwirq));
 
 			/*
@@ -486,7 +489,6 @@ static int rtd1625_gpio_setup_irq(struct platform_device *pdev, struct rtd1625_g
 		return irq;
 
 	num_irqs = (data->info->irq_type_support & IRQ_TYPE_LEVEL_MASK) ? 3 : 2;
-
 	for (unsigned int i = 0; i < num_irqs; i++) {
 		irq = platform_get_irq(pdev, i);
 		if (irq < 0)
@@ -544,6 +546,8 @@ static int rtd1625_gpio_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	data->dev = dev;
+
 	data->info = device_get_match_data(dev);
 	if (!data->info)
 		return -ENODATA;
@@ -612,8 +616,7 @@ static const struct rtd1625_gpio_info rtd1625_iso_gpio_info = {
 
 static const struct rtd1625_gpio_info rtd1625_isom_gpio_info = {
 	.num_gpios        = 4,
-	.irq_type_support = IRQ_TYPE_EDGE_BOTH | IRQ_TYPE_LEVEL_LOW |
-			    IRQ_TYPE_LEVEL_HIGH,
+	.irq_type_support = IRQ_TYPE_DEFAULT,
 	.base_offset      = 0x20,
 	.gpa_offset       = 0x00,
 	.gpda_offset      = 0x04,
-- 
2.43.0

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

[Andy Shevchenko]

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