[PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Andy Shevchenko]

A few fixes to the documentation and some cleanups against extcon core
module.

Andy Shevchenko (14):
  extcon: Fix kernel doc of property fields to avoid warnings
  extcon: Fix kernel doc of property capability fields to avoid warnings
  extcon: Use DECLARE_BITMAP() to declare bit arrays
  extcon: use sysfs_emit() to instead of sprintf()
  extcon: Amend kernel documentation of struct extcon_dev
  extcon: Allow name to be assigned outside of the framework
  extcon: Use unique number for the extcon device ID
  extcon: Switch to use kasprintf_strarray()
  extcon: Use device_match_of_node() helper
  extcon: use dev_of_node(dev) instead of dev->of_node
  extcon: Remove dup device name in the message and unneeded error check
  extcon: Use sizeof(*pointer) instead of sizeof(type)
  extcon: Drop unneeded assignments
  extcon: Use positive conditional in ternary operator

 drivers/extcon/extcon.c | 126 +++++++++++++++++++++-------------------
 drivers/extcon/extcon.h |   9 ++-
 2 files changed, 71 insertions(+), 64 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 01/14] extcon: Fix kernel doc of property fields to avoid warnings

[Andy Shevchenko]

Kernel documentation has to be synchronized with a code, otherwise
the validator is not happy:

     Function parameter or member 'usb_propval' not described in 'extcon_cable'
     Function parameter or member 'chg_propval' not described in 'extcon_cable'
     Function parameter or member 'jack_propval' not described in 'extcon_cable'
     Function parameter or member 'disp_propval' not described in 'extcon_cable'

Describe the fields added in the past.

Fixes: 067c1652e7a7 ("extcon: Add the support for extcon property according to extcon type")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 3997b39680b7..1bc2639704c2 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -206,6 +206,10 @@ static const struct __extcon_info {
  * @attr_name:		"name" sysfs entry
  * @attr_state:		"state" sysfs entry
  * @attrs:		the array pointing to attr_name and attr_state for attr_g
+ * @usb_propval:	the array of USB connector properties
+ * @chg_propval:	the array of charger connector properties
+ * @jack_propval:	the array of jack connector properties
+ * @disp_propval:	the array of display connector properties
  */
 struct extcon_cable {
 	struct extcon_dev *edev;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 02/14] extcon: Fix kernel doc of property capability fields to avoid warnings

[Andy Shevchenko]

Kernel documentation has to be synchronized with a code, otherwise
the validator is not happy:

     Function parameter or member 'usb_bits' not described in 'extcon_cable'
     Function parameter or member 'chg_bits' not described in 'extcon_cable'
     Function parameter or member 'jack_bits' not described in 'extcon_cable'
     Function parameter or member 'disp_bits' not described in 'extcon_cable'

Describe the fields added in the past.

Fixes: ceaa98f442cf ("extcon: Add the support for the capability of each property")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 1bc2639704c2..79006ab5334b 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -210,6 +210,10 @@ static const struct __extcon_info {
  * @chg_propval:	the array of charger connector properties
  * @jack_propval:	the array of jack connector properties
  * @disp_propval:	the array of display connector properties
+ * @usb_bits:		the bit array of the USB connector property capabilities
+ * @chg_bits:		the bit array of the charger connector property capabilities
+ * @jack_bits:		the bit array of the jack connector property capabilities
+ * @disp_bits:		the bit array of the display connector property capabilities
  */
 struct extcon_cable {
 	struct extcon_dev *edev;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 03/14] extcon: Use DECLARE_BITMAP() to declare bit arrays

[Andy Shevchenko]

Bit arrays has a specific type helper for the declaration.
Use it instead of homegronw equivalent.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 79006ab5334b..70e9755ba2bc 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -230,10 +230,10 @@ struct extcon_cable {
 	union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
 	union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
 
-	unsigned long usb_bits[BITS_TO_LONGS(EXTCON_PROP_USB_CNT)];
-	unsigned long chg_bits[BITS_TO_LONGS(EXTCON_PROP_CHG_CNT)];
-	unsigned long jack_bits[BITS_TO_LONGS(EXTCON_PROP_JACK_CNT)];
-	unsigned long disp_bits[BITS_TO_LONGS(EXTCON_PROP_DISP_CNT)];
+	DECLARE_BITMAP(usb_bits, EXTCON_PROP_USB_CNT);
+	DECLARE_BITMAP(chg_bits, EXTCON_PROP_CHG_CNT);
+	DECLARE_BITMAP(jack_bits, EXTCON_PROP_JACK_CNT);
+	DECLARE_BITMAP(disp_bits, EXTCON_PROP_DISP_CNT);
 };
 
 static struct class *extcon_class;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 04/14] extcon: use sysfs_emit() to instead of sprintf()

[Andy Shevchenko]

Follow the advice of the Documentation/filesystems/sysfs.rst that
show() should only use sysfs_emit() or sysfs_emit_at() when formatting
the value to be returned to user space.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 70e9755ba2bc..ac84f4aafc69 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -370,12 +370,12 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
 	struct extcon_dev *edev = dev_get_drvdata(dev);
 
 	if (edev->max_supported == 0)
-		return sprintf(buf, "%u\n", edev->state);
+		return sysfs_emit(buf, "%u\n", edev->state);
 
 	for (i = 0; i < edev->max_supported; i++) {
-		count += sprintf(buf + count, "%s=%d\n",
-				extcon_info[edev->supported_cable[i]].name,
-				 !!(edev->state & BIT(i)));
+		count += sysfs_emit_at(buf, count, "%s=%d\n",
+				       extcon_info[edev->supported_cable[i]].name,
+				       !!(edev->state & BIT(i)));
 	}
 
 	return count;
@@ -387,7 +387,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
 {
 	struct extcon_dev *edev = dev_get_drvdata(dev);
 
-	return sprintf(buf, "%s\n", edev->name);
+	return sysfs_emit(buf, "%s\n", edev->name);
 }
 static DEVICE_ATTR_RO(name);
 
@@ -398,8 +398,8 @@ static ssize_t cable_name_show(struct device *dev,
 						  attr_name);
 	int i = cable->cable_index;
 
-	return sprintf(buf, "%s\n",
-			extcon_info[cable->edev->supported_cable[i]].name);
+	return sysfs_emit(buf, "%s\n",
+			  extcon_info[cable->edev->supported_cable[i]].name);
 }
 
 static ssize_t cable_state_show(struct device *dev,
@@ -410,8 +410,8 @@ static ssize_t cable_state_show(struct device *dev,
 
 	int i = cable->cable_index;
 
-	return sprintf(buf, "%d\n",
-		extcon_get_state(cable->edev, cable->edev->supported_cable[i]));
+	return sysfs_emit(buf, "%d\n",
+			  extcon_get_state(cable->edev, cable->edev->supported_cable[i]));
 }
 
 /**
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 05/14] extcon: Amend kernel documentation of struct extcon_dev

[Andy Shevchenko]

First of all, the @lock description is missing. Add it.
Second, correct the terminator value for the mutual exclusive
cabling.

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

diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 93b5e0306966..15616446140d 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -13,8 +13,8 @@
  *			are disabled.
  * @mutually_exclusive:	Array of mutually exclusive set of cables that cannot
  *			be attached simultaneously. The array should be
- *			ending with NULL or be NULL (no mutually exclusive
- *			cables). For example, if it is { 0x7, 0x30, 0}, then,
+ *			ending with 0 or be NULL (no mutually exclusive cables).
+ *			For example, if it is {0x7, 0x30, 0}, then,
  *			{0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot
  *			be attached simulataneously. {0x7, 0} is equivalent to
  *			{0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
@@ -27,7 +27,7 @@
  * @nh:			Notifier for the state change events from this extcon
  * @entry:		To support list of extcon devices so that users can
  *			search for extcon devices based on the extcon name.
- * @lock:
+ * @lock:		Protects device state and serialises device registration
  * @max_supported:	Internal value to store the number of cables.
  * @extcon_dev_type:	Device_type struct to provide attribute_groups
  *			customized for each extcon device.
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 06/14] extcon: Allow name to be assigned outside of the framework

[Andy Shevchenko]

The documentation states that name of the extcon can be assigned
outside of the framework, however code does something different.
Fix the code to be aligned with the documentation.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index ac84f4aafc69..14e66e21597f 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1269,10 +1269,10 @@ int extcon_dev_register(struct extcon_dev *edev)
 	edev->dev.class = extcon_class;
 	edev->dev.release = extcon_dev_release;
 
-	edev->name = dev_name(edev->dev.parent);
-	if (IS_ERR_OR_NULL(edev->name)) {
-		dev_err(&edev->dev,
-			"extcon device name is null\n");
+	if (!edev->name)
+		edev->name = dev_name(edev->dev.parent);
+	if (!edev->name) {
+		dev_err(&edev->dev, "extcon device name is null\n");
 		return -EINVAL;
 	}
 	dev_set_name(&edev->dev, "extcon%lu",
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 07/14] 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 | 15 ++++++++++++---
 drivers/extcon/extcon.h |  2 ++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 14e66e21597f..0d261ec6c473 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)
 		dev_err(&edev->dev, "extcon device name is null\n");
 		return -EINVAL;
 	}
-	dev_set_name(&edev->dev, "extcon%lu",
-			(unsigned long)atomic_inc_return(&edev_no));
+
+	ret = ida_simple_get(&extcon_dev_ids, 0, INT_MAX, 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)
@@ -1368,6 +1375,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 		return;
 	}
 
+	ida_simple_remove(&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..877c0860e300 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;
+	int id;
 	struct raw_notifier_head nh_all;
 	struct raw_notifier_head *nh;
 	struct list_head entry;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 08/14] extcon: Switch to use kasprintf_strarray()

[Andy Shevchenko]

Since we have a generic helper, switch the module to use it.
No functional change intended.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 0d261ec6c473..a63e7eef02fd 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -23,6 +23,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include <linux/sysfs.h>
 
 #include "extcon.h"
@@ -1104,19 +1105,17 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
 	if (!edev->cables)
 		return -ENOMEM;
 
+	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
+	if (!edev->cable_names) {
+		kfree(edev->cables);
+		return -ENOMEM;
+	}
+
 	for (index = 0; index < edev->max_supported; index++) {
 		cable = &edev->cables[index];
 
-		str = kasprintf(GFP_KERNEL, "cable.%d", index);
-		if (!str) {
-			for (index--; index >= 0; index--) {
-				cable = &edev->cables[index];
-				kfree(cable->attr_g.name);
-			}
-
-			kfree(edev->cables);
-			return -ENOMEM;
-		}
+		str = edev->cable_names[index];
+		strreplace(str, '-', '.');
 
 		cable->edev = edev;
 		cable->cable_index = index;
@@ -1341,8 +1340,7 @@ int extcon_dev_register(struct extcon_dev *edev)
 		kfree(edev->attrs_muex);
 	}
 err_alloc_muex:
-	for (index = 0; index < edev->max_supported; index++)
-		kfree(edev->cables[index].attr_g.name);
+	kfree_strarray(edev->cable_names, edev->max_supported);
 	if (edev->max_supported)
 		kfree(edev->cables);
 err_alloc_cables:
@@ -1387,8 +1385,7 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 		kfree(edev->attrs_muex);
 	}
 
-	for (index = 0; index < edev->max_supported; index++)
-		kfree(edev->cables[index].attr_g.name);
+	kfree_strarray(edev->cable_names, edev->max_supported);
 
 	if (edev->max_supported) {
 		kfree(edev->extcon_dev_type.groups);
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 877c0860e300..5624f65ba17a 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -58,6 +58,7 @@ struct extcon_dev {
 	/* /sys/class/extcon/.../cable.n/... */
 	struct device_type extcon_dev_type;
 	struct extcon_cable *cables;
+	char **cable_names;
 
 	/* /sys/class/extcon/.../mutually_exclusive/... */
 	struct attribute_group attr_g_muex;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 09/14] extcon: Use device_match_of_node() helper

[Andy Shevchenko]

Instead of open coding, use device_match_of_node() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index a63e7eef02fd..5cadbfc151e6 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1411,7 +1411,7 @@ struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
 
 	mutex_lock(&extcon_dev_list_lock);
 	list_for_each_entry(edev, &extcon_dev_list, entry)
-		if (edev->dev.parent && edev->dev.parent->of_node == node)
+		if (edev->dev.parent && device_match_of_node(edev->dev.parent, node))
 			goto out;
 	edev = ERR_PTR(-EPROBE_DEFER);
 out:
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 10/14] extcon: use dev_of_node(dev) instead of dev->of_node

[Andy Shevchenko]

The dev_of_node function should be preferred.
In the result we may drop unneeded NULL check
of the pointer to the device object.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 5cadbfc151e6..32e96cb49067 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1429,21 +1429,17 @@ struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
  */
 struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
 {
-	struct device_node *node;
+	struct device_node *node, *np = dev_of_node(dev);
 	struct extcon_dev *edev;
 
-	if (!dev)
-		return ERR_PTR(-EINVAL);
-
-	if (!dev->of_node) {
+	if (!np) {
 		dev_dbg(dev, "device does not have a device node entry\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	node = of_parse_phandle(dev->of_node, "extcon", index);
+	node = of_parse_phandle(np, "extcon", index);
 	if (!node) {
-		dev_dbg(dev, "failed to get phandle in %pOF node\n",
-			dev->of_node);
+		dev_dbg(dev, "failed to get phandle in %pOF node\n", np);
 		return ERR_PTR(-ENODEV);
 	}
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 11/14] extcon: Remove dup device name in the message and unneeded error check

[Andy Shevchenko]

The device name is already printed with dev_err(), no need to repeat.
The device pointer itself is not supposed to be an error point, drop
that check.

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

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 32e96cb49067..0e04ea185c26 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1367,9 +1367,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 	list_del(&edev->entry);
 	mutex_unlock(&extcon_dev_list_lock);
 
-	if (IS_ERR_OR_NULL(get_device(&edev->dev))) {
-		dev_err(&edev->dev, "Failed to unregister extcon_dev (%s)\n",
-				dev_name(&edev->dev));
+	if (!get_device(&edev->dev)) {
+		dev_err(&edev->dev, "Failed to unregister extcon_dev\n");
 		return;
 	}
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 12/14] 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 | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 0e04ea185c26..b3f038639cd6 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1099,9 +1099,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),
-			       GFP_KERNEL);
+	edev->cables = kcalloc(edev->max_supported, sizeof(*edev->cables), GFP_KERNEL);
 	if (!edev->cables)
 		return -ENOMEM;
 
@@ -1160,14 +1158,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);
@@ -1213,8 +1209,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;
 
@@ -1298,8 +1294,7 @@ int extcon_dev_register(struct extcon_dev *edev)
 
 	spin_lock_init(&edev->lock);
 	if (edev->max_supported) {
-		edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh),
-				GFP_KERNEL);
+		edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh), GFP_KERNEL);
 		if (!edev->nh) {
 			ret = -ENOMEM;
 			goto err_alloc_nh;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 13/14] 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>
---
 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 b3f038639cd6..edfa0450e605 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -246,7 +246,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;
@@ -1244,7 +1244,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)
@@ -1253,7 +1253,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

[PATCH v1 14/14] extcon: Use positive conditional in ternary operator

[Andy Shevchenko]

It's easier to parse by a human being the positive conditional.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index edfa0450e605..3e8522d522b4 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1466,7 +1466,7 @@ EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
  */
 const char *extcon_get_edev_name(struct extcon_dev *edev)
 {
-	return !edev ? NULL : edev->name;
+	return edev ? edev->name : NULL;
 }
 EXPORT_SYMBOL_GPL(extcon_get_edev_name);
 
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Andy Shevchenko]

On Wed, Mar 22, 2023 at 04:39:51PM +0200, Andy Shevchenko wrote:
> A few fixes to the documentation and some cleanups against extcon core
> module.

Anything I should do with the series?
Any comments on it?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Bumwoo Lee]

Hi Andy Shevchenko
> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, March 30, 2023 7:12 PM
> To: Bumwoo Lee <bw365.lee@samsung.com>; linux-kernel@vger.kernel.org
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>; Chanwoo Choi
> <cw00.choi@samsung.com>
> Subject: Re: [PATCH v1 00/14] extcon: Core cleanups and documentation
> fixes
> 
> On Wed, Mar 22, 2023 at 04:39:51PM +0200, Andy Shevchenko wrote:
> > A few fixes to the documentation and some cleanups against extcon core
> > module.
> 
> Anything I should do with the series?
> Any comments on it?
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Looks fine to me.

Acked-by: Bumwoo Lee <bw365.lee@samsung.com>

MR. Chanwoo, Would you please take a look at this patch series.

Re: [PATCH v1 01/14] extcon: Fix kernel doc of property fields to avoid warnings

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Kernel documentation has to be synchronized with a code, otherwise
> the validator is not happy:
> 
>      Function parameter or member 'usb_propval' not described in 'extcon_cable'
>      Function parameter or member 'chg_propval' not described in 'extcon_cable'
>      Function parameter or member 'jack_propval' not described in 'extcon_cable'
>      Function parameter or member 'disp_propval' not described in 'extcon_cable'
> 
> Describe the fields added in the past.
> 
> Fixes: 067c1652e7a7 ("extcon: Add the support for extcon property according to extcon type")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 3997b39680b7..1bc2639704c2 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -206,6 +206,10 @@ static const struct __extcon_info {
>   * @attr_name:		"name" sysfs entry
>   * @attr_state:		"state" sysfs entry
>   * @attrs:		the array pointing to attr_name and attr_state for attr_g
> + * @usb_propval:	the array of USB connector properties
> + * @chg_propval:	the array of charger connector properties
> + * @jack_propval:	the array of jack connector properties
> + * @disp_propval:	the array of display connector properties
>   */
>  struct extcon_cable {
>  	struct extcon_dev *edev;

Applied it. Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 02/14] extcon: Fix kernel doc of property capability fields to avoid warnings

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Kernel documentation has to be synchronized with a code, otherwise
> the validator is not happy:
> 
>      Function parameter or member 'usb_bits' not described in 'extcon_cable'
>      Function parameter or member 'chg_bits' not described in 'extcon_cable'
>      Function parameter or member 'jack_bits' not described in 'extcon_cable'
>      Function parameter or member 'disp_bits' not described in 'extcon_cable'
> 
> Describe the fields added in the past.
> 
> Fixes: ceaa98f442cf ("extcon: Add the support for the capability of each property")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 1bc2639704c2..79006ab5334b 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -210,6 +210,10 @@ static const struct __extcon_info {
>   * @chg_propval:	the array of charger connector properties
>   * @jack_propval:	the array of jack connector properties
>   * @disp_propval:	the array of display connector properties
> + * @usb_bits:		the bit array of the USB connector property capabilities
> + * @chg_bits:		the bit array of the charger connector property capabilities
> + * @jack_bits:		the bit array of the jack connector property capabilities
> + * @disp_bits:		the bit array of the display connector property capabilities
>   */
>  struct extcon_cable {
>  	struct extcon_dev *edev;

Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 06/14] extcon: Allow name to be assigned outside of the framework

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> The documentation states that name of the extcon can be assigned
> outside of the framework, however code does something different.
> Fix the code to be aligned with the documentation.

Actually, it is not possible to initialize the name outside of the framework
because commit 20f7b53dfc24 ("extcon: Move struct extcon_cable from header file to core")
moved the 'struct extcon_dev' into drivers/extcon/extcon.c in order to prevent
the direct accessing of struct extcon_dev.

Instead of this patch, need to change the description of struct extcon_dev.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index ac84f4aafc69..14e66e21597f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1269,10 +1269,10 @@ int extcon_dev_register(struct extcon_dev *edev)
>  	edev->dev.class = extcon_class;
>  	edev->dev.release = extcon_dev_release;
>  
> -	edev->name = dev_name(edev->dev.parent);
> -	if (IS_ERR_OR_NULL(edev->name)) {
> -		dev_err(&edev->dev,
> -			"extcon device name is null\n");
> +	if (!edev->name)
> +		edev->name = dev_name(edev->dev.parent);
> +	if (!edev->name) {
> +		dev_err(&edev->dev, "extcon device name is null\n");
>  		return -EINVAL;
>  	}
>  	dev_set_name(&edev->dev, "extcon%lu",

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Chanwoo Choi]

Hi,

I recommend that use the "./scripts/get_maintainer.pl" script
to get the accurate maintainer list to send the patches.


On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> A few fixes to the documentation and some cleanups against extcon core
> module.
> 
> Andy Shevchenko (14):
>   extcon: Fix kernel doc of property fields to avoid warnings
>   extcon: Fix kernel doc of property capability fields to avoid warnings
>   extcon: Use DECLARE_BITMAP() to declare bit arrays
>   extcon: use sysfs_emit() to instead of sprintf()
>   extcon: Amend kernel documentation of struct extcon_dev
>   extcon: Allow name to be assigned outside of the framework
>   extcon: Use unique number for the extcon device ID
>   extcon: Switch to use kasprintf_strarray()
>   extcon: Use device_match_of_node() helper
>   extcon: use dev_of_node(dev) instead of dev->of_node
>   extcon: Remove dup device name in the message and unneeded error check
>   extcon: Use sizeof(*pointer) instead of sizeof(type)
>   extcon: Drop unneeded assignments
>   extcon: Use positive conditional in ternary operator
> 
>  drivers/extcon/extcon.c | 126 +++++++++++++++++++++-------------------
>  drivers/extcon/extcon.h |   9 ++-
>  2 files changed, 71 insertions(+), 64 deletions(-)
> 


-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 03/14] extcon: Use DECLARE_BITMAP() to declare bit arrays

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Bit arrays has a specific type helper for the declaration.
> Use it instead of homegronw equivalent.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 79006ab5334b..70e9755ba2bc 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -230,10 +230,10 @@ struct extcon_cable {
>  	union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
>  	union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  
> -	unsigned long usb_bits[BITS_TO_LONGS(EXTCON_PROP_USB_CNT)];
> -	unsigned long chg_bits[BITS_TO_LONGS(EXTCON_PROP_CHG_CNT)];
> -	unsigned long jack_bits[BITS_TO_LONGS(EXTCON_PROP_JACK_CNT)];
> -	unsigned long disp_bits[BITS_TO_LONGS(EXTCON_PROP_DISP_CNT)];
> +	DECLARE_BITMAP(usb_bits, EXTCON_PROP_USB_CNT);
> +	DECLARE_BITMAP(chg_bits, EXTCON_PROP_CHG_CNT);
> +	DECLARE_BITMAP(jack_bits, EXTCON_PROP_JACK_CNT);
> +	DECLARE_BITMAP(disp_bits, EXTCON_PROP_DISP_CNT);
>  };
>  
>  static struct class *extcon_class;

Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 04/14] extcon: use sysfs_emit() to instead of sprintf()

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Follow the advice of the Documentation/filesystems/sysfs.rst that
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 70e9755ba2bc..ac84f4aafc69 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -370,12 +370,12 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>  	struct extcon_dev *edev = dev_get_drvdata(dev);
>  
>  	if (edev->max_supported == 0)
> -		return sprintf(buf, "%u\n", edev->state);
> +		return sysfs_emit(buf, "%u\n", edev->state);
>  
>  	for (i = 0; i < edev->max_supported; i++) {
> -		count += sprintf(buf + count, "%s=%d\n",
> -				extcon_info[edev->supported_cable[i]].name,
> -				 !!(edev->state & BIT(i)));
> +		count += sysfs_emit_at(buf, count, "%s=%d\n",
> +				       extcon_info[edev->supported_cable[i]].name,
> +				       !!(edev->state & BIT(i)));
>  	}
>  
>  	return count;
> @@ -387,7 +387,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
>  {
>  	struct extcon_dev *edev = dev_get_drvdata(dev);
>  
> -	return sprintf(buf, "%s\n", edev->name);
> +	return sysfs_emit(buf, "%s\n", edev->name);
>  }
>  static DEVICE_ATTR_RO(name);
>  
> @@ -398,8 +398,8 @@ static ssize_t cable_name_show(struct device *dev,
>  						  attr_name);
>  	int i = cable->cable_index;
>  
> -	return sprintf(buf, "%s\n",
> -			extcon_info[cable->edev->supported_cable[i]].name);
> +	return sysfs_emit(buf, "%s\n",
> +			  extcon_info[cable->edev->supported_cable[i]].name);
>  }
>  
>  static ssize_t cable_state_show(struct device *dev,
> @@ -410,8 +410,8 @@ static ssize_t cable_state_show(struct device *dev,
>  
>  	int i = cable->cable_index;
>  
> -	return sprintf(buf, "%d\n",
> -		extcon_get_state(cable->edev, cable->edev->supported_cable[i]));
> +	return sysfs_emit(buf, "%d\n",
> +			  extcon_get_state(cable->edev, cable->edev->supported_cable[i]));
>  }
>  
>  /**

Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 05/14] extcon: Amend kernel documentation of struct extcon_dev

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> First of all, the @lock description is missing. Add it.
> Second, correct the terminator value for the mutual exclusive
> cabling.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
> index 93b5e0306966..15616446140d 100644
> --- a/drivers/extcon/extcon.h
> +++ b/drivers/extcon/extcon.h
> @@ -13,8 +13,8 @@
>   *			are disabled.
>   * @mutually_exclusive:	Array of mutually exclusive set of cables that cannot
>   *			be attached simultaneously. The array should be
> - *			ending with NULL or be NULL (no mutually exclusive
> - *			cables). For example, if it is { 0x7, 0x30, 0}, then,
> + *			ending with 0 or be NULL (no mutually exclusive cables).
> + *			For example, if it is {0x7, 0x30, 0}, then,
>   *			{0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot
>   *			be attached simulataneously. {0x7, 0} is equivalent to
>   *			{0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
> @@ -27,7 +27,7 @@
>   * @nh:			Notifier for the state change events from this extcon
>   * @entry:		To support list of extcon devices so that users can
>   *			search for extcon devices based on the extcon name.
> - * @lock:
> + * @lock:		Protects device state and serialises device registration
>   * @max_supported:	Internal value to store the number of cables.
>   * @extcon_dev_type:	Device_type struct to provide attribute_groups
>   *			customized for each extcon device.


Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 09/14] extcon: Use device_match_of_node() helper

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> Instead of open coding, use device_match_of_node() helper.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index a63e7eef02fd..5cadbfc151e6 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1411,7 +1411,7 @@ struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
>  
>  	mutex_lock(&extcon_dev_list_lock);
>  	list_for_each_entry(edev, &extcon_dev_list, entry)
> -		if (edev->dev.parent && edev->dev.parent->of_node == node)
> +		if (edev->dev.parent && device_match_of_node(edev->dev.parent, node))
>  			goto out;
>  	edev = ERR_PTR(-EPROBE_DEFER);
>  out:

Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 10/14] extcon: use dev_of_node(dev) instead of dev->of_node

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> The dev_of_node function should be preferred.
> In the result we may drop unneeded NULL check
> of the pointer to the device object.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 5cadbfc151e6..32e96cb49067 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1429,21 +1429,17 @@ struct extcon_dev *extcon_find_edev_by_node(struct device_node *node)
>   */
>  struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
>  {
> -	struct device_node *node;
> +	struct device_node *node, *np = dev_of_node(dev);
>  	struct extcon_dev *edev;
>  
> -	if (!dev)
> -		return ERR_PTR(-EINVAL);
> -
> -	if (!dev->of_node) {
> +	if (!np) {
>  		dev_dbg(dev, "device does not have a device node entry\n");
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	node = of_parse_phandle(dev->of_node, "extcon", index);
> +	node = of_parse_phandle(np, "extcon", index);
>  	if (!node) {
> -		dev_dbg(dev, "failed to get phandle in %pOF node\n",
> -			dev->of_node);
> +		dev_dbg(dev, "failed to get phandle in %pOF node\n", np);
>  		return ERR_PTR(-ENODEV);
>  	}
>  

Applied it with the following patch title.
Just use capital letter at the beginning char of title

- extcon: Use dev_of_node(dev) instead of dev->of_node

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 12/14] extcon: Use sizeof(*pointer) instead of sizeof(type)

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> 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 | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 0e04ea185c26..b3f038639cd6 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1099,9 +1099,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),
> -			       GFP_KERNEL);
> +	edev->cables = kcalloc(edev->max_supported, sizeof(*edev->cables), GFP_KERNEL);

Even if there are strictly not limitation of the number of maximum char
at the one line, But, I recommend to keep the 80 char at the one line
for the readability as following.

-       edev->cables = kcalloc(edev->max_supported, sizeof(*edev->cables), GFP_KERNEL);
+       edev->cables = kcalloc(edev->max_supported,
+                       sizeof(*edev->cables), GFP_KERNEL);


>  	if (!edev->cables)
>  		return -ENOMEM;
>  
> @@ -1160,14 +1158,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);
> @@ -1213,8 +1209,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);

ditto.

        edev->extcon_dev_type.groups = kcalloc(edev->max_supported + 2,
-                                              sizeof(*edev->extcon_dev_type.groups),
-                                              GFP_KERNEL);
+                                      sizeof(*edev->extcon_dev_type.groups),
+                                      GFP_KERNEL);


>  	if (!edev->extcon_dev_type.groups)
>  		return -ENOMEM;
>  
> @@ -1298,8 +1294,7 @@ int extcon_dev_register(struct extcon_dev *edev)
>  
>  	spin_lock_init(&edev->lock);
>  	if (edev->max_supported) {
> -		edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh),
> -				GFP_KERNEL);
> +		edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh), GFP_KERNEL);

-               edev->nh = kcalloc(edev->max_supported, sizeof(*edev->nh), GFP_KERNEL);
+               edev->nh = kcalloc(edev->max_supported,
+                               sizeof(*edev->nh), GFP_KERNEL);


>  		if (!edev->nh) {
>  			ret = -ENOMEM;
>  			goto err_alloc_nh;

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 14/14] extcon: Use positive conditional in ternary operator

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> It's easier to parse by a human being the positive conditional.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index edfa0450e605..3e8522d522b4 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1466,7 +1466,7 @@ EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
>   */
>  const char *extcon_get_edev_name(struct extcon_dev *edev)
>  {
> -	return !edev ? NULL : edev->name;
> +	return edev ? edev->name : NULL;
>  }
>  EXPORT_SYMBOL_GPL(extcon_get_edev_name);
>  

It is not fix-up patch and I'm not sure that it is beneficial patch. 
I will not apply it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 07/14] extcon: Use unique number for the extcon device ID

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> 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 | 15 ++++++++++++---
>  drivers/extcon/extcon.h |  2 ++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 14e66e21597f..0d261ec6c473 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)
>  		dev_err(&edev->dev, "extcon device name is null\n");
>  		return -EINVAL;
>  	}
> -	dev_set_name(&edev->dev, "extcon%lu",
> -			(unsigned long)atomic_inc_return(&edev_no));
> +
> +	ret = ida_simple_get(&extcon_dev_ids, 0, INT_MAX, GFP_KERNEL);


ida_simple_get and ida_simple_remove are deprecated on 
commit 3264ceec8f17 ("lib/idr.c: document that ida_simple_{get,remove}()
are deprecated"). Instead of them, better to use ida_alloc and ida_free 
according to the comments.


> +	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)
> @@ -1368,6 +1375,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  		return;
>  	}
>  
> +	ida_simple_remove(&extcon_dev_ids, edev->id);

ditto.

> +
>  	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..877c0860e300 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;
> +	int id;
>  	struct raw_notifier_head nh_all;
>  	struct raw_notifier_head *nh;
>  	struct list_head entry;

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 08/14] extcon: Switch to use kasprintf_strarray()

[Chanwoo Choi]

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Since we have a generic helper, switch the module to use it.
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 25 +++++++++++--------------
>  drivers/extcon/extcon.h |  1 +
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 0d261ec6c473..a63e7eef02fd 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -23,6 +23,7 @@
>  #include <linux/err.h>
>  #include <linux/of.h>
>  #include <linux/slab.h>
> +#include <linux/string_helpers.h>
>  #include <linux/sysfs.h>
>  
>  #include "extcon.h"
> @@ -1104,19 +1105,17 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
>  	if (!edev->cables)
>  		return -ENOMEM;
>  
> +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> +	if (!edev->cable_names) {
> +		kfree(edev->cables);
> +		return -ENOMEM;
> +	}
> +
>  	for (index = 0; index < edev->max_supported; index++) {
>  		cable = &edev->cables[index];
>  
> -		str = kasprintf(GFP_KERNEL, "cable.%d", index);
> -		if (!str) {
> -			for (index--; index >= 0; index--) {
> -				cable = &edev->cables[index];
> -				kfree(cable->attr_g.name);
> -			}
> -
> -			kfree(edev->cables);
> -			return -ENOMEM;
> -		}
> +		str = edev->cable_names[index];
> +		strreplace(str, '-', '.');
>  
>  		cable->edev = edev;
>  		cable->cable_index = index;
> @@ -1341,8 +1340,7 @@ int extcon_dev_register(struct extcon_dev *edev)
>  		kfree(edev->attrs_muex);
>  	}
>  err_alloc_muex:
> -	for (index = 0; index < edev->max_supported; index++)
> -		kfree(edev->cables[index].attr_g.name);
> +	kfree_strarray(edev->cable_names, edev->max_supported);
>  	if (edev->max_supported)
>  		kfree(edev->cables);
>  err_alloc_cables:
> @@ -1387,8 +1385,7 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  		kfree(edev->attrs_muex);
>  	}
>  
> -	for (index = 0; index < edev->max_supported; index++)
> -		kfree(edev->cables[index].attr_g.name);
> +	kfree_strarray(edev->cable_names, edev->max_supported);
>  
>  	if (edev->max_supported) {
>  		kfree(edev->extcon_dev_type.groups);
> diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
> index 877c0860e300..5624f65ba17a 100644
> --- a/drivers/extcon/extcon.h
> +++ b/drivers/extcon/extcon.h
> @@ -58,6 +58,7 @@ struct extcon_dev {
>  	/* /sys/class/extcon/.../cable.n/... */
>  	struct device_type extcon_dev_type;
>  	struct extcon_cable *cables;
> +	char **cable_names;

The extcon cable information should be included in struct extcon_cable
in order to gather information into one point like encapsulation.

I don't prefer to add 'cable_names'.

>  
>  	/* /sys/class/extcon/.../mutually_exclusive/... */
>  	struct attribute_group attr_g_muex;

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 11/14] extcon: Remove dup device name in the message and unneeded error check

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> The device name is already printed with dev_err(), no need to repeat.
> The device pointer itself is not supposed to be an error point, drop
> that check.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 32e96cb49067..0e04ea185c26 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1367,9 +1367,8 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  	list_del(&edev->entry);
>  	mutex_unlock(&extcon_dev_list_lock);
>  
> -	if (IS_ERR_OR_NULL(get_device(&edev->dev))) {
> -		dev_err(&edev->dev, "Failed to unregister extcon_dev (%s)\n",
> -				dev_name(&edev->dev));
> +	if (!get_device(&edev->dev)) {
> +		dev_err(&edev->dev, "Failed to unregister extcon_dev\n");
>  		return;
>  	}
>  

Applied it. Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 13/14] extcon: Drop unneeded assignments

[Chanwoo Choi]

On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> 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>
> ---
>  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 b3f038639cd6..edfa0450e605 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -246,7 +246,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;
> @@ -1244,7 +1244,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)
> @@ -1253,7 +1253,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) {

It has the dependency with patch7. When I try to apply it, the conflict happen.
When you are sending v2 patches, I'll apply it if there are no conflict.
[1] [PATCH v1 07/14] extcon: Use unique number for the extcon device ID

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

Re: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Andy Shevchenko]

On Mon, Apr 03, 2023 at 10:58:20PM +0900, Chanwoo Choi wrote:
> Hi,
> 
> I recommend that use the "./scripts/get_maintainer.pl" script
> to get the accurate maintainer list to send the patches.

That's what I'm using. What's wrong in your opinion with the Cc and/or To
lists?

> On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> > A few fixes to the documentation and some cleanups against extcon core
> > module.
> > 
> > Andy Shevchenko (14):
> >   extcon: Fix kernel doc of property fields to avoid warnings
> >   extcon: Fix kernel doc of property capability fields to avoid warnings
> >   extcon: Use DECLARE_BITMAP() to declare bit arrays
> >   extcon: use sysfs_emit() to instead of sprintf()
> >   extcon: Amend kernel documentation of struct extcon_dev
> >   extcon: Allow name to be assigned outside of the framework
> >   extcon: Use unique number for the extcon device ID
> >   extcon: Switch to use kasprintf_strarray()
> >   extcon: Use device_match_of_node() helper
> >   extcon: use dev_of_node(dev) instead of dev->of_node
> >   extcon: Remove dup device name in the message and unneeded error check
> >   extcon: Use sizeof(*pointer) instead of sizeof(type)
> >   extcon: Drop unneeded assignments
> >   extcon: Use positive conditional in ternary operator

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 08/14] extcon: Switch to use kasprintf_strarray()

[Andy Shevchenko]

On Mon, Apr 03, 2023 at 11:58:41PM +0900, Chanwoo Choi wrote:
> On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> > Since we have a generic helper, switch the module to use it.
> > No functional change intended.

...

> > +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> > +	if (!edev->cable_names) {
> > +		kfree(edev->cables);
> > +		return -ENOMEM;
> > +	}
> > +
> >  	for (index = 0; index < edev->max_supported; index++) {
> >  		cable = &edev->cables[index];
> >  

> > +		str = edev->cable_names[index];
> > +		strreplace(str, '-', '.');
> >  
> >  		cable->edev = edev;
> >  		cable->cable_index = index;

...

> >  	/* /sys/class/extcon/.../cable.n/... */
> >  	struct device_type extcon_dev_type;
> >  	struct extcon_cable *cables;
> > +	char **cable_names;
> 
> The extcon cable information should be included in struct extcon_cable
> in order to gather information into one point like encapsulation.
> 
> I don't prefer to add 'cable_names'.

I don't get this. The idea is to allocate the cable names in one call,
we have already API for that. The cable names are kept in the struct
extcon_cable as before. So, functionally it doesn't change anything,
it is a simple cleanup.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 14/14] extcon: Use positive conditional in ternary operator

[Andy Shevchenko]

On Mon, Apr 03, 2023 at 11:38:41PM +0900, Chanwoo Choi wrote:
> On 23. 3. 22. 23:40, Andy Shevchenko wrote:
> > It's easier to parse by a human being the positive conditional.

...

> >  const char *extcon_get_edev_name(struct extcon_dev *edev)
> >  {
> > -	return !edev ? NULL : edev->name;
> > +	return edev ? edev->name : NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(extcon_get_edev_name);
> 
> It is not fix-up patch and I'm not sure that it is beneficial patch.
> I will not apply it.

It improves the cognitive perception of the code, but I'm fine with
no patch being applied.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 07/14] extcon: Use unique number for the extcon device ID

[Andy Shevchenko]

On Mon, Apr 03, 2023 at 11:52:46PM +0900, Chanwoo Choi wrote:
> On 23. 3. 22. 23:39, Andy Shevchenko wrote:

...

> > +	ret = ida_simple_get(&extcon_dev_ids, 0, INT_MAX, GFP_KERNEL);
> 
> 
> ida_simple_get and ida_simple_remove are deprecated on 
> commit 3264ceec8f17 ("lib/idr.c: document that ida_simple_{get,remove}()
> are deprecated"). Instead of them, better to use ida_alloc and ida_free 
> according to the comments.

Done for v2.

...

> > +	ida_simple_remove(&extcon_dev_ids, edev->id);
> 
> ditto.

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 08/14] extcon: Switch to use kasprintf_strarray()

[Andy Shevchenko]

On Wed, Apr 05, 2023 at 11:16:36AM +0300, Andy Shevchenko wrote:
> On Mon, Apr 03, 2023 at 11:58:41PM +0900, Chanwoo Choi wrote:
> > On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> > > Since we have a generic helper, switch the module to use it.
> > > No functional change intended.

...

> > > +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> > > +	if (!edev->cable_names) {
> > > +		kfree(edev->cables);
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > >  	for (index = 0; index < edev->max_supported; index++) {
> > >  		cable = &edev->cables[index];
> > >  
> 
> > > +		str = edev->cable_names[index];
> > > +		strreplace(str, '-', '.');
> > >  
> > >  		cable->edev = edev;
> > >  		cable->cable_index = index;
> 
> ...
> 
> > >  	/* /sys/class/extcon/.../cable.n/... */
> > >  	struct device_type extcon_dev_type;
> > >  	struct extcon_cable *cables;
> > > +	char **cable_names;
> > 
> > The extcon cable information should be included in struct extcon_cable
> > in order to gather information into one point like encapsulation.
> > 
> > I don't prefer to add 'cable_names'.
> 
> I don't get this. The idea is to allocate the cable names in one call,
> we have already API for that. The cable names are kept in the struct
> extcon_cable as before. So, functionally it doesn't change anything,
> it is a simple cleanup.

Okay, I see now your point. I can redo this a bit better I think.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Chanwoo Choi]

Hi,

On 23. 3. 31. 09:48, Bumwoo Lee wrote:
> Hi Andy Shevchenko
>> -----Original Message-----
>> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Sent: Thursday, March 30, 2023 7:12 PM
>> To: Bumwoo Lee <bw365.lee@samsung.com>; linux-kernel@vger.kernel.org
>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>; Chanwoo Choi
>> <cw00.choi@samsung.com>
>> Subject: Re: [PATCH v1 00/14] extcon: Core cleanups and documentation
>> fixes
>>
>> On Wed, Mar 22, 2023 at 04:39:51PM +0200, Andy Shevchenko wrote:
>>> A few fixes to the documentation and some cleanups against extcon core
>>> module.
>>
>> Anything I should do with the series?
>> Any comments on it?
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>>
> 
> Looks fine to me.
> 
> Acked-by: Bumwoo Lee <bw365.lee@samsung.com>
> 
> MR. Chanwoo, Would you please take a look at this patch series.
> 

Actually, Acked tag will be replied by Maintainer or the driver owner.
If you want to review the mailing list patch, I think that Reviewed-by tag is proper.

Unfortunately, I could not see the any review comment from you even if this patchset
have the some review contents. Also you didn't review the any patches of extcon before.

I'm always welcome for many reviewers in order to improve the linux kernel.
But, in this case, I'm not sure that you are reviewing this patchset.

So that I'm sorry that I cannot take your acked-tag. 

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

RE: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

[Bumwoo Lee]

Hi

> -----Original Message-----
> From: Chanwoo Choi <cwchoi00@gmail.com>
> Sent: Thursday, April 6, 2023 8:17 AM
> To: Bumwoo Lee <bw365.lee@samsung.com>; 'Andy Shevchenko'
> <andriy.shevchenko@linux.intel.com>; linux-kernel@vger.kernel.org
> Cc: 'MyungJoo Ham' <myungjoo.ham@samsung.com>; 'Chanwoo Choi'
> <cw00.choi@samsung.com>
> Subject: Re: [PATCH v1 00/14] extcon: Core cleanups and documentation
> fixes
> 
> Hi,
> 
> On 23. 3. 31. 09:48, Bumwoo Lee wrote:
> > Hi Andy Shevchenko
> >> -----Original Message-----
> >> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Sent: Thursday, March 30, 2023 7:12 PM
> >> To: Bumwoo Lee <bw365.lee@samsung.com>; linux-kernel@vger.kernel.org
> >> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>; Chanwoo Choi
> >> <cw00.choi@samsung.com>
> >> Subject: Re: [PATCH v1 00/14] extcon: Core cleanups and documentation
> >> fixes
> >>
> >> On Wed, Mar 22, 2023 at 04:39:51PM +0200, Andy Shevchenko wrote:
> >>> A few fixes to the documentation and some cleanups against extcon
> >>> core module.
> >>
> >> Anything I should do with the series?
> >> Any comments on it?
> >>
> >> --
> >> With Best Regards,
> >> Andy Shevchenko
> >>
> >
> > Looks fine to me.
> >
> > Acked-by: Bumwoo Lee <bw365.lee@samsung.com>
> >
> > MR. Chanwoo, Would you please take a look at this patch series.
> >
> 
> Actually, Acked tag will be replied by Maintainer or the driver owner.
> If you want to review the mailing list patch, I think that Reviewed-by tag
> is proper.
> 

OK.
Thank you for your detail guidance. I will follow this guidance on next time.

> Unfortunately, I could not see the any review comment from you even if
> this patchset have the some review contents. Also you didn't review the
> any patches of extcon before.
> 
> I'm always welcome for many reviewers in order to improve the linux kernel.
> But, in this case, I'm not sure that you are reviewing this patchset.
> 
> So that I'm sorry that I cannot take your acked-tag.
> 

I agree your comment.
Thank you~

> --
> Best Regards,
> Samsung Electronics
> Chanwoo Choi

Re: [PATCH v1 00/14] extcon: Core cleanups and documentation fixes

['Andy Shevchenko']

On Thu, Apr 06, 2023 at 08:17:19AM +0900, Chanwoo Choi wrote:
> On 23. 3. 31. 09:48, Bumwoo Lee wrote:

...

> > Looks fine to me.
> > 
> > Acked-by: Bumwoo Lee <bw365.lee@samsung.com>
> > 
> > MR. Chanwoo, Would you please take a look at this patch series.
> 
> Actually, Acked tag will be replied by Maintainer or the driver owner.  If
> you want to review the mailing list patch, I think that Reviewed-by tag is
> proper.
> 
> Unfortunately, I could not see the any review comment from you even if this
> patchset have the some review contents. Also you didn't review the any
> patches of extcon before.
> 
> I'm always welcome for many reviewers in order to improve the linux kernel.
> But, in this case, I'm not sure that you are reviewing this patchset.
> 
> So that I'm sorry that I cannot take your acked-tag. 

I probably need to update this in the v3 of the rest of the patches I have sent
as v2. Btw, can you review those before, so if any comments I can address in
v3?

-- 
With Best Regards,
Andy Shevchenko