Re: [PATCH 5/7] platform/x86: Move dev from platdev to hsmp_socket

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12/12/23 11:36, Suma Hegde wrote:
> On an ACPI enabled platforms the probe is called for each socket
> and the struct dev is different for each socket. This change
> will help in handling both ACPI and non-ACPI platforms.
> 
> Also change pr_err to dev_err API.
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Co-developed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/platform/x86/amd/hsmp.c | 42 +++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
> index 44b15c1fab6a..87af1ad5c645 100644
> --- a/drivers/platform/x86/amd/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp.c
> @@ -69,13 +69,13 @@ struct hsmp_socket {
>  	struct semaphore hsmp_sem;
>  	char name[HSMP_ATTR_GRP_NAME_SIZE];
>  	struct pci_dev *root;
> +	struct device *dev;
>  	u16 sock_ind;
>  };
>  
>  struct hsmp_plat_device {
>  	struct miscdevice hsmp_device;
>  	struct hsmp_socket *sock;
> -	struct device *dev;
>  	u32 proto_ver;
>  	u16 num_sockets;
>  };
> @@ -279,8 +279,9 @@ static int hsmp_test(u16 sock_ind, u32 value)
>  
>  	/* Check the response value */
>  	if (msg.args[0] != (value + 1)) {
> -		pr_err("Socket %d test message failed, Expected 0x%08X, received 0x%08X\n",
> -		       sock_ind, (value + 1), msg.args[0]);
> +		dev_err(plat_dev.sock[sock_ind].dev,
> +			"Socket %d test message failed, Expected 0x%08X, received 0x%08X\n",
> +			sock_ind, (value + 1), msg.args[0]);
>  		return -EBADE;
>  	}
>  
> @@ -359,12 +360,12 @@ static ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
>  
>  	/* Do not support lseek(), reads entire metric table */
>  	if (count < bin_attr->size) {
> -		dev_err(plat_dev.dev, "Wrong buffer size\n");
> +		dev_err(sock->dev, "Wrong buffer size\n");
>  		return -EINVAL;
>  	}
>  
>  	if (!sock) {
> -		dev_err(plat_dev.dev, "Failed to read attribute private data\n");
> +		dev_err(sock->dev, "Failed to read attribute private data\n");
>  		return -EINVAL;
>  	}
>  
> @@ -400,13 +401,13 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
>  	 */
>  	dram_addr = msg.args[0] | ((u64)(msg.args[1]) << 32);
>  	if (!dram_addr) {
> -		dev_err(plat_dev.dev, "Invalid DRAM address for metric table\n");
> +		dev_err(sock->dev, "Invalid DRAM address for metric table\n");
>  		return -ENOMEM;
>  	}
> -	sock->metric_tbl_addr = devm_ioremap(plat_dev.dev, dram_addr,
> +	sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
>  					     sizeof(struct hsmp_metric_table));
>  	if (!sock->metric_tbl_addr) {
> -		dev_err(plat_dev.dev, "Failed to ioremap metric table addr\n");
> +		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
>  		return -ENOMEM;
>  	}
>  	return 0;
> @@ -454,14 +455,15 @@ static int hsmp_create_sysfs_interface(void)
>  	if (WARN_ON(plat_dev.num_sockets > U8_MAX))
>  		return -ERANGE;
>  
> -	hsmp_attr_grps = devm_kzalloc(plat_dev.dev, sizeof(struct attribute_group *) *
> +	hsmp_attr_grps = devm_kzalloc(plat_dev.sock[0].dev, sizeof(struct attribute_group *) *
>  				      (plat_dev.num_sockets + 1), GFP_KERNEL);
>  	if (!hsmp_attr_grps)
>  		return -ENOMEM;
>  
>  	/* Create a sysfs directory for each socket */
>  	for (i = 0; i < plat_dev.num_sockets; i++) {
> -		attr_grp = devm_kzalloc(plat_dev.dev, sizeof(struct attribute_group), GFP_KERNEL);
> +		attr_grp = devm_kzalloc(plat_dev.sock[i].dev, sizeof(struct attribute_group),
> +					GFP_KERNEL);
>  		if (!attr_grp)
>  			return -ENOMEM;
>  
> @@ -469,7 +471,7 @@ static int hsmp_create_sysfs_interface(void)
>  		attr_grp->name = plat_dev.sock[i].name;
>  
>  		/* Null terminated list of attributes */
> -		hsmp_bin_attrs = devm_kzalloc(plat_dev.dev, sizeof(struct bin_attribute *) *
> +		hsmp_bin_attrs = devm_kzalloc(plat_dev.sock[i].dev, sizeof(struct bin_attribute *) *
>  					      (NUM_HSMP_ATTRS + 1), GFP_KERNEL);
>  		if (!hsmp_bin_attrs)
>  			return -ENOMEM;
> @@ -483,7 +485,7 @@ static int hsmp_create_sysfs_interface(void)
>  		if (ret)
>  			return ret;
>  	}
> -	return devm_device_add_groups(plat_dev.dev, hsmp_attr_grps);
> +	return devm_device_add_groups(plat_dev.sock[0].dev, hsmp_attr_grps);
>  }
>  
>  static int hsmp_cache_proto_ver(void)
> @@ -502,7 +504,7 @@ static int hsmp_cache_proto_ver(void)
>  	return ret;
>  }
>  
> -static int initialize_platdev(void)
> +static int initialize_platdev(struct device *dev)
>  {
>  	int i;
>  
> @@ -511,6 +513,7 @@ static int initialize_platdev(void)
>  			return -ENODEV;
>  		plat_dev.sock[i].root			= node_to_amd_nb(i)->root;
>  		plat_dev.sock[i].sock_ind		= i;
> +		plat_dev.sock[i].dev			= dev;
>  		plat_dev.sock[i].mbinfo.base_addr	= SMN_HSMP_BASE;
>  		plat_dev.sock[i].mbinfo.msg_id_off	= SMN_HSMP_MSG_ID;
>  		plat_dev.sock[i].mbinfo.msg_resp_off    = SMN_HSMP_MSG_RESP;
> @@ -530,18 +533,17 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  				     GFP_KERNEL);
>  	if (!plat_dev.sock)
>  		return -ENOMEM;
> -	plat_dev.dev = &pdev->dev;
>  
> -	ret = initialize_platdev();
> +	ret = initialize_platdev(&pdev->dev);
>  	if (ret)
>  		return ret;
>  
>  	/* Test the hsmp interface */
>  	ret = hsmp_test(0, 0xDEADBEEF);
>  	if (ret) {
> -		pr_err("HSMP test message failed on Fam:%x model:%x\n",
> -		       boot_cpu_data.x86, boot_cpu_data.x86_model);
> -		pr_err("Is HSMP disabled in BIOS ?\n");
> +		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");
>  		return ret;
>  	}
>  
> @@ -554,13 +556,13 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  
>  	ret = hsmp_cache_proto_ver();
>  	if (ret) {
> -		dev_err(plat_dev.dev, "Failed to read HSMP protocol version\n");
> +		dev_err(&pdev->dev, "Failed to read HSMP protocol version\n");
>  		return ret;
>  	}
>  
>  	ret = hsmp_create_sysfs_interface();
>  	if (ret)
> -		dev_err(plat_dev.dev, "Failed to create HSMP sysfs interface\n");
> +		dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
>  
>  	return misc_register(&plat_dev.hsmp_device);
>  }