Re: [PATCH 3/4] platform/x86/amd/pmc: Do not fail probe when STB init fails

On Fri, 17 Jul 2026, Mario Limonciello wrote:

> STB (Spill to DRAM) is an optional debugging facility that is only
> enabled through the enable_stb module parameter.  On some platforms the

One whitespace is enough.

> SMU refuses the S2D setup outright, and on long-running systems the large
> telemetry region can fail to ioremap.  In either case amd_stb_s2d_init()
> returns an error and, because probe treated that as fatal, the entire
> PMC driver failed to load - silently disabling s0i3 support even though

There's odd change to past tense here.

treated -> treats
failed -> fails

> STB is only a debug aid.
> 
> Downgrade the failure to a warning and continue probing so that s0i3
> support via the LPS0 handler no longer depends on an optional debug
> feature.
> 
> Since probe no longer aborts on this path, the LPS0 and debugfs
> unwinding added by the earlier fix in this series becomes unreachable
> and is removed.
> 
> Assisted-by: Claude:opus
> Reported-by: Francis De Brabandere <[email protected]>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221759
> Tested-by: Francis De Brabandere <[email protected]>
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 630a664bdd2f4..5b6c90abd4e5f 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -917,19 +917,24 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	}
>  
>  	amd_pmc_dbgfs_register(dev);
> +
> +	/*
> +	 * STB is an optional debugging facility, only enabled via the
> +	 * enable_stb module parameter.  A failure to initialize it (e.g. the
> +	 * SMU refusing the request, or the telemetry region failing to map)
> +	 * must not prevent the rest of the driver - most importantly the s0i3
> +	 * LPS0 handler - from working, so treat it as non-fatal.
> +	 */
>  	err = amd_stb_s2d_init(dev);
>  	if (err)
> -		goto err_dbgfs_unregister;
> +		dev_warn(dev->dev, "STB initialization failed (%d), continuing without STB support\n",
> +			 err);

Please add include.

>  
>  	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
>  		amd_mp2_stb_init(dev);
>  	pm_report_max_hw_sleep(U64_MAX);
>  	return 0;
>  
> -err_dbgfs_unregister:
> -	amd_pmc_dbgfs_unregister(dev);
> -	if (IS_ENABLED(CONFIG_SUSPEND))
> -		acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops);
>  err_pci_dev_put:
>  	pci_dev_put(rdev);
>  	return err;
> 

-- 
 i.