Re: [PATCH v3] power: smb347_charger: Use device managed API's
From: Sebastian Reichel <sre@kernel.org>
Date: 2016-12-17 15:35:27
Hi, On Sat, Dec 10, 2016 at 04:38:31PM +0530, Atul Raj wrote:
quoted hunk ↗ jump to hunk
Using device managed API to make the code simpler using devm_gpio_request_one in place of gpio_request_one using devm_request_threaded_irq in place of request_threaded_irq using devm_power_supply_register in place of power_supply_register using gpiod_* API's in place of gpio_* API's Signed-off-by: Atul Raj <redacted> --- Changes in v2: - using gpiod_* API's in place of gpio_* API's Changes in v3 - chaning suject as per directory in file path in supply git adding signoff sorry for wrong v2 patch. Kindly ignore v2. drivers/power/smb347-charger.c | 59 ++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 35 deletions(-)diff --git a/drivers/power/smb347-charger.c b/drivers/power/smb347-charger.c index 072c518..5b2b5cb 100644 --- a/drivers/power/smb347-charger.c +++ b/drivers/power/smb347-charger.c
Please rebase your patch against a more modern kernel. The supply subsystem moved to drivers/power/supply.
quoted hunk ↗ jump to hunk
@@ -12,7 +12,7 @@ */ #include <linux/err.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/init.h>@@ -145,6 +145,7 @@ struct smb347_charger { bool mains_online; bool usb_online; bool charging_enabled; + struct gpio_desc *irq_gpio; const struct smb347_charger_platform_data *pdata; };@@ -835,21 +836,29 @@ static int smb347_irq_init(struct smb347_charger *smb, struct i2c_client *client) { const struct smb347_charger_platform_data *pdata = smb->pdata; - int ret, irq = gpio_to_irq(pdata->irq_gpio); + int ret, irq; + struct gpio_desc *gpiod; - ret = gpio_request_one(pdata->irq_gpio, GPIOF_IN, client->name); - if (ret < 0) + smb->irq_gpio = gpio_to_desc(pdata->irq_gpio); + + irq = gpiod_to_irq(smb->irq_gpio); + + gpiod = devm_gpiod_get_optional(&client->dev, NULL, GPIOD_IN); + if (IS_ERR(gpiod)) { + ret = PTR_ERR(gpiod); goto fail; + }
There seems to be a misunderstanding how to use the gpiod API. With the gpiod API the gpio description can be removed from the platform data. Instead gpios are described in the boardcode file similar to e.g. regulators. There are a few examples in the mainline tree, just check for board files with "struct gpiod_lookup_table". Then you get the correct one using devm_gpiod_get_optional(&client->dev, "gpio-name", GPIOD_IN); Actually checking the driver: Please just remove all gpio handling. The boardcode can do all of that and put the correct irq number into i2c_board_info.irq. This also makes it easier to add ACPI/DT bindings, which supports doing this transparently. -- Sebastian
Attachments
- signature.asc [application/pgp-signature] 833 bytes