[PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module

[PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module

[Akshay Gupta]

This change is in preparation for enabling SB-RMI access
over I3C along with present I2C bus.
Split the core APIs into a separate module, export those APIs
to be used by the I2C and I3C modules.
Introduce a config option in Kconfig and update Makefile accordingly.

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
 drivers/misc/amd-sbi/Kconfig     | 11 ++++++++++-
 drivers/misc/amd-sbi/Makefile    |  6 ++++--
 drivers/misc/amd-sbi/rmi-core.c  |  5 +++++
 drivers/misc/amd-sbi/rmi-hwmon.c |  4 ++++
 drivers/misc/amd-sbi/rmi-i2c.c   |  1 +
 5 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
index 4840831c84ca..9b1abeb6ab1a 100644
--- a/drivers/misc/amd-sbi/Kconfig
+++ b/drivers/misc/amd-sbi/Kconfig
@@ -1,7 +1,14 @@
 # SPDX-License-Identifier: GPL-2.0-only
+
+menu "AMD side band Interface(APML)"
+
+config AMD_SBRMI
+	bool
+
 config AMD_SBRMI_I2C
-	tristate "AMD side band RMI support"
+	tristate "AMD side band RMI support over I2C"
 	depends on I2C
+	select AMD_SBRMI
 	help
 	  Side band RMI over I2C support for AMD out of band management.
 
@@ -16,3 +23,5 @@ config AMD_SBRMI_HWMON
 	  This provides support for RMI device hardware monitoring. If enabled,
 	  a hardware monitoring device will be created for each socket in
 	  the system.
+
+endmenu
diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
index 38eaaa651fd9..6f3090fb9ff3 100644
--- a/drivers/misc/amd-sbi/Makefile
+++ b/drivers/misc/amd-sbi/Makefile
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
-sbrmi-i2c-objs  		+= rmi-i2c.o rmi-core.o
-sbrmi-i2c-$(CONFIG_AMD_SBRMI_HWMON)	+= rmi-hwmon.o
+obj-$(CONFIG_AMD_SBRMI)		+= sbrmi_core.o
+sbrmi_core-y			:= rmi-core.o
+obj-$(CONFIG_AMD_SBRMI_HWMON)	+= rmi-hwmon.o
+sbrmi-i2c-y			:= rmi-i2c.o
 obj-$(CONFIG_AMD_SBRMI_I2C)	+= sbrmi-i2c.o
diff --git a/drivers/misc/amd-sbi/rmi-core.c b/drivers/misc/amd-sbi/rmi-core.c
index 3dec2fc00124..6a7d0bb33bf8 100644
--- a/drivers/misc/amd-sbi/rmi-core.c
+++ b/drivers/misc/amd-sbi/rmi-core.c
@@ -352,6 +352,7 @@ int rmi_mailbox_xfer(struct sbrmi_data *data,
 	mutex_unlock(&data->lock);
 	return ret;
 }
+EXPORT_SYMBOL_NS_GPL(rmi_mailbox_xfer, "AMD_SBRMI");
 
 static int apml_rmi_reg_xfer(struct sbrmi_data *data,
 			     struct apml_reg_xfer_msg __user *arg)
@@ -482,3 +483,7 @@ int create_misc_rmi_device(struct sbrmi_data *data,
 
 	return misc_register(&data->sbrmi_misc_dev);
 }
+EXPORT_SYMBOL_NS_GPL(create_misc_rmi_device, "AMD_SBRMI");
+
+MODULE_DESCRIPTION("AMD SB-RMI common driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/misc/amd-sbi/rmi-hwmon.c b/drivers/misc/amd-sbi/rmi-hwmon.c
index f4f015605daa..e5d234549b5c 100644
--- a/drivers/misc/amd-sbi/rmi-hwmon.c
+++ b/drivers/misc/amd-sbi/rmi-hwmon.c
@@ -6,6 +6,7 @@
  */
 #include <linux/err.h>
 #include <linux/hwmon.h>
+#include <linux/module.h>
 #include <uapi/misc/amd-apml.h>
 #include "rmi-core.h"
 
@@ -118,3 +119,6 @@ int create_hwmon_sensor_device(struct device *dev, struct sbrmi_data *data)
 							 &sbrmi_chip_info, NULL);
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
+EXPORT_SYMBOL(create_hwmon_sensor_device);
+
+MODULE_IMPORT_NS("AMD_SBRMI");
diff --git a/drivers/misc/amd-sbi/rmi-i2c.c b/drivers/misc/amd-sbi/rmi-i2c.c
index f891f5af4bc6..e90ab0a5f8eb 100644
--- a/drivers/misc/amd-sbi/rmi-i2c.c
+++ b/drivers/misc/amd-sbi/rmi-i2c.c
@@ -127,6 +127,7 @@ static struct i2c_driver sbrmi_driver = {
 
 module_i2c_driver(sbrmi_driver);
 
+MODULE_IMPORT_NS("AMD_SBRMI");
 MODULE_AUTHOR("Akshay Gupta <akshay.gupta@amd.com>");
 MODULE_AUTHOR("Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>");
 MODULE_DESCRIPTION("Hwmon driver for AMD SB-RMI emulated sensor");
-- 
2.25.1

[PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C

[Akshay Gupta]

AMD EPYC platforms with zen5 and later support APML(SB-RMI)
connection to the BMC over I3C bus for improved efficiency.
Add the same functionality supported by rmi-i2c.c over I3C bus.

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
 drivers/misc/amd-sbi/Kconfig   |  15 +++-
 drivers/misc/amd-sbi/Makefile  |   2 +
 drivers/misc/amd-sbi/rmi-i3c.c | 133 +++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+), 2 deletions(-)
 create mode 100644 drivers/misc/amd-sbi/rmi-i3c.c

diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
index 9b1abeb6ab1a..838cf98805d9 100644
--- a/drivers/misc/amd-sbi/Kconfig
+++ b/drivers/misc/amd-sbi/Kconfig
@@ -15,10 +15,21 @@ config AMD_SBRMI_I2C
 	  This driver can also be built as a module. If so, the module will
 	  be called sbrmi-i2c.
 
+config AMD_SBRMI_I3C
+	tristate "AMD side band RMI support over I3C"
+	depends on I3C
+	select AMD_SBRMI
+	select REGMAP_I3C
+	help
+	  Side band RMI over I3C support for AMD out of band management.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called sbrmi-i3c.
+
 config AMD_SBRMI_HWMON
 	bool "SBRMI hardware monitoring"
-	depends on AMD_SBRMI_I2C && HWMON
-	depends on !(AMD_SBRMI_I2C=y && HWMON=m)
+	depends on (AMD_SBRMI_I2C || AMD_SBRMI_I3C) && HWMON
+	depends on !(AMD_SBRMI_I2C=y && HWMON=m) || !(AMD_SBRMI_I3C=y && HWMON=m)
 	help
 	  This provides support for RMI device hardware monitoring. If enabled,
 	  a hardware monitoring device will be created for each socket in
diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
index 6f3090fb9ff3..e43d4058a0f0 100644
--- a/drivers/misc/amd-sbi/Makefile
+++ b/drivers/misc/amd-sbi/Makefile
@@ -4,3 +4,5 @@ sbrmi_core-y			:= rmi-core.o
 obj-$(CONFIG_AMD_SBRMI_HWMON)	+= rmi-hwmon.o
 sbrmi-i2c-y			:= rmi-i2c.o
 obj-$(CONFIG_AMD_SBRMI_I2C)	+= sbrmi-i2c.o
+sbrmi-i3c-y  			:= rmi-i3c.o
+obj-$(CONFIG_AMD_SBRMI_I3C)	+= sbrmi-i3c.o
diff --git a/drivers/misc/amd-sbi/rmi-i3c.c b/drivers/misc/amd-sbi/rmi-i3c.c
new file mode 100644
index 000000000000..b960743afad1
--- /dev/null
+++ b/drivers/misc/amd-sbi/rmi-i3c.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * rmi-i3c.c - Side band RMI over I3C support for AMD out
+ *             of band management
+ *
+ * Copyright (C) 2025 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include "rmi-core.h"
+
+static int sbrmi_enable_alert(struct sbrmi_data *data)
+{
+	int ctrl, ret;
+
+	/*
+	 * Enable the SB-RMI Software alert status
+	 * by writing 0 to bit 4 of Control register(0x1)
+	 */
+	ret = regmap_read(data->regmap, SBRMI_CTRL, &ctrl);
+	if (ret < 0)
+		return ret;
+
+	if (ctrl & 0x10) {
+		ctrl &= ~0x10;
+		return regmap_write(data->regmap, SBRMI_CTRL, ctrl);
+	}
+
+	return 0;
+}
+
+static int sbrmi_get_max_pwr_limit(struct sbrmi_data *data)
+{
+	struct apml_mbox_msg msg = { 0 };
+	int ret;
+
+	msg.cmd = SBRMI_READ_PKG_MAX_PWR_LIMIT;
+	ret = rmi_mailbox_xfer(data, &msg);
+	if (ret < 0)
+		return ret;
+	data->pwr_limit_max = msg.mb_in_out;
+
+	return ret;
+}
+
+static int sbrmi_i3c_probe(struct i3c_device *i3cdev)
+{
+	struct device *dev = &i3cdev->dev;
+	struct sbrmi_data *data;
+	struct regmap_config sbrmi_i3c_regmap_config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+	};
+	int ret;
+
+	data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->lock);
+
+	data->regmap = devm_regmap_init_i3c(i3cdev, &sbrmi_i3c_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)
+		return ret;
+
+	/* Cache maximum power limit */
+	ret = sbrmi_get_max_pwr_limit(data);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * AMD APML I3C devices support static address
+	 */
+	if (i3cdev->desc->info.static_addr)
+		data->dev_static_addr = i3cdev->desc->info.static_addr;
+	else
+		data->dev_static_addr = i3cdev->desc->info.dyn_addr;
+
+	dev_set_drvdata(dev, data);
+
+	ret = create_hwmon_sensor_device(dev, data);
+	if (ret < 0)
+		return ret;
+	return create_misc_rmi_device(data, dev);
+}
+
+static void sbrmi_i3c_remove(struct i3c_device *i3cdev)
+{
+	struct sbrmi_data *data = dev_get_drvdata(&i3cdev->dev);
+
+	misc_deregister(&data->sbrmi_misc_dev);
+	/* Assign fops and parent of misc dev to NULL */
+	data->sbrmi_misc_dev.fops = NULL;
+	data->sbrmi_misc_dev.parent = NULL;
+	dev_info(&i3cdev->dev, "Removed sbrmi-i3c driver\n");
+}
+
+static const struct i3c_device_id sbrmi_i3c_id[] = {
+	/* PID for AMD SBRMI device */
+	I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x2, NULL),
+	{}
+};
+MODULE_DEVICE_TABLE(i3c, sbrmi_i3c_id);
+
+static struct i3c_driver sbrmi_i3c_driver = {
+	.driver = {
+		.name = "sbrmi-i3c",
+	},
+	.probe = sbrmi_i3c_probe,
+	.remove = sbrmi_i3c_remove,
+	.id_table = sbrmi_i3c_id,
+};
+
+module_i3c_driver(sbrmi_i3c_driver);
+
+MODULE_IMPORT_NS("AMD_SBRMI");
+MODULE_AUTHOR("Akshay Gupta <akshay.gupta@amd.com>");
+MODULE_AUTHOR("Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>");
+MODULE_DESCRIPTION("AMD SB-RMI driver over I3C");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc

[Akshay Gupta]

- AMD SB-RMI patches are moved from drivers/hwmon to
  drivers/misc to support additional functionality.
  Move the related bindings documentation files to misc.

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
 Documentation/devicetree/bindings/{hwmon => misc}/amd,sbrmi.yaml | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{hwmon => misc}/amd,sbrmi.yaml (100%)

diff --git a/Documentation/devicetree/bindings/hwmon/amd,sbrmi.yaml b/Documentation/devicetree/bindings/misc/amd,sbrmi.yaml
similarity index 100%
rename from Documentation/devicetree/bindings/hwmon/amd,sbrmi.yaml
rename to Documentation/devicetree/bindings/misc/amd,sbrmi.yaml
-- 
2.25.1

[PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path

[Akshay Gupta]

- Modify the "$id" path as file moves from hwmon to misc

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
 Documentation/devicetree/bindings/misc/amd,sbrmi.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/misc/amd,sbrmi.yaml b/Documentation/devicetree/bindings/misc/amd,sbrmi.yaml
index 353d81d89bf5..5fd221200e37 100644
--- a/Documentation/devicetree/bindings/misc/amd,sbrmi.yaml
+++ b/Documentation/devicetree/bindings/misc/amd,sbrmi.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/hwmon/amd,sbrmi.yaml#
+$id: http://devicetree.org/schemas/misc/amd,sbrmi.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: >
-- 
2.25.1

[PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C

[Akshay Gupta]

- Document the dt-binding for AMD SB-RMI

Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
---
 .../bindings/misc/amd,sbrmi-i3c.yaml          | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml

diff --git a/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
new file mode 100644
index 000000000000..1d19571c2095
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/amd,sbrmi-i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: >
+  Sideband Remote Management Interface (SB-RMI) compliant
+  AMD SoC.
+
+maintainers:
+  - Akshay Gupta <Akshay.Gupta@amd.com>
+
+description: |
+  SB Remote Management Interface (SB-RMI) is an SMBus compatible
+  interface that reports AMD SoC's Power (normalized Power) using,
+  Mailbox Service Request over I3C interface to BMC.
+  The power attributes in hwmon reports power in microwatts.
+
+properties:
+  reg:
+    - description: |
+        Encodes the static I2C address.
+      Socket 0: 0x3c
+      Socket 1: 0x38
+    - description: |
+        First half of the Provisioned ID (following the PID
+        definition provided by the I3C specification).
+        Contains the manufacturer ID left-shifted by 1 (0x224).
+    - description: |
+        Second half of the Provisioned ID (following the PID
+        definition provided by the I3C specification).
+        Contains the ORing of the part ID left-shifted by 16,
+        the instance ID left-shifted by 12 and extra information (0x00000002).
+
+  assigned-address:
+    - description: |
+	Dynamic address to be assigned to this device. As static address is
+	present, this address is assigned using SETDASA.
+
+required:
+  - reg
+  - assigned-address
+
+additionalProperties: false
+
+examples:
+  - |
+    i3c {
+        /* I3C device with a static I2C address and assigned address. */
+	sbrmi_p0_1: sbrmi@3c,22400000002 {
+		reg = <0x3c 0x224 0x00000002>;
+		assigned-address = <0x3c>;
+	};
+    };
+...
-- 
2.25.1

Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C

[Thomas Weißschuh]

On 2025-07-28 06:10:30+0000, Akshay Gupta wrote:
(...)

> diff --git a/drivers/misc/amd-sbi/rmi-i3c.c b/drivers/misc/amd-sbi/rmi-i3c.c
> new file mode 100644
> index 000000000000..b960743afad1
> --- /dev/null
> +++ b/drivers/misc/amd-sbi/rmi-i3c.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * rmi-i3c.c - Side band RMI over I3C support for AMD out
> + *             of band management
> + *
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/delay.h>

Unnecessary include.

> +#include <linux/err.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>

Ditto.

> +#include <linux/regmap.h>
> +#include "rmi-core.h"
> +
> +static int sbrmi_enable_alert(struct sbrmi_data *data)
> +{
> +	int ctrl, ret;
> +
> +	/*
> +	 * Enable the SB-RMI Software alert status
> +	 * by writing 0 to bit 4 of Control register(0x1)
> +	 */
> +	ret = regmap_read(data->regmap, SBRMI_CTRL, &ctrl);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ctrl & 0x10) {

Magic value? Use a define.

> +		ctrl &= ~0x10;
> +		return regmap_write(data->regmap, SBRMI_CTRL, ctrl);
> +	}
> +
> +	return 0;
> +}
> +
> +static int sbrmi_get_max_pwr_limit(struct sbrmi_data *data)
> +{
> +	struct apml_mbox_msg msg = { 0 };
> +	int ret;
> +
> +	msg.cmd = SBRMI_READ_PKG_MAX_PWR_LIMIT;
> +	ret = rmi_mailbox_xfer(data, &msg);
> +	if (ret < 0)
> +		return ret;
> +	data->pwr_limit_max = msg.mb_in_out;
> +
> +	return ret;
> +}
> +
> +static int sbrmi_i3c_probe(struct i3c_device *i3cdev)
> +{
> +	struct device *dev = &i3cdev->dev;

i3cdev_to_dev();

> +	struct sbrmi_data *data;
> +	struct regmap_config sbrmi_i3c_regmap_config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +	};
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	mutex_init(&data->lock);

devm_mutex_init().

> +
> +	data->regmap = devm_regmap_init_i3c(i3cdev, &sbrmi_i3c_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)
> +		return ret;
> +
> +	/* Cache maximum power limit */
> +	ret = sbrmi_get_max_pwr_limit(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * AMD APML I3C devices support static address
> +	 */
> +	if (i3cdev->desc->info.static_addr)
> +		data->dev_static_addr = i3cdev->desc->info.static_addr;
> +	else
> +		data->dev_static_addr = i3cdev->desc->info.dyn_addr;
> +
> +	dev_set_drvdata(dev, data);

If you get rid of _remove(), then this can go away.

> +
> +	ret = create_hwmon_sensor_device(dev, data);

That's a very generic name to have exported.

> +	if (ret < 0)
> +		return ret;
> +	return create_misc_rmi_device(data, dev);
> +}
> +
> +static void sbrmi_i3c_remove(struct i3c_device *i3cdev)
> +{
> +	struct sbrmi_data *data = dev_get_drvdata(&i3cdev->dev);
> +
> +	misc_deregister(&data->sbrmi_misc_dev);

create_misc_rmi_device() could use devm_add_action_or_reset() for the
misc deregister, simplifying the drivers code.

> +	/* Assign fops and parent of misc dev to NULL */
> +	data->sbrmi_misc_dev.fops = NULL;
> +	data->sbrmi_misc_dev.parent = NULL;

Why are these two needed? The data is freed anyways right after.

> +	dev_info(&i3cdev->dev, "Removed sbrmi-i3c driver\n");

Unnecessary.

> +}
> +
> +static const struct i3c_device_id sbrmi_i3c_id[] = {
> +	/* PID for AMD SBRMI device */
> +	I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x2, NULL),
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i3c, sbrmi_i3c_id);
> +
> +static struct i3c_driver sbrmi_i3c_driver = {
> +	.driver = {
> +		.name = "sbrmi-i3c",
> +	},
> +	.probe = sbrmi_i3c_probe,
> +	.remove = sbrmi_i3c_remove,
> +	.id_table = sbrmi_i3c_id,
> +};
> +
> +module_i3c_driver(sbrmi_i3c_driver);

You could have the i2c and i3c drivers in the same module using
module_i3c_i2c_driver().

> +
> +MODULE_IMPORT_NS("AMD_SBRMI");
> +MODULE_AUTHOR("Akshay Gupta <akshay.gupta@amd.com>");
> +MODULE_AUTHOR("Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>");
> +MODULE_DESCRIPTION("AMD SB-RMI driver over I3C");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc

[Krzysztof Kozlowski]

On 28/07/2025 08:10, Akshay Gupta wrote:
> - AMD SB-RMI patches are moved from drivers/hwmon to
>   drivers/misc to support additional functionality.
>   Move the related bindings documentation files to misc.
> 
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>

Really? What was reviewed here exactly? Poor style of commit msg or that
rename actually does rename?

> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
> ---
>  Documentation/devicetree/bindings/{hwmon => misc}/amd,sbrmi.yaml | 0

We don't put bindings into MISC.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>




Best regards,
Krzysztof

Re: [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path

[Krzysztof Kozlowski]

On 28/07/2025 08:10, Akshay Gupta wrote:
> - Modify the "$id" path as file moves from hwmon to misc
> 
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
> ---

So you broke everything in previous patch, introduced bugs, but code was
reviewed?

NAK

Best regards,
Krzysztof

Re: [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C

[Krzysztof Kozlowski]

On 28/07/2025 08:10, Akshay Gupta wrote:
> - Document the dt-binding for AMD SB-RMI

Why every commit is a one-item list?

> 
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>

I don't believe these reviews. Code was obviously buggy, but still reviewed.

> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
> ---
>  .../bindings/misc/amd,sbrmi-i3c.yaml          | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml

Your previous patches make no sense with this...

> 
> diff --git a/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
> new file mode 100644
> index 000000000000..1d19571c2095
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/amd,sbrmi-i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: >

From where did you take such syntax of title?

> +  Sideband Remote Management Interface (SB-RMI) compliant
> +  AMD SoC.
> +
> +maintainers:
> +  - Akshay Gupta <Akshay.Gupta@amd.com>
> +
> +description: |
> +  SB Remote Management Interface (SB-RMI) is an SMBus compatible
> +  interface that reports AMD SoC's Power (normalized Power) using,
> +  Mailbox Service Request over I3C interface to BMC.
> +  The power attributes in hwmon reports power in microwatts.
> +
> +properties:
> +  reg:
> +    - description: |
> +        Encodes the static I2C address.
> +      Socket 0: 0x3c
> +      Socket 1: 0x38
> +    - description: |
> +        First half of the Provisioned ID (following the PID
> +        definition provided by the I3C specification).
> +        Contains the manufacturer ID left-shifted by 1 (0x224).
> +    - description: |
> +        Second half of the Provisioned ID (following the PID
> +        definition provided by the I3C specification).
> +        Contains the ORing of the part ID left-shifted by 16,
> +        the instance ID left-shifted by 12 and extra information (0x00000002).
> +

And this entire patch is just noop. Makes no difference, makes no impact.

Drop it and write proper bindings with proper description of problem you
are solving.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C

[Krzysztof Kozlowski]

On 28/07/2025 08:10, Akshay Gupta wrote:
> AMD EPYC platforms with zen5 and later support APML(SB-RMI)
> connection to the BMC over I3C bus for improved efficiency.
> Add the same functionality supported by rmi-i2c.c over I3C bus.
> 
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
> ---
>  drivers/misc/amd-sbi/Kconfig   |  15 +++-
>  drivers/misc/amd-sbi/Makefile  |   2 +
>  drivers/misc/amd-sbi/rmi-i3c.c | 133 +++++++++++++++++++++++++++++++++
>  3 files changed, 148 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/misc/amd-sbi/rmi-i3c.c
> 
> diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
> index 9b1abeb6ab1a..838cf98805d9 100644
> --- a/drivers/misc/amd-sbi/Kconfig
> +++ b/drivers/misc/amd-sbi/Kconfig
> @@ -15,10 +15,21 @@ config AMD_SBRMI_I2C
>  	  This driver can also be built as a module. If so, the module will
>  	  be called sbrmi-i2c.
>  
> +config AMD_SBRMI_I3C
> +	tristate "AMD side band RMI support over I3C"
> +	depends on I3C
> +	select AMD_SBRMI
> +	select REGMAP_I3C
> +	help
> +	  Side band RMI over I3C support for AMD out of band management.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called sbrmi-i3c.
> +
>  config AMD_SBRMI_HWMON
>  	bool "SBRMI hardware monitoring"
> -	depends on AMD_SBRMI_I2C && HWMON
> -	depends on !(AMD_SBRMI_I2C=y && HWMON=m)
> +	depends on (AMD_SBRMI_I2C || AMD_SBRMI_I3C) && HWMON
> +	depends on !(AMD_SBRMI_I2C=y && HWMON=m) || !(AMD_SBRMI_I3C=y && HWMON=m)
>  	help
>  	  This provides support for RMI device hardware monitoring. If enabled,
>  	  a hardware monitoring device will be created for each socket in
> diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
> index 6f3090fb9ff3..e43d4058a0f0 100644
> --- a/drivers/misc/amd-sbi/Makefile
> +++ b/drivers/misc/amd-sbi/Makefile
> @@ -4,3 +4,5 @@ sbrmi_core-y			:= rmi-core.o
>  obj-$(CONFIG_AMD_SBRMI_HWMON)	+= rmi-hwmon.o
>  sbrmi-i2c-y			:= rmi-i2c.o
>  obj-$(CONFIG_AMD_SBRMI_I2C)	+= sbrmi-i2c.o
> +sbrmi-i3c-y  			:= rmi-i3c.o
> +obj-$(CONFIG_AMD_SBRMI_I3C)	+= sbrmi-i3c.o
> diff --git a/drivers/misc/amd-sbi/rmi-i3c.c b/drivers/misc/amd-sbi/rmi-i3c.c
> new file mode 100644
> index 000000000000..b960743afad1
> --- /dev/null
> +++ b/drivers/misc/amd-sbi/rmi-i3c.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * rmi-i3c.c - Side band RMI over I3C support for AMD out
> + *             of band management
> + *
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>

Where do you use it?

> +#include <linux/regmap.h>
> +#include "rmi-core.h"


...

> +static int sbrmi_i3c_probe(struct i3c_device *i3cdev)
> +{
> +	struct device *dev = &i3cdev->dev;
> +	struct sbrmi_data *data;
> +	struct regmap_config sbrmi_i3c_regmap_config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +	};
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);

sizeof(*). Use recent coding style, not some old, downstream code.

> +	if (!data)
> +		return -ENOMEM;
> +
> +	mutex_init(&data->lock);
> +
> +	data->regmap = devm_regmap_init_i3c(i3cdev, &sbrmi_i3c_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)
> +		return ret;
> +
> +	/* Cache maximum power limit */
> +	ret = sbrmi_get_max_pwr_limit(data);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * AMD APML I3C devices support static address
> +	 */
> +	if (i3cdev->desc->info.static_addr)
> +		data->dev_static_addr = i3cdev->desc->info.static_addr;
> +	else
> +		data->dev_static_addr = i3cdev->desc->info.dyn_addr;
> +
> +	dev_set_drvdata(dev, data);
> +
> +	ret = create_hwmon_sensor_device(dev, data);
> +	if (ret < 0)
> +		return ret;
> +	return create_misc_rmi_device(data, dev);
> +}
> +
> +static void sbrmi_i3c_remove(struct i3c_device *i3cdev)
> +{
> +	struct sbrmi_data *data = dev_get_drvdata(&i3cdev->dev);
> +
> +	misc_deregister(&data->sbrmi_misc_dev);
> +	/* Assign fops and parent of misc dev to NULL */
> +	data->sbrmi_misc_dev.fops = NULL;
> +	data->sbrmi_misc_dev.parent = NULL;
> +	dev_info(&i3cdev->dev, "Removed sbrmi-i3c driver\n");

No, drop. This is useless.



Best regards,
Krzysztof

Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C

[Gupta, Akshay]

On 7/28/2025 12:07 PM, Thomas Weißschuh wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 2025-07-28 06:10:30+0000, Akshay Gupta wrote:
> (...)
>
>> diff --git a/drivers/misc/amd-sbi/rmi-i3c.c b/drivers/misc/amd-sbi/rmi-i3c.c
>> new file mode 100644
>> index 000000000000..b960743afad1
>> --- /dev/null
>> +++ b/drivers/misc/amd-sbi/rmi-i3c.c
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * rmi-i3c.c - Side band RMI over I3C support for AMD out
>> + *             of band management
>> + *
>> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include <linux/delay.h>
> Unnecessary include.
Ack, will remove. Thank you.
>> +#include <linux/err.h>
>> +#include <linux/i3c/device.h>
>> +#include <linux/i3c/master.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
> Ditto.
Ack, will remove.
>
>> +#include <linux/regmap.h>
>> +#include "rmi-core.h"
>> +
>> +static int sbrmi_enable_alert(struct sbrmi_data *data)
>> +{
>> +     int ctrl, ret;
>> +
>> +     /*
>> +      * Enable the SB-RMI Software alert status
>> +      * by writing 0 to bit 4 of Control register(0x1)
>> +      */
>> +     ret = regmap_read(data->regmap, SBRMI_CTRL, &ctrl);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     if (ctrl & 0x10) {
> Magic value? Use a define.
Ack, will create #define
>> +             ctrl &= ~0x10;
>> +             return regmap_write(data->regmap, SBRMI_CTRL, ctrl);
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int sbrmi_get_max_pwr_limit(struct sbrmi_data *data)
>> +{
>> +     struct apml_mbox_msg msg = { 0 };
>> +     int ret;
>> +
>> +     msg.cmd = SBRMI_READ_PKG_MAX_PWR_LIMIT;
>> +     ret = rmi_mailbox_xfer(data, &msg);
>> +     if (ret < 0)
>> +             return ret;
>> +     data->pwr_limit_max = msg.mb_in_out;
>> +
>> +     return ret;
>> +}
>> +
>> +static int sbrmi_i3c_probe(struct i3c_device *i3cdev)
>> +{
>> +     struct device *dev = &i3cdev->dev;
> i3cdev_to_dev();
Ack, will update.
>
>> +     struct sbrmi_data *data;
>> +     struct regmap_config sbrmi_i3c_regmap_config = {
>> +             .reg_bits = 8,
>> +             .val_bits = 8,
>> +     };
>> +     int ret;
>> +
>> +     data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);
>> +     if (!data)
>> +             return -ENOMEM;
>> +
>> +     mutex_init(&data->lock);
> devm_mutex_init().
Will address with new patch to use guard(mutex)
>> +
>> +     data->regmap = devm_regmap_init_i3c(i3cdev, &sbrmi_i3c_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)
>> +             return ret;
>> +
>> +     /* Cache maximum power limit */
>> +     ret = sbrmi_get_max_pwr_limit(data);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     /*
>> +      * AMD APML I3C devices support static address
>> +      */
>> +     if (i3cdev->desc->info.static_addr)
>> +             data->dev_static_addr = i3cdev->desc->info.static_addr;
>> +     else
>> +             data->dev_static_addr = i3cdev->desc->info.dyn_addr;
>> +
>> +     dev_set_drvdata(dev, data);
> If you get rid of _remove(), then this can go away.
>
>> +
>> +     ret = create_hwmon_sensor_device(dev, data);
> That's a very generic name to have exported.
Will create separate patch to address this as this is used by i2c driver.
>
>> +     if (ret < 0)
>> +             return ret;
>> +     return create_misc_rmi_device(data, dev);
>> +}
>> +
>> +static void sbrmi_i3c_remove(struct i3c_device *i3cdev)
>> +{
>> +     struct sbrmi_data *data = dev_get_drvdata(&i3cdev->dev);
>> +
>> +     misc_deregister(&data->sbrmi_misc_dev);
> create_misc_rmi_device() could use devm_add_action_or_reset() for the
> misc deregister, simplifying the drivers code.
Ack , will update.
>
>> +     /* Assign fops and parent of misc dev to NULL */
>> +     data->sbrmi_misc_dev.fops = NULL;
>> +     data->sbrmi_misc_dev.parent = NULL;
> Why are these two needed? The data is freed anyways right after.
Ack, will update.
>
>> +     dev_info(&i3cdev->dev, "Removed sbrmi-i3c driver\n");
> Unnecessary.
Ack.
>
>> +}
>> +
>> +static const struct i3c_device_id sbrmi_i3c_id[] = {
>> +     /* PID for AMD SBRMI device */
>> +     I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x2, NULL),
>> +     {}
>> +};
>> +MODULE_DEVICE_TABLE(i3c, sbrmi_i3c_id);
>> +
>> +static struct i3c_driver sbrmi_i3c_driver = {
>> +     .driver = {
>> +             .name = "sbrmi-i3c",
>> +     },
>> +     .probe = sbrmi_i3c_probe,
>> +     .remove = sbrmi_i3c_remove,
>> +     .id_table = sbrmi_i3c_id,
>> +};
>> +
>> +module_i3c_driver(sbrmi_i3c_driver);
> You could have the i2c and i3c drivers in the same module using
> module_i3c_i2c_driver().

Idea was to keep I2C and I3C separate as the drivers will become complex 
with

new platform specific changes related to register address size.

I will update the existing i2c driver for now.

>> +
>> +MODULE_IMPORT_NS("AMD_SBRMI");
>> +MODULE_AUTHOR("Akshay Gupta <akshay.gupta@amd.com>");
>> +MODULE_AUTHOR("Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>");
>> +MODULE_DESCRIPTION("AMD SB-RMI driver over I3C");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.25.1
>>

Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C

[Gupta, Akshay]

On 7/29/2025 12:03 PM, Krzysztof Kozlowski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 28/07/2025 08:10, Akshay Gupta wrote:
>> AMD EPYC platforms with zen5 and later support APML(SB-RMI)
>> connection to the BMC over I3C bus for improved efficiency.
>> Add the same functionality supported by rmi-i2c.c over I3C bus.
>>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
>> ---
>>   drivers/misc/amd-sbi/Kconfig   |  15 +++-
>>   drivers/misc/amd-sbi/Makefile  |   2 +
>>   drivers/misc/amd-sbi/rmi-i3c.c | 133 +++++++++++++++++++++++++++++++++
>>   3 files changed, 148 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/misc/amd-sbi/rmi-i3c.c
>>
>> diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
>> index 9b1abeb6ab1a..838cf98805d9 100644
>> --- a/drivers/misc/amd-sbi/Kconfig
>> +++ b/drivers/misc/amd-sbi/Kconfig
>> @@ -15,10 +15,21 @@ config AMD_SBRMI_I2C
>>          This driver can also be built as a module. If so, the module will
>>          be called sbrmi-i2c.
>>
>> +config AMD_SBRMI_I3C
>> +     tristate "AMD side band RMI support over I3C"
>> +     depends on I3C
>> +     select AMD_SBRMI
>> +     select REGMAP_I3C
>> +     help
>> +       Side band RMI over I3C support for AMD out of band management.
>> +
>> +       This driver can also be built as a module. If so, the module will
>> +       be called sbrmi-i3c.
>> +
>>   config AMD_SBRMI_HWMON
>>        bool "SBRMI hardware monitoring"
>> -     depends on AMD_SBRMI_I2C && HWMON
>> -     depends on !(AMD_SBRMI_I2C=y && HWMON=m)
>> +     depends on (AMD_SBRMI_I2C || AMD_SBRMI_I3C) && HWMON
>> +     depends on !(AMD_SBRMI_I2C=y && HWMON=m) || !(AMD_SBRMI_I3C=y && HWMON=m)
>>        help
>>          This provides support for RMI device hardware monitoring. If enabled,
>>          a hardware monitoring device will be created for each socket in
>> diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
>> index 6f3090fb9ff3..e43d4058a0f0 100644
>> --- a/drivers/misc/amd-sbi/Makefile
>> +++ b/drivers/misc/amd-sbi/Makefile
>> @@ -4,3 +4,5 @@ sbrmi_core-y                  := rmi-core.o
>>   obj-$(CONFIG_AMD_SBRMI_HWMON)        += rmi-hwmon.o
>>   sbrmi-i2c-y                  := rmi-i2c.o
>>   obj-$(CONFIG_AMD_SBRMI_I2C)  += sbrmi-i2c.o
>> +sbrmi-i3c-y                          := rmi-i3c.o
>> +obj-$(CONFIG_AMD_SBRMI_I3C)  += sbrmi-i3c.o
>> diff --git a/drivers/misc/amd-sbi/rmi-i3c.c b/drivers/misc/amd-sbi/rmi-i3c.c
>> new file mode 100644
>> index 000000000000..b960743afad1
>> --- /dev/null
>> +++ b/drivers/misc/amd-sbi/rmi-i3c.c
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * rmi-i3c.c - Side band RMI over I3C support for AMD out
>> + *             of band management
>> + *
>> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/i3c/device.h>
>> +#include <linux/i3c/master.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
> Where do you use it?
Ack, will remove.
>
>> +#include <linux/regmap.h>
>> +#include "rmi-core.h"
>
> ...
>
>> +static int sbrmi_i3c_probe(struct i3c_device *i3cdev)
>> +{
>> +     struct device *dev = &i3cdev->dev;
>> +     struct sbrmi_data *data;
>> +     struct regmap_config sbrmi_i3c_regmap_config = {
>> +             .reg_bits = 8,
>> +             .val_bits = 8,
>> +     };
>> +     int ret;
>> +
>> +     data = devm_kzalloc(dev, sizeof(struct sbrmi_data), GFP_KERNEL);
> sizeof(*). Use recent coding style, not some old, downstream code.
Ack, will update.
>
>> +     if (!data)
>> +             return -ENOMEM;
>> +
>> +     mutex_init(&data->lock);
>> +
>> +     data->regmap = devm_regmap_init_i3c(i3cdev, &sbrmi_i3c_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)
>> +             return ret;
>> +
>> +     /* Cache maximum power limit */
>> +     ret = sbrmi_get_max_pwr_limit(data);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     /*
>> +      * AMD APML I3C devices support static address
>> +      */
>> +     if (i3cdev->desc->info.static_addr)
>> +             data->dev_static_addr = i3cdev->desc->info.static_addr;
>> +     else
>> +             data->dev_static_addr = i3cdev->desc->info.dyn_addr;
>> +
>> +     dev_set_drvdata(dev, data);
>> +
>> +     ret = create_hwmon_sensor_device(dev, data);
>> +     if (ret < 0)
>> +             return ret;
>> +     return create_misc_rmi_device(data, dev);
>> +}
>> +
>> +static void sbrmi_i3c_remove(struct i3c_device *i3cdev)
>> +{
>> +     struct sbrmi_data *data = dev_get_drvdata(&i3cdev->dev);
>> +
>> +     misc_deregister(&data->sbrmi_misc_dev);
>> +     /* Assign fops and parent of misc dev to NULL */
>> +     data->sbrmi_misc_dev.fops = NULL;
>> +     data->sbrmi_misc_dev.parent = NULL;
>> +     dev_info(&i3cdev->dev, "Removed sbrmi-i3c driver\n");
> No, drop. This is useless.
Ack, will update.
>
>
>
> Best regards,
> Krzysztof

Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc

[Gupta, Akshay]

On 7/29/2025 11:59 AM, Krzysztof Kozlowski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 28/07/2025 08:10, Akshay Gupta wrote:
>> - AMD SB-RMI patches are moved from drivers/hwmon to
>>    drivers/misc to support additional functionality.
>>    Move the related bindings documentation files to misc.
>>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> Really? What was reviewed here exactly? Poor style of commit msg or that
> rename actually does rename?
>
>> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
>> ---
>>   Documentation/devicetree/bindings/{hwmon => misc}/amd,sbrmi.yaml | 0
> We don't put bindings into MISC.

We moved the SB-RMI driver from hwmon to misc, and hence moved the 
binding documentation.

Referred the newly added yaml file, ti,fpc202.yaml through commit in the 
misc bindings.

If Documentation/devicetree/bindings/misc is not right path, can you 
please suggest the correct path for bindings for drivers in misc?

thank you.

>
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.
>
> Please kindly resend and include all necessary To/Cc entries.
> </form letter>
>
>
>
>
> Best regards,
> Krzysztof
Sorry, will take care of this.

Re: [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C

[Gupta, Akshay]

On 7/29/2025 12:01 PM, Krzysztof Kozlowski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On 28/07/2025 08:10, Akshay Gupta wrote:
>> - Document the dt-binding for AMD SB-RMI
> Why every commit is a one-item list?
Ack, will add more information related to changes
>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> I don't believe these reviews. Code was obviously buggy, but still reviewed.
>
>> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
>> ---
>>   .../bindings/misc/amd,sbrmi-i3c.yaml          | 56 +++++++++++++++++++
>>   1 file changed, 56 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
> Your previous patches make no sense with this...
This will push with I3C driver changes
>
>> diff --git a/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
>> new file mode 100644
>> index 000000000000..1d19571c2095
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/amd,sbrmi-i3c.yaml
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/misc/amd,sbrmi-i3c.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: >
>  From where did you take such syntax of title?
Ack, will update.
>
>> +  Sideband Remote Management Interface (SB-RMI) compliant
>> +  AMD SoC.
>> +
>> +maintainers:
>> +  - Akshay Gupta <Akshay.Gupta@amd.com>
>> +
>> +description: |
>> +  SB Remote Management Interface (SB-RMI) is an SMBus compatible
>> +  interface that reports AMD SoC's Power (normalized Power) using,
>> +  Mailbox Service Request over I3C interface to BMC.
>> +  The power attributes in hwmon reports power in microwatts.
>> +
>> +properties:
>> +  reg:
>> +    - description: |
>> +        Encodes the static I2C address.
>> +      Socket 0: 0x3c
>> +      Socket 1: 0x38
>> +    - description: |
>> +        First half of the Provisioned ID (following the PID
>> +        definition provided by the I3C specification).
>> +        Contains the manufacturer ID left-shifted by 1 (0x224).
>> +    - description: |
>> +        Second half of the Provisioned ID (following the PID
>> +        definition provided by the I3C specification).
>> +        Contains the ORing of the part ID left-shifted by 16,
>> +        the instance ID left-shifted by 12 and extra information (0x00000002).
>> +
> And this entire patch is just noop. Makes no difference, makes no impact.
>
> Drop it and write proper bindings with proper description of problem you
> are solving.
Ack, will modify the content as mentioned.
>
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.
>
> Please kindly resend and include all necessary To/Cc entries.
> </form letter>
>
> Best regards,
> Krzysztof

Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc

[Krzysztof Kozlowski]

On 11/08/2025 13:54, Gupta, Akshay wrote:
> 
> On 7/29/2025 11:59 AM, Krzysztof Kozlowski wrote:
>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>
>>
>> On 28/07/2025 08:10, Akshay Gupta wrote:
>>> - AMD SB-RMI patches are moved from drivers/hwmon to
>>>    drivers/misc to support additional functionality.
>>>    Move the related bindings documentation files to misc.
>>>
>>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> Really? What was reviewed here exactly? Poor style of commit msg or that
>> rename actually does rename?
>>
>>> Signed-off-by: Akshay Gupta <akshay.gupta@amd.com>
>>> ---
>>>   Documentation/devicetree/bindings/{hwmon => misc}/amd,sbrmi.yaml | 0
>> We don't put bindings into MISC.
> 
> We moved the SB-RMI driver from hwmon to misc, and hence moved the 
> binding documentation.
> 
> Referred the newly added yaml file, ti,fpc202.yaml through commit in the 
> misc bindings.
> 
> If Documentation/devicetree/bindings/misc is not right path, can you 
> please suggest the correct path for bindings for drivers in misc?

I don't know what this hardware is, but "misc" does not sound like class
of hardware devices... Don't just dump stuff there.

Best regards,
Krzysztof