Re: [PATCH] platform/x86/amd/pmc: Detect when STB is not available

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 24 Oct 2024, Corey Hickey wrote:

> From: Corey Hickey <bugfood-c@fatooh.org>
> 
> Loading the amd_pmc module as:
> 
>     amd_pmc enable_stb=1
> 
> ...can result in the following messages in the kernel ring buffer:
> 
>     amd_pmc AMDI0009:00: SMU cmd failed. err: 0xff
>     ioremap on RAM at 0x0000000000000000 - 0x0000000000ffffff
>     WARNING: CPU: 10 PID: 2151 at arch/x86/mm/ioremap.c:217 __ioremap_caller+0x2cd/0x340
> 
> Additional debug shows that this happens when the calls to obtain
> S2D_PHYS_ADDR_LOW and S2D_PHYS_ADDR_HIGH return 0.
> 
> Per discussion on platform-driver-x86@vger.kernel.org, this condition
> indicates that the STB is not available.

If you want to refer to discussion, add it into a Link: tag. Only write 
the conclusion into the commit message (the part you have there after 
comma).

> In order to avoid the ioremap warning, and to help the user understand
> the situation, catch the invalid address and print an error.
> 
> Signed-off-by: Corey Hickey <bugfood-c@fatooh.org>

Isn't Fixes tag appropriate for this change?

> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index bbb8edb62e00..72b1dfc64bf1 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -998,6 +998,11 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
>  	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low, dev->s2d_msg_id, true);
>  	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi, dev->s2d_msg_id, true);
>  
> +	if (!phys_addr_hi && !phys_addr_low) {
> +		dev_err(dev->dev, "amd_pmc: STB is not enabled on the system; disable enable_stb or contact system vendor\n");

Won't that end up duplicating the prefix if you put one into the string? 
The prefix is handled for you by pr_fmt() which is already provided in 
this file.

-- 
 i.

> +		return -EINVAL;
> +	}
> +
>  	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
>  
>  	/* Clear msg_port for other SMU operation */
>