Message-ID: <20240704111624.1583460-7-akshay.gupta@amd.com>
Date: Thu, 4 Jul 2024 11:16:24 +0000
From: Akshay Gupta <akshay.gupta@....com>
To: <linux-hwmon@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux@...ck-us.net>, <gregkh@...uxfoundation.org>, <arnd@...db.de>
CC: <naveenkrishna.chatradhi@....com>, Akshay Gupta <akshay.gupta@....com>
Subject: [PATCH 6/6] misc: sbrmi: Add support for new revision

- RMI device supports v21 on Turin, which has increased register set.
  Hence, requires 2byte for register addressing.

- Both Genoa and Turin processors are SP5 compatible, often CPUs are
  interchanged on the base boards with BMC remaining the same.
  Hence, we need to identify correct regmap configuration. A mechanism
  was not defined to identify the RMI register address width.
- The address width can only be determined if the socket is powered ON.
- This patch also addresses CPUID and MCAMSR register read protocol,
  the modification is due to the increase in register address size and
  the modified thread input.

Signed-off-by: Akshay Gupta <akshay.gupta@....com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@....com>
---
 drivers/misc/amd-sb/sbrmi-core.c | 215 +++++++++++++++++++++++--------
 drivers/misc/amd-sb/sbrmi-i2c.c  |  94 ++++++++++++--
 2 files changed, 242 insertions(+), 67 deletions(-)

diff --git a/drivers/misc/amd-sb/sbrmi-core.c b/drivers/misc/amd-sb/sbrmi-core.c
index c180f58b92c3..dc6f4e806a91 100644
--- a/drivers/misc/amd-sb/sbrmi-core.c
+++ b/drivers/misc/amd-sb/sbrmi-core.c
@@ -29,13 +29,17 @@
 /* MSR */
 #define MSR_RD_REG_LEN		0xa
 #define MSR_WR_REG_LEN		0x8
+#define MSR_WR_REG_LEN_v21	0x9
 #define MSR_RD_DATA_LEN		0x8
 #define MSR_WR_DATA_LEN		0x7
+#define MSR_WR_DATA_LEN_v21	0x8
 /* CPUID */
 #define CPUID_RD_DATA_LEN	0x8
 #define CPUID_WR_DATA_LEN	0x8
+#define CPUID_WR_DATA_LEN_v21	0x9
 #define CPUID_RD_REG_LEN	0xa
 #define CPUID_WR_REG_LEN	0x9
+#define CPUID_WR_REG_LEN_v21	0xa
 
 /* CPUID MSR Command Ids */
 #define CPUID_MCA_CMD	0x73
@@ -55,6 +59,19 @@ struct cpu_msr_indata {
 	u8 ext; /* extended function */
 } __packed;
 
+/* input for bulk write to v21 of CPUID and MSR protocol */
+struct cpu_msr_indata_v21 {
+	u8 wr_len;	/* const value */
+	u8 rd_len;	/* const value */
+	u8 proto_cmd;	/* const value */
+	u16 thread;	/* thread number */
+	union {
+		u8 reg_offset[4];	/* input value */
+		u32 value;
+	};
+	u8 ext; /* extended function */
+} __packed;
+
 /* output for bulk read from CPUID and MSR protocol */
 struct cpu_msr_outdata {
 	u8 num_bytes;	/* number of bytes return */
@@ -65,16 +82,16 @@ struct cpu_msr_outdata {
 	};
 } __packed;
 
-#define prepare_mca_msr_input_message(input, thread_id, data_in)	\
+#define prepare_mca_msr_input_message(input, thread_id, data_in, wr_data_len)	\
 	input.rd_len = MSR_RD_DATA_LEN,					\
-	input.wr_len = MSR_WR_DATA_LEN,					\
+	input.wr_len = wr_data_len,					\
 	input.proto_cmd = RD_MCA_CMD,					\
 	input.thread = thread_id << 1,					\
 	input.value =  data_in
 
-#define prepare_cpuid_input_message(input, thread_id, func, ext_func)	\
+#define prepare_cpuid_input_message(input, thread_id, func, ext_func, wr_data_len)	\
 	input.rd_len = CPUID_RD_DATA_LEN,				\
-	input.wr_len = CPUID_WR_DATA_LEN,				\
+	input.wr_len = wr_data_len,				\
 	input.proto_cmd = RD_CPUID_CMD,					\
 	input.thread = thread_id << 1,					\
 	input.value =  func,						\
@@ -131,25 +148,13 @@ static int sbrmi_wait_status(struct sbrmi_data *data,
 }
 
 /* MCA MSR protocol */
-static int rmi_mca_msr_read(struct sbrmi_data *data,
-			    struct apml_message *msg)
+static int msr_datain_v20(struct sbrmi_data *data,
+			  struct apml_message *msg)
 {
-	struct cpu_msr_outdata output = {0};
 	struct cpu_msr_indata input = {0};
 	int ret, val = 0;
-	int hw_status;
 	u16 thread;
 
-	/* cache the rev value to identify if protocol is supported or not */
-	if (!data->rev) {
-		ret = sbrmi_get_rev(data);
-		if (ret < 0)
-			return ret;
-	}
-	/* MCA MSR protocol for REV 0x10 is not supported*/
-	if (data->rev == 0x10)
-		return -EOPNOTSUPP;
-
 	thread = msg->data_in.reg_in[THREAD_LOW_INDEX] |
 		 msg->data_in.reg_in[THREAD_HI_INDEX] << 8;
 
@@ -160,15 +165,71 @@ static int rmi_mca_msr_read(struct sbrmi_data *data,
 	}
 	ret = regmap_write(data->regmap, SBRMI_THREAD128CS, val);
 	if (ret < 0)
-		goto exit_unlock;
+		return ret;
 
 	prepare_mca_msr_input_message(input, thread,
-				      msg->data_in.mb_in[RD_WR_DATA_INDEX]);
+				      msg->data_in.mb_in[RD_WR_DATA_INDEX],
+				      MSR_WR_DATA_LEN);
 
 	ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD,
 				&input, MSR_WR_REG_LEN);
-	if (ret < 0)
-		goto exit_unlock;
+	return ret;
+}
+
+static int msr_datain_v21(struct sbrmi_data *data,
+			  struct apml_message *msg)
+{
+	struct cpu_msr_indata_v21 input = {0};
+	int ret;
+	u16 thread;
+
+	thread = msg->data_in.reg_in[THREAD_LOW_INDEX] |
+		 msg->data_in.reg_in[THREAD_HI_INDEX] << 8;
+
+	prepare_mca_msr_input_message(input, thread,
+				      msg->data_in.mb_in[RD_WR_DATA_INDEX],
+				      MSR_WR_DATA_LEN_v21);
+
+	ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD,
+				&input, MSR_WR_REG_LEN_v21);
+	return ret;
+}
+
+static int rmi_mca_msr_read(struct sbrmi_data *data,
+			    struct apml_message *msg)
+{
+	struct cpu_msr_outdata output = {0};
+	int ret;
+	int hw_status;
+
+	if (!data->regmap)
+		return -ENODEV;
+
+	/* cache the rev value to identify if protocol is supported or not */
+	if (!data->rev) {
+		ret = sbrmi_get_rev(data);
+		if (ret < 0)
+			return ret;
+	}
+
+	switch (data->rev) {
+	/* MCA MSR protocol for REV 0x10 is not supported*/
+	case 0x10:
+		return -EOPNOTSUPP;
+	case 0x20:
+		ret = msr_datain_v20(data, msg);
+		if (ret < 0)
+			goto exit_unlock;
+
+		break;
+	case 0x21:
+		ret = msr_datain_v21(data, msg);
+		if (ret < 0)
+			goto exit_unlock;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
 
 	ret = sbrmi_wait_status(data, &hw_status, HW_ALERT_MASK);
 	if (ret < 0)
@@ -200,25 +261,13 @@ static int rmi_mca_msr_read(struct sbrmi_data *data,
 }
 
 /* Read CPUID function protocol */
-static int rmi_cpuid_read(struct sbrmi_data *data,
-			  struct apml_message *msg)
+static int cpuid_datain_v20(struct sbrmi_data *data,
+			    struct apml_message *msg)
 {
 	struct cpu_msr_indata input = {0};
-	struct cpu_msr_outdata output = {0};
-	int val = 0;
-	int ret, hw_status;
+	int ret, val = 0;
 	u16 thread;
 
-	/* cache the rev value to identify if protocol is supported or not */
-	if (!data->rev) {
-		ret = sbrmi_get_rev(data);
-		if (ret < 0)
-			return ret;
-	}
-	/* CPUID protocol for REV 0x10 is not supported*/
-	if (data->rev == 0x10)
-		return -EOPNOTSUPP;
-
 	thread = msg->data_in.reg_in[THREAD_LOW_INDEX] |
 		 msg->data_in.reg_in[THREAD_HI_INDEX] << 8;
 
@@ -229,16 +278,71 @@ static int rmi_cpuid_read(struct sbrmi_data *data,
 	}
 	ret = regmap_write(data->regmap, SBRMI_THREAD128CS, val);
 	if (ret < 0)
-		goto exit_unlock;
-
+		return ret;
 	prepare_cpuid_input_message(input, thread,
 				    msg->data_in.mb_in[RD_WR_DATA_INDEX],
-				    msg->data_in.reg_in[EXT_FUNC_INDEX]);
+				    msg->data_in.reg_in[EXT_FUNC_INDEX],
+				    CPUID_WR_DATA_LEN);
 
 	ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD,
 				&input, CPUID_WR_REG_LEN);
-	if (ret < 0)
-		goto exit_unlock;
+	return ret;
+}
+
+static int cpuid_datain_v21(struct sbrmi_data *data,
+			    struct apml_message *msg)
+{
+	struct cpu_msr_indata_v21 input = {0};
+	int ret;
+	u16 thread;
+
+	thread = msg->data_in.reg_in[THREAD_LOW_INDEX] |
+		 msg->data_in.reg_in[THREAD_HI_INDEX] << 8;
+
+	prepare_cpuid_input_message(input, thread,
+				    msg->data_in.mb_in[RD_WR_DATA_INDEX],
+				    msg->data_in.reg_in[EXT_FUNC_INDEX],
+				    CPUID_WR_DATA_LEN_v21);
+
+	ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD,
+				&input, CPUID_WR_REG_LEN_v21);
+	return ret;
+}
+
+/* CPUID protocol */
+static int rmi_cpuid_read(struct sbrmi_data *data,
+			  struct apml_message *msg)
+{
+	struct cpu_msr_outdata output = {0};
+	int ret, hw_status;
+
+	if (!data->regmap)
+		return -ENODEV;
+
+	/* cache the rev value to identify if protocol is supported or not */
+	if (!data->rev) {
+		ret = sbrmi_get_rev(data);
+		if (ret < 0)
+			return ret;
+	}
+
+	switch (data->rev) {
+	/* CPUID protocol for REV 0x10 is not supported*/
+	case 0x10:
+		return -EOPNOTSUPP;
+	case 0x20:
+		ret = cpuid_datain_v20(data, msg);
+		if (ret < 0)
+			goto exit_unlock;
+		break;
+	case 0x21:
+		ret = cpuid_datain_v21(data, msg);
+		if (ret < 0)
+			goto exit_unlock;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
 
 	ret = sbrmi_wait_status(data, &hw_status, HW_ALERT_MASK);
 	if (ret < 0)
@@ -412,11 +516,11 @@ static int rmi_xfer(struct sbrmi_data *data,
 		/* REG R/W */
 		if (msg->data_in.reg_in[RD_FLAG_INDEX])
 			ret = regmap_read(data->regmap,
-					  msg->data_in.reg_in[REG_OFF_INDEX],
+					  msg->data_in.mb_in[REG_OFF_INDEX],
 					  &msg->data_out.mb_out[RD_WR_DATA_INDEX]);
 		else
 			ret = regmap_write(data->regmap,
-					   msg->data_in.reg_in[REG_OFF_INDEX],
+					   msg->data_in.mb_in[REG_OFF_INDEX],
 					   msg->data_in.reg_in[REG_VAL_INDEX]);
 		break;
 	default:
@@ -466,7 +570,6 @@ static long sbrmi_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 	if (copy_struct_from_user(&msg, sizeof(msg), arguser,
 				  sizeof(struct apml_message)))
 		return ret;
-
 	/* Is this a read/monitor/get request */
 	if (msg.data_in.reg_in[RD_FLAG_INDEX])
 		read = true;
@@ -492,18 +595,18 @@ int create_misc_rmi_device(struct sbrmi_data *data,
 {
 	int ret;
 
-	data->sbrmi_misc_dev.name	= devm_kasprintf(dev,
-							 GFP_KERNEL,
-							 "sbrmi-%x",
-							 data->dev_static_addr);
-	data->sbrmi_misc_dev.minor	= MISC_DYNAMIC_MINOR;
-	data->sbrmi_misc_dev.fops	= &sbrmi_fops;
-	data->sbrmi_misc_dev.parent	= dev;
+	data->sbrmi_misc_dev.name		= devm_kasprintf(dev,
+								 GFP_KERNEL,
+								 "sbrmi-%x",
+								 data->dev_static_addr);
+	data->sbrmi_misc_dev.minor		= MISC_DYNAMIC_MINOR;
+	data->sbrmi_misc_dev.fops		= &sbrmi_fops;
+	data->sbrmi_misc_dev.parent		= dev;
 	data->sbrmi_misc_dev.nodename	= devm_kasprintf(dev,
-							 GFP_KERNEL,
-							 "sbrmi-%x",
-							 data->dev_static_addr);
-	data->sbrmi_misc_dev.mode	= 0600;
+								 GFP_KERNEL,
+								 "sbrmi-%x",
+								 data->dev_static_addr);
+	data->sbrmi_misc_dev.mode		= 0600;
 
 	ret = misc_register(&data->sbrmi_misc_dev);
 	if (ret)
diff --git a/drivers/misc/amd-sb/sbrmi-i2c.c b/drivers/misc/amd-sb/sbrmi-i2c.c
index f0499b0ef5e3..f0645b3fa05b 100644
--- a/drivers/misc/amd-sb/sbrmi-i2c.c
+++ b/drivers/misc/amd-sb/sbrmi-i2c.c
@@ -19,6 +19,82 @@
 
 #define MAX_WAIT_TIME_SEC	(3)
 
+/* Try 2 byte address size before switching to 1 byte */
+#define MAX_RETRY	5
+
+/*
+ * There are processor with RMI address size as 2 byte and others with 1 byte,
+ * with no clear detection mechanism in place.
+ * Execute 2 bytes access first, and then fall back to 1 byte.
+ * Sending 2 bytes first is safer as sending 1 byte address size on
+ * Turin(2 bytes) can cause unrecoverable error and may requries to reboot
+ * the system.
+ */
+static int configure_regmap(struct i2c_client *client, struct sbrmi_data *data)
+{
+	struct regmap_config sbrmi_i2c_regmap_config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+	};
+	struct regmap_config sbrmi_i2c_regmap_config_2_bytes = {
+		.reg_bits = 16,
+		.val_bits = 8,
+		.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	};
+	u32 i;
+	int ret, rev;
+
+	if (!client || !data)
+		return -ENODEV;
+
+	/* Checking RMI register address size as 2 bytes */
+	data->regmap = devm_regmap_init_i2c(client,
+					    &sbrmi_i2c_regmap_config_2_bytes);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	for (i = 0; i < MAX_RETRY; i++) {
+		ret = regmap_read(data->regmap, SBRMI_REV, &rev);
+		if (ret < 0) {
+			usleep_range(10000, 20000);
+			continue;
+		} else {
+			break;
+		}
+	}
+
+	/*
+	 * Fall back to register address size as 1 byte
+	 * Check for revision as 0x21 to not break backward compatbility
+	 * for few Family:0x19 processors
+	 */
+	if (ret || rev != 0x21) {
+		data->regmap = devm_regmap_init_i2c(client,
+						    &sbrmi_i2c_regmap_config);
+		if (IS_ERR(data->regmap))
+			return PTR_ERR(data->regmap);
+
+		ret = regmap_read(data->regmap, SBRMI_REV, &rev);
+		if (ret < 0)
+			return ret;
+	}
+	data->rev = rev;
+
+	/*
+	 * For some Turin platforms, first 1 byte transaction can be successful,
+	 * verify if revision is 0x21, if yes, switch to 2 byte address size
+	 */
+	if (data->rev == 0x21)
+		data->regmap = devm_regmap_init_i2c(client,
+						    &sbrmi_i2c_regmap_config_2_bytes);
+	else
+		data->regmap = devm_regmap_init_i2c(client,
+						    &sbrmi_i2c_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+	return ret;
+}
+
 static int sbrmi_enable_alert(struct sbrmi_data *data)
 {
 	int ctrl, ret;
@@ -58,24 +134,20 @@ static int sbrmi_i2c_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct sbrmi_data *data;
-	struct regmap_config sbrmi_i2c_regmap_config = {
-		.reg_bits = 8,
-		.val_bits = 8,
-	};
 	int ret;
 
 	data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
+	ret = configure_regmap(client, data);
+	if (ret)
+		return -EPROBE_DEFER;
+
 	atomic_set(&data->in_progress, 0);
 	atomic_set(&data->no_new_trans, 0);
 	mutex_init(&data->lock);
 
-	data->regmap = devm_regmap_init_i2c(client, &sbrmi_i2c_regmap_config);
-	if (IS_ERR(data->regmap))
-		return PTR_ERR(data->regmap);
-
 	/* Enable alert for SB-RMI sequence */
 	ret = sbrmi_enable_alert(data);
 	if (ret < 0)
@@ -91,9 +163,9 @@ static int sbrmi_i2c_probe(struct i2c_client *client)
 	dev_set_drvdata(dev, (void *)data);
 
 	data->pdev = platform_device_register_data(dev, "sbrmi-hwmon",
-						   PLATFORM_DEVID_NONE,
-						   data,
-						   sizeof(struct sbrmi_data));
+						      PLATFORM_DEVID_NONE,
+						      data,
+						      sizeof(struct sbrmi_data));
 	if (IS_ERR(data->pdev)) {
 		pr_err("unable to register platform device for sbrmi-hwmon\n");
 		return PTR_ERR(data->pdev);
-- 
2.25.1

Powered by blists - more mailing lists