| Message ID | 074ddc8e211bc7466b5ceec2591cb220a44d8d3a.1565672482.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] PM / devfreq: passive: Use non-devm notifiers | expand |
Hi, Why do you miss the 'Fixes' tag as I commented? Is there any reason? On 19. 8. 13. 오후 2:06, Leonard Crestez wrote: > The devfreq passive governor registers and unregisters devfreq > transition notifiers on DEVFREQ_GOV_START/GOV_STOP using devm wrappers. > > If devfreq itself is registered with devm then a warning is triggered on > rmmod from devm_devfreq_unregister_notifier. Call stack looks like this: > > devm_devfreq_unregister_notifier+0x30/0x40 > devfreq_passive_event_handler+0x4c/0x88 > devfreq_remove_device.part.8+0x6c/0x9c > devm_devfreq_dev_release+0x18/0x20 > release_nodes+0x1b0/0x220 > devres_release_all+0x78/0x84 > device_release_driver_internal+0x100/0x1c0 > driver_detach+0x4c/0x90 > bus_remove_driver+0x7c/0xd0 > driver_unregister+0x2c/0x58 > platform_driver_unregister+0x10/0x18 > imx_devfreq_platdrv_exit+0x14/0xd40 [imx_devfreq] > > This happens because devres_release_all will first remove all the nodes > into a separate todo list so the nested devres_release from > devm_devfreq_unregister_notifier won't find anything. > > Fix the warning by calling the non-devm APIS for frequency notification. > Using devm wrappers is not actually useful for a governor anyway: it > relies on the devfreq core to correctly match the GOV_START/GOV_STOP > notifications. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Acked-by: Chanwoo Choi <cw00.choi@samsung.com> > Cc: stable@vger.kernel.org > > --- > Changes since v1: > * Remove unused variable "dev" and avoid warning > * Added ack and cc stable > Link to v1: https://patchwork.kernel.org/patch/11084541/ > > drivers/devfreq/governor_passive.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c > index 58308948b863..be6eeab9c814 100644 > --- a/drivers/devfreq/governor_passive.c > +++ b/drivers/devfreq/governor_passive.c > @@ -147,11 +147,10 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb, > } > > static int devfreq_passive_event_handler(struct devfreq *devfreq, > unsigned int event, void *data) > { > - struct device *dev = devfreq->dev.parent; > struct devfreq_passive_data *p_data > = (struct devfreq_passive_data *)devfreq->data; > struct devfreq *parent = (struct devfreq *)p_data->parent; > struct notifier_block *nb = &p_data->nb; > int ret = 0; > @@ -163,16 +162,16 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, > case DEVFREQ_GOV_START: > if (!p_data->this) > p_data->this = devfreq; > > nb->notifier_call = devfreq_passive_notifier_call; > - ret = devm_devfreq_register_notifier(dev, parent, nb, > + ret = devfreq_register_notifier(parent, nb, > DEVFREQ_TRANSITION_NOTIFIER); > break; > case DEVFREQ_GOV_STOP: > - devm_devfreq_unregister_notifier(dev, parent, nb, > - DEVFREQ_TRANSITION_NOTIFIER); > + WARN_ON(devfreq_unregister_notifier(parent, nb, > + DEVFREQ_TRANSITION_NOTIFIER)); > break; > default: > break; > } > >
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index 58308948b863..be6eeab9c814 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -147,11 +147,10 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb, } static int devfreq_passive_event_handler(struct devfreq *devfreq, unsigned int event, void *data) { - struct device *dev = devfreq->dev.parent; struct devfreq_passive_data *p_data = (struct devfreq_passive_data *)devfreq->data; struct devfreq *parent = (struct devfreq *)p_data->parent; struct notifier_block *nb = &p_data->nb; int ret = 0; @@ -163,16 +162,16 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, case DEVFREQ_GOV_START: if (!p_data->this) p_data->this = devfreq; nb->notifier_call = devfreq_passive_notifier_call; - ret = devm_devfreq_register_notifier(dev, parent, nb, + ret = devfreq_register_notifier(parent, nb, DEVFREQ_TRANSITION_NOTIFIER); break; case DEVFREQ_GOV_STOP: - devm_devfreq_unregister_notifier(dev, parent, nb, - DEVFREQ_TRANSITION_NOTIFIER); + WARN_ON(devfreq_unregister_notifier(parent, nb, + DEVFREQ_TRANSITION_NOTIFIER)); break; default: break; }