Re: [v3 06/11] platform/x86/amd/hsmp: Change generic plat_dev name to hsmp_pdev

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

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

On Sat, 20 Jul 2024, Suma Hegde wrote:

> plat_dev is a commonly used variable name, since its made as extern now,
> change it to more specific name.
> Also change miscdevice hsmp_device to mdev.
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> Changes since v2:
> None
> Changes since v1:
> 1. This is a new patch to rename generic name "plat_dev"
>    in hsmp.h, hsmp.c, plat.c and acpi.c to more specific name "hsmp_pdev"
> 2. Rename miscdevice hsmp_device to mdev.
> 
>  drivers/platform/x86/amd/hsmp/acpi.c | 10 +++---
>  drivers/platform/x86/amd/hsmp/hsmp.c | 22 ++++++------
>  drivers/platform/x86/amd/hsmp/hsmp.h |  4 +--
>  drivers/platform/x86/amd/hsmp/plat.c | 52 ++++++++++++++--------------
>  4 files changed, 44 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index eca324774141..46cb86d5d550 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -191,12 +191,12 @@ static int hsmp_read_acpi_crs(struct hsmp_socket *sock)
>  /* Parse the ACPI table to read the data */
>  static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>  {
> -	struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
> +	struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
>  	int ret;
>  
> -	sock->sock_ind		= sock_ind;
> -	sock->dev		= dev;
> -	plat_dev.is_acpi_device	= true;
> +	sock->sock_ind			= sock_ind;
> +	sock->dev			= dev;
> +	hsmp_pdev.is_acpi_device	= true;

This a good example why aligning = is usually bad idea because it 
tends to lead to extra code churn like this when the alignment has to be 
maintained.

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

-- 
 i.

>  	sema_init(&sock->hsmp_sem, 1);
>  
> @@ -240,7 +240,7 @@ int init_acpi(struct device *dev)
>  	ret = hsmp_get_uid(dev, &sock_ind);
>  	if (ret)
>  		return ret;
> -	if (sock_ind >= plat_dev.num_sockets)
> +	if (sock_ind >= hsmp_pdev.num_sockets)
>  		return -EINVAL;
>  
>  	ret = hsmp_parse_acpi_table(dev, sock_ind);
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 8473c2d1258b..14edaace4379 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -29,12 +29,12 @@
>  #define HSMP_WR			true
>  #define HSMP_RD			false
>  
> -struct hsmp_plat_device plat_dev;
> +struct hsmp_plat_device hsmp_pdev;
>  
>  static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
>  			 u32 *value, bool write)
>  {
> -	if (plat_dev.is_acpi_device)
> +	if (hsmp_pdev.is_acpi_device)
>  		amd_hsmp_acpi_rdwr(sock, offset, value, write);
>  	else
>  		return amd_hsmp_pci_rdwr(sock, offset, value, write);
> @@ -177,9 +177,9 @@ int hsmp_send_message(struct hsmp_message *msg)
>  	if (ret)
>  		return ret;
>  
> -	if (!plat_dev.sock || msg->sock_ind >= plat_dev.num_sockets)
> +	if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
>  		return -ENODEV;
> -	sock = &plat_dev.sock[msg->sock_ind];
> +	sock = &hsmp_pdev.sock[msg->sock_ind];
>  
>  	/*
>  	 * The time taken by smu operation to complete is between
> @@ -220,7 +220,7 @@ int hsmp_test(u16 sock_ind, u32 value)
>  
>  	/* Check the response value */
>  	if (msg.args[0] != (value + 1)) {
> -		dev_err(plat_dev.sock[sock_ind].dev,
> +		dev_err(hsmp_pdev.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;
> @@ -315,7 +315,7 @@ ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
>  
>  static int hsmp_get_tbl_dram_base(u16 sock_ind)
>  {
> -	struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
> +	struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
>  	struct hsmp_message msg = { 0 };
>  	phys_addr_t dram_addr;
>  	int ret;
> @@ -349,7 +349,7 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
>  umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>  				  struct bin_attribute *battr, int id)
>  {
> -	if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> +	if (hsmp_pdev.proto_ver == HSMP_PROTO_VER6)
>  		return battr->attr.mode;
>  	else
>  		return 0;
> @@ -357,17 +357,17 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>  
>  static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock_ind)
>  {
> -	struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
> +	struct bin_attribute *hattr = &hsmp_pdev.sock[sock_ind].hsmp_attr;
>  
>  	sysfs_bin_attr_init(hattr);
>  	hattr->attr.name	= HSMP_METRICS_TABLE_NAME;
>  	hattr->attr.mode	= 0444;
>  	hattr->read		= hsmp_metric_tbl_read;
>  	hattr->size		= sizeof(struct hsmp_metric_table);
> -	hattr->private		= &plat_dev.sock[sock_ind];
> +	hattr->private		= &hsmp_pdev.sock[sock_ind];
>  	hattrs[0]		= hattr;
>  
> -	if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> +	if (hsmp_pdev.proto_ver == HSMP_PROTO_VER6)
>  		return hsmp_get_tbl_dram_base(sock_ind);
>  	else
>  		return 0;
> @@ -404,7 +404,7 @@ int hsmp_cache_proto_ver(u16 sock_ind)
>  
>  	ret = hsmp_send_message(&msg);
>  	if (!ret)
> -		plat_dev.proto_ver = msg.args[0];
> +		hsmp_pdev.proto_ver = msg.args[0];
>  
>  	return ret;
>  }
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index 1ff652dc0ef0..a77887d298b6 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -44,7 +44,7 @@ struct hsmp_socket {
>  };
>  
>  struct hsmp_plat_device {
> -	struct miscdevice hsmp_device;
> +	struct miscdevice mdev;
>  	struct hsmp_socket *sock;
>  	u32 proto_ver;
>  	u16 num_sockets;
> @@ -52,7 +52,7 @@ struct hsmp_plat_device {
>  	bool is_probed;
>  };
>  
> -extern struct hsmp_plat_device plat_dev;
> +extern struct hsmp_plat_device hsmp_pdev;
>  
>  int init_acpi(struct device *dev);
>  ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index 2ed5a9452244..c297540bb64c 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -68,21 +68,21 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
>  	struct attribute_group *attr_grp;
>  	u16 i;
>  
> -	hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
> +	hsmp_attr_grps = devm_kcalloc(dev, hsmp_pdev.num_sockets + 1,
>  				      sizeof(*hsmp_attr_grps),
>  				      GFP_KERNEL);
>  	if (!hsmp_attr_grps)
>  		return -ENOMEM;
>  
>  	/* Create a sysfs directory for each socket */
> -	for (i = 0; i < plat_dev.num_sockets; i++) {
> +	for (i = 0; i < hsmp_pdev.num_sockets; i++) {
>  		attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
>  					GFP_KERNEL);
>  		if (!attr_grp)
>  			return -ENOMEM;
>  
> -		snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> -		attr_grp->name			= plat_dev.sock[i].name;
> +		snprintf(hsmp_pdev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> +		attr_grp->name			= hsmp_pdev.sock[i].name;
>  		attr_grp->is_bin_visible	= hsmp_is_sock_attr_visible;
>  		hsmp_attr_grps[i]		= attr_grp;
>  
> @@ -105,10 +105,10 @@ static int init_platform_device(struct device *dev)
>  	struct hsmp_socket *sock;
>  	int ret, i;
>  
> -	for (i = 0; i < plat_dev.num_sockets; i++) {
> +	for (i = 0; i < hsmp_pdev.num_sockets; i++) {
>  		if (!node_to_amd_nb(i))
>  			return -ENODEV;
> -		sock = &plat_dev.sock[i];
> +		sock = &hsmp_pdev.sock[i];
>  		sock->root			= node_to_amd_nb(i)->root;
>  		sock->sock_ind			= i;
>  		sock->dev			= dev;
> @@ -173,11 +173,11 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  	 * Hence allocate memory for all the sockets at once instead of allocating
>  	 * on each probe.
>  	 */
> -	if (!plat_dev.is_probed) {
> -		plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
> -					     sizeof(*plat_dev.sock),
> -					     GFP_KERNEL);
> -		if (!plat_dev.sock)
> +	if (!hsmp_pdev.is_probed) {
> +		hsmp_pdev.sock = devm_kcalloc(&pdev->dev, hsmp_pdev.num_sockets,
> +					      sizeof(*hsmp_pdev.sock),
> +					      GFP_KERNEL);
> +		if (!hsmp_pdev.sock)
>  			return -ENOMEM;
>  	}
>  
> @@ -201,19 +201,19 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
>  	}
>  
> -	if (!plat_dev.is_probed) {
> -		plat_dev.hsmp_device.name	= HSMP_CDEV_NAME;
> -		plat_dev.hsmp_device.minor	= MISC_DYNAMIC_MINOR;
> -		plat_dev.hsmp_device.fops	= &hsmp_fops;
> -		plat_dev.hsmp_device.parent	= &pdev->dev;
> -		plat_dev.hsmp_device.nodename	= HSMP_DEVNODE_NAME;
> -		plat_dev.hsmp_device.mode	= 0644;
> +	if (!hsmp_pdev.is_probed) {
> +		hsmp_pdev.mdev.name	= HSMP_CDEV_NAME;
> +		hsmp_pdev.mdev.minor	= MISC_DYNAMIC_MINOR;
> +		hsmp_pdev.mdev.fops	= &hsmp_fops;
> +		hsmp_pdev.mdev.parent	= &pdev->dev;
> +		hsmp_pdev.mdev.nodename	= HSMP_DEVNODE_NAME;
> +		hsmp_pdev.mdev.mode	= 0644;
>  
> -		ret = misc_register(&plat_dev.hsmp_device);
> +		ret = misc_register(&hsmp_pdev.mdev);
>  		if (ret)
>  			return ret;
>  
> -		plat_dev.is_probed = true;
> +		hsmp_pdev.is_probed = true;
>  	}
>  
>  	return 0;
> @@ -225,9 +225,9 @@ static void hsmp_pltdrv_remove(struct platform_device *pdev)
>  	 * We register only one misc_device even on multi socket system.
>  	 * So, deregister should happen only once.
>  	 */
> -	if (plat_dev.is_probed) {
> -		misc_deregister(&plat_dev.hsmp_device);
> -		plat_dev.is_probed = false;
> +	if (hsmp_pdev.is_probed) {
> +		misc_deregister(&hsmp_pdev.mdev);
> +		hsmp_pdev.is_probed = false;
>  	}
>  }
>  
> @@ -299,15 +299,15 @@ static int __init hsmp_plt_init(void)
>  	 * amd_nb_num() returns number of SMN/DF interfaces present in the system
>  	 * if we have N SMN/DF interfaces that ideally means N sockets
>  	 */
> -	plat_dev.num_sockets = amd_nb_num();
> -	if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
> +	hsmp_pdev.num_sockets = amd_nb_num();
> +	if (hsmp_pdev.num_sockets == 0 || hsmp_pdev.num_sockets > MAX_AMD_SOCKETS)
>  		return ret;
>  
>  	ret = platform_driver_register(&amd_hsmp_driver);
>  	if (ret)
>  		return ret;
>  
> -	if (!plat_dev.is_acpi_device) {
> +	if (!hsmp_pdev.is_acpi_device) {
>  		if (legacy_hsmp_support()) {
>  			/* Not ACPI device, but supports HSMP, register a plat_dev */
>  			ret = hsmp_plat_dev_register();
>