| Message ID | 25f46d76dc95d5509edd7bf9d1a2e0532faef4cc.1570044052.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | PM / devfreq: Add dev_pm_qos support | expand |
Hi Leonard, This patch didn't get the acked-by from devfreq maintainer. I think that we need to discuss this patch with more time. Also, it is possible to make it as the separate patch from this series. IMHO, if you make the separate patch for this and resend the separate patch on later, I think that we can merge the remained patch related to PM_QOS. On 19. 10. 3. 오전 4:25, Leonard Crestez wrote: > In general it is a better to initialize an object before making it > accessible externally (through device_register). > > This makes it possible to avoid remove locking the partially initialized > object and simplifies the code. However devm is not available before > device_register (only after the device_initialize step) so the two > allocations need to be managed manually. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > --- > drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 18 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 3e0e936185a3..0b40f40ee7aa 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) > mutex_unlock(&devfreq_list_lock); > > if (devfreq->profile->exit) > devfreq->profile->exit(devfreq->dev.parent); > > + kfree(devfreq->time_in_state); > + kfree(devfreq->trans_table); > mutex_destroy(&devfreq->lock); > kfree(devfreq); > } > > /** > @@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, > devfreq->max_freq = devfreq->scaling_max_freq; > > devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); > atomic_set(&devfreq->suspend_count, 0); > > - dev_set_name(&devfreq->dev, "devfreq%d", > - atomic_inc_return(&devfreq_no)); > - err = device_register(&devfreq->dev); > - if (err) { > - mutex_unlock(&devfreq->lock); > - put_device(&devfreq->dev); > - goto err_out; > - } > - > - devfreq->trans_table = devm_kzalloc(&devfreq->dev, > + devfreq->trans_table = kzalloc( > array3_size(sizeof(unsigned int), > devfreq->profile->max_state, > devfreq->profile->max_state), > GFP_KERNEL); > if (!devfreq->trans_table) { > mutex_unlock(&devfreq->lock); > err = -ENOMEM; > - goto err_devfreq; > + goto err_dev; > } > > - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, > - devfreq->profile->max_state, > - sizeof(unsigned long), > - GFP_KERNEL); > + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, > + sizeof(unsigned long), > + GFP_KERNEL); > if (!devfreq->time_in_state) { > mutex_unlock(&devfreq->lock); > err = -ENOMEM; > - goto err_devfreq; > + goto err_dev; > } > > devfreq->last_stat_updated = jiffies; > > srcu_init_notifier_head(&devfreq->transition_notifier_list); > > + dev_set_name(&devfreq->dev, "devfreq%d", > + atomic_inc_return(&devfreq_no)); > + err = device_register(&devfreq->dev); > + if (err) { > + mutex_unlock(&devfreq->lock); > + put_device(&devfreq->dev); > + goto err_out; > + } > + > mutex_unlock(&devfreq->lock); > > mutex_lock(&devfreq_list_lock); > > governor = try_then_request_governor(devfreq->governor_name); > @@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, > > return devfreq; > > err_init: > mutex_unlock(&devfreq_list_lock); > -err_devfreq: > devfreq_remove_device(devfreq); > - devfreq = NULL; > + return ERR_PTR(err); > + > err_dev: > + /* > + * Cleanup path for errors that happen before registration. > + * Otherwise we rely on devfreq_dev_release. > + */ > + kfree(devfreq->time_in_state); > + kfree(devfreq->trans_table); > kfree(devfreq); > err_out: > return ERR_PTR(err); > } > EXPORT_SYMBOL(devfreq_add_device); >
On 31.10.2019 05:10, Chanwoo Choi wrote: > Hi Leonard, > > This patch didn't get the acked-by from devfreq maintainer. > I think that we need to discuss this patch with more time. > Also, it is possible to make it as the separate patch > from this series. > > IMHO, if you make the separate patch for this and > resend the separate patch on later, I think that > we can merge the remained patch related to PM_QOS. The devfreq initialization cleanups are required for dev_pm_qos support, otherwise lockdep warnings are triggered. I can post the cleanups as a separate series but the PM QoS one would depend on the cleanups. Do you prefer multiple smaller series? I try to order my patches with uncontroversial fixes and cleanups first so in theory the earlier parts could be applied separately. It's very rare to see series partially applied though. Earlier objection was that devm should be kept, I think this can be accomplished by splitting device_register into device_initialize and device_add. > On 19. 10. 3. 오전 4:25, Leonard Crestez wrote: >> In general it is a better to initialize an object before making it >> accessible externally (through device_register). >> >> This makes it possible to avoid remove locking the partially initialized >> object and simplifies the code. However devm is not available before >> device_register (only after the device_initialize step) so the two >> allocations need to be managed manually. >> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >> --- >> drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++---------------- >> 1 file changed, 25 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index 3e0e936185a3..0b40f40ee7aa 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) >> mutex_unlock(&devfreq_list_lock); >> >> if (devfreq->profile->exit) >> devfreq->profile->exit(devfreq->dev.parent); >> >> + kfree(devfreq->time_in_state); >> + kfree(devfreq->trans_table); >> mutex_destroy(&devfreq->lock); >> kfree(devfreq); >> } >> >> /** >> @@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, >> devfreq->max_freq = devfreq->scaling_max_freq; >> >> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); >> atomic_set(&devfreq->suspend_count, 0); >> >> - dev_set_name(&devfreq->dev, "devfreq%d", >> - atomic_inc_return(&devfreq_no)); >> - err = device_register(&devfreq->dev); >> - if (err) { >> - mutex_unlock(&devfreq->lock); >> - put_device(&devfreq->dev); >> - goto err_out; >> - } >> - >> - devfreq->trans_table = devm_kzalloc(&devfreq->dev, >> + devfreq->trans_table = kzalloc( >> array3_size(sizeof(unsigned int), >> devfreq->profile->max_state, >> devfreq->profile->max_state), >> GFP_KERNEL); >> if (!devfreq->trans_table) { >> mutex_unlock(&devfreq->lock); >> err = -ENOMEM; >> - goto err_devfreq; >> + goto err_dev; >> } >> >> - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, >> - devfreq->profile->max_state, >> - sizeof(unsigned long), >> - GFP_KERNEL); >> + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, >> + sizeof(unsigned long), >> + GFP_KERNEL); >> if (!devfreq->time_in_state) { >> mutex_unlock(&devfreq->lock); >> err = -ENOMEM; >> - goto err_devfreq; >> + goto err_dev; >> } >> >> devfreq->last_stat_updated = jiffies; >> >> srcu_init_notifier_head(&devfreq->transition_notifier_list); >> >> + dev_set_name(&devfreq->dev, "devfreq%d", >> + atomic_inc_return(&devfreq_no)); >> + err = device_register(&devfreq->dev); >> + if (err) { >> + mutex_unlock(&devfreq->lock); >> + put_device(&devfreq->dev); >> + goto err_out; >> + } >> + >> mutex_unlock(&devfreq->lock); >> >> mutex_lock(&devfreq_list_lock); >> >> governor = try_then_request_governor(devfreq->governor_name); >> @@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, >> >> return devfreq; >> >> err_init: >> mutex_unlock(&devfreq_list_lock); >> -err_devfreq: >> devfreq_remove_device(devfreq); >> - devfreq = NULL; >> + return ERR_PTR(err); >> + >> err_dev: >> + /* >> + * Cleanup path for errors that happen before registration. >> + * Otherwise we rely on devfreq_dev_release. >> + */ >> + kfree(devfreq->time_in_state); >> + kfree(devfreq->trans_table); >> kfree(devfreq); >> err_out: >> return ERR_PTR(err); >> } >> EXPORT_SYMBOL(devfreq_add_device);
On 19. 10. 31. 오후 10:31, Leonard Crestez wrote: > On 31.10.2019 05:10, Chanwoo Choi wrote: >> Hi Leonard, >> >> This patch didn't get the acked-by from devfreq maintainer. >> I think that we need to discuss this patch with more time. >> Also, it is possible to make it as the separate patch >> from this series. >> >> IMHO, if you make the separate patch for this and >> resend the separate patch on later, I think that >> we can merge the remained patch related to PM_QOS. > > The devfreq initialization cleanups are required for dev_pm_qos support, > otherwise lockdep warnings are triggered. I can post the cleanups as a > separate series but the PM QoS one would depend on the cleanups. > > Do you prefer multiple smaller series? After read the v10, I think v9 is better than v10 for this issue. > > I try to order my patches with uncontroversial fixes and cleanups first > so in theory the earlier parts could be applied separately. It's very > rare to see series partially applied though. > > Earlier objection was that devm should be kept, I think this can be > accomplished by splitting device_register into device_initialize and > device_add. > >> On 19. 10. 3. 오전 4:25, Leonard Crestez wrote: >>> In general it is a better to initialize an object before making it >>> accessible externally (through device_register). >>> >>> This makes it possible to avoid remove locking the partially initialized >>> object and simplifies the code. However devm is not available before >>> device_register (only after the device_initialize step) so the two >>> allocations need to be managed manually. >>> >>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >>> --- >>> drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++---------------- >>> 1 file changed, 25 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index 3e0e936185a3..0b40f40ee7aa 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) >>> mutex_unlock(&devfreq_list_lock); >>> >>> if (devfreq->profile->exit) >>> devfreq->profile->exit(devfreq->dev.parent); >>> >>> + kfree(devfreq->time_in_state); >>> + kfree(devfreq->trans_table); >>> mutex_destroy(&devfreq->lock); >>> kfree(devfreq); >>> } >>> >>> /** >>> @@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, >>> devfreq->max_freq = devfreq->scaling_max_freq; >>> >>> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); >>> atomic_set(&devfreq->suspend_count, 0); >>> >>> - dev_set_name(&devfreq->dev, "devfreq%d", >>> - atomic_inc_return(&devfreq_no)); >>> - err = device_register(&devfreq->dev); >>> - if (err) { >>> - mutex_unlock(&devfreq->lock); >>> - put_device(&devfreq->dev); >>> - goto err_out; >>> - } >>> - >>> - devfreq->trans_table = devm_kzalloc(&devfreq->dev, >>> + devfreq->trans_table = kzalloc( >>> array3_size(sizeof(unsigned int), >>> devfreq->profile->max_state, >>> devfreq->profile->max_state), >>> GFP_KERNEL); >>> if (!devfreq->trans_table) { >>> mutex_unlock(&devfreq->lock); >>> err = -ENOMEM; >>> - goto err_devfreq; >>> + goto err_dev; >>> } >>> >>> - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, >>> - devfreq->profile->max_state, >>> - sizeof(unsigned long), >>> - GFP_KERNEL); >>> + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, >>> + sizeof(unsigned long), >>> + GFP_KERNEL); >>> if (!devfreq->time_in_state) { >>> mutex_unlock(&devfreq->lock); >>> err = -ENOMEM; >>> - goto err_devfreq; >>> + goto err_dev; >>> } >>> >>> devfreq->last_stat_updated = jiffies; >>> >>> srcu_init_notifier_head(&devfreq->transition_notifier_list); >>> >>> + dev_set_name(&devfreq->dev, "devfreq%d", >>> + atomic_inc_return(&devfreq_no)); >>> + err = device_register(&devfreq->dev); >>> + if (err) { >>> + mutex_unlock(&devfreq->lock); >>> + put_device(&devfreq->dev); >>> + goto err_out; err_out -> err_dev When failed to register, have to free resource. >>> + } >>> + >>> mutex_unlock(&devfreq->lock); >>> >>> mutex_lock(&devfreq_list_lock); >>> >>> governor = try_then_request_governor(devfreq->governor_name); >>> @@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, >>> >>> return devfreq; >>> >>> err_init: >>> mutex_unlock(&devfreq_list_lock); >>> -err_devfreq: >>> devfreq_remove_device(devfreq); >>> - devfreq = NULL; >>> + return ERR_PTR(err); It is not proper to return on the middle of the exception handling. Need to consider more clean method. >>> + >>> err_dev: >>> + /* >>> + * Cleanup path for errors that happen before registration. >>> + * Otherwise we rely on devfreq_dev_release. >>> + */ >>> + kfree(devfreq->time_in_state); >>> + kfree(devfreq->trans_table); >>> kfree(devfreq); >>> err_out: >>> return ERR_PTR(err); >>> } >>> EXPORT_SYMBOL(devfreq_add_device);
On 01.11.2019 10:25, Chanwoo Choi wrote: > On 19. 10. 31. 오후 10:31, Leonard Crestez wrote: >> On 31.10.2019 05:10, Chanwoo Choi wrote: >>> Hi Leonard, >>> >>> This patch didn't get the acked-by from devfreq maintainer. >>> I think that we need to discuss this patch with more time. >>> Also, it is possible to make it as the separate patch >>> from this series. >>> >>> IMHO, if you make the separate patch for this and >>> resend the separate patch on later, I think that >>> we can merge the remained patch related to PM_QOS. >> >> The devfreq initialization cleanups are required for dev_pm_qos support, >> otherwise lockdep warnings are triggered. I can post the cleanups as a >> separate series but the PM QoS one would depend on the cleanups. >> >> Do you prefer multiple smaller series? > > After read the v10, I think v9 is better than v10 > for this issue. >> >> I try to order my patches with uncontroversial fixes and cleanups first >> so in theory the earlier parts could be applied separately. It's very >> rare to see series partially applied though. >> >> Earlier objection was that devm should be kept, I think this can be >> accomplished by splitting device_register into device_initialize and >> device_add. >> >>> On 19. 10. 3. 오전 4:25, Leonard Crestez wrote: >>>> In general it is a better to initialize an object before making it >>>> accessible externally (through device_register). >>>> >>>> This makes it possible to avoid remove locking the partially initialized >>>> object and simplifies the code. However devm is not available before >>>> device_register (only after the device_initialize step) so the two >>>> allocations need to be managed manually. >>>> >>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >>>> --- >>>> drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++---------------- >>>> 1 file changed, 25 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>>> index 3e0e936185a3..0b40f40ee7aa 100644 >>>> --- a/drivers/devfreq/devfreq.c >>>> +++ b/drivers/devfreq/devfreq.c >>>> @@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) >>>> mutex_unlock(&devfreq_list_lock); >>>> >>>> if (devfreq->profile->exit) >>>> devfreq->profile->exit(devfreq->dev.parent); >>>> >>>> + kfree(devfreq->time_in_state); >>>> + kfree(devfreq->trans_table); >>>> mutex_destroy(&devfreq->lock); >>>> kfree(devfreq); >>>> } >>>> >>>> /** >>>> @@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, >>>> devfreq->max_freq = devfreq->scaling_max_freq; >>>> >>>> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); >>>> atomic_set(&devfreq->suspend_count, 0); >>>> >>>> - dev_set_name(&devfreq->dev, "devfreq%d", >>>> - atomic_inc_return(&devfreq_no)); >>>> - err = device_register(&devfreq->dev); >>>> - if (err) { >>>> - mutex_unlock(&devfreq->lock); >>>> - put_device(&devfreq->dev); >>>> - goto err_out; >>>> - } >>>> - >>>> - devfreq->trans_table = devm_kzalloc(&devfreq->dev, >>>> + devfreq->trans_table = kzalloc( >>>> array3_size(sizeof(unsigned int), >>>> devfreq->profile->max_state, >>>> devfreq->profile->max_state), >>>> GFP_KERNEL); >>>> if (!devfreq->trans_table) { >>>> mutex_unlock(&devfreq->lock); >>>> err = -ENOMEM; >>>> - goto err_devfreq; >>>> + goto err_dev; >>>> } >>>> >>>> - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, >>>> - devfreq->profile->max_state, >>>> - sizeof(unsigned long), >>>> - GFP_KERNEL); >>>> + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, >>>> + sizeof(unsigned long), >>>> + GFP_KERNEL); >>>> if (!devfreq->time_in_state) { >>>> mutex_unlock(&devfreq->lock); >>>> err = -ENOMEM; >>>> - goto err_devfreq; >>>> + goto err_dev; >>>> } >>>> >>>> devfreq->last_stat_updated = jiffies; >>>> >>>> srcu_init_notifier_head(&devfreq->transition_notifier_list); >>>> >>>> + dev_set_name(&devfreq->dev, "devfreq%d", >>>> + atomic_inc_return(&devfreq_no)); >>>> + err = device_register(&devfreq->dev); >>>> + if (err) { >>>> + mutex_unlock(&devfreq->lock); >>>> + put_device(&devfreq->dev); >>>> + goto err_out; > > err_out -> err_dev > When failed to register, have to free resource. According documentation on device_register resources need to be freed via put_device (which calls dev.release = devfreq_dev_release). This chunk isn't new; it just appears so because other code was moved around it. > >>>> + } >>>> + >>>> mutex_unlock(&devfreq->lock); >>>> >>>> mutex_lock(&devfreq_list_lock); >>>> >>>> governor = try_then_request_governor(devfreq->governor_name); >>>> @@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, >>>> >>>> return devfreq; >>>> >>>> err_init: >>>> mutex_unlock(&devfreq_list_lock); >>>> -err_devfreq: >>>> devfreq_remove_device(devfreq); >>>> - devfreq = NULL; >>>> + return ERR_PTR(err); > > > It is not proper to return on the middle > of the exception handling. Need to consider more clean method. Current code already does "devfreq = NULL" in order to skip the kfree below. There are already many cleanup/exit paths, it's just not very obvious. Cleanup for errors before and after "device_register" is different and this is required by device core. This can be improved by splitting device_register into device_initialize and device_add: this means that all early error paths do put_device and there is no longer any need to manually kfree(devfreq) because devfreq_dev_release is called on all cleanup paths. It also preserves devm: https://patchwork.kernel.org/patch/11221873/ However there is still a difference between cleanup before/after device_add: if something fails after device_add you need to call devfreq_remove_device (which does device_unregister) and otherwise device_put. >>>> + >>>> err_dev: >>>> + /* >>>> + * Cleanup path for errors that happen before registration. >>>> + * Otherwise we rely on devfreq_dev_release. >>>> + */ >>>> + kfree(devfreq->time_in_state); >>>> + kfree(devfreq->trans_table); >>>> kfree(devfreq); >>>> err_out: >>>> return ERR_PTR(err); >>>> } >>>> EXPORT_SYMBOL(devfreq_add_device);
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 3e0e936185a3..0b40f40ee7aa 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) mutex_unlock(&devfreq_list_lock); if (devfreq->profile->exit) devfreq->profile->exit(devfreq->dev.parent); + kfree(devfreq->time_in_state); + kfree(devfreq->trans_table); mutex_destroy(&devfreq->lock); kfree(devfreq); } /** @@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, devfreq->max_freq = devfreq->scaling_max_freq; devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); atomic_set(&devfreq->suspend_count, 0); - dev_set_name(&devfreq->dev, "devfreq%d", - atomic_inc_return(&devfreq_no)); - err = device_register(&devfreq->dev); - if (err) { - mutex_unlock(&devfreq->lock); - put_device(&devfreq->dev); - goto err_out; - } - - devfreq->trans_table = devm_kzalloc(&devfreq->dev, + devfreq->trans_table = kzalloc( array3_size(sizeof(unsigned int), devfreq->profile->max_state, devfreq->profile->max_state), GFP_KERNEL); if (!devfreq->trans_table) { mutex_unlock(&devfreq->lock); err = -ENOMEM; - goto err_devfreq; + goto err_dev; } - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, - devfreq->profile->max_state, - sizeof(unsigned long), - GFP_KERNEL); + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, + sizeof(unsigned long), + GFP_KERNEL); if (!devfreq->time_in_state) { mutex_unlock(&devfreq->lock); err = -ENOMEM; - goto err_devfreq; + goto err_dev; } devfreq->last_stat_updated = jiffies; srcu_init_notifier_head(&devfreq->transition_notifier_list); + dev_set_name(&devfreq->dev, "devfreq%d", + atomic_inc_return(&devfreq_no)); + err = device_register(&devfreq->dev); + if (err) { + mutex_unlock(&devfreq->lock); + put_device(&devfreq->dev); + goto err_out; + } + mutex_unlock(&devfreq->lock); mutex_lock(&devfreq_list_lock); governor = try_then_request_governor(devfreq->governor_name); @@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, return devfreq; err_init: mutex_unlock(&devfreq_list_lock); -err_devfreq: devfreq_remove_device(devfreq); - devfreq = NULL; + return ERR_PTR(err); + err_dev: + /* + * Cleanup path for errors that happen before registration. + * Otherwise we rely on devfreq_dev_release. + */ + kfree(devfreq->time_in_state); + kfree(devfreq->trans_table); kfree(devfreq); err_out: return ERR_PTR(err); } EXPORT_SYMBOL(devfreq_add_device);