From: Jack Cheng <cheng.jackhy@inventec.com>
The Q50SN12072 and Q54SN120A1 are high-efficiency, high-density DC-DC power
module from Delta Power Modules.
The Q50SN12072, quarter brick, single output 12V. This product provides up
to 1200 watts of output power at 38~60V. The Q50SN12072 offers peak
efficiency up to 98.3%@54Vin.
The Q54SN120A1, quarter brick, single output 12V. This product provides up
to 1300 watts of output power at 40~60V. The Q54SN120A1 offers peak
efficiency up to 98.1%@54Vin.
Add support for them to q54sj108a2 driver.
Signed-off-by: Jack Cheng <cheng.jackhy@inventec.com>
Co-developed-by: Brian Chiang <chiang.brian@inventec.com>
Signed-off-by: Brian Chiang <chiang.brian@inventec.com>
---
drivers/hwmon/pmbus/q54sj108a2.c | 97 ++++++++++++++++++++++++++++------------
1 file changed, 68 insertions(+), 29 deletions(-)
diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c
index d5d60a9af8c5..cc2b32ad935c 100644
--- a/drivers/hwmon/pmbus/q54sj108a2.c
+++ b/drivers/hwmon/pmbus/q54sj108a2.c
@@ -22,7 +22,9 @@
#define PMBUS_FLASH_KEY_WRITE 0xEC
enum chips {
- q54sj108a2
+ q50sn12072,
+ q54sj108a2,
+ q54sn120a1
};
enum {
@@ -55,10 +57,24 @@ struct q54sj108a2_data {
#define to_psu(x, y) container_of((x), struct q54sj108a2_data, debugfs_entries[(y)])
static struct pmbus_driver_info q54sj108a2_info[] = {
- [q54sj108a2] = {
+ [q50sn12072] = {
.pages = 1,
+ /* Source : Delta Q50SN12072 */
+ .format[PSC_VOLTAGE_OUT] = linear,
+ .format[PSC_TEMPERATURE] = linear,
+ .format[PSC_VOLTAGE_IN] = linear,
+ .format[PSC_CURRENT_OUT] = linear,
+ .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
+ PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
+ PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
+ PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
+ },
+ [q54sj108a2] = {
+ .pages = 1,
/* Source : Delta Q54SJ108A2 */
+ .format[PSC_VOLTAGE_OUT] = linear,
.format[PSC_TEMPERATURE] = linear,
.format[PSC_VOLTAGE_IN] = linear,
.format[PSC_CURRENT_OUT] = linear,
@@ -69,6 +85,20 @@ static struct pmbus_driver_info q54sj108a2_info[] = {
PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
PMBUS_HAVE_STATUS_INPUT,
},
+ [q54sn120a1] = {
+ .pages = 1,
+ /* Source : Delta Q54SN120A1 */
+ .format[PSC_VOLTAGE_OUT] = linear,
+ .format[PSC_TEMPERATURE] = linear,
+ .format[PSC_VOLTAGE_IN] = linear,
+ .format[PSC_CURRENT_OUT] = linear,
+
+ .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
+ PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
+ PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
+ PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
+ },
};
static ssize_t q54sj108a2_debugfs_read(struct file *file, char __user *buf,
@@ -270,7 +300,9 @@ static const struct file_operations q54sj108a2_fops = {
};
static const struct i2c_device_id q54sj108a2_id[] = {
+ { "q50sn12072", q50sn12072 },
{ "q54sj108a2", q54sj108a2 },
+ { "q54sn120a1", q54sn120a1 },
{ },
};
@@ -280,6 +312,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
+ const struct i2c_device_id *mid;
enum chips chip_id;
int ret, i;
struct dentry *debugfs;
@@ -292,14 +325,9 @@ static int q54sj108a2_probe(struct i2c_client *client)
I2C_FUNC_SMBUS_BLOCK_DATA))
return -ENODEV;
- if (client->dev.of_node)
- chip_id = (enum chips)(unsigned long)of_device_get_match_data(dev);
- else
- chip_id = i2c_match_id(q54sj108a2_id, client)->driver_data;
-
ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
if (ret < 0) {
- dev_err(&client->dev, "Failed to read Manufacturer ID\n");
+ dev_err(dev, "Failed to read Manufacturer ID\n");
return ret;
}
if (ret != 6 || strncmp(buf, "DELTA", 5)) {
@@ -308,19 +336,25 @@ static int q54sj108a2_probe(struct i2c_client *client)
return -ENODEV;
}
- /*
- * The chips support reading PMBUS_MFR_MODEL.
- */
ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
if (ret < 0) {
dev_err(dev, "Failed to read Manufacturer Model\n");
return ret;
}
- if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10)) {
+ for (mid = q54sj108a2_id; mid->name[0]; mid++) {
+ if (ret == strlen(mid->name) && !strncasecmp(mid->name, buf, ret))
+ break;
+ }
+ if (!mid->name[0]) {
buf[ret] = '\0';
dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
return -ENODEV;
}
+ chip_id = mid->driver_data;
+
+ if (strcmp(client->name, mid->name) != 0)
+ dev_notice(dev, "Device mismatch: Configured %s, detected %s\n",
+ client->name, mid->name);
ret = i2c_smbus_read_block_data(client, PMBUS_MFR_REVISION, buf);
if (ret < 0) {
@@ -341,6 +375,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
if (!psu)
return 0;
+ psu->chip = chip_id;
psu->client = client;
debugfs = pmbus_get_debugfs_dir(client);
@@ -359,9 +394,6 @@ static int q54sj108a2_probe(struct i2c_client *client)
debugfs_create_file("write_protect", 0444, q54sj108a2_dir,
&psu->debugfs_entries[Q54SJ108A2_DEBUGFS_WRITEPROTECT],
&q54sj108a2_fops);
- debugfs_create_file("store_default", 0200, q54sj108a2_dir,
- &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
- &q54sj108a2_fops);
debugfs_create_file("vo_ov_response", 0644, q54sj108a2_dir,
&psu->debugfs_entries[Q54SJ108A2_DEBUGFS_VOOV_RESPONSE],
&q54sj108a2_fops);
@@ -383,27 +415,34 @@ static int q54sj108a2_probe(struct i2c_client *client)
debugfs_create_file("mfr_location", 0444, q54sj108a2_dir,
&psu->debugfs_entries[Q54SJ108A2_DEBUGFS_MFR_LOCATION],
&q54sj108a2_fops);
- debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
- &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
- &q54sj108a2_fops);
debugfs_create_file("blackbox_read_offset", 0444, q54sj108a2_dir,
&psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ_OFFSET],
&q54sj108a2_fops);
- debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
- &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
- &q54sj108a2_fops);
- debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
- &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
- &q54sj108a2_fops);
- debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
- &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
- &q54sj108a2_fops);
+ if (psu->chip == q54sj108a2) {
+ debugfs_create_file("store_default", 0200, q54sj108a2_dir,
+ &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
+ &q54sj108a2_fops);
+ debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
+ &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
+ &q54sj108a2_fops);
+ debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
+ &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
+ &q54sj108a2_fops);
+ debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
+ &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
+ &q54sj108a2_fops);
+ debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
+ &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
+ &q54sj108a2_fops);
+ }
return 0;
}
static const struct of_device_id q54sj108a2_of_match[] = {
- { .compatible = "delta,q54sj108a2", .data = (void *)q54sj108a2 },
+ { .compatible = "delta,q50sn12072" },
+ { .compatible = "delta,q54sj108a2" },
+ { .compatible = "delta,q54sn120a1" },
{ },
};
@@ -421,6 +460,6 @@ static struct i2c_driver q54sj108a2_driver = {
module_i2c_driver(q54sj108a2_driver);
MODULE_AUTHOR("Xiao.Ma <xiao.mx.ma@deltaww.com>");
-MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 series modules");
+MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 and compatibles");
MODULE_LICENSE("GPL");
MODULE_IMPORT_NS("PMBUS");
--
2.43.0On Thu, Mar 26, 2026 at 01:48:06PM +0000, Brian Chiang wrote:
> From: Jack Cheng <cheng.jackhy@inventec.com>
>
> The Q50SN12072 and Q54SN120A1 are high-efficiency, high-density DC-DC power
> module from Delta Power Modules.
>
> The Q50SN12072, quarter brick, single output 12V. This product provides up
> to 1200 watts of output power at 38~60V. The Q50SN12072 offers peak
> efficiency up to 98.3%@54Vin.
>
> The Q54SN120A1, quarter brick, single output 12V. This product provides up
> to 1300 watts of output power at 40~60V. The Q54SN120A1 offers peak
> efficiency up to 98.1%@54Vin.
>
> Add support for them to q54sj108a2 driver.
>
> Signed-off-by: Jack Cheng <cheng.jackhy@inventec.com>
> Co-developed-by: Brian Chiang <chiang.brian@inventec.com>
> Signed-off-by: Brian Chiang <chiang.brian@inventec.com>
> ---
> drivers/hwmon/pmbus/q54sj108a2.c | 97 ++++++++++++++++++++++++++++------------
> 1 file changed, 68 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c
> index d5d60a9af8c5..cc2b32ad935c 100644
> --- a/drivers/hwmon/pmbus/q54sj108a2.c
> +++ b/drivers/hwmon/pmbus/q54sj108a2.c
> @@ -22,7 +22,9 @@
> #define PMBUS_FLASH_KEY_WRITE 0xEC
>
> enum chips {
> - q54sj108a2
> + q50sn12072,
> + q54sj108a2,
> + q54sn120a1
> };
>
> enum {
> @@ -55,10 +57,24 @@ struct q54sj108a2_data {
> #define to_psu(x, y) container_of((x), struct q54sj108a2_data, debugfs_entries[(y)])
>
> static struct pmbus_driver_info q54sj108a2_info[] = {
> - [q54sj108a2] = {
> + [q50sn12072] = {
> .pages = 1,
> + /* Source : Delta Q50SN12072 */
> + .format[PSC_VOLTAGE_OUT] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_CURRENT_OUT] = linear,
>
> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
> + },
> + [q54sj108a2] = {
> + .pages = 1,
> /* Source : Delta Q54SJ108A2 */
> + .format[PSC_VOLTAGE_OUT] = linear,
> .format[PSC_TEMPERATURE] = linear,
> .format[PSC_VOLTAGE_IN] = linear,
> .format[PSC_CURRENT_OUT] = linear,
> @@ -69,6 +85,20 @@ static struct pmbus_driver_info q54sj108a2_info[] = {
> PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> PMBUS_HAVE_STATUS_INPUT,
> },
> + [q54sn120a1] = {
> + .pages = 1,
> + /* Source : Delta Q54SN120A1 */
> + .format[PSC_VOLTAGE_OUT] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_CURRENT_OUT] = linear,
> +
> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
> + },
> };
>
> static ssize_t q54sj108a2_debugfs_read(struct file *file, char __user *buf,
> @@ -270,7 +300,9 @@ static const struct file_operations q54sj108a2_fops = {
> };
>
> static const struct i2c_device_id q54sj108a2_id[] = {
> + { "q50sn12072", q50sn12072 },
> { "q54sj108a2", q54sj108a2 },
> + { "q54sn120a1", q54sn120a1 },
> { },
> };
>
> @@ -280,6 +312,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
> + const struct i2c_device_id *mid;
> enum chips chip_id;
> int ret, i;
> struct dentry *debugfs;
> @@ -292,14 +325,9 @@ static int q54sj108a2_probe(struct i2c_client *client)
> I2C_FUNC_SMBUS_BLOCK_DATA))
> return -ENODEV;
>
> - if (client->dev.of_node)
> - chip_id = (enum chips)(unsigned long)of_device_get_match_data(dev);
> - else
> - chip_id = i2c_match_id(q54sj108a2_id, client)->driver_data;
> -
> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> if (ret < 0) {
> - dev_err(&client->dev, "Failed to read Manufacturer ID\n");
> + dev_err(dev, "Failed to read Manufacturer ID\n");
> return ret;
> }
> if (ret != 6 || strncmp(buf, "DELTA", 5)) {
> @@ -308,19 +336,25 @@ static int q54sj108a2_probe(struct i2c_client *client)
> return -ENODEV;
> }
>
> - /*
> - * The chips support reading PMBUS_MFR_MODEL.
> - */
> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> if (ret < 0) {
> dev_err(dev, "Failed to read Manufacturer Model\n");
> return ret;
> }
> - if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10)) {
> + for (mid = q54sj108a2_id; mid->name[0]; mid++) {
> + if (ret == strlen(mid->name) && !strncasecmp(mid->name, buf, ret))
> + break;
> + }
> + if (!mid->name[0]) {
> buf[ret] = '\0';
> dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
> return -ENODEV;
> }
From Sashiko feedback:
In the original driver, the PMBUS_MFR_MODEL response length was explicitly
expected to be 14 bytes (if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10))),
indicating the hardware returns the 10-character name padded with 4 extra
bytes. The patch changes the detection logic to loop through supported
devices and strictly require the returned length to match the device name's
length exactly (`ret == strlen(mid->name)`). Since `ret` will be 14 for the
original hardware, and strlen("q54sj108a2") is 10, the condition 14 == 10
evaluates to false. This causes the loop to exit without matching,
erroneously printing 'Unsupported Manufacturer Model' and returning -ENODEV.
This completely breaks driver probing and hardware monitoring for the
pre-existing Q54SJ108A2 device."
> + chip_id = mid->driver_data;
> +
> + if (strcmp(client->name, mid->name) != 0)
> + dev_notice(dev, "Device mismatch: Configured %s, detected %s\n",
> + client->name, mid->name);
>
> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_REVISION, buf);
> if (ret < 0) {
> @@ -341,6 +375,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
> if (!psu)
> return 0;
>
> + psu->chip = chip_id;
> psu->client = client;
>
> debugfs = pmbus_get_debugfs_dir(client);
> @@ -359,9 +394,6 @@ static int q54sj108a2_probe(struct i2c_client *client)
> debugfs_create_file("write_protect", 0444, q54sj108a2_dir,
> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_WRITEPROTECT],
> &q54sj108a2_fops);
> - debugfs_create_file("store_default", 0200, q54sj108a2_dir,
> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
> - &q54sj108a2_fops);
> debugfs_create_file("vo_ov_response", 0644, q54sj108a2_dir,
> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_VOOV_RESPONSE],
> &q54sj108a2_fops);
> @@ -383,27 +415,34 @@ static int q54sj108a2_probe(struct i2c_client *client)
> debugfs_create_file("mfr_location", 0444, q54sj108a2_dir,
> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_MFR_LOCATION],
> &q54sj108a2_fops);
> - debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
> - &q54sj108a2_fops);
> debugfs_create_file("blackbox_read_offset", 0444, q54sj108a2_dir,
> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ_OFFSET],
> &q54sj108a2_fops);
What is the purpose/value of keeping this file outside the if() block ?
> - debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
> - &q54sj108a2_fops);
> - debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
> - &q54sj108a2_fops);
> - debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
> - &q54sj108a2_fops);
> + if (psu->chip == q54sj108a2) {
> + debugfs_create_file("store_default", 0200, q54sj108a2_dir,
> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
> + &q54sj108a2_fops);
> + debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
> + &q54sj108a2_fops);
> + debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
> + &q54sj108a2_fops);
> + debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
> + &q54sj108a2_fops);
> + debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
> + &q54sj108a2_fops);
> + }
>
> return 0;
> }
>
> static const struct of_device_id q54sj108a2_of_match[] = {
> - { .compatible = "delta,q54sj108a2", .data = (void *)q54sj108a2 },
> + { .compatible = "delta,q50sn12072" },
> + { .compatible = "delta,q54sj108a2" },
> + { .compatible = "delta,q54sn120a1" },
Why drop .data here ?
Thanks,
Guenter
> { },
> };
>
> @@ -421,6 +460,6 @@ static struct i2c_driver q54sj108a2_driver = {
> module_i2c_driver(q54sj108a2_driver);
>
> MODULE_AUTHOR("Xiao.Ma <xiao.mx.ma@deltaww.com>");
> -MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 series modules");
> +MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 and compatibles");
> MODULE_LICENSE("GPL");
> MODULE_IMPORT_NS("PMBUS");
>
> --
> 2.43.0
>
On Thu, Mar 26, 2026 at 09:29:38AM -0700, Guenter Roeck wrote:
>On Thu, Mar 26, 2026 at 01:48:06PM +0000, Brian Chiang wrote:
>> From: Jack Cheng <cheng.jackhy@inventec.com>
>>
>> The Q50SN12072 and Q54SN120A1 are high-efficiency, high-density DC-DC power
>> module from Delta Power Modules.
>>
>> The Q50SN12072, quarter brick, single output 12V. This product provides up
>> to 1200 watts of output power at 38~60V. The Q50SN12072 offers peak
>> efficiency up to 98.3%@54Vin.
>>
>> The Q54SN120A1, quarter brick, single output 12V. This product provides up
>> to 1300 watts of output power at 40~60V. The Q54SN120A1 offers peak
>> efficiency up to 98.1%@54Vin.
>>
>> Add support for them to q54sj108a2 driver.
>>
>> Signed-off-by: Jack Cheng <cheng.jackhy@inventec.com>
>> Co-developed-by: Brian Chiang <chiang.brian@inventec.com>
>> Signed-off-by: Brian Chiang <chiang.brian@inventec.com>
>> ---
>> drivers/hwmon/pmbus/q54sj108a2.c | 97 ++++++++++++++++++++++++++++------------
>> 1 file changed, 68 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c
>> index d5d60a9af8c5..cc2b32ad935c 100644
>> --- a/drivers/hwmon/pmbus/q54sj108a2.c
>> +++ b/drivers/hwmon/pmbus/q54sj108a2.c
>> @@ -22,7 +22,9 @@
>> #define PMBUS_FLASH_KEY_WRITE 0xEC
>>
>> enum chips {
>> - q54sj108a2
>> + q50sn12072,
>> + q54sj108a2,
>> + q54sn120a1
>> };
>>
>> enum {
>> @@ -55,10 +57,24 @@ struct q54sj108a2_data {
>> #define to_psu(x, y) container_of((x), struct q54sj108a2_data, debugfs_entries[(y)])
>>
>> static struct pmbus_driver_info q54sj108a2_info[] = {
>> - [q54sj108a2] = {
>> + [q50sn12072] = {
>> .pages = 1,
>> + /* Source : Delta Q50SN12072 */
>> + .format[PSC_VOLTAGE_OUT] = linear,
>> + .format[PSC_TEMPERATURE] = linear,
>> + .format[PSC_VOLTAGE_IN] = linear,
>> + .format[PSC_CURRENT_OUT] = linear,
>>
>> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
>> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
>> + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
>> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
>> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
>> + },
>> + [q54sj108a2] = {
>> + .pages = 1,
>> /* Source : Delta Q54SJ108A2 */
>> + .format[PSC_VOLTAGE_OUT] = linear,
>> .format[PSC_TEMPERATURE] = linear,
>> .format[PSC_VOLTAGE_IN] = linear,
>> .format[PSC_CURRENT_OUT] = linear,
>> @@ -69,6 +85,20 @@ static struct pmbus_driver_info q54sj108a2_info[] = {
>> PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
>> PMBUS_HAVE_STATUS_INPUT,
>> },
>> + [q54sn120a1] = {
>> + .pages = 1,
>> + /* Source : Delta Q54SN120A1 */
>> + .format[PSC_VOLTAGE_OUT] = linear,
>> + .format[PSC_TEMPERATURE] = linear,
>> + .format[PSC_VOLTAGE_IN] = linear,
>> + .format[PSC_CURRENT_OUT] = linear,
>> +
>> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
>> + PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
>> + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
>> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
>> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_POUT,
>> + },
>> };
>>
>> static ssize_t q54sj108a2_debugfs_read(struct file *file, char __user *buf,
>> @@ -270,7 +300,9 @@ static const struct file_operations q54sj108a2_fops = {
>> };
>>
>> static const struct i2c_device_id q54sj108a2_id[] = {
>> + { "q50sn12072", q50sn12072 },
>> { "q54sj108a2", q54sj108a2 },
>> + { "q54sn120a1", q54sn120a1 },
>> { },
>> };
>>
>> @@ -280,6 +312,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> {
>> struct device *dev = &client->dev;
>> u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
>> + const struct i2c_device_id *mid;
>> enum chips chip_id;
>> int ret, i;
>> struct dentry *debugfs;
>> @@ -292,14 +325,9 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> I2C_FUNC_SMBUS_BLOCK_DATA))
>> return -ENODEV;
>>
>> - if (client->dev.of_node)
>> - chip_id = (enum chips)(unsigned long)of_device_get_match_data(dev);
>> - else
>> - chip_id = i2c_match_id(q54sj108a2_id, client)->driver_data;
>> -
>> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
>> if (ret < 0) {
>> - dev_err(&client->dev, "Failed to read Manufacturer ID\n");
>> + dev_err(dev, "Failed to read Manufacturer ID\n");
>> return ret;
>> }
>> if (ret != 6 || strncmp(buf, "DELTA", 5)) {
>> @@ -308,19 +336,25 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> return -ENODEV;
>> }
>>
>> - /*
>> - * The chips support reading PMBUS_MFR_MODEL.
>> - */
>> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
>> if (ret < 0) {
>> dev_err(dev, "Failed to read Manufacturer Model\n");
>> return ret;
>> }
>> - if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10)) {
>> + for (mid = q54sj108a2_id; mid->name[0]; mid++) {
>> + if (ret == strlen(mid->name) && !strncasecmp(mid->name, buf, ret))
>> + break;
>> + }
>> + if (!mid->name[0]) {
>> buf[ret] = '\0';
>> dev_err(dev, "Unsupported Manufacturer Model '%s'\n", buf);
>> return -ENODEV;
>> }
>
>>From Sashiko feedback:
>
>In the original driver, the PMBUS_MFR_MODEL response length was explicitly
>expected to be 14 bytes (if (ret != 14 || strncmp(buf, "Q54SJ108A2", 10))),
>indicating the hardware returns the 10-character name padded with 4 extra
>bytes. The patch changes the detection logic to loop through supported
>devices and strictly require the returned length to match the device name's
>length exactly (`ret == strlen(mid->name)`). Since `ret` will be 14 for the
>original hardware, and strlen("q54sj108a2") is 10, the condition 14 == 10
>evaluates to false. This causes the loop to exit without matching,
>erroneously printing 'Unsupported Manufacturer Model' and returning -ENODEV.
>This completely breaks driver probing and hardware monitoring for the
>pre-existing Q54SJ108A2 device."
Thank you for the explanation. Maybe we should simply match the manufacturer
model and remove the strict length comparison to prevent it from breaking
the driver probing. I will add this change in v3.
>
>> + chip_id = mid->driver_data;
>> +
>> + if (strcmp(client->name, mid->name) != 0)
>> + dev_notice(dev, "Device mismatch: Configured %s, detected %s\n",
>> + client->name, mid->name);
>>
>> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_REVISION, buf);
>> if (ret < 0) {
>> @@ -341,6 +375,7 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> if (!psu)
>> return 0;
>>
>> + psu->chip = chip_id;
>> psu->client = client;
>>
>> debugfs = pmbus_get_debugfs_dir(client);
>> @@ -359,9 +394,6 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> debugfs_create_file("write_protect", 0444, q54sj108a2_dir,
>> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_WRITEPROTECT],
>> &q54sj108a2_fops);
>> - debugfs_create_file("store_default", 0200, q54sj108a2_dir,
>> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
>> - &q54sj108a2_fops);
>> debugfs_create_file("vo_ov_response", 0644, q54sj108a2_dir,
>> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_VOOV_RESPONSE],
>> &q54sj108a2_fops);
>> @@ -383,27 +415,34 @@ static int q54sj108a2_probe(struct i2c_client *client)
>> debugfs_create_file("mfr_location", 0444, q54sj108a2_dir,
>> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_MFR_LOCATION],
>> &q54sj108a2_fops);
>> - debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
>> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
>> - &q54sj108a2_fops);
>> debugfs_create_file("blackbox_read_offset", 0444, q54sj108a2_dir,
>> &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ_OFFSET],
>> &q54sj108a2_fops);
>
>What is the purpose/value of keeping this file outside the if() block ?
Thank you for catching this. It was an oversight in v2, the
`blackbox_read_offset` should have been kept inside of the block.
I'll add this change in v3.
>
>> - debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
>> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
>> - &q54sj108a2_fops);
>> - debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
>> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
>> - &q54sj108a2_fops);
>> - debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
>> - &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
>> - &q54sj108a2_fops);
>> + if (psu->chip == q54sj108a2) {
>> + debugfs_create_file("store_default", 0200, q54sj108a2_dir,
>> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_STOREDEFAULT],
>> + &q54sj108a2_fops);
>> + debugfs_create_file("blackbox_erase", 0200, q54sj108a2_dir,
>> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_ERASE],
>> + &q54sj108a2_fops);
>> + debugfs_create_file("blackbox_set_offset", 0200, q54sj108a2_dir,
>> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_SET_OFFSET],
>> + &q54sj108a2_fops);
>> + debugfs_create_file("blackbox_read", 0444, q54sj108a2_dir,
>> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_BLACKBOX_READ],
>> + &q54sj108a2_fops);
>> + debugfs_create_file("flash_key", 0444, q54sj108a2_dir,
>> + &psu->debugfs_entries[Q54SJ108A2_DEBUGFS_FLASH_KEY],
>> + &q54sj108a2_fops);
>> + }
>>
>> return 0;
>> }
>>
>> static const struct of_device_id q54sj108a2_of_match[] = {
>> - { .compatible = "delta,q54sj108a2", .data = (void *)q54sj108a2 },
>> + { .compatible = "delta,q50sn12072" },
>> + { .compatible = "delta,q54sj108a2" },
>> + { .compatible = "delta,q54sn120a1" },
>
>Why drop .data here ?
I would like to drop .data since it was previously consumed by
`of_device_get_match_data()`, which has been removed in v2 changes.
>
>Thanks,
>Guenter
>
>> { },
>> };
>>
>> @@ -421,6 +460,6 @@ static struct i2c_driver q54sj108a2_driver = {
>> module_i2c_driver(q54sj108a2_driver);
>>
>> MODULE_AUTHOR("Xiao.Ma <xiao.mx.ma@deltaww.com>");
>> -MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 series modules");
>> +MODULE_DESCRIPTION("PMBus driver for Delta Q54SJ108A2 and compatibles");
>> MODULE_LICENSE("GPL");
>> MODULE_IMPORT_NS("PMBUS");
>>
>> --
>> 2.43.0
>>
© 2016 - 2026 Red Hat, Inc.