* [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C
2025-07-28 6:10 [PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module Akshay Gupta
@ 2025-07-28 6:10 ` Akshay Gupta
2025-07-28 6:37 ` Thomas Weißschuh
2025-07-29 6:33 ` Krzysztof Kozlowski
2025-07-28 6:10 ` [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc Akshay Gupta
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Akshay Gupta @ 2025-07-28 6:10 UTC (permalink / raw)
To: linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Akshay Gupta,
Naveen Krishna Chatradhi
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C
2025-07-28 6:10 ` [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C Akshay Gupta
@ 2025-07-28 6:37 ` Thomas Weißschuh
2025-08-11 11:02 ` Gupta, Akshay
2025-07-29 6:33 ` Krzysztof Kozlowski
1 sibling, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2025-07-28 6:37 UTC (permalink / raw)
To: Akshay Gupta
Cc: linux-kernel, linux-hwmon, gregkh, arnd, linux, Anand.Umarji,
Naveen Krishna Chatradhi
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C
2025-07-28 6:37 ` Thomas Weißschuh
@ 2025-08-11 11:02 ` Gupta, Akshay
0 siblings, 0 replies; 15+ messages in thread
From: Gupta, Akshay @ 2025-08-11 11:02 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: linux-kernel, linux-hwmon, gregkh, arnd, linux, Anand.Umarji,
Naveen Krishna Chatradhi
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
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C
2025-07-28 6:10 ` [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C Akshay Gupta
2025-07-28 6:37 ` Thomas Weißschuh
@ 2025-07-29 6:33 ` Krzysztof Kozlowski
2025-08-11 11:03 ` Gupta, Akshay
1 sibling, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-29 6:33 UTC (permalink / raw)
To: Akshay Gupta, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C
2025-07-29 6:33 ` Krzysztof Kozlowski
@ 2025-08-11 11:03 ` Gupta, Akshay
0 siblings, 0 replies; 15+ messages in thread
From: Gupta, Akshay @ 2025-08-11 11:03 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc
2025-07-28 6:10 [PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module Akshay Gupta
2025-07-28 6:10 ` [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C Akshay Gupta
@ 2025-07-28 6:10 ` Akshay Gupta
2025-07-29 6:29 ` Krzysztof Kozlowski
2025-07-28 6:10 ` [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path Akshay Gupta
2025-07-28 6:10 ` [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C Akshay Gupta
3 siblings, 1 reply; 15+ messages in thread
From: Akshay Gupta @ 2025-07-28 6:10 UTC (permalink / raw)
To: linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Akshay Gupta,
Naveen Krishna Chatradhi
- 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc
2025-07-28 6:10 ` [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc Akshay Gupta
@ 2025-07-29 6:29 ` Krzysztof Kozlowski
2025-08-11 11:54 ` Gupta, Akshay
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-29 6:29 UTC (permalink / raw)
To: Akshay Gupta, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc
2025-07-29 6:29 ` Krzysztof Kozlowski
@ 2025-08-11 11:54 ` Gupta, Akshay
2025-08-11 11:56 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Gupta, Akshay @ 2025-08-11 11:54 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc
2025-08-11 11:54 ` Gupta, Akshay
@ 2025-08-11 11:56 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-11 11:56 UTC (permalink / raw)
To: Gupta, Akshay, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path
2025-07-28 6:10 [PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module Akshay Gupta
2025-07-28 6:10 ` [PATCH v1 2/5] misc: amd-sbi: Add support for SB-RMI over I3C Akshay Gupta
2025-07-28 6:10 ` [PATCH v1 3/5] dt-bindings: misc: Move amd,sbrmi.yaml from hwmon to misc Akshay Gupta
@ 2025-07-28 6:10 ` Akshay Gupta
2025-07-29 6:29 ` Krzysztof Kozlowski
2025-07-28 6:10 ` [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C Akshay Gupta
3 siblings, 1 reply; 15+ messages in thread
From: Akshay Gupta @ 2025-07-28 6:10 UTC (permalink / raw)
To: linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Akshay Gupta,
Naveen Krishna Chatradhi
- 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path
2025-07-28 6:10 ` [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path Akshay Gupta
@ 2025-07-29 6:29 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-29 6:29 UTC (permalink / raw)
To: Akshay Gupta, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C
2025-07-28 6:10 [PATCH v1 1/5] misc: amd-sbi: Split core driver as separate module Akshay Gupta
` (2 preceding siblings ...)
2025-07-28 6:10 ` [PATCH v1 4/5] dt-bindings: misc: Update the "$id:" to misc path Akshay Gupta
@ 2025-07-28 6:10 ` Akshay Gupta
2025-07-29 6:31 ` Krzysztof Kozlowski
3 siblings, 1 reply; 15+ messages in thread
From: Akshay Gupta @ 2025-07-28 6:10 UTC (permalink / raw)
To: linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Akshay Gupta,
Naveen Krishna Chatradhi
- 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C
2025-07-28 6:10 ` [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C Akshay Gupta
@ 2025-07-29 6:31 ` Krzysztof Kozlowski
2025-08-11 11:55 ` Gupta, Akshay
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-29 6:31 UTC (permalink / raw)
To: Akshay Gupta, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v1 5/5] dt-bindings: misc: Add binding document for SB-RMI I3C
2025-07-29 6:31 ` Krzysztof Kozlowski
@ 2025-08-11 11:55 ` Gupta, Akshay
0 siblings, 0 replies; 15+ messages in thread
From: Gupta, Akshay @ 2025-08-11 11:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-kernel, linux-hwmon
Cc: gregkh, arnd, linux, Anand.Umarji, Naveen Krishna Chatradhi
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
^ permalink raw reply [flat|nested] 15+ messages in thread