From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751088AbdEBMQC (ORCPT ); Tue, 2 May 2017 08:16:02 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:33354 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750708AbdEBMQA (ORCPT ); Tue, 2 May 2017 08:16:00 -0400 Date: Tue, 2 May 2017 20:15:56 +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" Subject: Re: [PATCH v2 3/4] iio: accel: adxl345: Setup DATA_READY trigger Message-ID: <20170502121554.GD3030@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" References: 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 On Mon, May 01, 2017 at 02:31:00PM +0300, Andy Shevchenko wrote: [...] > > -int adxl345_core_probe(struct device *dev, struct regmap *regmap, > > +int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq, > > const char *name); > > I think I commented this once. Instead of increasing parameters, > please introduce a new struct (as separate preparatory patch) which > will hold current parameters. Let's call it > strut adxl345_chip { > struct device *dev; > struct regmap *regmap; > const char *name; > }; > > I insisnt in this chage. I'm not sure if what you want is more simpler, is it something like what this driver does? http://lxr.free-electrons.com/source/drivers/iio/gyro/mpu3050.h#L41 http://lxr.free-electrons.com/source/drivers/iio/gyro/mpu3050-i2c.c#L34 > > > #include > > +#include > > #include > > > +#include > > Can we get rid of gnostic resource providers? > I'm uninformed and still learning. Please let me know how to approach this in some other way. > > +static const struct iio_trigger_ops adxl345_trigger_ops = { > > > + .owner = THIS_MODULE, > > Do we still need this kind of lines? > I'm not sure either. Jonathan, is it OK to omit this and also the one below? > > + .set_trigger_state = adxl345_drdy_trigger_set_state, > > +}; > > > static const struct iio_info adxl345_info = { > > > .driver_module = THIS_MODULE, > > Ditto, though it's in the current code. > > > .read_raw = adxl345_read_raw, > > }; > > > + /* > > + * Any bits set to 0 send their respective interrupts to the INT1 pin, > > + * whereas bits set to 1 send their respective interrupts to the INT2 > > + * pin. Map all interrupts to the specified pin. > > + */ > > + of_irq = of_irq_get_byname(dev->of_node, "INT2"); > > So, can we get it in resourse provider agnostic way? > > > + if (of_irq == irq) > > + regval = 0xFF; > > + else > > + regval = 0x00; > > regval = of_irq == irq ? 0xff : 0x00; ? > OK. Thanks, Eva > > + > > + ret = regmap_write(data->regmap, ADXL345_REG_INT_MAP, regval); > > + if (ret < 0) { > > + dev_err(dev, "Failed to set up interrupts: %d\n", ret); > > + return ret; > > + } > > -- > With Best Regards, > Andy Shevchenko