Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 7 Oct 2018 20:23:58 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Renato Lui Geh <renatogeh@gmail.com>
Cc:     lars@metafoo.de, michael.hennerich@analog.com, knaack.h@gmx.de,
        pmeerw@pmeerw.net, gregkh@linuxfoundation.org,
        sonic.zhang@analog.com, linux-iio@vger.kernel.org,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: iio: adt7316: fix parenthesis alignment
Message-ID: <20181007202358.53ed37e8@archlinux>
In-Reply-To: <20181004230651.wcqz4fp2cycyrq6t@renatolg>
References: <20181004230651.wcqz4fp2cycyrq6t@renatolg>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 4 Oct 2018 20:06:55 -0300
Renato Lui Geh <renatogeh@gmail.com> wrote:

> This patch fixes all "Alignment should match open parenthesis" check
> warnings found by checkpatch.pl in the addac/adt7316 driver.
> 
> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com>
Hi,

Mostly good, but alignment doesn't override other considerations such
as consistency in argument naming.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/addac/adt7316.c | 462 ++++++++++++++--------------
>  drivers/staging/iio/addac/adt7316.h |   2 +-
>  2 files changed, 233 insertions(+), 231 deletions(-)
> 
...
> @@ -1363,9 +1365,9 @@ static ssize_t adt7316_show_in_analog_temp_offset(struct device *dev,
>  }
>  
>  static ssize_t adt7316_store_in_analog_temp_offset(struct device *dev,
> -		struct device_attribute *attr,
> -		const char *buf,
> -		size_t len)
> +						   struct device_attribute *att,
I wasn't expecting to see a rename in here.  I know it's not a big thing 
as the parameter isn't even used, but it is better to have consistency
of naming than a minor improvement in alignment.

> +						   const char *buf,
> +						   size_t len)
>  {
>  
>  static ssize_t adt7316_store_ex_analog_temp_offset(struct device *dev,
> -		struct device_attribute *attr,
> -		const char *buf,
> -		size_t len)
> +						   struct device_attribute *att,
Same here...

> +						   const char *buf,
> +						   size_t len)
>  {
>  	struct iio_dev *dev_info = dev_to_iio_dev(dev);
>  	struct adt7316_chip_info *chip = iio_priv(dev_info);

...