[v4,2/4] extcon: Added extcon_alloc_cables to simplify extcon register function
Commit Message
The cable allocation part is functionalized from extcon_dev_register.
Signed-off-by: Bumwoo Lee <bw365.lee@samsung.com>
---
drivers/extcon/extcon.c | 108 +++++++++++++++++++++++-----------------
1 file changed, 62 insertions(+), 46 deletions(-)
Comments
On 23. 3. 2. 18:01, Bumwoo Lee wrote:
> The cable allocation part is functionalized from extcon_dev_register.
>
> Signed-off-by: Bumwoo Lee <bw365.lee@samsung.com>
> ---
> drivers/extcon/extcon.c | 108 +++++++++++++++++++++++-----------------
> 1 file changed, 62 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index adcf01132f70..49605e96bedd 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1070,6 +1070,63 @@ void extcon_dev_free(struct extcon_dev *edev)
> }
> EXPORT_SYMBOL_GPL(extcon_dev_free);
>
> +/**
> + * extcon_alloc_cables() - alloc the cables for extcon device
> + * @edev: extcon device which has cables
> + *
> + * Returns 0 if success or error number if fail.
> + */
> +static int extcon_alloc_cables(struct extcon_dev *edev)
> +{
> + int index;
> + char *str;
> + struct extcon_cable *cable;
> +
> + if (!edev->max_supported)
> + return 0;
Need to check whether edev is NULL or not
because make the separate function.
if (!edev && !edev->max_supported)
> +
> + edev->cables = kcalloc(edev->max_supported,
> + sizeof(struct extcon_cable),
> + GFP_KERNEL);
> + if (!edev->cables)
> + return -ENOMEM;
> +
> + for (index = 0; index < edev->max_supported; index++) {
> + cable = &edev->cables[index];
> +
> + str = kasprintf(GFP_KERNEL, "cable.%d", index);
> + if (!str) {
> + for (index--; index >= 0; index--) {
> + cable = &edev->cables[index];
> + kfree(cable->attr_g.name);
> + }
> +
> + kfree(edev->cables);
> + return -ENOMEM;
> + }
> +
> + cable->edev = edev;
> + cable->cable_index = index;
> + cable->attrs[0] = &cable->attr_name.attr;
> + cable->attrs[1] = &cable->attr_state.attr;
> + cable->attrs[2] = NULL;
> + cable->attr_g.name = str;
> + cable->attr_g.attrs = cable->attrs;
> +
> + sysfs_attr_init(&cable->attr_name.attr);
> + cable->attr_name.attr.name = "name";
> + cable->attr_name.attr.mode = 0444;
> + cable->attr_name.show = cable_name_show;
> +
> + sysfs_attr_init(&cable->attr_state.attr);
> + cable->attr_state.attr.name = "state";
> + cable->attr_state.attr.mode = 0444;
> + cable->attr_state.show = cable_state_show;
> + }
> +
> + return 0;
> +}
> +
> /**
> * extcon_dev_register() - Register an new extcon device
> * @edev: the extcon device to be registered
> @@ -1117,50 +1174,9 @@ int extcon_dev_register(struct extcon_dev *edev)
> dev_set_name(&edev->dev, "extcon%lu",
> (unsigned long)atomic_inc_return(&edev_no));
>
> - if (edev->max_supported) {
> - char *str;
> - struct extcon_cable *cable;
> -
> - edev->cables = kcalloc(edev->max_supported,
> - sizeof(struct extcon_cable),
> - GFP_KERNEL);
> - if (!edev->cables) {
> - ret = -ENOMEM;
> - goto err_sysfs_alloc;
> - }
> - for (index = 0; index < edev->max_supported; index++) {
> - cable = &edev->cables[index];
> -
> - str = kasprintf(GFP_KERNEL, "cable.%d", index);
> - if (!str) {
> - for (index--; index >= 0; index--) {
> - cable = &edev->cables[index];
> - kfree(cable->attr_g.name);
> - }
> - ret = -ENOMEM;
> -
> - goto err_alloc_cables;
> - }
> -
> - cable->edev = edev;
> - cable->cable_index = index;
> - cable->attrs[0] = &cable->attr_name.attr;
> - cable->attrs[1] = &cable->attr_state.attr;
> - cable->attrs[2] = NULL;
> - cable->attr_g.name = str;
> - cable->attr_g.attrs = cable->attrs;
> -
> - sysfs_attr_init(&cable->attr_name.attr);
> - cable->attr_name.attr.name = "name";
> - cable->attr_name.attr.mode = 0444;
> - cable->attr_name.show = cable_name_show;
> -
> - sysfs_attr_init(&cable->attr_state.attr);
> - cable->attr_state.attr.name = "state";
> - cable->attr_state.attr.mode = 0444;
> - cable->attr_state.show = cable_state_show;
> - }
> - }
> + ret = extcon_alloc_cables(edev);
> + if (ret < 0)
> + goto err_alloc_cables;
>
> if (edev->max_supported && edev->mutually_exclusive) {
> char *name;
> @@ -1279,10 +1295,10 @@ int extcon_dev_register(struct extcon_dev *edev)
> err_muex:
> for (index = 0; index < edev->max_supported; index++)
> kfree(edev->cables[index].attr_g.name);
> -err_alloc_cables:
> if (edev->max_supported)
> kfree(edev->cables);
> -err_sysfs_alloc:
> +err_alloc_cables:
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(extcon_dev_register);
On 23. 3. 13. 20:06, Chanwoo Choi wrote:
> On 23. 3. 2. 18:01, Bumwoo Lee wrote:
>> The cable allocation part is functionalized from extcon_dev_register.
>>
>> Signed-off-by: Bumwoo Lee <bw365.lee@samsung.com>
>> ---
>> drivers/extcon/extcon.c | 108 +++++++++++++++++++++++-----------------
>> 1 file changed, 62 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index adcf01132f70..49605e96bedd 100644
>> --- a/drivers/extcon/extcon.c
>> +++ b/drivers/extcon/extcon.c
>> @@ -1070,6 +1070,63 @@ void extcon_dev_free(struct extcon_dev *edev)
>> }
>> EXPORT_SYMBOL_GPL(extcon_dev_free);
>>
>> +/**
>> + * extcon_alloc_cables() - alloc the cables for extcon device
>> + * @edev: extcon device which has cables
>> + *
>> + * Returns 0 if success or error number if fail.
>> + */
>> +static int extcon_alloc_cables(struct extcon_dev *edev)
>> +{
>> + int index;
>> + char *str;
>> + struct extcon_cable *cable;
>> +
>> + if (!edev->max_supported)
>> + return 0;
>
>
> Need to check whether edev is NULL or not
> because make the separate function.
>
> if (!edev && !edev->max_supported)
Instead of previous my comment, need to check it as following:
if (!edev)
return -EINVAL;
if (!edev->max_supported)
return 0;
>
>> +
>> + edev->cables = kcalloc(edev->max_supported,
>> + sizeof(struct extcon_cable),
>> + GFP_KERNEL);
>> + if (!edev->cables)
>> + return -ENOMEM;
>> +
>> + for (index = 0; index < edev->max_supported; index++) {
>> + cable = &edev->cables[index];
>> +
>> + str = kasprintf(GFP_KERNEL, "cable.%d", index);
>> + if (!str) {
>> + for (index--; index >= 0; index--) {
>> + cable = &edev->cables[index];
>> + kfree(cable->attr_g.name);
>> + }
>> +
>> + kfree(edev->cables);
>> + return -ENOMEM;
>> + }
>> +
>> + cable->edev = edev;
>> + cable->cable_index = index;
>> + cable->attrs[0] = &cable->attr_name.attr;
>> + cable->attrs[1] = &cable->attr_state.attr;
>> + cable->attrs[2] = NULL;
>> + cable->attr_g.name = str;
>> + cable->attr_g.attrs = cable->attrs;
>> +
>> + sysfs_attr_init(&cable->attr_name.attr);
>> + cable->attr_name.attr.name = "name";
>> + cable->attr_name.attr.mode = 0444;
>> + cable->attr_name.show = cable_name_show;
>> +
>> + sysfs_attr_init(&cable->attr_state.attr);
>> + cable->attr_state.attr.name = "state";
>> + cable->attr_state.attr.mode = 0444;
>> + cable->attr_state.show = cable_state_show;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * extcon_dev_register() - Register an new extcon device
>> * @edev: the extcon device to be registered
>> @@ -1117,50 +1174,9 @@ int extcon_dev_register(struct extcon_dev *edev)
>> dev_set_name(&edev->dev, "extcon%lu",
>> (unsigned long)atomic_inc_return(&edev_no));
>>
>> - if (edev->max_supported) {
>> - char *str;
>> - struct extcon_cable *cable;
>> -
>> - edev->cables = kcalloc(edev->max_supported,
>> - sizeof(struct extcon_cable),
>> - GFP_KERNEL);
>> - if (!edev->cables) {
>> - ret = -ENOMEM;
>> - goto err_sysfs_alloc;
>> - }
>> - for (index = 0; index < edev->max_supported; index++) {
>> - cable = &edev->cables[index];
>> -
>> - str = kasprintf(GFP_KERNEL, "cable.%d", index);
>> - if (!str) {
>> - for (index--; index >= 0; index--) {
>> - cable = &edev->cables[index];
>> - kfree(cable->attr_g.name);
>> - }
>> - ret = -ENOMEM;
>> -
>> - goto err_alloc_cables;
>> - }
>> -
>> - cable->edev = edev;
>> - cable->cable_index = index;
>> - cable->attrs[0] = &cable->attr_name.attr;
>> - cable->attrs[1] = &cable->attr_state.attr;
>> - cable->attrs[2] = NULL;
>> - cable->attr_g.name = str;
>> - cable->attr_g.attrs = cable->attrs;
>> -
>> - sysfs_attr_init(&cable->attr_name.attr);
>> - cable->attr_name.attr.name = "name";
>> - cable->attr_name.attr.mode = 0444;
>> - cable->attr_name.show = cable_name_show;
>> -
>> - sysfs_attr_init(&cable->attr_state.attr);
>> - cable->attr_state.attr.name = "state";
>> - cable->attr_state.attr.mode = 0444;
>> - cable->attr_state.show = cable_state_show;
>> - }
>> - }
>> + ret = extcon_alloc_cables(edev);
>> + if (ret < 0)
>> + goto err_alloc_cables;
>>
>> if (edev->max_supported && edev->mutually_exclusive) {
>> char *name;
>> @@ -1279,10 +1295,10 @@ int extcon_dev_register(struct extcon_dev *edev)
>> err_muex:
>> for (index = 0; index < edev->max_supported; index++)
>> kfree(edev->cables[index].attr_g.name);
>> -err_alloc_cables:
>> if (edev->max_supported)
>> kfree(edev->cables);
>> -err_sysfs_alloc:
>> +err_alloc_cables:
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(extcon_dev_register);
>
@@ -1070,6 +1070,63 @@ void extcon_dev_free(struct extcon_dev *edev)
}
EXPORT_SYMBOL_GPL(extcon_dev_free);
+/**
+ * extcon_alloc_cables() - alloc the cables for extcon device
+ * @edev: extcon device which has cables
+ *
+ * Returns 0 if success or error number if fail.
+ */
+static int extcon_alloc_cables(struct extcon_dev *edev)
+{
+ int index;
+ char *str;
+ struct extcon_cable *cable;
+
+ if (!edev->max_supported)
+ return 0;
+
+ edev->cables = kcalloc(edev->max_supported,
+ sizeof(struct extcon_cable),
+ GFP_KERNEL);
+ if (!edev->cables)
+ return -ENOMEM;
+
+ for (index = 0; index < edev->max_supported; index++) {
+ cable = &edev->cables[index];
+
+ str = kasprintf(GFP_KERNEL, "cable.%d", index);
+ if (!str) {
+ for (index--; index >= 0; index--) {
+ cable = &edev->cables[index];
+ kfree(cable->attr_g.name);
+ }
+
+ kfree(edev->cables);
+ return -ENOMEM;
+ }
+
+ cable->edev = edev;
+ cable->cable_index = index;
+ cable->attrs[0] = &cable->attr_name.attr;
+ cable->attrs[1] = &cable->attr_state.attr;
+ cable->attrs[2] = NULL;
+ cable->attr_g.name = str;
+ cable->attr_g.attrs = cable->attrs;
+
+ sysfs_attr_init(&cable->attr_name.attr);
+ cable->attr_name.attr.name = "name";
+ cable->attr_name.attr.mode = 0444;
+ cable->attr_name.show = cable_name_show;
+
+ sysfs_attr_init(&cable->attr_state.attr);
+ cable->attr_state.attr.name = "state";
+ cable->attr_state.attr.mode = 0444;
+ cable->attr_state.show = cable_state_show;
+ }
+
+ return 0;
+}
+
/**
* extcon_dev_register() - Register an new extcon device
* @edev: the extcon device to be registered
@@ -1117,50 +1174,9 @@ int extcon_dev_register(struct extcon_dev *edev)
dev_set_name(&edev->dev, "extcon%lu",
(unsigned long)atomic_inc_return(&edev_no));
- if (edev->max_supported) {
- char *str;
- struct extcon_cable *cable;
-
- edev->cables = kcalloc(edev->max_supported,
- sizeof(struct extcon_cable),
- GFP_KERNEL);
- if (!edev->cables) {
- ret = -ENOMEM;
- goto err_sysfs_alloc;
- }
- for (index = 0; index < edev->max_supported; index++) {
- cable = &edev->cables[index];
-
- str = kasprintf(GFP_KERNEL, "cable.%d", index);
- if (!str) {
- for (index--; index >= 0; index--) {
- cable = &edev->cables[index];
- kfree(cable->attr_g.name);
- }
- ret = -ENOMEM;
-
- goto err_alloc_cables;
- }
-
- cable->edev = edev;
- cable->cable_index = index;
- cable->attrs[0] = &cable->attr_name.attr;
- cable->attrs[1] = &cable->attr_state.attr;
- cable->attrs[2] = NULL;
- cable->attr_g.name = str;
- cable->attr_g.attrs = cable->attrs;
-
- sysfs_attr_init(&cable->attr_name.attr);
- cable->attr_name.attr.name = "name";
- cable->attr_name.attr.mode = 0444;
- cable->attr_name.show = cable_name_show;
-
- sysfs_attr_init(&cable->attr_state.attr);
- cable->attr_state.attr.name = "state";
- cable->attr_state.attr.mode = 0444;
- cable->attr_state.show = cable_state_show;
- }
- }
+ ret = extcon_alloc_cables(edev);
+ if (ret < 0)
+ goto err_alloc_cables;
if (edev->max_supported && edev->mutually_exclusive) {
char *name;
@@ -1279,10 +1295,10 @@ int extcon_dev_register(struct extcon_dev *edev)
err_muex:
for (index = 0; index < edev->max_supported; index++)
kfree(edev->cables[index].attr_g.name);
-err_alloc_cables:
if (edev->max_supported)
kfree(edev->cables);
-err_sysfs_alloc:
+err_alloc_cables:
+
return ret;
}
EXPORT_SYMBOL_GPL(extcon_dev_register);