From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752469AbbCXLvO (ORCPT ); Tue, 24 Mar 2015 07:51:14 -0400 Received: from mail-wg0-f48.google.com ([74.125.82.48]:36185 "EHLO mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751643AbbCXLvK convert rfc822-to-8bit (ORCPT ); Tue, 24 Mar 2015 07:51:10 -0400 MIME-Version: 1.0 In-Reply-To: <20150323151841.GA1878@lahna.fi.intel.com> References: <1427118025-4380-1-git-send-email-robert.dolca@intel.com> <20150323151841.GA1878@lahna.fi.intel.com> Date: Tue, 24 Mar 2015 13:51:08 +0200 X-Google-Sender-Auth: 20Pby5q0zZQUFTi8-NQT5mjk2zQ Message-ID: Subject: Re: [PATCH] IIO: Adds ACPI support for ST gyroscopes From: Daniel Baluta To: Mika Westerberg , Denis CIOCCA Cc: Robert Dolca , "linux-iio@vger.kernel.org" , Jonathan Cameron , Linux Kernel Mailing List , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Linus Walleij Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 23, 2015 at 5:18 PM, Mika Westerberg wrote: > On Mon, Mar 23, 2015 at 03:40:24PM +0200, Robert Dolca wrote: >> Signed-off-by: Robert Dolca >> --- >> drivers/iio/common/st_sensors/st_sensors_i2c.c | 35 ++++++++++++++++++++++++++ >> drivers/iio/gyro/st_gyro_i2c.c | 29 ++++++++++++++++++++- >> include/linux/iio/common/st_sensors_i2c.h | 3 +++ >> 3 files changed, 66 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c >> index 98cfee29..2f612ec 100644 >> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c >> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c >> @@ -13,6 +13,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> >> #include >> >> @@ -107,6 +109,39 @@ void st_sensors_of_i2c_probe(struct i2c_client *client, >> EXPORT_SYMBOL(st_sensors_of_i2c_probe); >> #endif >> >> +int st_sensors_acpi_i2c_probe(struct i2c_client *client, >> + const struct acpi_device_id *match) >> +{ >> + const struct acpi_device_id *id; >> + struct gpio_desc *gpiod_irq; >> + int ret; >> + >> + id = acpi_match_device(match, &client->dev); >> + if (!id) >> + return -ENODEV; >> + >> + /* Get IRQ GPIO */ >> + gpiod_irq = devm_gpiod_get_index(&client->dev, 0, 0); > > Please use plain devm_gpiod_get(&client->dev, NULL). That should work > with DT and ACPI _DSD as well. > >> + if (IS_ERR(gpiod_irq)) >> + return -ENODEV; > > Why not return the original error here? Now you lose things like > -EPROBE_DEFER. > >> + /* Configure IRQ GPIO */ >> + ret = gpiod_direction_input(gpiod_irq); >> + if (ret) >> + return ret; >> + >> + /* Map the pin to an IRQ */ >> + client->irq = gpiod_to_irq(gpiod_irq); >> + >> + /* The name from the ACPI match takes precedence if present */ >> + memset(client->name, 0, sizeof(client->name)); >> + strncpy(client->name, (char *) id->driver_data, >> + min(sizeof(client->name), strlen((char *) id->driver_data))); > > Hmm, the above should not be required at all. If the device has an ACPI > companion the I2C core will match first with that. > > Also you now modify the original i2c_client structure. Should you at > least restore the original values when the driver is unbound? Hi Mika, This is similar with device tree aproach. Looking at st_sensors_of_i2c_probe, we have: » /* The name from the OF match takes precedence if present */ » strncpy(client->name, of_id->data, sizeof(client->name)); » client->name[sizeof(client->name) - 1] = '\0'; We want to keep the compatibility with the device tree implementation. Not sure if the custom name is really required. Denis, perhaps you can offer more info on this. Daniel.