[PATCH v4 1/4] Input: adp5589: Add default platform data

[PATCH v4 1/4] Input: adp5589: Add default platform data

[Alexandru Ardelean]

From: Lars-Peter Clausen <lars@metafoo.de>

If no platform data is supplied use a dummy platform data that configures
the device in GPIO only mode. This change adds a adp5589_kpad_pdata_get()
helper that returns the default platform-data. This can be later extended
to load configuration from device-trees or ACPI.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v3 -> v4:
* Fixed smatch warnings about potential leak being created; resulted in 2
  more patches:
  - Input: adp5589: unify ret & error variables
  - Input: adp5589: fix possible memleak of 'kpad'

Changelog v2 -> v3:
* fix `-Wpointer-to-int-cast` warnings for patch `input: adp5589: Add basic
  devicetree support` ; warnings shows up on 64 bit ARCHs

Changelog v1 -> v2:
* adjusted patch `input: adp5589: Add default platform data` by
  introducting a `adp5589_kpad_pdata_get()` helper, which returns the
  platform-data; the previos patch was based on an older version of the
  kernel from the ADI kernel-tree; the driver was sync-ed with the upstream
  version

 drivers/input/keyboard/adp5589-keys.c | 36 +++++++++++++++++++--------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index e7d58e7f0257..c6a801bcdf90 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -369,6 +369,25 @@ static const struct adp_constants const_adp5585 = {
 	.reg			= adp5585_reg,
 };
 
+static const struct adp5589_gpio_platform_data adp5589_default_gpio_pdata = {
+	.gpio_start = -1,
+};
+
+static const struct adp5589_kpad_platform_data adp5589_default_pdata = {
+	.gpio_data = &adp5589_default_gpio_pdata,
+};
+
+static const struct adp5589_kpad_platform_data *adp5589_kpad_pdata_get(
+	struct device *dev)
+{
+	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
+
+	if (!pdata)
+		pdata = &adp5589_default_pdata;
+
+	return pdata;
+}
+
 static int adp5589_read(struct i2c_client *client, u8 reg)
 {
 	int ret = i2c_smbus_read_byte_data(client, reg);
@@ -498,7 +517,8 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
 static int adp5589_gpio_add(struct adp5589_kpad *kpad)
 {
 	struct device *dev = &kpad->client->dev;
-	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
+	const struct adp5589_kpad_platform_data *pdata =
+		adp5589_kpad_pdata_get(dev);
 	const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
 	int i, error;
 
@@ -553,7 +573,8 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
 static void adp5589_gpio_remove(struct adp5589_kpad *kpad)
 {
 	struct device *dev = &kpad->client->dev;
-	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
+	const struct adp5589_kpad_platform_data *pdata =
+		adp5589_kpad_pdata_get(dev);
 	const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
 	int error;
 
@@ -656,7 +677,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
 {
 	struct i2c_client *client = kpad->client;
 	const struct adp5589_kpad_platform_data *pdata =
-		dev_get_platdata(&client->dev);
+		adp5589_kpad_pdata_get(&client->dev);
 	u8 (*reg) (u8) = kpad->var->reg;
 	unsigned char evt_mode1 = 0, evt_mode2 = 0, evt_mode3 = 0;
 	unsigned char pull_mask = 0;
@@ -861,7 +882,7 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
 {
 	struct i2c_client *client = kpad->client;
 	const struct adp5589_kpad_platform_data *pdata =
-		dev_get_platdata(&client->dev);
+		adp5589_kpad_pdata_get(&client->dev);
 	struct input_dev *input;
 	unsigned int i;
 	int error;
@@ -992,7 +1013,7 @@ static int adp5589_probe(struct i2c_client *client,
 {
 	struct adp5589_kpad *kpad;
 	const struct adp5589_kpad_platform_data *pdata =
-		dev_get_platdata(&client->dev);
+		adp5589_kpad_pdata_get(&client->dev);
 	unsigned int revid;
 	int error, ret;
 
@@ -1002,11 +1023,6 @@ static int adp5589_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	if (!pdata) {
-		dev_err(&client->dev, "no platform data?\n");
-		return -EINVAL;
-	}
-
 	kpad = kzalloc(sizeof(*kpad), GFP_KERNEL);
 	if (!kpad)
 		return -ENOMEM;
-- 
2.20.1

[PATCH v4 2/4] Input: adp5589: Add basic devicetree support

[Alexandru Ardelean]

From: Lars-Peter Clausen <lars@metafoo.de>

Add very basic devicetree suppport to the adp5589 allowing the device to be
registered from devicetree.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/input/keyboard/adp5589-keys.c | 33 ++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index c6a801bcdf90..56f4ff7fa9c3 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -1008,6 +1008,25 @@ static void adp5589_keypad_remove(struct adp5589_kpad *kpad)
 	}
 }
 
+static int adp5589_i2c_get_driver_data(struct i2c_client *i2c,
+				       const struct i2c_device_id *id)
+{
+	const struct of_device_id *match;
+
+	if (id)
+		return id->driver_data;
+
+	if (!IS_ENABLED(CONFIG_OF) || !i2c->dev.of_node)
+		return -ENODEV;
+
+	match = of_match_node(i2c->dev.driver->of_match_table,
+			      i2c->dev.of_node);
+	if (match)
+		return (uintptr_t)match->data;
+
+	return -ENODEV;
+}
+
 static int adp5589_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -1029,7 +1048,11 @@ static int adp5589_probe(struct i2c_client *client,
 
 	kpad->client = client;
 
-	switch (id->driver_data) {
+	ret = adp5589_i2c_get_driver_data(client, id);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
 	case ADP5585_02:
 		kpad->support_row5 = true;
 		/* fall through */
@@ -1129,6 +1152,13 @@ static int adp5589_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(adp5589_dev_pm_ops, adp5589_suspend, adp5589_resume);
 
+static const struct of_device_id adp5589_of_match[] = {
+	{ .compatible = "adi,adp5585", .data = (void *)ADP5585_01 },
+	{ .compatible = "adi,adp5585-02", .data = (void *)ADP5585_02 },
+	{ .compatible = "adi,adp5589", .data = (void *)ADP5589 },
+	{}
+};
+
 static const struct i2c_device_id adp5589_id[] = {
 	{"adp5589-keys", ADP5589},
 	{"adp5585-keys", ADP5585_01},
@@ -1142,6 +1172,7 @@ static struct i2c_driver adp5589_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.pm = &adp5589_dev_pm_ops,
+		.of_match_table = adp5589_of_match,
 	},
 	.probe = adp5589_probe,
 	.remove = adp5589_remove,
-- 
2.20.1

[PATCH v4 3/4] Input: adp5589: unify ret & error variables

[Alexandru Ardelean]

Both variables are used mostly in the same way in the probe function.
Having both means that we need to copy 'ret' to 'error' before exiting, so
just use 'ret' everywhere.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/input/keyboard/adp5589-keys.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 56f4ff7fa9c3..1fd36c581a91 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -1034,7 +1034,7 @@ static int adp5589_probe(struct i2c_client *client,
 	const struct adp5589_kpad_platform_data *pdata =
 		adp5589_kpad_pdata_get(&client->dev);
 	unsigned int revid;
-	int error, ret;
+	int ret;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -1067,28 +1067,26 @@ static int adp5589_probe(struct i2c_client *client,
 	}
 
 	ret = adp5589_read(client, ADP5589_5_ID);
-	if (ret < 0) {
-		error = ret;
+	if (ret < 0)
 		goto err_free_mem;
-	}
 
 	revid = (u8) ret & ADP5589_5_DEVICE_ID_MASK;
 
 	if (pdata->keymapsize) {
-		error = adp5589_keypad_add(kpad, revid);
-		if (error)
+		ret = adp5589_keypad_add(kpad, revid);
+		if (ret)
 			goto err_free_mem;
 	}
 
-	error = adp5589_setup(kpad);
-	if (error)
+	ret = adp5589_setup(kpad);
+	if (ret)
 		goto err_keypad_remove;
 
 	if (kpad->gpimapsize)
 		adp5589_report_switch_state(kpad);
 
-	error = adp5589_gpio_add(kpad);
-	if (error)
+	ret = adp5589_gpio_add(kpad);
+	if (ret)
 		goto err_keypad_remove;
 
 	i2c_set_clientdata(client, kpad);
@@ -1101,7 +1099,7 @@ static int adp5589_probe(struct i2c_client *client,
 err_free_mem:
 	kfree(kpad);
 
-	return error;
+	return ret;
 }
 
 static int adp5589_remove(struct i2c_client *client)
-- 
2.20.1

[PATCH v4 4/4] Input: adp5589: fix possible memleak of 'kpad'

[Alexandru Ardelean]

If 'adp5589_i2c_get_driver_data()' returns an error, the exit path should
be to also free the 'kpad' object.
This change fixes that.

Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/input/keyboard/adp5589-keys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 1fd36c581a91..5cef5a13b776 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -1050,7 +1050,7 @@ static int adp5589_probe(struct i2c_client *client,
 
 	ret = adp5589_i2c_get_driver_data(client, id);
 	if (ret < 0)
-		return ret;
+		goto err_free_mem;
 
 	switch (ret) {
 	case ADP5585_02:
-- 
2.20.1

Re: [PATCH v4 3/4] Input: adp5589: unify ret & error variables

[Dan Carpenter]

On Thu, Mar 05, 2020 at 03:14:04PM +0200, Alexandru Ardelean wrote:
> Both variables are used mostly in the same way in the probe function.
> Having both means that we need to copy 'ret' to 'error' before exiting, so
> just use 'ret' everywhere.
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---

Looks good.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v4 1/4] Input: adp5589: Add default platform data

[Ardelean, Alexandru]

On Thu, 2020-03-05 at 15:14 +0200, Alexandru Ardelean wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> If no platform data is supplied use a dummy platform data that configures
> the device in GPIO only mode. This change adds a adp5589_kpad_pdata_get()
> helper that returns the default platform-data. This can be later extended
> to load configuration from device-trees or ACPI.

ping

> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> 
> Changelog v3 -> v4:
> * Fixed smatch warnings about potential leak being created; resulted in 2
>   more patches:
>   - Input: adp5589: unify ret & error variables
>   - Input: adp5589: fix possible memleak of 'kpad'
> 
> Changelog v2 -> v3:
> * fix `-Wpointer-to-int-cast` warnings for patch `input: adp5589: Add basic
>   devicetree support` ; warnings shows up on 64 bit ARCHs
> 
> Changelog v1 -> v2:
> * adjusted patch `input: adp5589: Add default platform data` by
>   introducting a `adp5589_kpad_pdata_get()` helper, which returns the
>   platform-data; the previos patch was based on an older version of the
>   kernel from the ADI kernel-tree; the driver was sync-ed with the upstream
>   version
> 
>  drivers/input/keyboard/adp5589-keys.c | 36 +++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adp5589-keys.c
> b/drivers/input/keyboard/adp5589-keys.c
> index e7d58e7f0257..c6a801bcdf90 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -369,6 +369,25 @@ static const struct adp_constants const_adp5585 = {
>  	.reg			= adp5585_reg,
>  };
>  
> +static const struct adp5589_gpio_platform_data adp5589_default_gpio_pdata = {
> +	.gpio_start = -1,
> +};
> +
> +static const struct adp5589_kpad_platform_data adp5589_default_pdata = {
> +	.gpio_data = &adp5589_default_gpio_pdata,
> +};
> +
> +static const struct adp5589_kpad_platform_data *adp5589_kpad_pdata_get(
> +	struct device *dev)
> +{
> +	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
> +
> +	if (!pdata)
> +		pdata = &adp5589_default_pdata;
> +
> +	return pdata;
> +}
> +
>  static int adp5589_read(struct i2c_client *client, u8 reg)
>  {
>  	int ret = i2c_smbus_read_byte_data(client, reg);
> @@ -498,7 +517,8 @@ static int adp5589_build_gpiomap(struct adp5589_kpad
> *kpad,
>  static int adp5589_gpio_add(struct adp5589_kpad *kpad)
>  {
>  	struct device *dev = &kpad->client->dev;
> -	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
> +	const struct adp5589_kpad_platform_data *pdata =
> +		adp5589_kpad_pdata_get(dev);
>  	const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
>  	int i, error;
>  
> @@ -553,7 +573,8 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
>  static void adp5589_gpio_remove(struct adp5589_kpad *kpad)
>  {
>  	struct device *dev = &kpad->client->dev;
> -	const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
> +	const struct adp5589_kpad_platform_data *pdata =
> +		adp5589_kpad_pdata_get(dev);
>  	const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
>  	int error;
>  
> @@ -656,7 +677,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
>  {
>  	struct i2c_client *client = kpad->client;
>  	const struct adp5589_kpad_platform_data *pdata =
> -		dev_get_platdata(&client->dev);
> +		adp5589_kpad_pdata_get(&client->dev);
>  	u8 (*reg) (u8) = kpad->var->reg;
>  	unsigned char evt_mode1 = 0, evt_mode2 = 0, evt_mode3 = 0;
>  	unsigned char pull_mask = 0;
> @@ -861,7 +882,7 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad,
> unsigned int revid)
>  {
>  	struct i2c_client *client = kpad->client;
>  	const struct adp5589_kpad_platform_data *pdata =
> -		dev_get_platdata(&client->dev);
> +		adp5589_kpad_pdata_get(&client->dev);
>  	struct input_dev *input;
>  	unsigned int i;
>  	int error;
> @@ -992,7 +1013,7 @@ static int adp5589_probe(struct i2c_client *client,
>  {
>  	struct adp5589_kpad *kpad;
>  	const struct adp5589_kpad_platform_data *pdata =
> -		dev_get_platdata(&client->dev);
> +		adp5589_kpad_pdata_get(&client->dev);
>  	unsigned int revid;
>  	int error, ret;
>  
> @@ -1002,11 +1023,6 @@ static int adp5589_probe(struct i2c_client *client,
>  		return -EIO;
>  	}
>  
> -	if (!pdata) {
> -		dev_err(&client->dev, "no platform data?\n");
> -		return -EINVAL;
> -	}
> -
>  	kpad = kzalloc(sizeof(*kpad), GFP_KERNEL);
>  	if (!kpad)
>  		return -ENOMEM;