From: Jishnu Prakash <quic_jprakash@quicinc.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: <agross@kernel.org>, <devicetree@vger.kernel.org>,
<robh+dt@kernel.org>, <linus.walleij@linaro.org>,
<quic_kamalw@quicinc.com>, <sboyd@kernel.org>,
<quic_subbaram@quicinc.com>, <quic_collinsd@quicinc.com>,
<linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
<linux-arm-msm-owner@vger.kernel.org>
Subject: Re: [PATCH] spmi: Add check for remove callback in spmi_drv_remove API
Date: Wed, 21 Dec 2022 11:09:03 +0530 [thread overview]
Message-ID: <e63e27df-7e46-337c-f22a-ccd1bbcd0c28@quicinc.com> (raw)
In-Reply-To: <Y5iVqrnlX8NoiOkl@kroah.com>
Hi Greg
On 12/13/2022 8:39 PM, Greg KH wrote:
> On Tue, Dec 13, 2022 at 07:12:10PM +0530, Jishnu Prakash wrote:
>> Hi Greg
>
> Hi, please do not top-post :(
>
>> These are two SPMI drivers without remove callbacks defined:
>>
>> drivers/mfd/qcom-spmi-pmic.c
>> drivers/mfd/hi6421-spmi-pmic.c
>
> Great, they should be fixed up now, right?
>
Our QCOM SPMI PMIC driver allocates resources in its probe using only
devm_() APIs and does not require any other cleanup. It doesn't seem
right to add an empty remove callback to it just to avoid this crash, it
seems the better solution architecturally is to call the remove function
only if it's defined.
In addition, I could see that other buses like PCI and AMBA do check for
the remove API being defined for their drivers before calling it:
AMBA example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/amba/bus.c#n328
PCI example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-driver.c#n474
>> We made this change after noticing an issue internally with the first one
>> above, there was a crash when trying to remove it with rmmod, which is fixed
>> by this change.
>
> Then please say that in the changelog text, otherwise we have no idea
> _why_ this is needed. All you said was "add this new check _IF_" and we
> have no idea what the answer to "if" is :(
>
I have uploaded the change with an updated title and commit text, can
you please have a look?
> thanks,
>
> greg k-h
Thanks,
Jishnu
prev parent reply other threads:[~2022-12-21 5:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-04 9:22 [PATCH] Update spmi driver removal API Jishnu Prakash
2022-12-04 9:23 ` [PATCH] spmi: Add check for remove callback in spmi_drv_remove API Jishnu Prakash
2022-12-13 12:04 ` Greg KH
2022-12-13 13:42 ` Jishnu Prakash
2022-12-13 15:09 ` Greg KH
2022-12-21 5:39 ` Jishnu Prakash [this message]
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=e63e27df-7e46-337c-f22a-ccd1bbcd0c28@quicinc.com \
--to=quic_jprakash@quicinc.com \
--cc=agross@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-msm-owner@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_collinsd@quicinc.com \
--cc=quic_kamalw@quicinc.com \
--cc=quic_subbaram@quicinc.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox