On Wed, 20 Dec 2023, 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@xxxxxxx>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@xxxxxxx>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> Changes since v1:
> Add "Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>"
>
> ---
Just noticed this... Please put the version history under --- line because
then you'll save us maintainers time, if you put it above that separator,
the tools will not cut it away automatically.
> 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 6beca5787e55..3508399c6aa9 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;
> };
> @@ -278,8 +278,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;
> }
>
> @@ -358,12 +359,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;
> }
>
> @@ -399,13 +400,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;
> @@ -453,14 +454,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;
>
> @@ -468,7 +470,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;
> @@ -482,7 +484,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)
> @@ -501,7 +503,7 @@ static int hsmp_cache_proto_ver(void)
> return ret;
> }
>
> -static int initialize_platdev(void)
> +static int initialize_platdev(struct device *dev)
> {
> int ret, i;
>
> @@ -510,6 +512,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;
> @@ -519,9 +522,9 @@ static int initialize_platdev(void)
> /* Test the hsmp interface on each socket */
> ret = hsmp_test(i, 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(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;
> }
> }
> @@ -538,9 +541,8 @@ 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);
It would feel a bit more natural to pass pdev to initialize_platdev() and
deref ->dev only inside it but this is not a correctness issue.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
