Re: [PATCH 1/4] extcon: Modify the device name as extcon[X] for sysfs

[Chanwoo Choi]

Hi Charles,

On Tue, Apr 28, 2015 at 11:17 PM, Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> 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 <cw00.choi@xxxxxxxxxxx>
>> ---
>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/