The runtime PM callback may be called as soon as the runtime PM facility
is enabled and activated. It means that ->suspend() may be called before
we finish probing the device in the ACPI case. Hence, NULL pointer
dereference:
intel-lpss INT34BA:00: IRQ index 0 not found
BUG: kernel NULL pointer dereference, address: 0000000000000030
...
Workqueue: pm pm_runtime_work
RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
To fix this, first try to register the device and only after that enable
runtime PM facility.
Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
Reported-by: Orlando Chamberlain <[email protected]>
Reported-by: Aditya Garg <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
Tested-by: Aditya Garg <[email protected]>
---
v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
drivers/mfd/intel-lpss-acpi.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
index 3f1d976eb67c..f2ea6540a01e 100644
--- a/drivers/mfd/intel-lpss-acpi.c
+++ b/drivers/mfd/intel-lpss-acpi.c
@@ -136,6 +136,7 @@ static int intel_lpss_acpi_probe(struct platform_device *pdev)
{
struct intel_lpss_platform_info *info;
const struct acpi_device_id *id;
+ int ret;
id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
if (!id)
@@ -149,10 +150,14 @@ static int intel_lpss_acpi_probe(struct platform_device *pdev)
info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
info->irq = platform_get_irq(pdev, 0);
+ ret = intel_lpss_probe(&pdev->dev, info);
+ if (ret)
+ return ret;
+
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
- return intel_lpss_probe(&pdev->dev, info);
+ return 0;
}
static int intel_lpss_acpi_remove(struct platform_device *pdev)
--
2.33.0
> On 02-Nov-2021, at 12:30 AM, Andy Shevchenko <[email protected]> wrote:
>
> The runtime PM callback may be called as soon as the runtime PM facility
> is enabled and activated. It means that ->suspend() may be called before
> we finish probing the device in the ACPI case. Hence, NULL pointer
> dereference:
>
> intel-lpss INT34BA:00: IRQ index 0 not found
> BUG: kernel NULL pointer dereference, address: 0000000000000030
> ...
> Workqueue: pm pm_runtime_work
> RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
>
> To fix this, first try to register the device and only after that enable
> runtime PM facility.
>
> Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> Reported-by: Orlando Chamberlain <[email protected]>
> Reported-by: Aditya Garg <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> Tested-by: Aditya Garg <[email protected]>
> ---
> v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
It works
> drivers/mfd/intel-lpss-acpi.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/intel-lpss-acpi.c b/drivers/mfd/intel-lpss-acpi.c
> index 3f1d976eb67c..f2ea6540a01e 100644
> --- a/drivers/mfd/intel-lpss-acpi.c
> +++ b/drivers/mfd/intel-lpss-acpi.c
> @@ -136,6 +136,7 @@ static int intel_lpss_acpi_probe(struct platform_device *pdev)
> {
> struct intel_lpss_platform_info *info;
> const struct acpi_device_id *id;
> + int ret;
>
> id = acpi_match_device(intel_lpss_acpi_ids, &pdev->dev);
> if (!id)
> @@ -149,10 +150,14 @@ static int intel_lpss_acpi_probe(struct platform_device *pdev)
> info->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> info->irq = platform_get_irq(pdev, 0);
>
> + ret = intel_lpss_probe(&pdev->dev, info);
> + if (ret)
> + return ret;
> +
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> - return intel_lpss_probe(&pdev->dev, info);
> + return 0;
> }
>
> static int intel_lpss_acpi_remove(struct platform_device *pdev)
> --
> 2.33.0
>
On Tue, Nov 02, 2021 at 05:42:35AM +0000, Aditya Garg wrote:
> > On 02-Nov-2021, at 12:30 AM, Andy Shevchenko <[email protected]> wrote:
> > The runtime PM callback may be called as soon as the runtime PM facility
> > is enabled and activated. It means that ->suspend() may be called before
> > we finish probing the device in the ACPI case. Hence, NULL pointer
> > dereference:
> >
> > intel-lpss INT34BA:00: IRQ index 0 not found
> > BUG: kernel NULL pointer dereference, address: 0000000000000030
> > ...
> > Workqueue: pm pm_runtime_work
> > RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
> >
> > To fix this, first try to register the device and only after that enable
> > runtime PM facility.
> >
> > Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> > Reported-by: Orlando Chamberlain <[email protected]>
> > Reported-by: Aditya Garg <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
> > Tested-by: Aditya Garg <[email protected]>
> > ---
> > v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
> It works
Thanks for testing again!
Lee, can we have this as a fix material for v5.16-rcX?
--
With Best Regards,
Andy Shevchenko
On Mon, 15 Nov 2021, Aditya Garg wrote:
>
> ________________________________
> From: Andy Shevchenko <[email protected]>
> Sent: Monday, November 15, 2021 4:06 PM
> To: Aditya Garg
> Cc: Lee Jones; [email protected]; Orlando Chamberlain
> Subject: Re: [PATCH v2 1/1] mfd: intel-lpss: Fix too early PM enablement in the ACPI ->probe()
>
> On Tue, Nov 02, 2021 at 05:42:35AM +0000, Aditya Garg wrote:
> > > On 02-Nov-2021, at 12:30 AM, Andy Shevchenko <[email protected]> wrote:
>
> > > The runtime PM callback may be called as soon as the runtime PM facility
> > > is enabled and activated. It means that ->suspend() may be called before
> > > we finish probing the device in the ACPI case. Hence, NULL pointer
> > > dereference:
> > >
> > > intel-lpss INT34BA:00: IRQ index 0 not found
> > > BUG: kernel NULL pointer dereference, address: 0000000000000030
> > > ...
> > > Workqueue: pm pm_runtime_work
> > > RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
> > >
> > > To fix this, first try to register the device and only after that enable
> > > runtime PM facility.
> > >
> > > Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> > > Reported-by: Orlando Chamberlain <[email protected]>
> > > Reported-by: Aditya Garg <[email protected]>
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > Tested-by: Aditya Garg <[email protected]>
> > > ---
> > > v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
>
> > It works
>
> Thanks for testing again!
>
> Lee, can we have this as a fix material for v5.16-rcX?
Generally not, no.
We usually only push patches for the -rcs if they fix something that
was broken during the merge window. Not 6+ years ago. :)
However, if other valid fixes appear, I'll shove it into the PR too.
> I would like to have it backported to stable 5.15 too.
Yes, once it's merged into mainline, it will be back-ported as far
back as it applies cleanly.
If you have a specific kernel in mind, you should indicate it on the
end of the Fixes line.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 17 Nov 2021, Aditya Garg wrote:
>
>
> On 16-Nov-2021, at 9:23 PM, Lee Jones <[email protected]<mailto:[email protected]>> wrote:
>
> On Mon, 15 Nov 2021, Aditya Garg wrote:
>
>
> ________________________________
> From: Andy Shevchenko <[email protected]<mailto:[email protected]>>
> Sent: Monday, November 15, 2021 4:06 PM
> To: Aditya Garg
> Cc: Lee Jones; [email protected]<mailto:[email protected]>; Orlando Chamberlain
> Subject: Re: [PATCH v2 1/1] mfd: intel-lpss: Fix too early PM enablement in the ACPI ->probe()
>
> On Tue, Nov 02, 2021 at 05:42:35AM +0000, Aditya Garg wrote:
> On 02-Nov-2021, at 12:30 AM, Andy Shevchenko <[email protected]<mailto:[email protected]>> wrote:
>
> The runtime PM callback may be called as soon as the runtime PM facility
> is enabled and activated. It means that ->suspend() may be called before
> we finish probing the device in the ACPI case. Hence, NULL pointer
> dereference:
>
> intel-lpss INT34BA:00: IRQ index 0 not found
> BUG: kernel NULL pointer dereference, address: 0000000000000030
> ...
> Workqueue: pm pm_runtime_work
> RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
>
> To fix this, first try to register the device and only after that enable
> runtime PM facility.
>
> Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> Reported-by: Orlando Chamberlain <[email protected]<mailto:[email protected]>>
> Reported-by: Aditya Garg <[email protected]<mailto:[email protected]>>
> Signed-off-by: Andy Shevchenko <[email protected]<mailto:[email protected]>>
> Tested-by: Aditya Garg <[email protected]<mailto:[email protected]>>
> ---
> v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
>
> It works
>
> Thanks for testing again!
>
> Lee, can we have this as a fix material for v5.16-rcX?
>
> Generally not, no.
>
> We usually only push patches for the -rcs if they fix something that
> was broken during the merge window. Not 6+ years ago. :)
>
> However, if other valid fixes appear, I'll shove it into the PR too.
>
> I would like to have it backported to stable 5.15 too.
>
> Yes, once it's merged into mainline, it will be back-ported as far
> back as it applies cleanly.
>
> If you have a specific kernel in mind, you should indicate it on the
> end of the Fixes line.
> No specific kernel in mind. The issue was observed since 5.15.
That was really hard to read!
Please fix your mailer to set previous replies as quotes.
Not only did you not quote the mail you were replying to, you also
stripped out *all* previous quotes too. I didn't even know you could
do that!
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, 01 Nov 2021, Andy Shevchenko wrote:
> The runtime PM callback may be called as soon as the runtime PM facility
> is enabled and activated. It means that ->suspend() may be called before
> we finish probing the device in the ACPI case. Hence, NULL pointer
> dereference:
>
> intel-lpss INT34BA:00: IRQ index 0 not found
> BUG: kernel NULL pointer dereference, address: 0000000000000030
> ...
> Workqueue: pm pm_runtime_work
> RIP: 0010:intel_lpss_suspend+0xb/0x40 [intel_lpss]
>
> To fix this, first try to register the device and only after that enable
> runtime PM facility.
>
> Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> Reported-by: Orlando Chamberlain <[email protected]>
> Reported-by: Aditya Garg <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> Tested-by: Aditya Garg <[email protected]>
> ---
> v2: added tag (Aditya), returned 0 explicitly at the end of ->probe()
> drivers/mfd/intel-lpss-acpi.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog