Re: [PATCH v2 14/14] gpio: dwapb: Amend indentation in some cases

From: Serge Semin <fancer.lancer@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH v2 14/14] gpio: dwapb: Amend indentation in some cases
Date: Wed, 15 Apr 2020 20:15:16 +0300	[thread overview]
Message-ID: <20200415171516.cugbzhvjua6cygnq@mobilestation> (raw)
In-Reply-To: <20200415141534.31240-15-andriy.shevchenko@linux.intel.com>

On Wed, Apr 15, 2020 at 05:15:34PM +0300, Andy Shevchenko wrote:
> In some cases indentation makes code harder to read. Amend indentation
> in those cases despite of lines go a bit over 80 character limit.
> 
> Cc: Serge Semin <fancer.lancer@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  drivers/gpio/gpio-dwapb.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 84c971e0adf0..a09332d9c7fe 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -437,7 +437,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>  		}
>  	}
>  
> -	for (hwirq = 0 ; hwirq < ngpio ; hwirq++)
> +	for (hwirq = 0; hwirq < ngpio; hwirq++)
>  		irq_create_mapping(gpio->domain, hwirq);
>  
>  	port->gc.to_irq = dwapb_gpio_to_irq;
> @@ -453,7 +453,7 @@ static void dwapb_irq_teardown(struct dwapb_gpio *gpio)
>  	if (!gpio->domain)
>  		return;
>  
> -	for (hwirq = 0 ; hwirq < ngpio ; hwirq++)
> +	for (hwirq = 0; hwirq < ngpio; hwirq++)
>  		irq_dispose_mapping(irq_find_mapping(gpio->domain, hwirq));
>  
>  	irq_domain_remove(gpio->domain);
> @@ -478,10 +478,9 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>  		return -ENOMEM;
>  #endif
>  
> -	dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_STRIDE);
> -	set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_STRIDE);
> -	dirout = gpio->regs + GPIO_SWPORTA_DDR +
> -		(pp->idx * GPIO_SWPORT_DDR_STRIDE);
> +	dat = gpio->regs + GPIO_EXT_PORTA + pp->idx * GPIO_EXT_PORT_STRIDE;
> +	set = gpio->regs + GPIO_SWPORTA_DR + pp->idx * GPIO_SWPORT_DR_STRIDE;
> +	dirout = gpio->regs + GPIO_SWPORTA_DDR + pp->idx * GPIO_SWPORT_DDR_STRIDE;
>  
>  	/* This registers 32 GPIO lines per port */
>  	err = bgpio_init(&port->gc, gpio->dev, 4, dat, set, NULL, dirout,
> @@ -582,17 +581,13 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
>  
>  		if (fwnode_property_read_u32(fwnode, "reg", &pp->idx) ||
>  		    pp->idx >= DWAPB_MAX_PORTS) {
> -			dev_err(dev,
> -				"missing/invalid port index for port%d\n", i);

> +			dev_err(dev, "missing/invalid port index for port%d\n", i);

What about shortening the message text to fit the 80 chars per line rule?
I suppose the "missing" word could be omitted.

>  			fwnode_handle_put(fwnode);
>  			return ERR_PTR(-EINVAL);
>  		}
>  
> -		if (fwnode_property_read_u32(fwnode, "snps,nr-gpios",
> -					 &pp->ngpio)) {
> -			dev_info(dev,
> -				 "failed to get number of gpios for port%d\n",
> -				 i);
> +		if (fwnode_property_read_u32(fwnode, "snps,nr-gpios", &pp->ngpio)) {

> +			dev_info(dev, "failed to get number of gpios for port%d\n", i);

The same here. For instance "no snps,nr-gpios found for port%d" would work here.

>  			pp->ngpio = 32;
>  		}
>  
> @@ -743,8 +738,7 @@ static int dwapb_gpio_suspend(struct device *dev)
>  			ctx->int_deb	= dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
>  
>  			/* Mask out interrupts */
> -			dwapb_write(gpio, GPIO_INTMASK,
> -				    0xffffffff & ~ctx->wake_en);

> +			dwapb_write(gpio, GPIO_INTMASK, 0xffffffff & ~ctx->wake_en);

Hm, do I need some rest and missing something or the &-operation with 1s here
does nothing seeing the operands data types have the same width?

(the change introduced by commit 6437c7ba69c3 ("gpio: dwapb: Add wakeup source support"))

-Sergey

>  		}
>  	}
>  	spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> -- 
> 2.25.1
> 