[PATCH v3 0/3] extcon: Core cleanups and documentation fixes

[PATCH v3 0/3] extcon: Core cleanups and documentation fixes

[Andy Shevchenko]

A few fixes and some cleanups against extcon core module.

Changelog v3:
- dropped NAKed patches
- added missing tag (Chanwoo)
- dropped unrelated change in patch 2 (Chanwoo)
- dropped misplaced tags (Chanwoo)

Changelog v2:
- dropped applied patches
- completely rewrote the patch to handle name field
- dropped kasprintf_strarray() patch for now (Chanwoo)
- used new IDA APIs (Chanwoo)
- added tag (Bumwoo) to the patches that haven't changed

Cc: Chanwoo Choi <cwchoi00@gmail.com>

Note, MAINTAINERS shows what it has and hence the above Cc is manually
added. If the database has issues it should be updated, but it's out of
scope of this series.

Andy Shevchenko (3):
  extcon: Use unique number for the extcon device ID
  extcon: Use sizeof(*pointer) instead of sizeof(type)
  extcon: Drop unneeded assignments

 drivers/extcon/extcon.c | 35 +++++++++++++++++++++--------------
 drivers/extcon/extcon.h |  2 ++
 2 files changed, 23 insertions(+), 14 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v3 1/3] extcon: Use unique number for the extcon device ID

[Andy Shevchenko]

The use of atomic variable is still racy when we do not control which
device has been unregistered and there is a (theoretical) possibility
of the overflow that may cause a duplicate extcon device ID number
to be allocated next time a device is registered.

Replace above mentioned approach by using IDA framework.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 16 +++++++++++++---
 drivers/extcon/extcon.h |  2 ++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 47819c5144d5..5da1cc60582a 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -16,6 +16,7 @@
 
 #include <linux/module.h>
 #include <linux/types.h>
+#include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/fs.h>
@@ -238,6 +239,7 @@ struct extcon_cable {
 
 static struct class *extcon_class;
 
+static DEFINE_IDA(extcon_dev_ids);
 static LIST_HEAD(extcon_dev_list);
 static DEFINE_MUTEX(extcon_dev_list_lock);
 
@@ -1248,7 +1250,6 @@ static int extcon_alloc_groups(struct extcon_dev *edev)
 int extcon_dev_register(struct extcon_dev *edev)
 {
 	int ret, index = 0;
-	static atomic_t edev_no = ATOMIC_INIT(-1);
 
 	ret = create_extcon_class();
 	if (ret < 0)
@@ -1275,8 +1276,14 @@ int extcon_dev_register(struct extcon_dev *edev)
 			"extcon device name is null\n");
 		return -EINVAL;
 	}
-	dev_set_name(&edev->dev, "extcon%lu",
-			(unsigned long)atomic_inc_return(&edev_no));
+
+	ret = ida_alloc(&extcon_dev_ids, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	edev->id = ret;
+
+	dev_set_name(&edev->dev, "extcon%d", edev->id);
 
 	ret = extcon_alloc_cables(edev);
 	if (ret < 0)
@@ -1339,6 +1346,7 @@ int extcon_dev_register(struct extcon_dev *edev)
 	if (edev->max_supported)
 		kfree(edev->cables);
 err_alloc_cables:
+	ida_free(&extcon_dev_ids, edev->id);
 
 	return ret;
 }
@@ -1367,6 +1375,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 		return;
 	}
 
+	ida_free(&extcon_dev_ids, edev->id);
+
 	device_unregister(&edev->dev);
 
 	if (edev->mutually_exclusive && edev->max_supported) {
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 15616446140d..946182687786 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -20,6 +20,7 @@
  *			{0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
  *			can be no simultaneous connections.
  * @dev:		Device of this extcon.
+ * @id:			Unique device ID of this extcon.
  * @state:		Attach/detach state of this extcon. Do not provide at
  *			register-time.
  * @nh_all:		Notifier for the state change events for all supported
@@ -46,6 +47,7 @@ struct extcon_dev {
 
 	/* Internal data. Please do not set. */
 	struct device dev;
+	unsigned int id;
 	struct raw_notifier_head nh_all;
 	struct raw_notifier_head *nh;
 	struct list_head entry;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v3 2/3] extcon: Use sizeof(*pointer) instead of sizeof(type)

[Andy Shevchenko]

It is preferred to use sizeof(*pointer) instead of sizeof(type).
The type of the variable can change and one needs not change
the former (unlike the latter). No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 5da1cc60582a..76dc41e8f250 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1098,8 +1098,7 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
 	if (!edev->max_supported)
 		return 0;
 
-	edev->cables = kcalloc(edev->max_supported,
-			       sizeof(struct extcon_cable),
+	edev->cables = kcalloc(edev->max_supported, sizeof(*edev->cables),
 			       GFP_KERNEL);
 	if (!edev->cables)
 		return -ENOMEM;
@@ -1161,14 +1160,12 @@ static int extcon_alloc_muex(struct extcon_dev *edev)
 	for (index = 0; edev->mutually_exclusive[index]; index++)
 		;
 
-	edev->attrs_muex = kcalloc(index + 1,
-				   sizeof(struct attribute *),
+	edev->attrs_muex = kcalloc(index + 1, sizeof(*edev->attrs_muex),
 				   GFP_KERNEL);
 	if (!edev->attrs_muex)
 		return -ENOMEM;
 
-	edev->d_attrs_muex = kcalloc(index,
-				     sizeof(struct device_attribute),
+	edev->d_attrs_muex = kcalloc(index, sizeof(*edev->d_attrs_muex),
 				     GFP_KERNEL);
 	if (!edev->d_attrs_muex) {
 		kfree(edev->attrs_muex);
@@ -1214,8 +1211,8 @@ static int extcon_alloc_groups(struct extcon_dev *edev)
 		return 0;
 
 	edev->extcon_dev_type.groups = kcalloc(edev->max_supported + 2,
-			sizeof(struct attribute_group *),
-			GFP_KERNEL);
+					  sizeof(*edev->extcon_dev_type.groups),
+					  GFP_KERNEL);
 	if (!edev->extcon_dev_type.groups)
 		return -ENOMEM;
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v3 3/3] extcon: Drop unneeded assignments

[Andy Shevchenko]

In one case the assignment is duplicative, in the other,
it's better to move it into the loop — the user of it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 76dc41e8f250..6f7a60d2ed91 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -245,7 +245,7 @@ static DEFINE_MUTEX(extcon_dev_list_lock);
 
 static int check_mutually_exclusive(struct extcon_dev *edev, u32 new_state)
 {
-	int i = 0;
+	int i;
 
 	if (!edev->mutually_exclusive)
 		return 0;
@@ -1246,7 +1246,7 @@ static int extcon_alloc_groups(struct extcon_dev *edev)
  */
 int extcon_dev_register(struct extcon_dev *edev)
 {
-	int ret, index = 0;
+	int ret, index;
 
 	ret = create_extcon_class();
 	if (ret < 0)
@@ -1255,7 +1255,7 @@ int extcon_dev_register(struct extcon_dev *edev)
 	if (!edev || !edev->supported_cable)
 		return -EINVAL;
 
-	for (; edev->supported_cable[index] != EXTCON_NONE; index++);
+	for (index = 0; edev->supported_cable[index] != EXTCON_NONE; index++);
 
 	edev->max_supported = index;
 	if (index > SUPPORTED_CABLE_MAX) {
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v3 0/3] extcon: Core cleanups and documentation fixes

[Chanwoo Choi]

On 23. 4. 11. 20:48, Andy Shevchenko wrote:
> A few fixes and some cleanups against extcon core module.
> 
> Changelog v3:
> - dropped NAKed patches
> - added missing tag (Chanwoo)
> - dropped unrelated change in patch 2 (Chanwoo)
> - dropped misplaced tags (Chanwoo)
> 
> Changelog v2:
> - dropped applied patches
> - completely rewrote the patch to handle name field
> - dropped kasprintf_strarray() patch for now (Chanwoo)
> - used new IDA APIs (Chanwoo)
> - added tag (Bumwoo) to the patches that haven't changed
> 
> Cc: Chanwoo Choi <cwchoi00@gmail.com>
> 
> Note, MAINTAINERS shows what it has and hence the above Cc is manually
> added. If the database has issues it should be updated, but it's out of
> scope of this series.
> 
> Andy Shevchenko (3):
>   extcon: Use unique number for the extcon device ID
>   extcon: Use sizeof(*pointer) instead of sizeof(type)
>   extcon: Drop unneeded assignments
> 
>  drivers/extcon/extcon.c | 35 +++++++++++++++++++++--------------
>  drivers/extcon/extcon.h |  2 ++
>  2 files changed, 23 insertions(+), 14 deletions(-)
> 

Applied them. Thanks for your clean-up.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi