From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: vipulkumar.samar@st.com, viresh.kumar@linaro.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/2] Input: spear-keyboard - Fix a double put in spear_kbd_remove()
Date: Thu, 25 Jul 2024 14:55:46 -0700 [thread overview]
Message-ID: <ZqLJ4mZaKgzPVMFb@google.com> (raw)
In-Reply-To: <0e4f8499-97b1-49a7-b5af-11203cf625d7@wanadoo.fr>
On Thu, Jul 25, 2024 at 11:34:14PM +0200, Christophe JAILLET wrote:
> Le 25/07/2024 à 22:52, Dmitry Torokhov a écrit :
> > Hi Christophe,
> >
> > On Thu, Jul 25, 2024 at 10:46:49PM +0200, Christophe JAILLET wrote:
> > > The 'input_dev' is a managed resource allocated with
> > > devm_input_allocate_device(), so there is no need to call
> > > input_unregister_device() in the remove function.
> > >
> > > In fact, this call was correctly removed in commit 6102752eb354 ("Input:
> > > spear-keyboard - switch to using managed resources"), but silently
> > > re-introduced later in the commit in Fixes.
> >
> > This change is incorrect as it leads to an active and enabled clock
> > being unprepared to early. We need to unregister input device which in
> > turn will call spear_kbd_close() if needed which will disable the clock
> > in question. Only after that we can unprepare it.
> >
> > There is also no double put as input core will recognize that input
> > device was unregistered explicitly and will not attempt to unregister it
> > 2nd time through devm:
>
> Got it.
>
> Thanks for the review and the detailed explanation.
> Sorry for the noise.
>
> I'll resend as asked in patch 2/2, if saving some lines of code makes enough
> sense for you.
> But as said in the cover letter, if there is no issue, I'm not sure it worth
> the time for an old driver.
I generally like infrastructure cleanups, unless it is too much trouble.
Thanks.
--
Dmitry
next prev parent reply other threads:[~2024-07-25 21:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-25 20:46 [PATCH 0/2] Input: spear-keyboard - Fix an issue and clean-up code Christophe JAILLET
2024-07-25 20:46 ` [PATCH 1/2] Input: spear-keyboard - Fix a double put in spear_kbd_remove() Christophe JAILLET
2024-07-25 20:52 ` Dmitry Torokhov
2024-07-25 21:34 ` Christophe JAILLET
2024-07-25 21:55 ` Dmitry Torokhov [this message]
2024-07-25 20:46 ` [PATCH 2/2] Input: spear-keyboard - Switch to devm_clk_get_prepared() Christophe JAILLET
2024-07-25 20:55 ` Dmitry Torokhov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZqLJ4mZaKgzPVMFb@google.com \
--to=dmitry.torokhov@gmail.com \
--cc=christophe.jaillet@wanadoo.fr \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vipulkumar.samar@st.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.