| Message ID | 72a97f7989f56c50e1ca417633fe703593d49189.1721941953.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | platform/x86/amd/pmf: Fix a double put in amd_pmf_remove() | expand |
Le 25/07/2024 à 23:13, Christophe JAILLET a écrit : > The 'input_dev' is a managed resource allocated with > devm_input_allocate_device(), so there is no need to call > input_unregister_device() explicitly. It will be called automatically > when the driver is removed. > > Fixes: 4c92d448e3e6 ("platform/x86/amd/pmf: Use existing input event codes to update system states") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Compile tested-only > > I'm not 100% confident with this change. The error handling scheme is not > a clear to me as what I usually see. For example, the last calls from > amd_pmf_probe() don't handle error at all. So the probe just succeeds in > these cases. > > So, because of it, it is maybe fine to call input_unregister_device() in > amd_pmf_deinit_smart_pc(), even if it looks strange to me. > > Review with care! NACK. Things have been explained to me in another similar patch proposal. Sorry for the noise. CJ > --- > drivers/platform/x86/amd/pmf/tee-if.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c > index e246367aacee..cc721fbc3e0b 100644 > --- a/drivers/platform/x86/amd/pmf/tee-if.c > +++ b/drivers/platform/x86/amd/pmf/tee-if.c > @@ -515,9 +515,6 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) > > void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) > { > - if (dev->pmf_idev) > - input_unregister_device(dev->pmf_idev); > - > if (pb_side_load && dev->esbin) > amd_pmf_remove_pb(dev); >
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c index e246367aacee..cc721fbc3e0b 100644 --- a/drivers/platform/x86/amd/pmf/tee-if.c +++ b/drivers/platform/x86/amd/pmf/tee-if.c @@ -515,9 +515,6 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) { - if (dev->pmf_idev) - input_unregister_device(dev->pmf_idev); - if (pb_side_load && dev->esbin) amd_pmf_remove_pb(dev);
The 'input_dev' is a managed resource allocated with devm_input_allocate_device(), so there is no need to call input_unregister_device() explicitly. It will be called automatically when the driver is removed. Fixes: 4c92d448e3e6 ("platform/x86/amd/pmf: Use existing input event codes to update system states") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Compile tested-only I'm not 100% confident with this change. The error handling scheme is not a clear to me as what I usually see. For example, the last calls from amd_pmf_probe() don't handle error at all. So the probe just succeeds in these cases. So, because of it, it is maybe fine to call input_unregister_device() in amd_pmf_deinit_smart_pc(), even if it looks strange to me. Review with care! --- drivers/platform/x86/amd/pmf/tee-if.c | 3 --- 1 file changed, 3 deletions(-)