[PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Amitai Gottlieb]

In function `scmi_devm_notifier_unregister` the notifier-block parameter
was unused and therefore never passed to `devres_release`. This causes
the function to always return -ENOENT and fail to unregister the
notifier.

In drivers that rely on this function for cleanup this causes
unexpected failures including kernel-panic.

Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres
notification ops")
Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
---

This is not needed upstream becaues the bug was fixed
in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
6.6 kernels.

 drivers/firmware/arm_scmi/notify.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 0efd20cd9d69..4782b115e6ec 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -1539,6 +1539,7 @@ static int scmi_devm_notifier_unregister(struct scmi_device *sdev,
 	dres.handle = sdev->handle;
 	dres.proto_id = proto_id;
 	dres.evt_id = evt_id;
+	dres.nb = nb;
 	if (src_id) {
 		dres.__src_id = *src_id;
 		dres.src_id = &dres.__src_id;
-- 
2.34.1

Re: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Cristian Marussi]

On Thu, Dec 11, 2025 at 01:49:27PM +0200, Amitai Gottlieb wrote:
> In function `scmi_devm_notifier_unregister` the notifier-block parameter
> was unused and therefore never passed to `devres_release`. This causes
> the function to always return -ENOENT and fail to unregister the
> notifier.

[ CC-ing also Sudeep ]

Hi,

thanks for this.

LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

> 
> In drivers that rely on this function for cleanup this causes
> unexpected failures including kernel-panic.
> 
> Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres
> notification ops")
> Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
> ---
> 
> This is not needed upstream becaues the bug was fixed
> in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
> 6.6 kernels.

I think there is some sort of syntax to refer to the applicable Kernel
versions in the tag line....not sure it can be easily followed here...so
maybe the above comment is enough...

More details here...

https://docs.kernel.org/process/stable-kernel-rules.html

Thanks,
Cristian

Re: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Dan Carpenter]

On Thu, Dec 11, 2025 at 12:17:01PM +0000, Cristian Marussi wrote:
> On Thu, Dec 11, 2025 at 01:49:27PM +0200, Amitai Gottlieb wrote:
> > In function `scmi_devm_notifier_unregister` the notifier-block parameter
> > was unused and therefore never passed to `devres_release`. This causes
> > the function to always return -ENOENT and fail to unregister the
> > notifier.
> 
> [ CC-ing also Sudeep ]
> 
> Hi,
> 
> thanks for this.
> 
> LGTM.
> 
> Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
> 
> > 
> > In drivers that rely on this function for cleanup this causes
> > unexpected failures including kernel-panic.
> > 
> > Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres
> > notification ops")

This Fixes tag needs to be all on one line.
Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres notification ops")

> > Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
> > ---
> > 
> > This is not needed upstream becaues the bug was fixed
> > in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> > scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
> > 6.6 kernels.
> 
> I think there is some sort of syntax to refer to the applicable Kernel
> versions in the tag line....not sure it can be easily followed here...so
> maybe the above comment is enough...
> 
> More details here...
> 
> https://docs.kernel.org/process/stable-kernel-rules.html

I'm pretty sure those little comments aren't parsed automatically...
It feels duplicative.  I don't love those little notes because it feels
like the Fixes tag should normally be sufficient otherwise I want a full
explanation.  There was an embarrassing bug where the notes were wrong
and the patch was missing a Fixes tag so it wasn't backported far enough
in distro kernels.

It can't hurt to add them to the tag section, I suppose.

Cc: <stable@vger.kernel.org> # 5.15.x, 6.1.x, and 6.6.x

Leaving them out is also fine, really.

We definitely still need the note.  I said before that note about
"This is not needed upstream becaues the bug was fixed
in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
scmi_devm_notifier_unregister")" should go under the --- cut off line,
but actually I think now that it should go into the actual commit
message itself.  Otherwise people who work on distro kernels, for
example, will wonder why the commit doesn't have an upstream hash.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

Wait until Sudeep has responded, and then resend the patch to
stable@vger.kernel.org with all the Reviewed-by tags.

regards,
dan carpenter

Re: Fw: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[amitaigottlieb]

On 25/12/15 11:27AM, Amitai Gottlieb wrote:
> 
> ________________________________
> From: Dan Carpenter <dan.carpenter@linaro.org>
> Sent: Thursday, December 11, 2025 14:53
> To: Cristian Marussi <cristian.marussi@arm.com>
> Cc: Amitai Gottlieb <amitaig@hailo.ai>; arm-scmi@vger.kernel.org <arm-scmi@vger.kernel.org>; sudeep.holla@arm.com <sudeep.holla@arm.com>
> Subject: Re: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister
> 
> On Thu, Dec 11, 2025 at 12:17:01PM +0000, Cristian Marussi wrote:
> > On Thu, Dec 11, 2025 at 01:49:27PM +0200, Amitai Gottlieb wrote:
> > > In function `scmi_devm_notifier_unregister` the notifier-block parameter
> > > was unused and therefore never passed to `devres_release`. This causes
> > > the function to always return -ENOENT and fail to unregister the
> > > notifier.
> >
> > [ CC-ing also Sudeep ]
> >
> > Hi,
> >
> > thanks for this.
> >
> > LGTM.
> >
> > Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
> >
> > >
> > > In drivers that rely on this function for cleanup this causes
> > > unexpected failures including kernel-panic.
> > >
> > > Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres
> > > notification ops")
> 
> This Fixes tag needs to be all on one line.
> Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres notification ops")
> 
> > > Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
> > > ---
> > >
> > > This is not needed upstream becaues the bug was fixed
> > > in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> > > scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
> > > 6.6 kernels.
> >
> > I think there is some sort of syntax to refer to the applicable Kernel
> > versions in the tag line....not sure it can be easily followed here...so
> > maybe the above comment is enough...
> >
> > More details here...
> >
> > https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fprocess%2Fstable-kernel-rules.html&data=05%7C02%7Camitaig%40hailo.ai%7C6aec4657c2364f78663408de38b45031%7C6ae4a5f7546741898f6af2928ed536de%7C0%7C0%7C639010544284877259%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Kxmnl4F4KpZgBjzGakndjp9EK9A2GfrXFj04F5%2FPIxE%3D&reserved=0<https://docs.kernel.org/process/stable-kernel-rules.html>
> 
> I'm pretty sure those little comments aren't parsed automatically...
> It feels duplicative.  I don't love those little notes because it feels
> like the Fixes tag should normally be sufficient otherwise I want a full
> explanation.  There was an embarrassing bug where the notes were wrong
> and the patch was missing a Fixes tag so it wasn't backported far enough
> in distro kernels.
> 
> It can't hurt to add them to the tag section, I suppose.
> 
> Cc: <stable@vger.kernel.org> # 5.15.x, 6.1.x, and 6.6.x
> 
> Leaving them out is also fine, really.
> 
> We definitely still need the note.  I said before that note about
> "This is not needed upstream becaues the bug was fixed
> in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> scmi_devm_notifier_unregister")" should go under the --- cut off line,
> but actually I think now that it should go into the actual commit
> message itself.  Otherwise people who work on distro kernels, for
> example, will wonder why the commit doesn't have an upstream hash.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> Wait until Sudeep has responded, and then resend the patch to
> stable@vger.kernel.org with all the Reviewed-by tags.
> 
> regards,
> dan carpenter

I switched to my personal mail as my work was making my life difficult
while using an external mail client.

I've waited a few days and Sudeep hasn't responded, is it fine with you
if I send it off as is?

thanks,
Amitai
> 

Re: Fw: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Sudeep Holla]

On Mon, Dec 15, 2025 at 02:19:29PM +0200, amitaigottlieb@gmail.com wrote:
> On 25/12/15 11:27AM, Amitai Gottlieb wrote:
> > 
> > ________________________________
> > From: Dan Carpenter <dan.carpenter@linaro.org>
> > Sent: Thursday, December 11, 2025 14:53
> > To: Cristian Marussi <cristian.marussi@arm.com>
> > Cc: Amitai Gottlieb <amitaig@hailo.ai>; arm-scmi@vger.kernel.org <arm-scmi@vger.kernel.org>; sudeep.holla@arm.com <sudeep.holla@arm.com>
> > Subject: Re: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister
> > 
> > On Thu, Dec 11, 2025 at 12:17:01PM +0000, Cristian Marussi wrote:
> > > On Thu, Dec 11, 2025 at 01:49:27PM +0200, Amitai Gottlieb wrote:
> > > > In function `scmi_devm_notifier_unregister` the notifier-block parameter
> > > > was unused and therefore never passed to `devres_release`. This causes
> > > > the function to always return -ENOENT and fail to unregister the
> > > > notifier.
> > >
> > > [ CC-ing also Sudeep ]
> > >
> > > Hi,
> > >
> > > thanks for this.
> > >
> > > LGTM.
> > >
> > > Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
> > >
> > > >
> > > > In drivers that rely on this function for cleanup this causes
> > > > unexpected failures including kernel-panic.
> > > >
> > > > Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres
> > > > notification ops")
> > 
> > This Fixes tag needs to be all on one line.
> > Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres notification ops")
> > 
> > > > Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
> > > > ---
> > > >
> > > > This is not needed upstream becaues the bug was fixed
> > > > in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> > > > scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
> > > > 6.6 kernels.
> > >
> > > I think there is some sort of syntax to refer to the applicable Kernel
> > > versions in the tag line....not sure it can be easily followed here...so
> > > maybe the above comment is enough...
> > >
> > > More details here...
> > >
> > > https://docs.kernel.org/process/stable-kernel-rules.html<https://docs.kernel.org/process/stable-kernel-rules.html>
> > 
> > I'm pretty sure those little comments aren't parsed automatically...
> > It feels duplicative.  I don't love those little notes because it feels
> > like the Fixes tag should normally be sufficient otherwise I want a full
> > explanation.  There was an embarrassing bug where the notes were wrong
> > and the patch was missing a Fixes tag so it wasn't backported far enough
> > in distro kernels.
> > 
> > It can't hurt to add them to the tag section, I suppose.
> > 
> > Cc: <stable@vger.kernel.org> # 5.15.x, 6.1.x, and 6.6.x
> > 
> > Leaving them out is also fine, really.
> > 
> > We definitely still need the note.  I said before that note about
> > "This is not needed upstream becaues the bug was fixed
> > in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> > scmi_devm_notifier_unregister")" should go under the --- cut off line,
> > but actually I think now that it should go into the actual commit
> > message itself.  Otherwise people who work on distro kernels, for
> > example, will wonder why the commit doesn't have an upstream hash.
> > 
> > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> > 
> > Wait until Sudeep has responded, and then resend the patch to
> > stable@vger.kernel.org with all the Reviewed-by tags.
> > 
> > regards,
> > dan carpenter
> 
> I switched to my personal mail as my work was making my life difficult
> while using an external mail client.
> 
> I've waited a few days and Sudeep hasn't responded, is it fine with you
> if I send it off as is?
> 

Sorry, I am fine with the change and was thinking of acking it when you
posted version for stable kernel following the rules. Or did I miss that ?
Please point me with a link to the posting if already done.

-- 
Regards,
Sudeep

[PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Amitai Gottlieb]

In function `scmi_devm_notifier_unregister` the notifier-block parameter
was unused and therefore never passed to `devres_release`. This causes
the function to always return -ENOENT and fail to unregister the
notifier.

In drivers that rely on this function for cleanup this causes
unexpected failures including kernel-panic.

This is not needed upstream becaues the bug was fixed
in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
6.6 kernels.

Cc: <stable@vger.kernel.org> # 5.15.x, 6.1.x, and 6.6.x
Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres notification ops")
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
---

 drivers/firmware/arm_scmi/notify.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 0efd20cd9d69..4782b115e6ec 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -1539,6 +1539,7 @@ static int scmi_devm_notifier_unregister(struct scmi_device *sdev,
 	dres.handle = sdev->handle;
 	dres.proto_id = proto_id;
 	dres.evt_id = evt_id;
+	dres.nb = nb;
 	if (src_id) {
 		dres.__src_id = *src_id;
 		dres.src_id = &dres.__src_id;
-- 
2.34.1

Re: [PATCH] firmware: arm_scmi: Fix unused notifier-block in unregister

[Sudeep Holla]

Add [STABLE ONLY] in the subject please along with [PATCH] and repost
it.

Also add Greg Kroah-Hartman <gregkh@linuxfoundation.org> to cc as well.

On Mon, Dec 15, 2025 at 06:37:32PM +0200, Amitai Gottlieb wrote:
> In function `scmi_devm_notifier_unregister` the notifier-block parameter
> was unused and therefore never passed to `devres_release`. This causes
> the function to always return -ENOENT and fail to unregister the
> notifier.
> 
> In drivers that rely on this function for cleanup this causes
> unexpected failures including kernel-panic.
> 
> This is not needed upstream becaues the bug was fixed
> in a refactor by commit 264a2c520628 ("firmware: arm_scmi: Simplify
> scmi_devm_notifier_unregister").  It is needed for the 5.15, 6.1 and
> 6.6 kernels.
> 

I would slightly reword the commit message like below.

 | In scmi_devm_notifier_unregister(), the notifier-block argument was ignored
 | and never passed to devres_release(). As a result, the function always
 | returned -ENOENT and failed to unregister the notifier.
 |
 | Drivers that depend on this helper for teardown could therefore hit
 | unexpected failures, including kernel panics.
 |
 | Commit 264a2c520628 ("firmware: arm_scmi: Simplify scmi_devm_notifier_unregister")
 | removed the faulty code path during refactoring and hence this fix is not
 | required upstream.

(add a panic log if you have seen one, that would help)

With that you can add,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Cc: <stable@vger.kernel.org> # 5.15.x, 6.1.x, and 6.6.x
> Fixes: 5ad3d1cf7d34 ("firmware: arm_scmi: Introduce new devres notification ops")
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Amitai Gottlieb <amitaig@hailo.ai>
> ---
> 
>  drivers/firmware/arm_scmi/notify.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
> index 0efd20cd9d69..4782b115e6ec 100644
> --- a/drivers/firmware/arm_scmi/notify.c
> +++ b/drivers/firmware/arm_scmi/notify.c
> @@ -1539,6 +1539,7 @@ static int scmi_devm_notifier_unregister(struct scmi_device *sdev,
>  	dres.handle = sdev->handle;
>  	dres.proto_id = proto_id;
>  	dres.evt_id = evt_id;
> +	dres.nb = nb;
>  	if (src_id) {
>  		dres.__src_id = *src_id;
>  		dres.src_id = &dres.__src_id;
> -- 
> 2.34.1
> 

-- 
Regards,
Sudeep