[PATCHv3 0/9] extcon: Support devm_extcon_dev_allocate/free()

[PATCHv3 0/9] extcon: Support devm_extcon_dev_allocate/free()

[Chanwoo Choi]

This patchset add devm_extcon_dev_allocate/free() for the resource management
of extcon device. And devm_extcon_dev_allocate() handles all of supported cables.

Changes from v2:
- Resolve bisectable issue about patch-1
  extcon: Add extcon_dev_allocate/free() to control the memory of extcon device
- extcon_dev_allocate/free() don't handle all behaviors for supported cables

Changes from v1:
- Rebase it on latest extcon-next branch

Chanwoo Choi (9):
  extcon: Add extcon_dev_allocate/free() to control the memory of extcon device
  extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device
  extcon: max8997: Use devm_extcon_dev_allocate for extcon_dev
  extcon: max77693: Use devm_extcon_dev_allocate for extcon_dev
  extcon: max14577: Use devm_extcon_dev_allocate for extcon_dev
  extcon: arizona: Use devm_extcon_dev_allocate for extcon_dev
  extcon: adc-jack: Use devm_extcon_dev_allocate for extcon_dev
  extcon: gpio: Use devm_extcon_dev_allocate for extcon_dev
  extcon: palmas: Use devm_extcon_dev_allocate for extcon_dev

 drivers/extcon/extcon-adc-jack.c |  21 ++++----
 drivers/extcon/extcon-arizona.c  |  31 ++++++-----
 drivers/extcon/extcon-class.c    | 109 ++++++++++++++++++++++++++++++++++-----
 drivers/extcon/extcon-gpio.c     |  23 ++++++---
 drivers/extcon/extcon-max14577.c |   5 +-
 drivers/extcon/extcon-max77693.c |   7 ++-
 drivers/extcon/extcon-max8997.c  |   5 +-
 drivers/extcon/extcon-palmas.c   |  35 +++++++------
 include/linux/extcon.h           |  24 +++++++++
 include/linux/mfd/palmas.h       |   2 +-
 10 files changed, 195 insertions(+), 67 deletions(-)

-- 
1.8.0

[PATCHv3 1/9] extcon: Add extcon_dev_allocate/free() to control the memory of extcon device

[Chanwoo Choi]

This patch add APIs to control the extcon device on extcon provider driver.
The extcon_dev_allocate() allocates the memory of extcon device and initializes
supported cables. And then extcon_dev_free() decrement the reference of the
device of extcon device and free the memory of the extcon device. This APIs
must need to implement devm_extcon_dev_allocate()/free() APIs.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-class.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/extcon.h        | 13 +++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index f6df689..bec66d4 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -565,6 +565,45 @@ static void dummy_sysfs_dev_release(struct device *dev)
 {
 }
 
+/*
+ * extcon_dev_allocate() - Allocate the memory of extcon device.
+ * @supported_cable:	Array of supported cable names ending with NULL.
+ *			If supported_cable is NULL, cable name related APIs
+ *			are disabled.
+ *
+ * This function allocates the memory for extcon device without allocating
+ * memory in each extcon provider driver and initialize default setting for
+ * extcon device.
+ *
+ * Return the pointer of extcon device if success or ERR_PTR(err) if fail
+ */
+struct extcon_dev *extcon_dev_allocate(const char **supported_cable)
+{
+	struct extcon_dev *edev;
+
+	edev = kzalloc(sizeof(*edev), GFP_KERNEL);
+	if (!edev) {
+		pr_err("Failed to allocate the memory for extcon device\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	edev->max_supported = 0;
+	edev->supported_cable = supported_cable;
+
+	return edev;
+}
+
+/*
+ * extcon_dev_free() - Free the memory of extcon device.
+ * @edev:	the extcon device to free
+ */
+void extcon_dev_free(struct extcon_dev *edev)
+{
+	if (edev)
+		kfree(edev);
+}
+EXPORT_SYMBOL_GPL(extcon_dev_free);
+
 /**
  * extcon_dev_register() - Register a new extcon device
  * @edev	: the new extcon device (should be allocated before calling)
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 548447b..f4fc983 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -192,6 +192,12 @@ extern void devm_extcon_dev_unregister(struct device *dev,
 extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
 
 /*
+ * Following APIs control the memory of extcon device.
+ */
+extern struct extcon_dev *extcon_dev_allocate(const char **cables);
+extern void extcon_dev_free(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
@@ -267,6 +273,13 @@ static inline int devm_extcon_dev_register(struct device *dev,
 static inline void devm_extcon_dev_unregister(struct device *dev,
 					      struct extcon_dev *edev) { }
 
+static inline struct extcon_dev *extcon_dev_allocate(const char **cables)
+{
+	return ERR_PTR(-ENOMEM);
+}
+
+static inline void extcon_dev_free(struct extcon_dev *edev) { }
+
 static inline u32 extcon_get_state(struct extcon_dev *edev)
 {
 	return 0;
-- 
1.8.0

Re: [PATCHv3 1/9] extcon: Add extcon_dev_allocate/free() to control the memory of extcon device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

On Thu, Apr 24, 2014 at 09:34:49PM +0900, Chanwoo Choi wrote:
> This patch add APIs to control the extcon device on extcon provider driver.
> The extcon_dev_allocate() allocates the memory of extcon device and initializes
> supported cables. And then extcon_dev_free() decrement the reference of the
> device of extcon device and free the memory of the extcon device. This APIs
> must need to implement devm_extcon_dev_allocate()/free() APIs.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/extcon/extcon-class.c | 39 +++++++++++++++++++++++++++++++++++++++
>  include/linux/extcon.h        | 13 +++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index f6df689..bec66d4 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -565,6 +565,45 @@ static void dummy_sysfs_dev_release(struct device *dev)
>  {
>  }
>  
> +/*
> + * extcon_dev_allocate() - Allocate the memory of extcon device.
> + * @supported_cable:	Array of supported cable names ending with NULL.
> + *			If supported_cable is NULL, cable name related APIs
> + *			are disabled.
> + *
> + * This function allocates the memory for extcon device without allocating
> + * memory in each extcon provider driver and initialize default setting for
> + * extcon device.
> + *
> + * Return the pointer of extcon device if success or ERR_PTR(err) if fail
> + */
> +struct extcon_dev *extcon_dev_allocate(const char **supported_cable)
> +{
> +	struct extcon_dev *edev;
> +
> +	edev = kzalloc(sizeof(*edev), GFP_KERNEL);
> +	if (!edev) {
> +		pr_err("Failed to allocate the memory for extcon device\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	edev->max_supported = 0;
> +	edev->supported_cable = supported_cable;
> +
> +	return edev;
> +}
> +
> +/*
> + * extcon_dev_free() - Free the memory of extcon device.
> + * @edev:	the extcon device to free
> + */
> +void extcon_dev_free(struct extcon_dev *edev)
> +{
> +	if (edev)
> +		kfree(edev);

kfree(NULL) is safe

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 1/9] extcon: Add extcon_dev_allocate/free() to control the memory of extcon device

[Chanwoo Choi]

Hi,

On Thu, Apr 24, 2014 at 11:42 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Apr 24, 2014 at 09:34:49PM +0900, Chanwoo Choi wrote:
>> This patch add APIs to control the extcon device on extcon provider driver.
>> The extcon_dev_allocate() allocates the memory of extcon device and initializes
>> supported cables. And then extcon_dev_free() decrement the reference of the
>> device of extcon device and free the memory of the extcon device. This APIs
>> must need to implement devm_extcon_dev_allocate()/free() APIs.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/extcon/extcon-class.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  include/linux/extcon.h        | 13 +++++++++++++
>>  2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index f6df689..bec66d4 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -565,6 +565,45 @@ static void dummy_sysfs_dev_release(struct device *dev)
>>  {
>>  }
>>
>> +/*
>> + * extcon_dev_allocate() - Allocate the memory of extcon device.
>> + * @supported_cable: Array of supported cable names ending with NULL.
>> + *                   If supported_cable is NULL, cable name related APIs
>> + *                   are disabled.
>> + *
>> + * This function allocates the memory for extcon device without allocating
>> + * memory in each extcon provider driver and initialize default setting for
>> + * extcon device.
>> + *
>> + * Return the pointer of extcon device if success or ERR_PTR(err) if fail
>> + */
>> +struct extcon_dev *extcon_dev_allocate(const char **supported_cable)
>> +{
>> +     struct extcon_dev *edev;
>> +
>> +     edev = kzalloc(sizeof(*edev), GFP_KERNEL);
>> +     if (!edev) {
>> +             pr_err("Failed to allocate the memory for extcon device\n");
>> +             return ERR_PTR(-ENOMEM);
>> +     }
>> +
>> +     edev->max_supported = 0;
>> +     edev->supported_cable = supported_cable;
>> +
>> +     return edev;
>> +}
>> +
>> +/*
>> + * extcon_dev_free() - Free the memory of extcon device.
>> + * @edev:    the extcon device to free
>> + */
>> +void extcon_dev_free(struct extcon_dev *edev)
>> +{
>> +     if (edev)
>> +             kfree(edev);
>
> kfree(NULL) is safe

I don't understand about meaning 'kfree(NULL)'.
Why do I free 'NULL' pointer' instead of 'edev' pointer?

Re: [PATCHv3 1/9] extcon: Add extcon_dev_allocate/free() to control the memory of extcon device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Hi,

On Thu, Apr 24, 2014 at 11:58:29PM +0900, Chanwoo Choi wrote:
> >> +void extcon_dev_free(struct extcon_dev *edev)
> >> +{
> >> +     if (edev)
> >> +             kfree(edev);
> >
> > kfree(NULL) is safe
> 
> I don't understand about meaning 'kfree(NULL)'.
> Why do I free 'NULL' pointer' instead of 'edev' pointer?

you don't need to check if evdev is valid before calling kfree() on it
because if it happens to be NULL, no cute bunnies will be sacrificed.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 1/9] extcon: Add extcon_dev_allocate/free() to control the memory of extcon device

[Chanwoo Choi]

On Fri, Apr 25, 2014 at 12:03 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Apr 24, 2014 at 11:58:29PM +0900, Chanwoo Choi wrote:
>> >> +void extcon_dev_free(struct extcon_dev *edev)
>> >> +{
>> >> +     if (edev)
>> >> +             kfree(edev);
>> >
>> > kfree(NULL) is safe
>>
>> I don't understand about meaning 'kfree(NULL)'.
>> Why do I free 'NULL' pointer' instead of 'edev' pointer?
>
> you don't need to check if evdev is valid before calling kfree() on it
> because if it happens to be NULL, no cute bunnies will be sacrificed.
>

OK, I'll fix it as following code without checking whether 'edev' is
NULL or not.

void extcon_dev_free(struct extcon_dev *edev)
{
          kfree(edev);
}

Thanks,
Chanwoo Choi

[PATCHv3 2/9] extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device

[Chanwoo Choi]

This patch add device managed devm_extcon_dev_{allocate,free} to automatically
free the memory of extcon_dev structure without handling free operation.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-class.c | 70 +++++++++++++++++++++++++++++++++++--------
 include/linux/extcon.h        | 11 +++++++
 2 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index bec66d4..f369168 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -604,6 +604,64 @@ void extcon_dev_free(struct extcon_dev *edev)
 }
 EXPORT_SYMBOL_GPL(extcon_dev_free);
 
+static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
+{
+	struct extcon_dev **r = res;
+
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+
+	return *r == data;
+}
+
+static void devm_extcon_dev_release(struct device *dev, void *res)
+{
+	extcon_dev_free(*(struct extcon_dev **)res);
+}
+
+/**
+ * devm_extcon_dev_allocate - Allocate managed extcon device
+ * @dev:		device owning the extcon device being created
+ * @supported_cable:	Array of supported cable names ending with NULL.
+ *			If supported_cable is NULL, cable name related APIs
+ *			are disabled.
+ *
+ * This function manages automatically the memory of extcon device using device
+ * resource management and simplify the control of freeing the memory of extcon
+ * device.
+ *
+ * Returns the pointer memory of allocated extcon_dev if success or NULL if fail
+ */
+struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
+					    const char **supported_cable)
+{
+	struct extcon_dev **ptr, *edev;
+
+	ptr = devres_alloc(devm_extcon_dev_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	edev = extcon_dev_allocate(supported_cable);
+	if (!IS_ERR(edev)) {
+		*ptr = edev;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return edev;
+}
+EXPORT_SYMBOL_GPL(devm_extcon_dev_allocate);
+
+void devm_extcon_dev_free(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_free);
+
 /**
  * extcon_dev_register() - Register a new extcon device
  * @edev	: the new extcon device (should be allocated before calling)
@@ -863,18 +921,6 @@ static void devm_extcon_dev_unreg(struct device *dev, void *res)
 	extcon_dev_unregister(*(struct extcon_dev **)res);
 }
 
-static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
-{
-	struct extcon_dev **r = res;
-
-	if (!r || !*r) {
-		WARN_ON(!r || !*r);
-		return 0;
-	}
-
-	return *r == data;
-}
-
 /**
  * devm_extcon_dev_register() - Resource-managed extcon_dev_register()
  * @dev:	device to allocate extcon device
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index f4fc983..3fd831b 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -196,6 +196,9 @@ extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
  */
 extern struct extcon_dev *extcon_dev_allocate(const char **cables);
 extern void extcon_dev_free(struct extcon_dev *edev);
+extern struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
+						   const char **cables);
+extern void devm_extcon_dev_free(struct device *dev, struct extcon_dev *edev);
 
 /*
  * get/set/update_state access the 32b encoded state value, which represents
@@ -280,6 +283,14 @@ static inline struct extcon_dev *extcon_dev_allocate(const char **cables)
 
 static inline void extcon_dev_free(struct extcon_dev *edev) { }
 
+static inline struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
+							  const char **cables)
+{
+	return ERR_PTR(-ENOMEM);
+}
+
+static inline void devm_extcon_dev_free(struct extcon_dev *edev) { }
+
 static inline u32 extcon_get_state(struct extcon_dev *edev)
 {
 	return 0;
-- 
1.8.0

Re: [PATCHv3 2/9] extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

On Thu, Apr 24, 2014 at 09:34:50PM +0900, Chanwoo Choi wrote:
> This patch add device managed devm_extcon_dev_{allocate,free} to automatically
> free the memory of extcon_dev structure without handling free operation.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/extcon/extcon-class.c | 70 +++++++++++++++++++++++++++++++++++--------
>  include/linux/extcon.h        | 11 +++++++
>  2 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index bec66d4..f369168 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -604,6 +604,64 @@ void extcon_dev_free(struct extcon_dev *edev)
>  }
>  EXPORT_SYMBOL_GPL(extcon_dev_free);
>  
> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
> +{
> +	struct extcon_dev **r = res;
> +
> +	if (!r || !*r) {
> +		WARN_ON(!r || !*r);
> +		return 0;
> +	}
> +
> +	return *r == data;
> +}
> +
> +static void devm_extcon_dev_release(struct device *dev, void *res)
> +{
> +	extcon_dev_free(*(struct extcon_dev **)res);
> +}
> +
> +/**
> + * devm_extcon_dev_allocate - Allocate managed extcon device
> + * @dev:		device owning the extcon device being created
> + * @supported_cable:	Array of supported cable names ending with NULL.
> + *			If supported_cable is NULL, cable name related APIs
> + *			are disabled.
> + *
> + * This function manages automatically the memory of extcon device using device
> + * resource management and simplify the control of freeing the memory of extcon
> + * device.
> + *
> + * Returns the pointer memory of allocated extcon_dev if success or NULL if fail
> + */
> +struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
> +					    const char **supported_cable)
> +{
> +	struct extcon_dev **ptr, *edev;
> +
> +	ptr = devres_alloc(devm_extcon_dev_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return NULL;

if this fails you return NULL;

> +
> +	edev = extcon_dev_allocate(supported_cable);

if this fails, you return -ENOMEM. Not very nice, if you ask me.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 2/9] extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device

[Chanwoo Choi]

Hi,

On Thu, Apr 24, 2014 at 11:44 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Apr 24, 2014 at 09:34:50PM +0900, Chanwoo Choi wrote:
>> This patch add device managed devm_extcon_dev_{allocate,free} to automatically
>> free the memory of extcon_dev structure without handling free operation.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/extcon/extcon-class.c | 70 +++++++++++++++++++++++++++++++++++--------
>>  include/linux/extcon.h        | 11 +++++++
>>  2 files changed, 69 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> index bec66d4..f369168 100644
>> --- a/drivers/extcon/extcon-class.c
>> +++ b/drivers/extcon/extcon-class.c
>> @@ -604,6 +604,64 @@ void extcon_dev_free(struct extcon_dev *edev)
>>  }
>>  EXPORT_SYMBOL_GPL(extcon_dev_free);
>>
>> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
>> +{
>> +     struct extcon_dev **r = res;
>> +
>> +     if (!r || !*r) {
>> +             WARN_ON(!r || !*r);
>> +             return 0;
>> +     }
>> +
>> +     return *r == data;
>> +}
>> +
>> +static void devm_extcon_dev_release(struct device *dev, void *res)
>> +{
>> +     extcon_dev_free(*(struct extcon_dev **)res);
>> +}
>> +
>> +/**
>> + * devm_extcon_dev_allocate - Allocate managed extcon device
>> + * @dev:             device owning the extcon device being created
>> + * @supported_cable: Array of supported cable names ending with NULL.
>> + *                   If supported_cable is NULL, cable name related APIs
>> + *                   are disabled.
>> + *
>> + * This function manages automatically the memory of extcon device using device
>> + * resource management and simplify the control of freeing the memory of extcon
>> + * device.
>> + *
>> + * Returns the pointer memory of allocated extcon_dev if success or NULL if fail
>> + */
>> +struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
>> +                                         const char **supported_cable)
>> +{
>> +     struct extcon_dev **ptr, *edev;
>> +
>> +     ptr = devres_alloc(devm_extcon_dev_release, sizeof(*ptr), GFP_KERNEL);
>> +     if (!ptr)
>> +             return NULL;
>
> if this fails you return NULL;
>
>> +
>> +     edev = extcon_dev_allocate(supported_cable);
>
> if this fails, you return -ENOMEM. Not very nice, if you ask me.

I'll fix it as following:

If  both extcon_dev_allocate() and devm_excon_dev_allocate() failed to
allocate memory,
two functions would return NULL pointer instead of  ERR_PTR(-ENOMEM).

Thanks,
Chanwoo Choi

Re: [PATCHv3 2/9] extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]

Hi,

On Fri, Apr 25, 2014 at 12:03:31AM +0900, Chanwoo Choi wrote:
> On Thu, Apr 24, 2014 at 11:44 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Thu, Apr 24, 2014 at 09:34:50PM +0900, Chanwoo Choi wrote:
> >> This patch add device managed devm_extcon_dev_{allocate,free} to automatically
> >> free the memory of extcon_dev structure without handling free operation.
> >>
> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> >> ---
> >>  drivers/extcon/extcon-class.c | 70 +++++++++++++++++++++++++++++++++++--------
> >>  include/linux/extcon.h        | 11 +++++++
> >>  2 files changed, 69 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> >> index bec66d4..f369168 100644
> >> --- a/drivers/extcon/extcon-class.c
> >> +++ b/drivers/extcon/extcon-class.c
> >> @@ -604,6 +604,64 @@ void extcon_dev_free(struct extcon_dev *edev)
> >>  }
> >>  EXPORT_SYMBOL_GPL(extcon_dev_free);
> >>
> >> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
> >> +{
> >> +     struct extcon_dev **r = res;
> >> +
> >> +     if (!r || !*r) {
> >> +             WARN_ON(!r || !*r);
> >> +             return 0;
> >> +     }
> >> +
> >> +     return *r == data;
> >> +}
> >> +
> >> +static void devm_extcon_dev_release(struct device *dev, void *res)
> >> +{
> >> +     extcon_dev_free(*(struct extcon_dev **)res);
> >> +}
> >> +
> >> +/**
> >> + * devm_extcon_dev_allocate - Allocate managed extcon device
> >> + * @dev:             device owning the extcon device being created
> >> + * @supported_cable: Array of supported cable names ending with NULL.
> >> + *                   If supported_cable is NULL, cable name related APIs
> >> + *                   are disabled.
> >> + *
> >> + * This function manages automatically the memory of extcon device using device
> >> + * resource management and simplify the control of freeing the memory of extcon
> >> + * device.
> >> + *
> >> + * Returns the pointer memory of allocated extcon_dev if success or NULL if fail
> >> + */
> >> +struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
> >> +                                         const char **supported_cable)
> >> +{
> >> +     struct extcon_dev **ptr, *edev;
> >> +
> >> +     ptr = devres_alloc(devm_extcon_dev_release, sizeof(*ptr), GFP_KERNEL);
> >> +     if (!ptr)
> >> +             return NULL;
> >
> > if this fails you return NULL;
> >
> >> +
> >> +     edev = extcon_dev_allocate(supported_cable);
> >
> > if this fails, you return -ENOMEM. Not very nice, if you ask me.
> 
> I'll fix it as following:
> 
> If  both extcon_dev_allocate() and devm_excon_dev_allocate() failed to
> allocate memory,
> two functions would return NULL pointer instead of  ERR_PTR(-ENOMEM).

your call, I'd rather see them both returning ERR_PTR(-ENOMEM), but no
strong feelings.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 2/9] extcon: Add devm_extcon_dev_allocate/free to manage the resource of extcon device

[Chanwoo Choi]

On Fri, Apr 25, 2014 at 12:06 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Apr 25, 2014 at 12:03:31AM +0900, Chanwoo Choi wrote:
>> On Thu, Apr 24, 2014 at 11:44 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Thu, Apr 24, 2014 at 09:34:50PM +0900, Chanwoo Choi wrote:
>> >> This patch add device managed devm_extcon_dev_{allocate,free} to automatically
>> >> free the memory of extcon_dev structure without handling free operation.
>> >>
>> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> >> ---
>> >>  drivers/extcon/extcon-class.c | 70 +++++++++++++++++++++++++++++++++++--------
>> >>  include/linux/extcon.h        | 11 +++++++
>> >>  2 files changed, 69 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
>> >> index bec66d4..f369168 100644
>> >> --- a/drivers/extcon/extcon-class.c
>> >> +++ b/drivers/extcon/extcon-class.c
>> >> @@ -604,6 +604,64 @@ void extcon_dev_free(struct extcon_dev *edev)
>> >>  }
>> >>  EXPORT_SYMBOL_GPL(extcon_dev_free);
>> >>
>> >> +static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
>> >> +{
>> >> +     struct extcon_dev **r = res;
>> >> +
>> >> +     if (!r || !*r) {
>> >> +             WARN_ON(!r || !*r);
>> >> +             return 0;
>> >> +     }
>> >> +
>> >> +     return *r == data;
>> >> +}
>> >> +
>> >> +static void devm_extcon_dev_release(struct device *dev, void *res)
>> >> +{
>> >> +     extcon_dev_free(*(struct extcon_dev **)res);
>> >> +}
>> >> +
>> >> +/**
>> >> + * devm_extcon_dev_allocate - Allocate managed extcon device
>> >> + * @dev:             device owning the extcon device being created
>> >> + * @supported_cable: Array of supported cable names ending with NULL.
>> >> + *                   If supported_cable is NULL, cable name related APIs
>> >> + *                   are disabled.
>> >> + *
>> >> + * This function manages automatically the memory of extcon device using device
>> >> + * resource management and simplify the control of freeing the memory of extcon
>> >> + * device.
>> >> + *
>> >> + * Returns the pointer memory of allocated extcon_dev if success or NULL if fail
>> >> + */
>> >> +struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
>> >> +                                         const char **supported_cable)
>> >> +{
>> >> +     struct extcon_dev **ptr, *edev;
>> >> +
>> >> +     ptr = devres_alloc(devm_extcon_dev_release, sizeof(*ptr), GFP_KERNEL);
>> >> +     if (!ptr)
>> >> +             return NULL;
>> >
>> > if this fails you return NULL;
>> >
>> >> +
>> >> +     edev = extcon_dev_allocate(supported_cable);
>> >
>> > if this fails, you return -ENOMEM. Not very nice, if you ask me.
>>
>> I'll fix it as following:
>>
>> If  both extcon_dev_allocate() and devm_excon_dev_allocate() failed to
>> allocate memory,
>> two functions would return NULL pointer instead of  ERR_PTR(-ENOMEM).
>
> your call, I'd rather see them both returning ERR_PTR(-ENOMEM), but no
> strong feelings.

OK, I'll fix it as your comment.

Thanks,
Chanwoo Choi

[PATCHv3 3/9] extcon: max8997: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max8997.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 804a446..e2ee2f5 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -699,8 +699,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
 	}
 
 	/* External connector */
-	info->edev = devm_kzalloc(&pdev->dev, sizeof(struct extcon_dev),
-				  GFP_KERNEL);
+	info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable);
 	if (!info->edev) {
 		dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
 		ret = -ENOMEM;
@@ -708,7 +707,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 = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
-- 
1.8.0

Re: [PATCHv3 3/9] extcon: max8997: Use devm_extcon_dev_allocate for extcon_dev

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Thu, Apr 24, 2014 at 09:34:51PM +0900, Chanwoo Choi wrote:
> This patch use devm_extcon_dev_allocate() to simplify the memory control
> of extcon device.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/extcon/extcon-max8997.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
> index 804a446..e2ee2f5 100644
> --- a/drivers/extcon/extcon-max8997.c
> +++ b/drivers/extcon/extcon-max8997.c
> @@ -699,8 +699,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
>  	}
>  
>  	/* External connector */
> -	info->edev = devm_kzalloc(&pdev->dev, sizeof(struct extcon_dev),
> -				  GFP_KERNEL);
> +	info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable);
>  	if (!info->edev) {

and here you never check for IS_ERR(info->evdev)

NAK

I suspect all following patches have the same defect.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 3/9] extcon: max8997: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

On Thu, Apr 24, 2014 at 11:45 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Apr 24, 2014 at 09:34:51PM +0900, Chanwoo Choi wrote:
>> This patch use devm_extcon_dev_allocate() to simplify the memory control
>> of extcon device.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/extcon/extcon-max8997.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
>> index 804a446..e2ee2f5 100644
>> --- a/drivers/extcon/extcon-max8997.c
>> +++ b/drivers/extcon/extcon-max8997.c
>> @@ -699,8 +699,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
>>       }
>>
>>       /* External connector */
>> -     info->edev = devm_kzalloc(&pdev->dev, sizeof(struct extcon_dev),
>> -                               GFP_KERNEL);
>> +     info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable);
>>       if (!info->edev) {
>
> and here you never check for IS_ERR(info->evdev)
>
> NAK
>
> I suspect all following patches have the same defect.

OK, I'll use IS_ERR() instead of checking NULL.

Thanks,
Chanwoo Choi

[PATCHv3 4/9] extcon: max77693: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max77693.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index f0f18e2..2e15a24 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -1175,8 +1175,8 @@ static int max77693_muic_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize extcon device */
-	info->edev = devm_kzalloc(&pdev->dev, sizeof(struct extcon_dev),
-				  GFP_KERNEL);
+	info->edev = devm_extcon_dev_allocate(&pdev->dev,
+					      max77693_extcon_cable);
 	if (!info->edev) {
 		dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
 		ret = -ENOMEM;
@@ -1184,14 +1184,13 @@ 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 = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
 		goto err_irq;
 	}
 
-
 	/* Initialize MUIC register by using platform data or default data */
 	if (pdata && pdata->muic_data) {
 		init_data = pdata->muic_data->init_data;
-- 
1.8.0

[PATCHv3 5/9] extcon: max14577: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/extcon/extcon-max14577.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
index d17dd4f..7d4aa5a 100644
--- a/drivers/extcon/extcon-max14577.c
+++ b/drivers/extcon/extcon-max14577.c
@@ -739,14 +739,15 @@ static int max14577_muic_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize extcon device */
-	info->edev = devm_kzalloc(&pdev->dev, sizeof(*info->edev), GFP_KERNEL);
+	info->edev = devm_extcon_dev_allocate(&pdev->dev,
+					      max14577_extcon_cable);
 	if (!info->edev) {
 		dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
 		return -ENOMEM;
 	}
 
 	info->edev->name = dev_name(&pdev->dev);
-	info->edev->supported_cable = max14577_extcon_cable;
+
 	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register extcon device\n");
-- 
1.8.0

[PATCHv3 6/9] extcon: arizona: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: patches@opensource.wolfsonmicro.com
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index f63fa6f..78931f1 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -91,7 +91,7 @@ struct arizona_extcon_info {
 
 	int hpdet_ip;
 
-	struct extcon_dev edev;
+	struct extcon_dev *edev;
 };
 
 static const struct arizona_micd_config micd_default_modes[] = {
@@ -546,7 +546,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	}
 
 	/* If the cable was removed while measuring ignore the result */
-	ret = extcon_get_cable_state_(&info->edev, ARIZONA_CABLE_MECHANICAL);
+	ret = extcon_get_cable_state_(info->edev, ARIZONA_CABLE_MECHANICAL);
 	if (ret < 0) {
 		dev_err(arizona->dev, "Failed to check cable state: %d\n",
 			ret);
@@ -581,7 +581,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	else
 		report = ARIZONA_CABLE_HEADPHONE;
 
-	ret = extcon_set_cable_state_(&info->edev, report, true);
+	ret = extcon_set_cable_state_(info->edev, report, true);
 	if (ret != 0)
 		dev_err(arizona->dev, "Failed to report HP/line: %d\n",
 			ret);
@@ -664,7 +664,7 @@ err:
 			   ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
 
 	/* Just report headphone */
-	ret = extcon_update_state(&info->edev,
+	ret = extcon_update_state(info->edev,
 				  1 << ARIZONA_CABLE_HEADPHONE,
 				  1 << ARIZONA_CABLE_HEADPHONE);
 	if (ret != 0)
@@ -723,7 +723,7 @@ err:
 			   ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
 
 	/* Just report headphone */
-	ret = extcon_update_state(&info->edev,
+	ret = extcon_update_state(info->edev,
 				  1 << ARIZONA_CABLE_HEADPHONE,
 				  1 << ARIZONA_CABLE_HEADPHONE);
 	if (ret != 0)
@@ -764,7 +764,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	mutex_lock(&info->lock);
 
 	/* If the cable was removed while measuring ignore the result */
-	ret = extcon_get_cable_state_(&info->edev, ARIZONA_CABLE_MECHANICAL);
+	ret = extcon_get_cable_state_(info->edev, ARIZONA_CABLE_MECHANICAL);
 	if (ret < 0) {
 		dev_err(arizona->dev, "Failed to check cable state: %d\n",
 				ret);
@@ -812,7 +812,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	if (info->detecting && (val & ARIZONA_MICD_LVL_8)) {
 		arizona_identify_headphone(info);
 
-		ret = extcon_update_state(&info->edev,
+		ret = extcon_update_state(info->edev,
 					  1 << ARIZONA_CABLE_MICROPHONE,
 					  1 << ARIZONA_CABLE_MICROPHONE);
 
@@ -999,7 +999,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 
 	if (info->last_jackdet == present) {
 		dev_dbg(arizona->dev, "Detected jack\n");
-		ret = extcon_set_cable_state_(&info->edev,
+		ret = extcon_set_cable_state_(info->edev,
 					      ARIZONA_CABLE_MECHANICAL, true);
 
 		if (ret != 0)
@@ -1038,7 +1038,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 					 info->micd_ranges[i].key, 0);
 		input_sync(info->input);
 
-		ret = extcon_update_state(&info->edev, 0xffffffff, 0);
+		ret = extcon_update_state(info->edev, 0xffffffff, 0);
 		if (ret != 0)
 			dev_err(arizona->dev, "Removal report failed: %d\n",
 				ret);
@@ -1150,11 +1150,16 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		break;
 	}
 
-	info->edev.name = "Headset Jack";
-	info->edev.dev.parent = arizona->dev;
-	info->edev.supported_cable = arizona_cable;
+	info->edev = devm_extcon_dev_allocate(&pdev->dev, arizona_cable);
+	if (!info->edev) {
+		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		return -ENOMEM;
+	}
+	info->edev->name = "Headset Jack";
+	info->edev->dev.parent = arizona->dev;
+
 
-	ret = devm_extcon_dev_register(&pdev->dev, &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);
-- 
1.8.0

[PATCHv3 7/9] extcon: adc-jack: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-adc-jack.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 549d820..fd3bc661 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -39,7 +39,7 @@
  * @chan:		iio channel being queried.
  */
 struct adc_jack_data {
-	struct extcon_dev edev;
+	struct extcon_dev *edev;
 
 	const char **cable_names;
 	int num_cables;
@@ -64,7 +64,7 @@ static void adc_jack_handler(struct work_struct *work)
 
 	ret = iio_read_channel_raw(data->chan, &adc_val);
 	if (ret < 0) {
-		dev_err(&data->edev.dev, "read channel() error: %d\n", ret);
+		dev_err(&data->edev->dev, "read channel() error: %d\n", ret);
 		return;
 	}
 
@@ -80,7 +80,7 @@ static void adc_jack_handler(struct work_struct *work)
 	}
 	/* if no def has met, it means state = 0 (no cables attached) */
 
-	extcon_set_state(&data->edev, state);
+	extcon_set_state(data->edev, state);
 }
 
 static irqreturn_t adc_jack_irq_thread(int irq, void *_data)
@@ -102,18 +102,21 @@ static int adc_jack_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
-	data->edev.name = pdata->name;
-
 	if (!pdata->cable_names) {
 		dev_err(&pdev->dev, "error: cable_names not defined.\n");
 		return -EINVAL;
 	}
 
-	data->edev.dev.parent = &pdev->dev;
-	data->edev.supported_cable = pdata->cable_names;
+	data->edev = devm_extcon_dev_allocate(&pdev->dev, pdata->cable_names);
+	if (!data->edev) {
+		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		return -ENOMEM;
+	}
+	data->edev->dev.parent = &pdev->dev;
+	data->edev->name = pdata->name;
 
 	/* Check the length of array and set num_cables */
-	for (i = 0; data->edev.supported_cable[i]; i++)
+	for (i = 0; data->edev->supported_cable[i]; i++)
 		;
 	if (i == 0 || i > SUPPORTED_CABLE_MAX) {
 		dev_err(&pdev->dev, "error: pdata->cable_names size = %d\n",
@@ -144,7 +147,7 @@ static int adc_jack_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
-	err = devm_extcon_dev_register(&pdev->dev, &data->edev);
+	err = devm_extcon_dev_register(&pdev->dev, data->edev);
 	if (err)
 		return err;
 
-- 
1.8.0

[PATCHv3 8/9] extcon: gpio: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 43af34c..41e2157 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -32,7 +32,7 @@
 #include <linux/extcon/extcon-gpio.h>
 
 struct gpio_extcon_data {
-	struct extcon_dev edev;
+	struct extcon_dev *edev;
 	unsigned gpio;
 	bool gpio_active_low;
 	const char *state_on;
@@ -53,7 +53,7 @@ static void gpio_extcon_work(struct work_struct *work)
 	state = gpio_get_value(data->gpio);
 	if (data->gpio_active_low)
 		state = !state;
-	extcon_set_state(&data->edev, state);
+	extcon_set_state(data->edev, state);
 }
 
 static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
@@ -67,9 +67,10 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
 
 static ssize_t extcon_gpio_print_state(struct extcon_dev *edev, char *buf)
 {
-	struct gpio_extcon_data	*extcon_data =
-		container_of(edev, struct gpio_extcon_data, edev);
+	struct device *dev = edev->dev.parent;
+	struct gpio_extcon_data *extcon_data = dev_get_drvdata(dev);
 	const char *state;
+
 	if (extcon_get_state(edev))
 		state = extcon_data->state_on;
 	else
@@ -98,15 +99,21 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	if (!extcon_data)
 		return -ENOMEM;
 
-	extcon_data->edev.name = pdata->name;
-	extcon_data->edev.dev.parent = &pdev->dev;
+	extcon_data->edev = devm_extcon_dev_allocate(&pdev->dev, NULL);
+	if (!extcon_data->edev) {
+		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		return -ENOMEM;
+	}
+	extcon_data->edev->name = pdata->name;
+	extcon_data->edev->dev.parent = &pdev->dev;
+
 	extcon_data->gpio = pdata->gpio;
 	extcon_data->gpio_active_low = pdata->gpio_active_low;
 	extcon_data->state_on = pdata->state_on;
 	extcon_data->state_off = pdata->state_off;
 	extcon_data->check_on_resume = pdata->check_on_resume;
 	if (pdata->state_on && pdata->state_off)
-		extcon_data->edev.print_state = extcon_gpio_print_state;
+		extcon_data->edev->print_state = extcon_gpio_print_state;
 
 	ret = devm_gpio_request_one(&pdev->dev, extcon_data->gpio, GPIOF_DIR_IN,
 				    pdev->name);
@@ -121,7 +128,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 				msecs_to_jiffies(pdata->debounce);
 	}
 
-	ret = devm_extcon_dev_register(&pdev->dev, &extcon_data->edev);
+	ret = devm_extcon_dev_register(&pdev->dev, extcon_data->edev);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.0

[PATCHv3 9/9] extcon: palmas: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

This patch use devm_extcon_dev_allocate() to simplify the memory control
of extcon device.

Cc: Graeme Gregory <gg@slimlogic.co.uk>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/extcon/extcon-palmas.c | 35 ++++++++++++++++++++---------------
 include/linux/mfd/palmas.h     |  2 +-
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 1a770e0..a9299ea 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -57,7 +57,7 @@ static irqreturn_t palmas_vbus_irq_handler(int irq, void *_palmas_usb)
 	if (vbus_line_state & PALMAS_INT3_LINE_STATE_VBUS) {
 		if (palmas_usb->linkstat != PALMAS_USB_STATE_VBUS) {
 			palmas_usb->linkstat = PALMAS_USB_STATE_VBUS;
-			extcon_set_cable_state(&palmas_usb->edev, "USB", true);
+			extcon_set_cable_state(palmas_usb->edev, "USB", true);
 			dev_info(palmas_usb->dev, "USB cable is attached\n");
 		} else {
 			dev_dbg(palmas_usb->dev,
@@ -66,7 +66,7 @@ static irqreturn_t palmas_vbus_irq_handler(int irq, void *_palmas_usb)
 	} else if (!(vbus_line_state & PALMAS_INT3_LINE_STATE_VBUS)) {
 		if (palmas_usb->linkstat == PALMAS_USB_STATE_VBUS) {
 			palmas_usb->linkstat = PALMAS_USB_STATE_DISCONNECT;
-			extcon_set_cable_state(&palmas_usb->edev, "USB", false);
+			extcon_set_cable_state(palmas_usb->edev, "USB", false);
 			dev_info(palmas_usb->dev, "USB cable is detached\n");
 		} else {
 			dev_dbg(palmas_usb->dev,
@@ -93,7 +93,7 @@ static irqreturn_t palmas_id_irq_handler(int irq, void *_palmas_usb)
 			PALMAS_USB_ID_INT_LATCH_CLR,
 			PALMAS_USB_ID_INT_EN_HI_CLR_ID_GND);
 		palmas_usb->linkstat = PALMAS_USB_STATE_ID;
-		extcon_set_cable_state(&palmas_usb->edev, "USB-HOST", true);
+		extcon_set_cable_state(palmas_usb->edev, "USB-HOST", true);
 		dev_info(palmas_usb->dev, "USB-HOST cable is attached\n");
 	} else if ((set & PALMAS_USB_ID_INT_SRC_ID_FLOAT) &&
 				(id_src & PALMAS_USB_ID_INT_SRC_ID_FLOAT)) {
@@ -101,17 +101,17 @@ static irqreturn_t palmas_id_irq_handler(int irq, void *_palmas_usb)
 			PALMAS_USB_ID_INT_LATCH_CLR,
 			PALMAS_USB_ID_INT_EN_HI_CLR_ID_FLOAT);
 		palmas_usb->linkstat = PALMAS_USB_STATE_DISCONNECT;
-		extcon_set_cable_state(&palmas_usb->edev, "USB-HOST", false);
+		extcon_set_cable_state(palmas_usb->edev, "USB-HOST", false);
 		dev_info(palmas_usb->dev, "USB-HOST cable is detached\n");
 	} else if ((palmas_usb->linkstat == PALMAS_USB_STATE_ID) &&
 				(!(set & PALMAS_USB_ID_INT_SRC_ID_GND))) {
 		palmas_usb->linkstat = PALMAS_USB_STATE_DISCONNECT;
-		extcon_set_cable_state(&palmas_usb->edev, "USB-HOST", false);
+		extcon_set_cable_state(palmas_usb->edev, "USB-HOST", false);
 		dev_info(palmas_usb->dev, "USB-HOST cable is detached\n");
 	} else if ((palmas_usb->linkstat == PALMAS_USB_STATE_DISCONNECT) &&
 				(id_src & PALMAS_USB_ID_INT_SRC_ID_GND)) {
 		palmas_usb->linkstat = PALMAS_USB_STATE_ID;
-		extcon_set_cable_state(&palmas_usb->edev, "USB-HOST", true);
+		extcon_set_cable_state(palmas_usb->edev, "USB-HOST", true);
 		dev_info(palmas_usb->dev, " USB-HOST cable is attached\n");
 	}
 
@@ -187,15 +187,20 @@ static int palmas_usb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, palmas_usb);
 
-	palmas_usb->edev.supported_cable = palmas_extcon_cable;
-	palmas_usb->edev.dev.parent = palmas_usb->dev;
-	palmas_usb->edev.name = kstrdup(node->name, GFP_KERNEL);
-	palmas_usb->edev.mutually_exclusive = mutually_exclusive;
+	palmas_usb->edev = devm_extcon_dev_allocate(&pdev->dev,
+						    palmas_extcon_cable);
+	if (!palmas_usb->edev) {
+		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		return -ENOMEM;
+	}
+	palmas_usb->edev->name = kstrdup(node->name, GFP_KERNEL);
+	palmas_usb->edev->dev.parent = palmas_usb->dev;
+	palmas_usb->edev->mutually_exclusive = mutually_exclusive;
 
-	status = devm_extcon_dev_register(&pdev->dev, &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);
+		kfree(palmas_usb->edev->name);
 		return status;
 	}
 
@@ -209,7 +214,7 @@ 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);
-			kfree(palmas_usb->edev.name);
+			kfree(palmas_usb->edev->name);
 			return status;
 		}
 	}
@@ -224,7 +229,7 @@ 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);
-			kfree(palmas_usb->edev.name);
+			kfree(palmas_usb->edev->name);
 			return status;
 		}
 	}
@@ -238,7 +243,7 @@ static int palmas_usb_remove(struct platform_device *pdev)
 {
 	struct palmas_usb *palmas_usb = platform_get_drvdata(pdev);
 
-	kfree(palmas_usb->edev.name);
+	kfree(palmas_usb->edev->name);
 
 	return 0;
 }
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 9974e38..b8f87b7 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -415,7 +415,7 @@ struct palmas_usb {
 	struct palmas *palmas;
 	struct device *dev;
 
-	struct extcon_dev edev;
+	struct extcon_dev *edev;
 
 	int id_otg_irq;
 	int id_irq;
-- 
1.8.0

Re: [PATCHv3 9/9] extcon: palmas: Use devm_extcon_dev_allocate for extcon_dev

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Thu, Apr 24, 2014 at 09:34:57PM +0900, Chanwoo Choi wrote:
> This patch use devm_extcon_dev_allocate() to simplify the memory control
> of extcon device.
> 
> Cc: Graeme Gregory <gg@slimlogic.co.uk>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Tested-by: Felipe Balbi <balbi@ti.com>

*BUT* you need to fix NULL vs ERR_PTR return in your code. The way this
is, we can easily dereference an ERR_PTR because the code doesn't test
for it. I suggest always returning valid pointer or ERR_PTR and avoid
returning NULL at all.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCHv3 9/9] extcon: palmas: Use devm_extcon_dev_allocate for extcon_dev

[Chanwoo Choi]

On Thu, Apr 24, 2014 at 11:47 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Apr 24, 2014 at 09:34:57PM +0900, Chanwoo Choi wrote:
>> This patch use devm_extcon_dev_allocate() to simplify the memory control
>> of extcon device.
>>
>> Cc: Graeme Gregory <gg@slimlogic.co.uk>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Cc: Samuel Ortiz <sameo@linux.intel.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>
> Tested-by: Felipe Balbi <balbi@ti.com>

Thanks for your test.

>
> *BUT* you need to fix NULL vs ERR_PTR return in your code. The way this
> is, we can easily dereference an ERR_PTR because the code doesn't test
> for it. I suggest always returning valid pointer or ERR_PTR and avoid
> returning NULL at all.

OK, as my reply on other mail, I'll fix return type by using ERR_PTR
instead of NULL.

Thanks,
Chanwoo Choi