Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751580AbdCDGAc (ORCPT ); Sat, 4 Mar 2017 01:00:32 -0500 Received: from mail-pg0-f65.google.com ([74.125.83.65]:34818 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750731AbdCDGAa (ORCPT ); Sat, 4 Mar 2017 01:00:30 -0500 Date: Sat, 4 Mar 2017 14:00:12 +0800 From: Eva Rachel Retuya To: Andy Shevchenko Cc: Jonathan Cameron , linux-iio@vger.kernel.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Dmitry Torokhov , Michael Hennerich , Daniel Baluta , Alison Schofield , Florian Vaussard , "linux-kernel@vger.kernel.org" , Rob Herring , Mark Rutland , devicetree Subject: Re: [PATCH v5 4/4] iio: accel: adxl345: Add SPI support Message-ID: <20170304060010.GC6948@Socrates-UM> Mail-Followup-To: Andy Shevchenko , Jonathan Cameron , linux-iio@vger.kernel.org, Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Dmitry Torokhov , Michael Hennerich , Daniel Baluta , Alison Schofield , Florian Vaussard , "linux-kernel@vger.kernel.org" , Rob Herring , Mark Rutland , devicetree References: <698e2cf848231eacde91dd72aa6f73059c9a7752.1488248485.git.eraretuya@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1316 Lines: 51 On Fri, Mar 03, 2017 at 06:55:26PM +0200, Andy Shevchenko wrote: > On Tue, Feb 28, 2017 at 4:37 AM, Eva Rachel Retuya wrote: > > Add SPI driver that initializes SPI regmap for the adxl345 core driver. > > The driver supports the same functionality as I2C namely the x, y, z and > > scale readings. > > Portion of minor comments. > > > +config ADXL345_SPI > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver" > > > + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > Same question. Would it be just > > depends on INPUT_ADXL34X=n > > ? > > > + /* Setting bits 7 and 6 enables multiple-byte read */ > > + .read_flag_mask = BIT(7) | BIT(6), > > GENMASK(7, 6) ? > True, but I would like to keep this as is. It is more readable and obvious than GENMASK(). > > +static int adxl345_spi_probe(struct spi_device *spi) > > +{ > > > + struct regmap *regmap; > > + const struct spi_device_id *id = spi_get_device_id(spi); > > Reverse order. > > And usually we do assignments from function parameters first. Ack. Thanks, Eva > > > + dev_err(&spi->dev, "Error initializing spi regmap: %d\n", > > + (int)PTR_ERR(regmap)); > > Ugly casting! > > -- > With Best Regards, > Andy Shevchenko