[PATCHv3 0/8] Resource-managed extcon device register function

[PATCHv3 0/8] Resource-managed extcon device register function

[Sangjung Woo]

These patches add resource-managed extcon device register functions for
developers' convenience and apply them to related device driver files.
This work can make the code more tidy since extcon device is automatically
unregistered on driver detach so tiresome managing codes could be removed.


Changelog:

v3:
* send the right version instead of previous v1
* add the credit for reviewers according to the review rules

v2:
* modify and clean up all unnecessary code reported by Chanwoo
* fix the bug reported by Seung-Woo
* add the credit for reviewers

v1:
* initial version

Sangjung Woo (8):
  extcon: Add resource-managed extcon register function
  extcon: adc-jack: Use devm_extcon_dev_register()
  extcon: gpio: Use devm_extcon_dev_register()
  extcon: max14577: Use devm_extcon_dev_register()
  extcon: max77693: Use devm_extcon_dev_register()
  extcon: max8997: Use devm_extcon_dev_register()
  extcon: palmas: Use devm_extcon_dev_register()
  extcon: arizona: Use devm_extcon_dev_register()

 drivers/extcon/extcon-adc-jack.c |   30 +++++-----------
 drivers/extcon/extcon-arizona.c  |   12 +++----
 drivers/extcon/extcon-class.c    |   72 ++++++++++++++++++++++++++++++++++++++
 drivers/extcon/extcon-gpio.c     |   16 +++------
 drivers/extcon/extcon-max14577.c |    9 ++---
 drivers/extcon/extcon-max77693.c |    7 ++--
 drivers/extcon/extcon-max8997.c  |    4 +--
 drivers/extcon/extcon-palmas.c   |   15 +++-----
 include/linux/extcon.h           |   17 +++++++++
 9 files changed, 116 insertions(+), 66 deletions(-)

-- 
1.7.9.5

[PATCHv3 1/8] extcon: Add resource-managed extcon register function

[Sangjung Woo]

Add resource-managed extcon device register function for convenience.
For example, if a extcon device is attached with new
devm_extcon_dev_register(), that extcon device is automatically
unregistered on driver detach.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
---
 drivers/extcon/extcon-class.c |   72 +++++++++++++++++++++++++++++++++++++++++
 include/linux/extcon.h        |   17 ++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 7ab21aa..e177edb6 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -819,6 +819,78 @@ void extcon_dev_unregister(struct extcon_dev *edev)
 }
 EXPORT_SYMBOL_GPL(extcon_dev_unregister);
 
+
+/*
+ * Device resource management
+ */
+static void devm_extcon_dev_release(struct device *dev, void *res)
+{
+	struct extcon_dev *devres = res;
+
+	extcon_dev_unregister(devres);
+}
+
+static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
+{
+	struct extcon_dev *devres = res;
+	struct extcon_dev *match = data;
+	return devres == match;
+}
+
+/**
+ * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
+ * @dev:	device to allocate extcon device
+ * @edev:	the new extcon device to register
+ *
+ * Managed extcon_dev_register() function. If extcon device is attached with
+ * this function, that extcon device is automatically unregistered on driver
+ * detach. Internally this function calls extcon_dev_register() function.
+ * To get more information, refer that function.
+ *
+ * If extcon device is registered with this function and the device needs to be
+ * unregistered separately, devm_extcon_dev_unregister() should be used.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int devm_extcon_dev_register(struct device *dev, struct extcon_dev *edev)
+{
+	struct extcon_dev *devres;
+	int ret;
+
+	devres = devres_alloc(devm_extcon_dev_release, sizeof(*devres),
+			GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = extcon_dev_register(edev);
+	if (ret) {
+		devres_free(devres);
+		return ret;
+	}
+
+	devres = edev;
+	devres_add(dev, devres);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_extcon_dev_register);
+
+/**
+ * devm_extcon_dev_unregister() - Resource-managed extcon_dev_unregister()
+ * @dev:	device the extcon belongs to
+ * @edev:	the extcon device to unregister
+ *
+ * Unregister extcon device that is registered with devm_extcon_dev_register()
+ * function.
+ */
+void devm_extcon_dev_unregister(struct device *dev, struct extcon_dev *edev)
+{
+	WARN_ON(devres_release(dev, devm_extcon_dev_release,
+			devm_extcon_dev_match, edev));
+}
+EXPORT_SYMBOL_GPL(devm_extcon_dev_unregister);
+
 #ifdef CONFIG_OF
 /*
  * extcon_get_edev_by_phandle - Get the extcon device from devicetree
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index f488145..35f3343 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -188,6 +188,14 @@ extern void extcon_dev_unregister(struct extcon_dev *edev);
 extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
 
 /*
+ * Resource-managed extcon device register function.
+ */
+extern int devm_extcon_dev_register(struct device *dev,
+				struct extcon_dev *edev);
+extern void devm_extcon_dev_unregister(struct device *dev,
+				struct extcon_dev *edev);
+
+/*
  * get/set/update_state access the 32b encoded state value, which represents
  * states of all possible cables of the multistate port. For example, if one
  * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
@@ -254,6 +262,15 @@ static inline int extcon_dev_register(struct extcon_dev *edev)
 
 static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
 
+static inline devm_extcon_dev_register(struct device *dev,
+		struct extcon_dev *edev)
+{
+	return 0;
+}
+
+static inline devm_extcon_dev_unregister(struct device *dev,
+		struct extcon_dev *edev) { }
+
 static inline u32 extcon_get_state(struct extcon_dev *edev)
 {
 	return 0;
-- 
1.7.9.5

Re: [PATCHv3 1/8] extcon: Add resource-managed extcon register function

[Chanwoo Choi]

Hi Sangjung,

On Fri, Apr 18, 2014 at 9:32 AM, Sangjung Woo <sangjung.woo@samsung.com> wrote:
> Add resource-managed extcon device register function for convenience.
> For example, if a extcon device is attached with new
> devm_extcon_dev_register(), that extcon device is automatically
> unregistered on driver detach.
>
> Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
> ---
>  drivers/extcon/extcon-class.c |   72 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/extcon.h        |   17 ++++++++++
>  2 files changed, 89 insertions(+)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 7ab21aa..e177edb6 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -819,6 +819,78 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  }
>  EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>
> +
> +/*
> + * Device resource management
> + */

This comment is un-necessary because this patchset(v3) remove 'struct
extcon_devres'.

> +static void devm_extcon_dev_release(struct device *dev, void *res)
> +{
> +       struct extcon_dev *devres = res;
> +
> +       extcon_dev_unregister(devres);

I prefer following function call withou defining separate 'devres' variable.
But, this casting on the first argument is only for readability.
  extcon_dev_unregister((strcut extcon_dev *)res);

> +}
> +
> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
> +{
> +       struct extcon_dev *devres = res;
> +       struct extcon_dev *match = data;
> +       return devres == match;

To simplify code, I think you could change checking code as following:
            return res == data;

> +}
> +
> +/**
> + * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
> + * @dev:       device to allocate extcon device
> + * @edev:      the new extcon device to register
> + *
> + * Managed extcon_dev_register() function. If extcon device is attached with
> + * this function, that extcon device is automatically unregistered on driver
> + * detach. Internally this function calls extcon_dev_register() function.
> + * To get more information, refer that function.
> + *
> + * If extcon device is registered with this function and the device needs to be
> + * unregistered separately, devm_extcon_dev_unregister() should be used.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int devm_extcon_dev_register(struct device *dev, struct extcon_dev *edev)
> +{
> +       struct extcon_dev *devres;

The 'devres' in this function don't mean 'device resource structure'.
So I think it is not proper name.
I think you should use other general name (e.g.,  'ptr' or 'res'
instead of 'devres')

> +       int ret;
> +
> +       devres = devres_alloc(devm_extcon_dev_release, sizeof(*devres)

Other subsytem used double pointer to get device resource from
devres_alloc() instead of
single pointer.(devres is single pointer)  I can't find subsystem
using single pointer of devm function.
First of all, We have to analyze the correct reason using only double
pointer instead of single pointer whether single pointer use is good
or not.

> +                       GFP_KERNEL);
> +       if (!devres)
> +               return -ENOMEM;
> +
> +       ret = extcon_dev_register(edev);
> +       if (ret) {
> +               devres_free(devres);
> +               return ret;
> +       }
> +
> +       devres = edev;
> +       devres_add(dev, devres);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_extcon_dev_register);
> +
> +/**
> + * devm_extcon_dev_unregister() - Resource-managed extcon_dev_unregister()
> + * @dev:       device the extcon belongs to
> + * @edev:      the extcon device to unregister
> + *
> + * Unregister extcon device that is registered with devm_extcon_dev_register()
> + * function.
> + */
> +void devm_extcon_dev_unregister(struct device *dev, struct extcon_dev *edev)
> +{
> +       WARN_ON(devres_release(dev, devm_extcon_dev_release,
> +                       devm_extcon_dev_match, edev));
> +}
> +EXPORT_SYMBOL_GPL(devm_extcon_dev_unregister);
> +
>  #ifdef CONFIG_OF
>  /*
>   * extcon_get_edev_by_phandle - Get the extcon device from devicetree
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index f488145..35f3343 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -188,6 +188,14 @@ extern void extcon_dev_unregister(struct extcon_dev *edev);
>  extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
>
>  /*
> + * Resource-managed extcon device register function.
> + */
> +extern int devm_extcon_dev_register(struct device *dev,
> +                               struct extcon_dev *edev);
> +extern void devm_extcon_dev_unregister(struct device *dev,
> +                               struct extcon_dev *edev);

I prefer that this function meet indentation of function definition
needs as following:

extern int devm_extcon_dev_register(struct device *dev,

struct extcon_dev *edev);
extern void devm_extcon_dev_unregister(struct device *dev,

  struct extcon_dev *edev);

> +
> +/*
>   * get/set/update_state access the 32b encoded state value, which represents
>   * states of all possible cables of the multistate port. For example, if one
>   * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
> @@ -254,6 +262,15 @@ static inline int extcon_dev_register(struct extcon_dev *edev)
>
>  static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
>
> +static inline devm_extcon_dev_register(struct device *dev,
> +               struct extcon_dev *edev)

Missing the type of return value.
-> static inline int devm_extcon_dev_register(...

Also, ditto about function definition identification.

> +{
> +       return 0;
> +}
> +
> +static inline devm_extcon_dev_unregister(struct device *dev,

ditto.
-> static inline void devm_extcon_dev_unregister(...

Thanks,
Chanwoo Choi

Re: [PATCHv3 1/8] extcon: Add resource-managed extcon register function

[Sangjung]

Hi Chanwoo.

Thanks for your comments. I also add my opinion too.


On 04/19/2014 04:13 PM, Chanwoo Choi wrote:
> Hi Sangjung,
>
> On Fri, Apr 18, 2014 at 9:32 AM, Sangjung Woo <sangjung.woo@samsung.com> wrote:
>> Add resource-managed extcon device register function for convenience.
>> For example, if a extcon device is attached with new
>> devm_extcon_dev_register(), that extcon device is automatically
>> unregistered on driver detach.
>>
>> Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
>> ---
>>   drivers/extcon/extcon-class.c |   72 +++++++++++++++++++++++++++++++++++++++++
>>   include/linux/extcon.h        |   17 ++++++++++
>>   2 files changed, 89 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index 7ab21aa..e177edb6 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -819,6 +819,78 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>>   }
>>   EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>>
>> +
>> +/*
>> + * Device resource management
>> + */
> This comment is un-necessary because this patchset(v3) remove 'struct
> extcon_devres'.
>
>> +static void devm_extcon_dev_release(struct device *dev, void *res)
>> +{
>> +       struct extcon_dev *devres = res;
>> +
>> +       extcon_dev_unregister(devres);
> I prefer following function call withou defining separate 'devres' variable.
> But, this casting on the first argument is only for readability.
>    extcon_dev_unregister((strcut extcon_dev *)res);
>
OK. I'll fix it.

>> +}
>> +
>> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
>> +{
>> +       struct extcon_dev *devres = res;
>> +       struct extcon_dev *match = data;
>> +       return devres == match;
> To simplify code, I think you could change checking code as following:
>              return res == data;
Right. Simple is better than others.

>> +}
>> +
>> +/**
>> + * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
>> + * @dev:       device to allocate extcon device
>> + * @edev:      the new extcon device to register
>> + *
>> + * Managed extcon_dev_register() function. If extcon device is attached with
>> + * this function, that extcon device is automatically unregistered on driver
>> + * detach. Internally this function calls extcon_dev_register() function.
>> + * To get more information, refer that function.
>> + *
>> + * If extcon device is registered with this function and the device needs to be
>> + * unregistered separately, devm_extcon_dev_unregister() should be used.
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error number on failure.
>> + */
>> +int devm_extcon_dev_register(struct device *dev, struct extcon_dev *edev)
>> +{
>> +       struct extcon_dev *devres;
> The 'devres' in this function don't mean 'device resource structure'.
> So I think it is not proper name.
> I think you should use other general name (e.g.,  'ptr' or 'res'
> instead of 'devres')
>
>> +       int ret;
>> +
>> +       devres = devres_alloc(devm_extcon_dev_release, sizeof(*devres)
> Other subsytem used double pointer to get device resource from
> devres_alloc() instead of
> single pointer.(devres is single pointer)  I can't find subsystem
> using single pointer of devm function.
> First of all, We have to analyze the correct reason using only double
> pointer instead of single pointer whether single pointer use is good
> or not.

IMO, other subsystem should return the memory pointer that is allocated 
by devres_alloc().
However, in our case, we need not do that since the pointer is used only 
in extcon core.
You can refer the way that I did to gpio subsystem (devm_gpio_request() 
in /drivers/gpio/devres.c).

>> +                       GFP_KERNEL);
>> +       if (!devres)
>> +               return -ENOMEM;
>> +
>> +       ret = extcon_dev_register(edev);
>> +       if (ret) {
>> +               devres_free(devres);
>> +               return ret;
>> +       }
>> +
>> +       devres = edev;
>> +       devres_add(dev, devres);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_extcon_dev_register);
>> +
>> +/**
>> + * devm_extcon_dev_unregister() - Resource-managed extcon_dev_unregister()
>> + * @dev:       device the extcon belongs to
>> + * @edev:      the extcon device to unregister
>> + *
>> + * Unregister extcon device that is registered with devm_extcon_dev_register()
>> + * function.
>> + */
>> +void devm_extcon_dev_unregister(struct device *dev, struct extcon_dev *edev)
>> +{
>> +       WARN_ON(devres_release(dev, devm_extcon_dev_release,
>> +                       devm_extcon_dev_match, edev));
>> +}
>> +EXPORT_SYMBOL_GPL(devm_extcon_dev_unregister);
>> +
>>   #ifdef CONFIG_OF
>>   /*
>>    * extcon_get_edev_by_phandle - Get the extcon device from devicetree
>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>> index f488145..35f3343 100644
>> --- a/include/linux/extcon.h
>> +++ b/include/linux/extcon.h
>> @@ -188,6 +188,14 @@ extern void extcon_dev_unregister(struct extcon_dev *edev);
>>   extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
>>
>>   /*
>> + * Resource-managed extcon device register function.
>> + */
>> +extern int devm_extcon_dev_register(struct device *dev,
>> +                               struct extcon_dev *edev);
>> +extern void devm_extcon_dev_unregister(struct device *dev,
>> +                               struct extcon_dev *edev);
> I prefer that this function meet indentation of function definition
> needs as following:
>
> extern int devm_extcon_dev_register(struct device *dev,
>
> struct extcon_dev *edev);
> extern void devm_extcon_dev_unregister(struct device *dev,
>
>    struct extcon_dev *edev);

I have a question about the indentation issue
since my Thunderbird email client does not show me the below line tidy.

You want me to adjust the start point of the second line
to the right after parentheses of the first line. right?

>> +
>> +/*
>>    * get/set/update_state access the 32b encoded state value, which represents
>>    * states of all possible cables of the multistate port. For example, if one
>>    * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
>> @@ -254,6 +262,15 @@ static inline int extcon_dev_register(struct extcon_dev *edev)
>>
>>   static inline void extcon_dev_unregister(struct extcon_dev *edev) { }
>>
>> +static inline devm_extcon_dev_register(struct device *dev,
>> +               struct extcon_dev *edev)
> Missing the type of return value.
> -> static inline int devm_extcon_dev_register(...
>
> Also, ditto about function definition identification.

Right. I made a mistake.
>
>> +{
>> +       return 0;
>> +}
>> +
>> +static inline devm_extcon_dev_unregister(struct device *dev,
> ditto.
> -> static inline void devm_extcon_dev_unregister(...
>
> Thanks,
> Chanwoo Choi
>

Re: [PATCHv3 1/8] extcon: Add resource-managed extcon register function

[Chanwoo Choi]

Hi,

On Sat, Apr 19, 2014 at 6:50 PM, Sangjung <sangjung.woo@samsung.com> wrote:
> Hi Chanwoo.
>
> Thanks for your comments. I also add my opinion too.
>
>
>
> On 04/19/2014 04:13 PM, Chanwoo Choi wrote:
>>
>> Hi Sangjung,
>>
>> On Fri, Apr 18, 2014 at 9:32 AM, Sangjung Woo <sangjung.woo@samsung.com>
>> wrote:
>>>
>>> Add resource-managed extcon device register function for convenience.
>>> For example, if a extcon device is attached with new
>>> devm_extcon_dev_register(), that extcon device is automatically
>>> unregistered on driver detach.
>>>
>>> Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
>>> ---
>>>   drivers/extcon/extcon-class.c |   72
>>> +++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/extcon.h        |   17 ++++++++++
>>>   2 files changed, 89 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon-class.c
>>> b/drivers/extcon/extcon-class.c
>>> index 7ab21aa..e177edb6 100644
>>> --- a/drivers/extcon/extcon-class.c
>>> +++ b/drivers/extcon/extcon-class.c
>>> @@ -819,6 +819,78 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>>>   }
>>>   EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>>>
>>> +
>>> +/*
>>> + * Device resource management
>>> + */
>>
>> This comment is un-necessary because this patchset(v3) remove 'struct
>> extcon_devres'.
>>
>>> +static void devm_extcon_dev_release(struct device *dev, void *res)
>>> +{
>>> +       struct extcon_dev *devres = res;
>>> +
>>> +       extcon_dev_unregister(devres);
>>
>> I prefer following function call withou defining separate 'devres'
>> variable.
>> But, this casting on the first argument is only for readability.
>>    extcon_dev_unregister((strcut extcon_dev *)res);
>>
> OK. I'll fix it.
>
>
>>> +}
>>> +
>>> +static int devm_extcon_dev_match(struct device *dev, void *res, void
>>> *data)
>>> +{
>>> +       struct extcon_dev *devres = res;
>>> +       struct extcon_dev *match = data;
>>> +       return devres == match;
>>
>> To simplify code, I think you could change checking code as following:
>>              return res == data;
>
> Right. Simple is better than others.
>
>
>>> +}
>>> +
>>> +/**
>>> + * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
>>> + * @dev:       device to allocate extcon device
>>> + * @edev:      the new extcon device to register
>>> + *
>>> + * Managed extcon_dev_register() function. If extcon device is attached
>>> with
>>> + * this function, that extcon device is automatically unregistered on
>>> driver
>>> + * detach. Internally this function calls extcon_dev_register()
>>> function.
>>> + * To get more information, refer that function.
>>> + *
>>> + * If extcon device is registered with this function and the device
>>> needs to be
>>> + * unregistered separately, devm_extcon_dev_unregister() should be used.
>>> + *
>>> + * RETURNS:
>>> + * 0 on success, negative error number on failure.
>>> + */
>>> +int devm_extcon_dev_register(struct device *dev, struct extcon_dev
>>> *edev)
>>> +{
>>> +       struct extcon_dev *devres;
>>
>> The 'devres' in this function don't mean 'device resource structure'.
>> So I think it is not proper name.
>> I think you should use other general name (e.g.,  'ptr' or 'res'
>> instead of 'devres')
>>
>>> +       int ret;
>>> +
>>> +       devres = devres_alloc(devm_extcon_dev_release, sizeof(*devres)
>>
>> Other subsytem used double pointer to get device resource from
>> devres_alloc() instead of
>> single pointer.(devres is single pointer)  I can't find subsystem
>> using single pointer of devm function.
>> First of all, We have to analyze the correct reason using only double
>> pointer instead of single pointer whether single pointer use is good
>> or not.
>
>
> IMO, other subsystem should return the memory pointer that is allocated by
> devres_alloc().
> However, in our case, we need not do that since the pointer is used only in
> extcon core.
> You can refer the way that I did to gpio subsystem (devm_gpio_request() in
> /drivers/gpio/devres.c).

As you comment, I checked 'devm_gpio_request' in drivers/gpio/devres.c
. There are a little difference between devm_extcon_dev_register and
devm_gpio_request.

The second argument (unsigned gpio) is not pointer type in
devm_gpio_request() But, devm_extcon_dev_register() needs the pointer
type for second argument(struct extcon_dev *edev).

>
>
>>> +                       GFP_KERNEL);
>>> +       if (!devres)
>>> +               return -ENOMEM;
>>> +
>>> +       ret = extcon_dev_register(edev);
>>> +       if (ret) {
>>> +               devres_free(devres);
>>> +               return ret;
>>> +       }
>>> +
>>> +       devres = edev;
>>> +       devres_add(dev, devres);
>>> +
>>> +       return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_extcon_dev_register);
>>> +
>>> +/**
>>> + * devm_extcon_dev_unregister() - Resource-managed
>>> extcon_dev_unregister()
>>> + * @dev:       device the extcon belongs to
>>> + * @edev:      the extcon device to unregister
>>> + *
>>> + * Unregister extcon device that is registered with
>>> devm_extcon_dev_register()
>>> + * function.
>>> + */
>>> +void devm_extcon_dev_unregister(struct device *dev, struct extcon_dev
>>> *edev)
>>> +{
>>> +       WARN_ON(devres_release(dev, devm_extcon_dev_release,
>>> +                       devm_extcon_dev_match, edev));
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_extcon_dev_unregister);
>>> +
>>>   #ifdef CONFIG_OF
>>>   /*
>>>    * extcon_get_edev_by_phandle - Get the extcon device from devicetree
>>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>>> index f488145..35f3343 100644
>>> --- a/include/linux/extcon.h
>>> +++ b/include/linux/extcon.h
>>> @@ -188,6 +188,14 @@ extern void extcon_dev_unregister(struct extcon_dev
>>> *edev);
>>>   extern struct extcon_dev *extcon_get_extcon_dev(const char
>>> *extcon_name);
>>>
>>>   /*
>>> + * Resource-managed extcon device register function.
>>> + */
>>> +extern int devm_extcon_dev_register(struct device *dev,
>>> +                               struct extcon_dev *edev);
>>> +extern void devm_extcon_dev_unregister(struct device *dev,
>>> +                               struct extcon_dev *edev);
>>
>> I prefer that this function meet indentation of function definition
>> needs as following:
>>
>> extern int devm_extcon_dev_register(struct device *dev,
>>
>> struct extcon_dev *edev);
>> extern void devm_extcon_dev_unregister(struct device *dev,
>>
>>    struct extcon_dev *edev);
>
>
> I have a question about the indentation issue
> since my Thunderbird email client does not show me the below line tidy.
>
> You want me to adjust the start point of the second line
> to the right after parentheses of the first line. right?

Right.

Thanks,
Chanwoo Choi

Re: [PATCHv3 1/8] extcon: Add resource-managed extcon register function

[Chanwoo Choi]

Hi,

On Sat, Apr 19, 2014 at 10:36 PM, Chanwoo Choi <cwchoi00@gmail.com> wrote:
> Hi,
>
> On Sat, Apr 19, 2014 at 6:50 PM, Sangjung <sangjung.woo@samsung.com> wrote:
>> Hi Chanwoo.
>>
>> Thanks for your comments. I also add my opinion too.
>>
>>
>>
>> On 04/19/2014 04:13 PM, Chanwoo Choi wrote:
>>>
>>> Hi Sangjung,
>>>
>>> On Fri, Apr 18, 2014 at 9:32 AM, Sangjung Woo <sangjung.woo@samsung.com>
>>> wrote:
>>>>
>>>> Add resource-managed extcon device register function for convenience.
>>>> For example, if a extcon device is attached with new
>>>> devm_extcon_dev_register(), that extcon device is automatically
>>>> unregistered on driver detach.
>>>>
>>>> Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
>>>> ---
>>>>   drivers/extcon/extcon-class.c |   72
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>   include/linux/extcon.h        |   17 ++++++++++
>>>>   2 files changed, 89 insertions(+)
>>>>
>>>> diff --git a/drivers/extcon/extcon-class.c
>>>> b/drivers/extcon/extcon-class.c
>>>> index 7ab21aa..e177edb6 100644
>>>> --- a/drivers/extcon/extcon-class.c
>>>> +++ b/drivers/extcon/extcon-class.c
>>>> @@ -819,6 +819,78 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>>>>
>>>> +
>>>> +/*
>>>> + * Device resource management
>>>> + */
>>>
>>> This comment is un-necessary because this patchset(v3) remove 'struct
>>> extcon_devres'.
>>>
>>>> +static void devm_extcon_dev_release(struct device *dev, void *res)
>>>> +{
>>>> +       struct extcon_dev *devres = res;
>>>> +
>>>> +       extcon_dev_unregister(devres);
>>>
>>> I prefer following function call withou defining separate 'devres'
>>> variable.
>>> But, this casting on the first argument is only for readability.
>>>    extcon_dev_unregister((strcut extcon_dev *)res);
>>>
>> OK. I'll fix it.
>>
>>
>>>> +}
>>>> +
>>>> +static int devm_extcon_dev_match(struct device *dev, void *res, void
>>>> *data)
>>>> +{
>>>> +       struct extcon_dev *devres = res;
>>>> +       struct extcon_dev *match = data;
>>>> +       return devres == match;
>>>
>>> To simplify code, I think you could change checking code as following:
>>>              return res == data;
>>
>> Right. Simple is better than others.
>>
>>
>>>> +}
>>>> +
>>>> +/**
>>>> + * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
>>>> + * @dev:       device to allocate extcon device
>>>> + * @edev:      the new extcon device to register
>>>> + *
>>>> + * Managed extcon_dev_register() function. If extcon device is attached
>>>> with
>>>> + * this function, that extcon device is automatically unregistered on
>>>> driver
>>>> + * detach. Internally this function calls extcon_dev_register()
>>>> function.
>>>> + * To get more information, refer that function.
>>>> + *
>>>> + * If extcon device is registered with this function and the device
>>>> needs to be
>>>> + * unregistered separately, devm_extcon_dev_unregister() should be used.
>>>> + *
>>>> + * RETURNS:
>>>> + * 0 on success, negative error number on failure.
>>>> + */
>>>> +int devm_extcon_dev_register(struct device *dev, struct extcon_dev
>>>> *edev)
>>>> +{
>>>> +       struct extcon_dev *devres;
>>>
>>> The 'devres' in this function don't mean 'device resource structure'.
>>> So I think it is not proper name.
>>> I think you should use other general name (e.g.,  'ptr' or 'res'
>>> instead of 'devres')
>>>
>>>> +       int ret;
>>>> +
>>>> +       devres = devres_alloc(devm_extcon_dev_release, sizeof(*devres)
>>>
>>> Other subsytem used double pointer to get device resource from
>>> devres_alloc() instead of
>>> single pointer.(devres is single pointer)  I can't find subsystem
>>> using single pointer of devm function.
>>> First of all, We have to analyze the correct reason using only double
>>> pointer instead of single pointer whether single pointer use is good
>>> or not.
>>
>>
>> IMO, other subsystem should return the memory pointer that is allocated by
>> devres_alloc().
>> However, in our case, we need not do that since the pointer is used only in
>> extcon core.
>> You can refer the way that I did to gpio subsystem (devm_gpio_request() in
>> /drivers/gpio/devres.c).
>
> As you comment, I checked 'devm_gpio_request' in drivers/gpio/devres.c
> . There are a little difference between devm_extcon_dev_register and
> devm_gpio_request.
>
> The second argument (unsigned gpio) is not pointer type in
> devm_gpio_request() But, devm_extcon_dev_register() needs the pointer
> type for second argument(struct extcon_dev *edev).

This patch have to use double pointer for extcon device from devres_alloc().

Because in this following case:
struct extcon_dev *ptr;

ptr = devres_alloc(...)
...
ptr = edev;
-> edev would override the 'ptr' which is allocated memory from devres_alloc()
In result, this patch cannot access original allocated memory for 'ptr'.

Thanks,
Chanwoo Choi

[PATCHv3 2/8] extcon: adc-jack: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
---
 drivers/extcon/extcon-adc-jack.c |   30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index e23f1c2..549d820 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -105,9 +105,8 @@ static int adc_jack_probe(struct platform_device *pdev)
 	data->edev.name = pdata->name;
 
 	if (!pdata->cable_names) {
-		err = -EINVAL;
 		dev_err(&pdev->dev, "error: cable_names not defined.\n");
-		goto out;
+		return -EINVAL;
 	}
 
 	data->edev.dev.parent = &pdev->dev;
@@ -117,18 +116,16 @@ static int adc_jack_probe(struct platform_device *pdev)
 	for (i = 0; data->edev.supported_cable[i]; i++)
 		;
 	if (i == 0 || i > SUPPORTED_CABLE_MAX) {
-		err = -EINVAL;
 		dev_err(&pdev->dev, "error: pdata->cable_names size = %d\n",
 				i - 1);
-		goto out;
+		return -EINVAL;
 	}
 	data->num_cables = i;
 
 	if (!pdata->adc_conditions ||
 			!pdata->adc_conditions[0].state) {
-		err = -EINVAL;
 		dev_err(&pdev->dev, "error: adc_conditions not defined.\n");
-		goto out;
+		return -EINVAL;
 	}
 	data->adc_conditions = pdata->adc_conditions;
 
@@ -138,10 +135,8 @@ static int adc_jack_probe(struct platform_device *pdev)
 	data->num_conditions = i;
 
 	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
-	if (IS_ERR(data->chan)) {
-		err = PTR_ERR(data->chan);
-		goto out;
-	}
+	if (IS_ERR(data->chan))
+		return PTR_ERR(data->chan);
 
 	data->handling_delay = msecs_to_jiffies(pdata->handling_delay_ms);
 
@@ -149,15 +144,14 @@ static int adc_jack_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
-	err = extcon_dev_register(&data->edev);
+	err = devm_extcon_dev_register(&pdev->dev, &data->edev);
 	if (err)
-		goto out;
+		return err;
 
 	data->irq = platform_get_irq(pdev, 0);
 	if (!data->irq) {
 		dev_err(&pdev->dev, "platform_get_irq failed\n");
-		err = -ENODEV;
-		goto err_irq;
+		return -ENODEV;
 	}
 
 	err = request_any_context_irq(data->irq, adc_jack_irq_thread,
@@ -165,15 +159,10 @@ static int adc_jack_probe(struct platform_device *pdev)
 
 	if (err < 0) {
 		dev_err(&pdev->dev, "error: irq %d\n", data->irq);
-		goto err_irq;
+		return err;
 	}
 
 	return 0;
-
-err_irq:
-	extcon_dev_unregister(&data->edev);
-out:
-	return err;
 }
 
 static int adc_jack_remove(struct platform_device *pdev)
@@ -182,7 +171,6 @@ static int adc_jack_remove(struct platform_device *pdev)
 
 	free_irq(data->irq, data);
 	cancel_work_sync(&data->handler.work);
-	extcon_dev_unregister(&data->edev);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCHv3 3/8] extcon: gpio: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
---
 drivers/extcon/extcon-gpio.c |   16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 13d5222..43af34c 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -121,34 +121,27 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 				msecs_to_jiffies(pdata->debounce);
 	}
 
-	ret = extcon_dev_register(&extcon_data->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, &extcon_data->edev);
 	if (ret < 0)
 		return ret;
 
 	INIT_DELAYED_WORK(&extcon_data->work, gpio_extcon_work);
 
 	extcon_data->irq = gpio_to_irq(extcon_data->gpio);
-	if (extcon_data->irq < 0) {
-		ret = extcon_data->irq;
-		goto err;
-	}
+	if (extcon_data->irq < 0)
+		return extcon_data->irq;
 
 	ret = request_any_context_irq(extcon_data->irq, gpio_irq_handler,
 				      pdata->irq_flags, pdev->name,
 				      extcon_data);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	platform_set_drvdata(pdev, extcon_data);
 	/* Perform initial detection */
 	gpio_extcon_work(&extcon_data->work.work);
 
 	return 0;
-
-err:
-	extcon_dev_unregister(&extcon_data->edev);
-
-	return ret;
 }
 
 static int gpio_extcon_remove(struct platform_device *pdev)
@@ -157,7 +150,6 @@ static int gpio_extcon_remove(struct platform_device *pdev)
 
 	cancel_delayed_work_sync(&extcon_data->work);
 	free_irq(extcon_data->irq, extcon_data);
-	extcon_dev_unregister(&extcon_data->edev);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCHv3 4/8] extcon: max14577: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max14577.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
index 1fef08d..c6166e7 100644
--- a/drivers/extcon/extcon-max14577.c
+++ b/drivers/extcon/extcon-max14577.c
@@ -675,7 +675,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
 	}
 	info->edev->name = DEV_NAME;
 	info->edev->supported_cable = max14577_extcon_cable;
-	ret = extcon_dev_register(info->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
 		return ret;
@@ -694,7 +694,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
 			MAX14577_REG_DEVICEID, &id);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to read revision number\n");
-		goto err_extcon;
+		return ret;
 	}
 	dev_info(info->dev, "device ID : 0x%x\n", id);
 
@@ -714,10 +714,6 @@ static int max14577_muic_probe(struct platform_device *pdev)
 			delay_jiffies);
 
 	return ret;
-
-err_extcon:
-	extcon_dev_unregister(info->edev);
-	return ret;
 }
 
 static int max14577_muic_remove(struct platform_device *pdev)
@@ -725,7 +721,6 @@ static int max14577_muic_remove(struct platform_device *pdev)
 	struct max14577_muic_info *info = platform_get_drvdata(pdev);
 
 	cancel_work_sync(&info->irq_work);
-	extcon_dev_unregister(info->edev);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCHv3 5/8] extcon: max77693: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max77693.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 39cd095..f0f18e2 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -1185,7 +1185,7 @@ static int max77693_muic_probe(struct platform_device *pdev)
 	info->edev->name = DEV_NAME;
 	info->edev->dev.parent = &pdev->dev;
 	info->edev->supported_cable = max77693_extcon_cable;
-	ret = extcon_dev_register(info->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
 		goto err_irq;
@@ -1267,7 +1267,7 @@ static int max77693_muic_probe(struct platform_device *pdev)
 			MAX77693_MUIC_REG_ID, &id);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to read revision number\n");
-		goto err_extcon;
+		goto err_irq;
 	}
 	dev_info(info->dev, "device ID : 0x%x\n", id);
 
@@ -1288,8 +1288,6 @@ static int max77693_muic_probe(struct platform_device *pdev)
 
 	return ret;
 
-err_extcon:
-	extcon_dev_unregister(info->edev);
 err_irq:
 	while (--i >= 0)
 		free_irq(muic_irqs[i].virq, info);
@@ -1305,7 +1303,6 @@ static int max77693_muic_remove(struct platform_device *pdev)
 		free_irq(muic_irqs[i].virq, info);
 	cancel_work_sync(&info->irq_work);
 	input_unregister_device(info->dock);
-	extcon_dev_unregister(info->edev);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCHv3 6/8] extcon: max8997: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max8997.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 223e6b0..804a446 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -709,7 +709,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
 	info->edev->name = DEV_NAME;
 	info->edev->dev.parent = &pdev->dev;
 	info->edev->supported_cable = max8997_extcon_cable;
-	ret = extcon_dev_register(info->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
 		goto err_irq;
@@ -790,8 +790,6 @@ static int max8997_muic_remove(struct platform_device *pdev)
 		free_irq(muic_irqs[i].virq, info);
 	cancel_work_sync(&info->irq_work);
 
-	extcon_dev_unregister(info->edev);
-
 	return 0;
 }
 
-- 
1.7.9.5

[PATCHv3 7/8] extcon: palmas: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
---
 drivers/extcon/extcon-palmas.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 51db5bc..1a770e0 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -192,7 +192,7 @@ static int palmas_usb_probe(struct platform_device *pdev)
 	palmas_usb->edev.name = kstrdup(node->name, GFP_KERNEL);
 	palmas_usb->edev.mutually_exclusive = mutually_exclusive;
 
-	status = extcon_dev_register(&palmas_usb->edev);
+	status = devm_extcon_dev_register(&pdev->dev, &palmas_usb->edev);
 	if (status) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
 		kfree(palmas_usb->edev.name);
@@ -209,7 +209,8 @@ static int palmas_usb_probe(struct platform_device *pdev)
 		if (status < 0) {
 			dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
 					palmas_usb->id_irq, status);
-			goto fail_extcon;
+			kfree(palmas_usb->edev.name);
+			return status;
 		}
 	}
 
@@ -223,26 +224,20 @@ static int palmas_usb_probe(struct platform_device *pdev)
 		if (status < 0) {
 			dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
 					palmas_usb->vbus_irq, status);
-			goto fail_extcon;
+			kfree(palmas_usb->edev.name);
+			return status;
 		}
 	}
 
 	palmas_enable_irq(palmas_usb);
 	device_set_wakeup_capable(&pdev->dev, true);
 	return 0;
-
-fail_extcon:
-	extcon_dev_unregister(&palmas_usb->edev);
-	kfree(palmas_usb->edev.name);
-
-	return status;
 }
 
 static int palmas_usb_remove(struct platform_device *pdev)
 {
 	struct palmas_usb *palmas_usb = platform_get_drvdata(pdev);
 
-	extcon_dev_unregister(&palmas_usb->edev);
 	kfree(palmas_usb->edev.name);
 
 	return 0;
-- 
1.7.9.5

[PATCHv3 8/8] extcon: arizona: Use devm_extcon_dev_register()

[Sangjung Woo]

Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically unregistered
on driver detach. That reduces tiresome managing code.

Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
---
 drivers/extcon/extcon-arizona.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 98a14f6..f63fa6f 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1105,15 +1105,14 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		dev_err(&pdev->dev, "Failed to allocate memory\n");
-		ret = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	info->micvdd = devm_regulator_get(arizona->dev, "MICVDD");
 	if (IS_ERR(info->micvdd)) {
 		ret = PTR_ERR(info->micvdd);
 		dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
-		goto err;
+		return ret;
 	}
 
 	mutex_init(&info->lock);
@@ -1155,11 +1154,11 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	info->edev.dev.parent = arizona->dev;
 	info->edev.supported_cable = arizona_cable;
 
-	ret = extcon_dev_register(&info->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, &info->edev);
 	if (ret < 0) {
 		dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
 			ret);
-		goto err;
+		return ret;
 	}
 
 	info->input = devm_input_allocate_device(&pdev->dev);
@@ -1410,8 +1409,6 @@ err_rise:
 err_input:
 err_register:
 	pm_runtime_disable(&pdev->dev);
-	extcon_dev_unregister(&info->edev);
-err:
 	return ret;
 }
 
@@ -1445,7 +1442,6 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 	regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
 			   ARIZONA_JD1_ENA, 0);
 	arizona_clk32k_disable(arizona);
-	extcon_dev_unregister(&info->edev);
 
 	return 0;
 }
-- 
1.7.9.5