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

[Charles Keepax <ckeepax@opensource.wolfsonmicro.com>]

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
> <ckeepax@opensource.wolfsonmicro.com> 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@samsung.com>
> >> ---
> >>  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