diff mbox series

[3/5] mfd/hwmon sbrmi: Use regmap subsystem

Message ID 20240530112307.3089696-4-naveenkrishna.chatradhi@amd.com (mailing list archive)
State Handled Elsewhere
Headers show
Series mfd: add amd side-band functionality | expand

Commit Message

Chatradhi, Naveen Krishna May 30, 2024, 11:23 a.m. UTC 
From: Akshay Gupta <akshay.gupta@amd.com>

- regmap subsystem provides multiple benefits over direct smbus APIs
- The susbsytem can be helpful in following cases
  - Differnet types of bus (i2c/i3c)
  - Different Register address size (1byte/2byte)

Signed-off-by: Akshay Gupta <Akshay.Gupta@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
 drivers/mfd/Kconfig        |  2 +-
 drivers/mfd/sbrmi-core.c   | 30 ++++++++++++------------------
 drivers/mfd/sbrmi-i2c.c    | 25 ++++++++++++++++---------
 include/linux/mfd/amd-sb.h |  6 +++---
 4 files changed, 32 insertions(+), 31 deletions(-)

Comments

Guenter Roeck June 10, 2024, 1:31 p.m. UTC | #1 
On 5/30/24 04:23, Naveen Krishna Chatradhi wrote:
> From: Akshay Gupta <akshay.gupta@amd.com>
> 
> - regmap subsystem provides multiple benefits over direct smbus APIs
> - The susbsytem can be helpful in following cases
>    - Differnet types of bus (i2c/i3c)
>    - Different Register address size (1byte/2byte)
> 
> Signed-off-by: Akshay Gupta <Akshay.Gupta@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>

The "hwmon" in the subject should be dropped.

> ---
>   drivers/mfd/Kconfig        |  2 +-
>   drivers/mfd/sbrmi-core.c   | 30 ++++++++++++------------------
>   drivers/mfd/sbrmi-i2c.c    | 25 ++++++++++++++++---------
>   include/linux/mfd/amd-sb.h |  6 +++---
>   4 files changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 0411cb29b6df..d89513e5a06b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1444,8 +1444,8 @@ config MFD_STMPE
>   
>   config MFD_SBRMI_I2C
>           tristate "APML SBRMI support"
> -        depends on I2C
>           select MFD_CORE
> +        select REGMAP_I2C
>           help
>             APML RMI core support for AMD out of band management
>   	  This driver can also be built as a module. If so, the module will
> diff --git a/drivers/mfd/sbrmi-core.c b/drivers/mfd/sbrmi-core.c
> index d872b5107b36..5617b91787ba 100644
> --- a/drivers/mfd/sbrmi-core.c
> +++ b/drivers/mfd/sbrmi-core.c
> @@ -7,9 +7,9 @@
>    */
>   #include <linux/delay.h>
>   #include <linux/err.h>
> -#include <linux/i2c.h>
>   #include <linux/mfd/amd-sb.h>
>   #include <linux/mutex.h>
> +#include <linux/regmap.h>
>   
>   /* Mask for Status Register bit[1] */
>   #define SW_ALERT_MASK	0x2
> @@ -44,6 +44,7 @@ enum sbrmi_reg {
>   int rmi_mailbox_xfer(struct sbrmi_data *data,
>   		     struct sbrmi_mailbox_msg *msg)
>   {
> +	unsigned int bytes;
>   	int i, ret, retry = 10;
>   	int sw_status;
>   	u8 byte;
> @@ -51,14 +52,12 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	mutex_lock(&data->lock);
>   
>   	/* Indicate firmware a command is to be serviced */
> -	ret = i2c_smbus_write_byte_data(data->client,
> -					SBRMI_INBNDMSG7, START_CMD);
> +	ret = regmap_write(data->regmap, SBRMI_INBNDMSG7, START_CMD);
>   	if (ret < 0)
>   		goto exit_unlock;
>   
>   	/* Write the command to SBRMI::InBndMsg_inst0 */
> -	ret = i2c_smbus_write_byte_data(data->client,
> -					SBRMI_INBNDMSG0, msg->cmd);
> +	ret = regmap_write(data->regmap, SBRMI_INBNDMSG0, msg->cmd);
>   	if (ret < 0)
>   		goto exit_unlock;
>   
> @@ -69,8 +68,7 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	 */
>   	for (i = 0; i < 4; i++) {
>   		byte = (msg->data_in >> i * 8) & 0xff;
> -		ret = i2c_smbus_write_byte_data(data->client,
> -						SBRMI_INBNDMSG1 + i, byte);
> +		ret = regmap_write(data->regmap, SBRMI_INBNDMSG1 + i, byte);
>   		if (ret < 0)
>   			goto exit_unlock;
>   	}
> @@ -79,8 +77,7 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	 * Write 0x01 to SBRMI::SoftwareInterrupt to notify firmware to
>   	 * perform the requested read or write command
>   	 */
> -	ret = i2c_smbus_write_byte_data(data->client,
> -					SBRMI_SW_INTERRUPT, TRIGGER_MAILBOX);
> +	ret = regmap_write(data->regmap, SBRMI_SW_INTERRUPT, TRIGGER_MAILBOX);
>   	if (ret < 0)
>   		goto exit_unlock;
>   
> @@ -90,8 +87,7 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	 * of the requested command
>   	 */
>   	do {
> -		sw_status = i2c_smbus_read_byte_data(data->client,
> -						     SBRMI_STATUS);
> +		ret = regmap_read(data->regmap, SBRMI_STATUS, &sw_status);
>   		if (sw_status < 0) {
>   			ret = sw_status;

This is wrong. It should be
		if (ret < 0)
			goto exit_unlock;

>   			goto exit_unlock;
> @@ -102,8 +98,6 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	} while (retry--);
>   
>   	if (retry < 0) {
> -		dev_err(&data->client->dev,
> -			"Firmware fail to indicate command completion\n");
>   		ret = -EIO;
>   		goto exit_unlock;
>   	}
> @@ -115,11 +109,11 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	 */
>   	if (msg->read) {
>   		for (i = 0; i < 4; i++) {
> -			ret = i2c_smbus_read_byte_data(data->client,
> -						       SBRMI_OUTBNDMSG1 + i);
> +			ret = regmap_read(data->regmap,
> +					  SBRMI_OUTBNDMSG1 + i, &bytes);
>   			if (ret < 0)
>   				goto exit_unlock;
> -			msg->data_out |= ret << i * 8;
> +			msg->data_out |= bytes << i * 8;
>   		}
>   	}
>   
> @@ -127,8 +121,8 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
>   	 * BMC must write 1'b1 to SBRMI::Status[SwAlertSts] to clear the
>   	 * ALERT to initiator
>   	 */
> -	ret = i2c_smbus_write_byte_data(data->client, SBRMI_STATUS,
> -					sw_status | SW_ALERT_MASK);
> +	ret = regmap_write(data->regmap, SBRMI_STATUS,
> +			   sw_status | SW_ALERT_MASK);
>   
>   exit_unlock:
>   	mutex_unlock(&data->lock);
> diff --git a/drivers/mfd/sbrmi-i2c.c b/drivers/mfd/sbrmi-i2c.c
> index c19f0b3eb0cd..bdf15a7a2167 100644
> --- a/drivers/mfd/sbrmi-i2c.c
> +++ b/drivers/mfd/sbrmi-i2c.c
> @@ -15,6 +15,7 @@
>   #include <linux/module.h>
>   #include <linux/mutex.h>
>   #include <linux/of.h>
> +#include <linux/regmap.h>
>   
>   #define SBRMI_CTRL	0x1
>   
> @@ -22,22 +23,21 @@ static struct mfd_cell apml_sbrmi[] = {
>   	{ .name = "sbrmi-hwmon" },
>   };
>   
> -static int sbrmi_enable_alert(struct i2c_client *client)
> +static int sbrmi_enable_alert(struct sbrmi_data *data)
>   {
> -	int ctrl;
> +	int ctrl, ret;
>   
>   	/*
>   	 * Enable the SB-RMI Software alert status
>   	 * by writing 0 to bit 4 of Control register(0x1)
>   	 */
> -	ctrl = i2c_smbus_read_byte_data(client, SBRMI_CTRL);
> -	if (ctrl < 0)
> -		return ctrl;
> +	ret = regmap_read(data->regmap, SBRMI_CTRL, &ctrl);
> +	if (ret < 0)
> +		return ret;
>   
>   	if (ctrl & 0x10) {
>   		ctrl &= ~0x10;
> -		return i2c_smbus_write_byte_data(client,
> -						 SBRMI_CTRL, ctrl);
> +		return regmap_write(data->regmap, SBRMI_CTRL, ctrl);
>   	}
>   
>   	return 0;
> @@ -62,17 +62,24 @@ 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;
>   
> -	data->client = client;
>   	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(client);
> +	ret = sbrmi_enable_alert(data);
>   	if (ret < 0)
>   		return ret;
>   
> diff --git a/include/linux/mfd/amd-sb.h b/include/linux/mfd/amd-sb.h
> index 7805f31fb6ea..977b8228ffa1 100644
> --- a/include/linux/mfd/amd-sb.h
> +++ b/include/linux/mfd/amd-sb.h
> @@ -7,7 +7,7 @@
>   #define _AMD_SB_H_
>   
>   #include <linux/mutex.h>
> -#include <linux/i2c.h>
> +#include <linux/regmap.h>
>   /*
>    * SB-RMI supports soft mailbox service request to MP1 (power management
>    * firmware) through SBRMI inbound/outbound message registers.
> @@ -22,10 +22,10 @@ enum sbrmi_msg_id {
>   
>   /* Each client has this additional data */
>   struct sbrmi_data {
> -	struct i2c_client *client;
> +	struct regmap *regmap;
>   	struct mutex lock;
>   	u32 pwr_limit_max;
> -};
> +} __packed;
>   
>   struct sbrmi_mailbox_msg {
>   	u8 cmd;
diff mbox series

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0411cb29b6df..d89513e5a06b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1444,8 +1444,8 @@  config MFD_STMPE
 
 config MFD_SBRMI_I2C
         tristate "APML SBRMI support"
-        depends on I2C
         select MFD_CORE
+        select REGMAP_I2C
         help
           APML RMI core support for AMD out of band management
 	  This driver can also be built as a module. If so, the module will
diff --git a/drivers/mfd/sbrmi-core.c b/drivers/mfd/sbrmi-core.c
index d872b5107b36..5617b91787ba 100644
--- a/drivers/mfd/sbrmi-core.c
+++ b/drivers/mfd/sbrmi-core.c
@@ -7,9 +7,9 @@ 
  */
 #include <linux/delay.h>
 #include <linux/err.h>
-#include <linux/i2c.h>
 #include <linux/mfd/amd-sb.h>
 #include <linux/mutex.h>
+#include <linux/regmap.h>
 
 /* Mask for Status Register bit[1] */
 #define SW_ALERT_MASK	0x2
@@ -44,6 +44,7 @@  enum sbrmi_reg {
 int rmi_mailbox_xfer(struct sbrmi_data *data,
 		     struct sbrmi_mailbox_msg *msg)
 {
+	unsigned int bytes;
 	int i, ret, retry = 10;
 	int sw_status;
 	u8 byte;
@@ -51,14 +52,12 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	mutex_lock(&data->lock);
 
 	/* Indicate firmware a command is to be serviced */
-	ret = i2c_smbus_write_byte_data(data->client,
-					SBRMI_INBNDMSG7, START_CMD);
+	ret = regmap_write(data->regmap, SBRMI_INBNDMSG7, START_CMD);
 	if (ret < 0)
 		goto exit_unlock;
 
 	/* Write the command to SBRMI::InBndMsg_inst0 */
-	ret = i2c_smbus_write_byte_data(data->client,
-					SBRMI_INBNDMSG0, msg->cmd);
+	ret = regmap_write(data->regmap, SBRMI_INBNDMSG0, msg->cmd);
 	if (ret < 0)
 		goto exit_unlock;
 
@@ -69,8 +68,7 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 */
 	for (i = 0; i < 4; i++) {
 		byte = (msg->data_in >> i * 8) & 0xff;
-		ret = i2c_smbus_write_byte_data(data->client,
-						SBRMI_INBNDMSG1 + i, byte);
+		ret = regmap_write(data->regmap, SBRMI_INBNDMSG1 + i, byte);
 		if (ret < 0)
 			goto exit_unlock;
 	}
@@ -79,8 +77,7 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 * Write 0x01 to SBRMI::SoftwareInterrupt to notify firmware to
 	 * perform the requested read or write command
 	 */
-	ret = i2c_smbus_write_byte_data(data->client,
-					SBRMI_SW_INTERRUPT, TRIGGER_MAILBOX);
+	ret = regmap_write(data->regmap, SBRMI_SW_INTERRUPT, TRIGGER_MAILBOX);
 	if (ret < 0)
 		goto exit_unlock;
 
@@ -90,8 +87,7 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 * of the requested command
 	 */
 	do {
-		sw_status = i2c_smbus_read_byte_data(data->client,
-						     SBRMI_STATUS);
+		ret = regmap_read(data->regmap, SBRMI_STATUS, &sw_status);
 		if (sw_status < 0) {
 			ret = sw_status;
 			goto exit_unlock;
@@ -102,8 +98,6 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	} while (retry--);
 
 	if (retry < 0) {
-		dev_err(&data->client->dev,
-			"Firmware fail to indicate command completion\n");
 		ret = -EIO;
 		goto exit_unlock;
 	}
@@ -115,11 +109,11 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 */
 	if (msg->read) {
 		for (i = 0; i < 4; i++) {
-			ret = i2c_smbus_read_byte_data(data->client,
-						       SBRMI_OUTBNDMSG1 + i);
+			ret = regmap_read(data->regmap,
+					  SBRMI_OUTBNDMSG1 + i, &bytes);
 			if (ret < 0)
 				goto exit_unlock;
-			msg->data_out |= ret << i * 8;
+			msg->data_out |= bytes << i * 8;
 		}
 	}
 
@@ -127,8 +121,8 @@  int rmi_mailbox_xfer(struct sbrmi_data *data,
 	 * BMC must write 1'b1 to SBRMI::Status[SwAlertSts] to clear the
 	 * ALERT to initiator
 	 */
-	ret = i2c_smbus_write_byte_data(data->client, SBRMI_STATUS,
-					sw_status | SW_ALERT_MASK);
+	ret = regmap_write(data->regmap, SBRMI_STATUS,
+			   sw_status | SW_ALERT_MASK);
 
 exit_unlock:
 	mutex_unlock(&data->lock);
diff --git a/drivers/mfd/sbrmi-i2c.c b/drivers/mfd/sbrmi-i2c.c
index c19f0b3eb0cd..bdf15a7a2167 100644
--- a/drivers/mfd/sbrmi-i2c.c
+++ b/drivers/mfd/sbrmi-i2c.c
@@ -15,6 +15,7 @@ 
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/regmap.h>
 
 #define SBRMI_CTRL	0x1
 
@@ -22,22 +23,21 @@  static struct mfd_cell apml_sbrmi[] = {
 	{ .name = "sbrmi-hwmon" },
 };
 
-static int sbrmi_enable_alert(struct i2c_client *client)
+static int sbrmi_enable_alert(struct sbrmi_data *data)
 {
-	int ctrl;
+	int ctrl, ret;
 
 	/*
 	 * Enable the SB-RMI Software alert status
 	 * by writing 0 to bit 4 of Control register(0x1)
 	 */
-	ctrl = i2c_smbus_read_byte_data(client, SBRMI_CTRL);
-	if (ctrl < 0)
-		return ctrl;
+	ret = regmap_read(data->regmap, SBRMI_CTRL, &ctrl);
+	if (ret < 0)
+		return ret;
 
 	if (ctrl & 0x10) {
 		ctrl &= ~0x10;
-		return i2c_smbus_write_byte_data(client,
-						 SBRMI_CTRL, ctrl);
+		return regmap_write(data->regmap, SBRMI_CTRL, ctrl);
 	}
 
 	return 0;
@@ -62,17 +62,24 @@  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;
 
-	data->client = client;
 	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(client);
+	ret = sbrmi_enable_alert(data);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/linux/mfd/amd-sb.h b/include/linux/mfd/amd-sb.h
index 7805f31fb6ea..977b8228ffa1 100644
--- a/include/linux/mfd/amd-sb.h
+++ b/include/linux/mfd/amd-sb.h
@@ -7,7 +7,7 @@ 
 #define _AMD_SB_H_
 
 #include <linux/mutex.h>
-#include <linux/i2c.h>
+#include <linux/regmap.h>
 /*
  * SB-RMI supports soft mailbox service request to MP1 (power management
  * firmware) through SBRMI inbound/outbound message registers.
@@ -22,10 +22,10 @@  enum sbrmi_msg_id {
 
 /* Each client has this additional data */
 struct sbrmi_data {
-	struct i2c_client *client;
+	struct regmap *regmap;
 	struct mutex lock;
 	u32 pwr_limit_max;
-};
+} __packed;
 
 struct sbrmi_mailbox_msg {
 	u8 cmd;