Re: [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi()

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

[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]

On Thu, 27 Jun 2024, Suma Hegde wrote:

> This is in preparation to splitting ACPI and platform device drivers.
> Having init_acpi() helps in smooth code movement.
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/hsmp.c | 91 ++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 8fcf38eed7f0..10ab9b2437f1 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -778,6 +778,11 @@ static int init_platform_device(struct device *dev)
>  			dev_err(dev, "Is HSMP disabled in BIOS ?\n");
>  			return ret;
>  		}
> +		ret = hsmp_cache_proto_ver(i);
> +		if (ret) {
> +			dev_err(dev, "Failed to read HSMP protocol version\n");
> +			return ret;
> +		}
>  	}
>  
>  	return 0;
> @@ -789,10 +794,53 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
>  
> +static bool check_acpi_support(struct device *dev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> +	if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> +		return true;
> +
> +	return false;
> +}
> +
> +static int init_acpi(struct device *dev)
> +{
> +	u16 sock_ind;
> +	int ret;
> +
> +	ret = hsmp_get_uid(dev, &sock_ind);
> +	if (ret)
> +		return ret;
> +	if (sock_ind >= plat_dev.num_sockets)
> +		return -EINVAL;
> +
> +	ret = hsmp_parse_acpi_table(dev, sock_ind);
> +	if (ret) {
> +		dev_err(dev, "Failed to parse ACPI table\n");
> +		return ret;
> +	}
> +
> +	/* Test the hsmp interface */
> +	ret = hsmp_test(sock_ind, 0xDEADBEEF);
> +	if (ret) {
> +		dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> +			boot_cpu_data.x86, boot_cpu_data.x86_model);
> +		dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> +		return ret;
> +	}
> +
> +	ret = hsmp_cache_proto_ver(sock_ind);
> +	if (ret) {
> +		dev_err(dev, "Failed to read HSMP protocol version\n");
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
>  static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  {
> -	struct acpi_device *adev;
> -	u16 sock_ind = 0;
>  	int ret;
>  
>  	/*
> @@ -809,46 +857,25 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  		if (!plat_dev.sock)
>  			return -ENOMEM;
>  	}
> -	adev = ACPI_COMPANION(&pdev->dev);
> -	if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) {
> -		ret = hsmp_get_uid(&pdev->dev, &sock_ind);
> -		if (ret)
> -			return ret;
> -		if (sock_ind >= plat_dev.num_sockets)
> -			return -EINVAL;
> -		ret = hsmp_parse_acpi_table(&pdev->dev, sock_ind);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Failed to parse ACPI table\n");
> -			return ret;
> -		}
> -		/* Test the hsmp interface */
> -		ret = hsmp_test(sock_ind, 0xDEADBEEF);
> +	if (check_acpi_support(&pdev->dev)) {
> +		ret = init_acpi(&pdev->dev);
>  		if (ret) {
> -			dev_err(&pdev->dev, "HSMP test message failed on Fam:%x model:%x\n",
> -				boot_cpu_data.x86, boot_cpu_data.x86_model);
> -			dev_err(&pdev->dev, "Is HSMP disabled in BIOS ?\n");
> +			dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
>  			return ret;
>  		}
> +		ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> +		if (ret)
> +			dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
>  	} else {
>  		ret = init_platform_device(&pdev->dev);
>  		if (ret) {
>  			dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
>  			return ret;
>  		}
> -	}
> -
> -	ret = hsmp_cache_proto_ver(sock_ind);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to read HSMP protocol version\n");
> -		return ret;
> -	}
> -
> -	if (plat_dev.is_acpi_device)
> -		ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> -	else
>  		ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
> -	if (ret)
> -		dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +		if (ret)
> +			dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +	}
>  
>  	if (!plat_dev.is_probed) {
>  		plat_dev.hsmp_device.name	= HSMP_CDEV_NAME;
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.