This patchset update the extcon core to maintain the consistency of device name
on sysfs and add the extcon_get_edev_name() API to access the filed of 'struct
extcon_dev' by only extcon core API. And this patchset fix the checkpat warning
and minor coding style issue.
Chanwoo Choi (4):
extcon: Modify the device name as extcon[X] for sysfs
extcon: Add extcon_get_edev_name() API to get the extcon device name
extcon: Fix the checkpatch warning and minor coding style issue
extcon: arizona: Remove the setting of device name
drivers/extcon/extcon-arizona.c | 1 -
drivers/extcon/extcon.c | 27 +++++++++++++++++++--------
include/linux/extcon.h | 11 ++++++++---
3 files changed, 27 insertions(+), 12 deletions(-)
--
1.8.5.5
This patch modify the device name as extcon[X] for sysfs by using the 'extcon'
prefix word instead of separate device name. On user-space aspect, user would
find the some extcon drvier with extcon[X] pattern. So, this patch modify the
device name as following:
- /sys/class/extcon/[device name] -> /sys/class/extcon/extcon[X]
Signed-off-by: Chanwoo Choi <[email protected]>
---
drivers/extcon/extcon.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 4c9f165..1a93229 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -163,7 +163,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
return ret;
}
- return sprintf(buf, "%s\n", dev_name(&edev->dev));
+ return sprintf(buf, "%s\n", edev->name);
}
static DEVICE_ATTR_RO(name);
@@ -701,6 +701,7 @@ EXPORT_SYMBOL_GPL(devm_extcon_dev_free);
int extcon_dev_register(struct extcon_dev *edev)
{
int ret, index = 0;
+ static atomic_t edev_no = ATOMIC_INIT(-1);
if (!extcon_class) {
ret = create_extcon_class();
@@ -725,13 +726,14 @@ int extcon_dev_register(struct extcon_dev *edev)
edev->dev.class = extcon_class;
edev->dev.release = extcon_dev_release;
- edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
+ edev->name = dev_name(edev->dev.parent);
if (IS_ERR_OR_NULL(edev->name)) {
dev_err(&edev->dev,
"extcon device name is null\n");
return -EINVAL;
}
- dev_set_name(&edev->dev, "%s", edev->name);
+ dev_set_name(&edev->dev, "extcon%lu",
+ (unsigned long)atomic_inc_return(&edev_no));
if (edev->max_supported) {
char buf[10];
--
1.8.5.5
This patch adds the extcon_get_edev_name() API to get the name of extcon device
because all information inclued in the structure extcon_dev should be accessed
by extcon core API instead of directly accessing the data.
Signed-off-by: Chanwoo Choi <[email protected]>
---
drivers/extcon/extcon.c | 9 +++++++++
include/linux/extcon.h | 4 ++++
2 files changed, 13 insertions(+)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 1a93229..c5e9ebf 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1046,6 +1046,15 @@ struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index)
#endif /* CONFIG_OF */
EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
+/**
+ * extcon_get_edev_name() - Get the name of the extcon device.
+ * @edev: the extcon device
+ */
+const char *extcon_get_edev_name(struct extcon_dev *edev)
+{
+ return !edev ? NULL : edev->name;
+}
+
static int __init extcon_class_init(void)
{
return create_extcon_class();
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 36f49c4..e2cf625 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -259,6 +259,10 @@ extern int extcon_unregister_notifier(struct extcon_dev *edev,
* This function use phandle of devicetree to get extcon device directly.
*/
extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index);
+
+/* Following API to get information of extcon device */
+extern const char *extcon_get_edev_name(struct extcon_dev *edev);
+
#else /* CONFIG_EXTCON */
static inline int extcon_dev_register(struct extcon_dev *edev)
{
--
1.8.5.5
This patch clean up the extcon core driver by fixing the checkpatch warning
and minor coding style issue.
Signed-off-by: Chanwoo Choi <[email protected]>
---
drivers/extcon/extcon.c | 10 +++++-----
include/linux/extcon.h | 7 ++++---
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index c5e9ebf..2fb5f75 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1,5 +1,5 @@
/*
- * drivers/extcon/extcon_class.c
+ * drivers/extcon/extcon.c - External Connector (extcon) framework.
*
* External connector (extcon) class driver
*
@@ -19,8 +19,7 @@
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
- *
-*/
+ */
#include <linux/module.h>
#include <linux/types.h>
@@ -469,7 +468,6 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
ret = raw_notifier_chain_register(&obj->edev->nh,
&obj->internal_nb);
spin_unlock_irqrestore(&obj->edev->lock, flags);
- return ret;
} else {
struct class_dev_iter iter;
struct extcon_dev *extd;
@@ -489,8 +487,10 @@ int extcon_register_interest(struct extcon_specific_cable_nb *obj,
cable_name, nb);
}
- return -ENODEV;
+ ret = -ENODEV;
}
+
+ return ret;
}
EXPORT_SYMBOL_GPL(extcon_register_interest);
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index e2cf625..799474d9d 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -97,8 +97,8 @@ struct extcon_cable;
* @state: Attach/detach state of this extcon. Do not provide at
* register-time.
* @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.
+ * @entry: To support list of extcon devices so that users can
+ * search for extcon devices based on the extcon name.
* @lock:
* @max_supported: Internal value to store the number of cables.
* @extcon_dev_type: Device_type struct to provide attribute_groups
@@ -258,7 +258,8 @@ extern int extcon_unregister_notifier(struct extcon_dev *edev,
* Following API get the extcon device from devicetree.
* This function use phandle of devicetree to get extcon device directly.
*/
-extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev, int index);
+extern struct extcon_dev *extcon_get_edev_by_phandle(struct device *dev,
+ int index);
/* Following API to get information of extcon device */
extern const char *extcon_get_edev_name(struct extcon_dev *edev);
--
1.8.5.5
This patch removes the setting of device name. Instead, extcon_dev_register()
set the device name such as 'extcon[number]' naming method.
- /sys/class/extcon/Headset Jack -> /sys/class/extcon/extcon[number]
Cc: Charles Keepax <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
---
drivers/extcon/extcon-arizona.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index a0ed35b..363e6b8 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1185,7 +1185,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to allocate extcon device\n");
return -ENOMEM;
}
- info->edev->name = "Headset Jack";
ret = devm_extcon_dev_register(&pdev->dev, info->edev);
if (ret < 0) {
--
1.8.5.5
On Mon, Apr 27, 2015 at 09:13:35PM +0900, Chanwoo Choi wrote:
> This patch modify the device name as extcon[X] for sysfs by using the 'extcon'
> prefix word instead of separate device name. On user-space aspect, user would
> find the some extcon drvier with extcon[X] pattern. So, this patch modify the
> device name as following:
> - /sys/class/extcon/[device name] -> /sys/class/extcon/extcon[X]
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
> drivers/extcon/extcon.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 4c9f165..1a93229 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -163,7 +163,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
> return ret;
> }
>
> - return sprintf(buf, "%s\n", dev_name(&edev->dev));
> + return sprintf(buf, "%s\n", edev->name);
> }
> static DEVICE_ATTR_RO(name);
>
> @@ -701,6 +701,7 @@ EXPORT_SYMBOL_GPL(devm_extcon_dev_free);
> int extcon_dev_register(struct extcon_dev *edev)
> {
> int ret, index = 0;
> + static atomic_t edev_no = ATOMIC_INIT(-1);
>
> if (!extcon_class) {
> ret = create_extcon_class();
> @@ -725,13 +726,14 @@ int extcon_dev_register(struct extcon_dev *edev)
> edev->dev.class = extcon_class;
> edev->dev.release = extcon_dev_release;
>
> - edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
> + edev->name = dev_name(edev->dev.parent);
> if (IS_ERR_OR_NULL(edev->name)) {
> dev_err(&edev->dev,
> "extcon device name is null\n");
> return -EINVAL;
> }
> - dev_set_name(&edev->dev, "%s", edev->name);
> + dev_set_name(&edev->dev, "extcon%lu",
> + (unsigned long)atomic_inc_return(&edev_no));
>
> if (edev->max_supported) {
> char buf[10];
> --
> 1.8.5.5
>
I am not quite sure I see the advantage of this. Why is naming
the node extcon[X] better than the old system? Seems like the
older more descriptive names are better on the face of it, unless
there is some problem with them I am missing.
It also looks problematic, it changes the ABI for the Arizona
extcon driver for a start, admittedly there arn't many users for
the extcon driver at the moment but still is far from ideal to
break the ABI. Secondly, the order of the extcon[X]'s will not be
guaranteed, add some new hardware to the system probe order
changes and now what was extcon0 is extcon1. So it somewhat
complicates the user-space code as it now has to work out if the
device is the one it wants whereas before it could just hard-code
the name.
Thanks,
Charles
Hi Charles,
On Tue, Apr 28, 2015 at 11:17 PM, Charles Keepax
<[email protected]> wrote:
> On Mon, Apr 27, 2015 at 09:13:35PM +0900, Chanwoo Choi wrote:
>> This patch modify the device name as extcon[X] for sysfs by using the 'extcon'
>> prefix word instead of separate device name. On user-space aspect, user would
>> find the some extcon drvier with extcon[X] pattern. So, this patch modify the
>> device name as following:
>> - /sys/class/extcon/[device name] -> /sys/class/extcon/extcon[X]
>>
>> Signed-off-by: Chanwoo Choi <[email protected]>
>> ---
>> drivers/extcon/extcon.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index 4c9f165..1a93229 100644
>> --- a/drivers/extcon/extcon.c
>> +++ b/drivers/extcon/extcon.c
>> @@ -163,7 +163,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
>> return ret;
>> }
>>
>> - return sprintf(buf, "%s\n", dev_name(&edev->dev));
>> + return sprintf(buf, "%s\n", edev->name);
>> }
>> static DEVICE_ATTR_RO(name);
>>
>> @@ -701,6 +701,7 @@ EXPORT_SYMBOL_GPL(devm_extcon_dev_free);
>> int extcon_dev_register(struct extcon_dev *edev)
>> {
>> int ret, index = 0;
>> + static atomic_t edev_no = ATOMIC_INIT(-1);
>>
>> if (!extcon_class) {
>> ret = create_extcon_class();
>> @@ -725,13 +726,14 @@ int extcon_dev_register(struct extcon_dev *edev)
>> edev->dev.class = extcon_class;
>> edev->dev.release = extcon_dev_release;
>>
>> - edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
>> + edev->name = dev_name(edev->dev.parent);
>> if (IS_ERR_OR_NULL(edev->name)) {
>> dev_err(&edev->dev,
>> "extcon device name is null\n");
>> return -EINVAL;
>> }
>> - dev_set_name(&edev->dev, "%s", edev->name);
>> + dev_set_name(&edev->dev, "extcon%lu",
>> + (unsigned long)atomic_inc_return(&edev_no));
>>
>> if (edev->max_supported) {
>> char buf[10];
>> --
>> 1.8.5.5
>>
>
> I am not quite sure I see the advantage of this. Why is naming
> the node extcon[X] better than the old system? Seems like the
> older more descriptive names are better on the face of it, unless
> there is some problem with them I am missing.
In the older method for device name, if some board have the
two more same extcon h/w, extcon driver will fail to probe because
of same device name. There is definitely critical problem with older method.
>
> It also looks problematic, it changes the ABI for the Arizona
> extcon driver for a start, admittedly there arn't many users for
> the extcon driver at the moment but still is far from ideal to
> break the ABI. Secondly, the order of the extcon[X]'s will not be
> guaranteed, add some new hardware to the system probe order
> changes and now what was extcon0 is extcon1. So it somewhat
> complicates the user-space code as it now has to work out if the
> device is the one it wants whereas before it could just hard-code
> the name.
If extcon use the device name as 'extcon[X]' pattern, user-space should
find the appropriate extcon device by using the 'extcon[X]' pattern.
This method is more flexible instead of static hard coded device name.
If extcon use the hard coded device name, user-space platform or
process have to modify the sysfs path of extcon device when changing
the user-space platform or process. It is the cause of strong dependency
of between hardware and user-space platform/process. The user-space
don't consider the h/w name.
So, I'll have the plan to add 'type' field which means the type of h/w
port of extcon device as following and add the 'type' sysfs entry.
#define EXTCON_PORT_USB 0x1
#define EXTCON_PORT_HEADSET 0x2
#define EXTCON_PORT_HDMI 0x3
#define EXTCON_PORT_MHL 0x4
...
For example, some board have the two headset jack port:
The user-space can find the jack device by 'type' entry.
/sys/class/extcon/extcon0/name : arizona-extcon
/sys/class/extcon/extcon0/type : 0x2
/sys/class/extcon/extcon1/name : arizona-extcon
/sys/class/extcon/extcon1/type : 0x2
Thanks,
Chanwoo Choi
On Thu, Apr 30, 2015 at 12:31:06AM +0900, Chanwoo Choi wrote:
> Hi Charles,
>
> On Tue, Apr 28, 2015 at 11:17 PM, Charles Keepax
> <[email protected]> wrote:
> > On Mon, Apr 27, 2015 at 09:13:35PM +0900, Chanwoo Choi wrote:
> >> This patch modify the device name as extcon[X] for sysfs by using the 'extcon'
> >> prefix word instead of separate device name. On user-space aspect, user would
> >> find the some extcon drvier with extcon[X] pattern. So, this patch modify the
> >> device name as following:
> >> - /sys/class/extcon/[device name] -> /sys/class/extcon/extcon[X]
> >>
> >> Signed-off-by: Chanwoo Choi <[email protected]>
> >> ---
> >> drivers/extcon/extcon.c | 8 +++++---
> >> 1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> >> index 4c9f165..1a93229 100644
> >> --- a/drivers/extcon/extcon.c
> >> +++ b/drivers/extcon/extcon.c
> >> @@ -163,7 +163,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
> >> return ret;
> >> }
> >>
> >> - return sprintf(buf, "%s\n", dev_name(&edev->dev));
> >> + return sprintf(buf, "%s\n", edev->name);
> >> }
> >> static DEVICE_ATTR_RO(name);
> >>
> >> @@ -701,6 +701,7 @@ EXPORT_SYMBOL_GPL(devm_extcon_dev_free);
> >> int extcon_dev_register(struct extcon_dev *edev)
> >> {
> >> int ret, index = 0;
> >> + static atomic_t edev_no = ATOMIC_INIT(-1);
> >>
> >> if (!extcon_class) {
> >> ret = create_extcon_class();
> >> @@ -725,13 +726,14 @@ int extcon_dev_register(struct extcon_dev *edev)
> >> edev->dev.class = extcon_class;
> >> edev->dev.release = extcon_dev_release;
> >>
> >> - edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
> >> + edev->name = dev_name(edev->dev.parent);
> >> if (IS_ERR_OR_NULL(edev->name)) {
> >> dev_err(&edev->dev,
> >> "extcon device name is null\n");
> >> return -EINVAL;
> >> }
> >> - dev_set_name(&edev->dev, "%s", edev->name);
> >> + dev_set_name(&edev->dev, "extcon%lu",
> >> + (unsigned long)atomic_inc_return(&edev_no));
> >>
> >> if (edev->max_supported) {
> >> char buf[10];
> >> --
> >> 1.8.5.5
> >>
> >
> > I am not quite sure I see the advantage of this. Why is naming
> > the node extcon[X] better than the old system? Seems like the
> > older more descriptive names are better on the face of it, unless
> > there is some problem with them I am missing.
>
> In the older method for device name, if some board have the
> two more same extcon h/w, extcon driver will fail to probe because
> of same device name. There is definitely critical problem with older method.
Hmm... ok that is probably reasonable enough reason to change the
ABI. I will add my ack to the Arizona patch.
Thanks,
Charles
On Mon, Apr 27, 2015 at 09:13:38PM +0900, Chanwoo Choi wrote:
> This patch removes the setting of device name. Instead, extcon_dev_register()
> set the device name such as 'extcon[number]' naming method.
> - /sys/class/extcon/Headset Jack -> /sys/class/extcon/extcon[number]
>
> Cc: Charles Keepax <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> ---
Acked-by: Charles Keepax <[email protected]>
Thanks,
Charles