Support ROHM BD127x0 hot-swap controllers
Subsystems (3)
cover Support ROHM BD127x0 hot-swap controllers
Support ROHM BD12780(A) and BD12790
The BD12780 and BD12780A hot-swap controllers are very similar to Analog Devices ADM1278. There are only some minor differences in the registers.
The BD12790 is largely similar to the ADM1272, with slightly different coefficients and minor register changes.
This series adds basic support for these ROHM ICs.
The last patch adds of_device_id table with entries for the newly added controllers. This fixes the module auto-load on the test board with old Debian user-space.
I have no idea if adding the of_device_id -entries for other ICs could cause problems in some existing systems. Hence only new ICs were added to the of_device_id tables.
---
Matti Vaittinen (7):
dt-bindings: adm1275: ROHM BD12780 hot-swap controller
doc: Add ROHM BD12780 and BD12780A
hwmon: adm1275: Support ROHM BD12780
dt-bindings: adm1275: ROHM BD12790 hot-swap controller
doc: adm1275: Add ROHM BD12790
hwmon: adm1275: Support ROHM BD12790
hwmon: adm1275: Support module auto-loading
.../bindings/hwmon/adi,adm1275.yaml | 44 ++++++--- Documentation/hwmon/adm1275.rst | 24 +++++ drivers/hwmon/pmbus/Kconfig | 4 +- drivers/hwmon/pmbus/adm1275.c | 91 +++++++++++++++++-- 4 files changed, 142 insertions(+), 21 deletions(-)
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
Signature · 1 line
2.54.0
PATCH 1/7 dt-bindings: adm1275: ROHM BD12780 hot-swap controller
From: Matti Vaittinen <[email protected]>
Support ROHM BD12780 and BD12780A hot-swap controllers, which are largely compatible with the Analog Devices adm1278. Main difference between the BD12780 and the BD12780A is, that the BD12780 has one I2C address configuration pin more (ADDR3) than the BD12780A.
Introduce own compatibles for both variants but require the BD12780A to always have the BD12780 as a fall-back.
--- .../bindings/hwmon/adi,adm1275.yaml | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-)
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +28 -11
@@ -25,19 +25,35 @@ description: | https://www.silergy.com/ download/downloadFile?id=5669&type=product&ftype=note + The BD12780 and BD12780A are hot-swap controllers from ROHM. They are + functionally compatible with the ADM1278. The main difference between + the BD12780A and the BD12780 is amount of configurable I2C addresses. + + Datasheets: + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + properties: compatible: - enum: - - adi,adm1075 - - adi,adm1272 - - adi,adm1273 - - adi,adm1275 - - adi,adm1276 - - adi,adm1278 - - adi,adm1281 - - adi,adm1293 - - adi,adm1294 - - silergy,mc09c + oneOf: + - items: + enum: + - adi,adm1075 + - adi,adm1272 + - adi,adm1273 + - adi,adm1275 + - adi,adm1276 + - adi,adm1278 + - adi,adm1281 + - adi,adm1293 + - adi,adm1294 + - rohm,bd12780 + - silergy,mc09c + +# Require BD12780 as a fall-back for BD12780A. + - items: + - const: rohm,bd12780a + - const: rohm,bd12780 reg: maxItems: 1
@@ -104,6 +120,7 @@ allOf: - adi,adm1281 - adi,adm1293 - adi,adm1294 + - rohm,bd12780 - silergy,mc09c then: properties:
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 1/7] dt-bindings: adm1275: ROHM BD12780 hot-swap controller
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Invalid use of the `items` object for the single-string compatible enum, allowing arrays of…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Invalid use of the items object for the single-string compatible enum, allowing arrays of multiple arbitrary compatible strings. --
commit 7132123aae357451c411a0cee036262cc9f62a45
Author: Matti Vaittinen <[email protected]>
dt-bindings: adm1275: ROHM BD12780 hot-swap controller
This commit adds support for the ROHM BD12780 and BD12780A hot-swap controllers to the adm1275 device tree bindings. It introduces new compatible strings, requiring BD12780A to use BD12780 as a fallback.
Quoted 4 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +0
-0
[ ... ]
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +0
-7@@ -25,19 +25,35 @@ description: | properties: compatible: - enum: - - adi,adm1075 - - adi,adm1272 - - adi,adm1273 - - adi,adm1275 - - adi,adm1276 - - adi,adm1278
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 32
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +6
-4- - adi,adm1281 - - adi,adm1293 - - adi,adm1294 - - silergy,mc09c + oneOf: + - items: + enum: + - adi,adm1075 + - adi,adm1272 + - adi,adm1273
[Severity: Low]
Does wrapping this enum inside an items list inadvertently allow multiple exclusive strings instead of restricting it to exactly one?
In dt-schema, applying an items object directly to a string-array property like compatible defines the schema for all elements of the array, but does not impose bounds on the array length. This might allow a device tree to define a compatible property with multiple strings from this list, such as:
compatible = "adi,adm1075", "adi,adm1272";
To enforce exactly one string from the list, should the items wrapper be removed so that the enum resides directly under oneOf?
Quoted 13 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +13
-0+ - adi,adm1275 + - adi,adm1276 + - adi,adm1278 + - adi,adm1281 + - adi,adm1293 + - adi,adm1294 + - rohm,bd12780 + - silergy,mc09c + +# Require BD12780 as a fall-back for BD12780A. + - items: + - const: rohm,bd12780a + - const: rohm,bd12780
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH 1/7] dt-bindings: adm1275: ROHM BD12780 hot-swap controller
In reply to Matti VaittinenOn Tue, Jun 16, 2026 at 09:35:35AM +0300, Matti Vaittinen wrote:
+
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +4
-4+ Datasheets: + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + properties: compatible: - enum: - - adi,adm1075 - - adi,adm1272 - - adi,adm1273
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 40
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +3
-7- - adi,adm1275 - - adi,adm1276 - - adi,adm1278 - - adi,adm1281 - - adi,adm1293 - - adi,adm1294 - - silergy,mc09c + oneOf: + - items: + enum:
s/items/enum/, so:
oneOf:
- enum:
....
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 53
Quoted 13 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +13
-0+ - adi,adm1075 + - adi,adm1272 + - adi,adm1273 + - adi,adm1275 + - adi,adm1276 + - adi,adm1278 + - adi,adm1281 + - adi,adm1293 + - adi,adm1294 + - rohm,bd12780 + - silergy,mc09c + +# Require BD12780 as a fall-back for BD12780A.
No need for the comment, schema is quite explicit.
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 56
Quoted 3 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +3
-0+ - items: + - const: rohm,bd12780a + - const: rohm,bd12780
Best regards,
Krzysztof
Re: [PATCH 1/7] dt-bindings: adm1275: ROHM BD12780 hot-swap controller
I think I (almost) missed this review... Sorry for the belated reply.
Quoted 24 lines · Krzysztof Kozlowski
On Tue, Jun 16, 2026 at 09:35:35AM +0300, Matti Vaittinen wrote:
+Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +4
-4+ Datasheets: + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf + https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + properties: compatible: - enum: - - adi,adm1075 - - adi,adm1272 - - adi,adm1273Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 40
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +3
-7- - adi,adm1275 - - adi,adm1276 - - adi,adm1278 - - adi,adm1281 - - adi,adm1293 - - adi,adm1294 - - silergy,mc09c + oneOf: + - items: + enum:
Quoted 5 lines
s/items/enum/, so:
oneOf:
- enum:....
Thanks Krzysztof. I am always so lost with these bindings. Giving the concrete suggestion(s) helps a lot!
Quoted 16 lines
Quoted 4 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +3
-0+ - adi,adm1075 + - adi,adm1272 + - adi,adm1273Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 53
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +10
-0+ - adi,adm1275 + - adi,adm1276 + - adi,adm1278 + - adi,adm1281 + - adi,adm1293 + - adi,adm1294 + - rohm,bd12780 + - silergy,mc09c + +# Require BD12780 as a fall-back for BD12780A.No need for the comment, schema is quite explicit.
Eh... I know it is explicit for one who fluently reads yaml. Not all of us do that :| (See my reply to the previous comment...) I am not sure the comment hurts - while I am sure it helps occasional binding reader like me. Can you please reconsider keeping the comment?
Although, I am not sure if Guenter suggested me to drop the compatible for the bd12780a and only use the bd12780 - or if his comment only applied to the i2c IDs.
https://lore.kernel.org/all/[email protected]/
Well, I will keep the bd12780a compatible and drop the I2C ID unless something else is suggested. Again, the BD12780 and BD12780A do have different hardware properties (at least in I2C slave address selection pins), and while it doesn't really matter for the Linux drivers, the DT bindings should ideally be generic and not Linux specific.
Yours,
-- Matti.
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH 1/7] dt-bindings: adm1275: ROHM BD12780 hot-swap controller
Quoted 16 lines · Matti Vaittinen
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +7
-0+ - adi,adm1075 + - adi,adm1272 + - adi,adm1273 + - adi,adm1275 + - adi,adm1276 + - adi,adm1278 + - adi,adm1281Quoted 8 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml description: | · line 53
Quoted 6 lines
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +6
-0+ - adi,adm1293 + - adi,adm1294 + - rohm,bd12780 + - silergy,mc09c + +# Require BD12780 as a fall-back for BD12780A.No need for the comment, schema is quite explicit.
Quoted 4 lines
Eh... I know it is explicit for one who fluently reads yaml. Not all of us do that :| (See my reply to the previous comment...) I am not sure the comment hurts - while I am sure it helps occasional binding reader like me. Can you please reconsider keeping the comment?
This one does not, but if people take the existing code as a starting point or even as an example in arguments ("he did like that, so I am allowed as well"), it gets multiplied and we have more bindings with redundant data.
That's said, if you insist then fine with me, keep it.
Best regards,
Krzysztof
PATCH 2/7 doc: Add ROHM BD12780 and BD12780A
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
Add the ROHM BD12780 and the BD12780A to the list of the ICs supported by the adm1275 driver.
--- Documentation/hwmon/adm1275.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Documentation/hwmon/adm1275.rst +16 -0
@@ -67,6 +67,22 @@ Supported chips: Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1293_1294.pdf + * ROHM Semiconductor BD12780 + + Prefix: 'bd12780' + + Addresses scanned: - + + Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf + + * ROHM Semiconductor BD12780A + + Prefix: 'bd12780' + + Addresses scanned: - + + Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + * Silergy SQ24905C Prefix: 'mc09c'
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 2/7] doc: Add ROHM BD12780 and BD12780A
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 6 lines
From: Matti Vaittinen <[email protected]>
Add the ROHM BD12780 and the BD12780A to the list of the ICs supported by the adm1275 driver.
Signed-off-by Matti Vaittinen <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH 3/7 hwmon: adm1275: Support ROHM BD12780
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark PMON_CONFIG bits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.
The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".
Support ROHM BD12780 and BD12780A controllers.
--- drivers/hwmon/pmbus/Kconfig | 2 +- drivers/hwmon/pmbus/adm1275.c | 46 +++++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/Kconfig +1 -1
@@ -52,7 +52,7 @@ config SENSORS_ADM1275 help If you say yes here you get hardware monitoring support for Analog Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281, - ADM1293, ADM1294 and SQ24905C Hot-Swap Controller and + ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and Digital Power Monitors. This driver can also be built as a module. If so, the module will
drivers/hwmon/pmbus/adm1275.c +38 -8
110 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 3/7] hwmon: adm1275: Support ROHM BD12780
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 7 lines
From: Matti Vaittinen <[email protected]>
ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark
PMON_CONFIGbits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.
Quoted 6 lines
The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".
Support ROHM BD12780 and BD12780A controllers.
Signed-off-by Matti Vaittinen <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Guenter Roeck
maintainerRe: [PATCH 3/7] hwmon: adm1275: Support ROHM BD12780
In reply to Matti VaittinenQuoted 62 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark
PMON_CONFIGbits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".
Support ROHM BD12780 and BD12780A controllers.
Signed-off-by Matti Vaittinen <[email protected]>--- drivers/hwmon/pmbus/Kconfig | 2 +- drivers/hwmon/pmbus/adm1275.c | 46 +++++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 9 deletions(-)drivers/hwmon/pmbus/Kconfig +1
-1@@ -52,7 +52,7 @@ config SENSORS_ADM1275 help If you say yes here you get hardware monitoring support for Analog Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281, - ADM1293, ADM1294 and SQ24905C Hot-Swap Controller and + ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and Digital Power Monitors. This driver can also be built as a module. If so, the module willdrivers/hwmon/pmbus/adm1275.c +8
-1@@ -19,7 +19,7 @@#include "pmbus.h" enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281, - adm1293, adm1294, sq24905c }; + adm1293, adm1294, bd12780, sq24905c };#define ADM1275_MFR_STATUS_IOUT_WARN2 BIT(0)#define ADM1293_MFR_STATUS_VAUX_UV_WARN BIT(5)@@ -47,6 +47,8 @@ enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281,#define ADM1278_VOUT_EN BIT(1)#define ADM1278_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN | ADM1278_TSFILT) +/* The BD12780 data sheets mark TSFILT bit as reserved. */ +#define BD12780_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)#define ADM1293_IRANGE_25 0#define ADM1293_IRANGE_50 BIT(6)@@ -487,6 +489,21 @@ static const struct i2c_device_id adm1275_id[] = { { "adm1281", adm1281 }, { "adm1293", adm1293 }, { "adm1294", adm1294 }, +/* +* The BD12780a is functionally identical to BD12780(*). Even the pmbus ID +* register contents are same. When instantiated from the DT, it is required +* to have the bd12780 as a fall-back. We still need the bd12780a ID here, +* because the i2c_device_id is created from the first compatible, not from
drivers/hwmon/pmbus/adm1275.c static const struct i2c_device_id adm1275_id[] = { · line 506
Quoted 10 lines
drivers/hwmon/pmbus/adm1275.c +10
-0+* the fall-back entry. +* (*)Until proven to differ. I prefer having own compatible for these +* variants for that day. Please note that even though the probe is called +* based on the 'bd12780a' -entry, the ID is picked at probe based on the +* pmbus register contents and not by DT entry. Thus, if the bd12780 and +* bd12780a are found to require different handling, then this needs to be +* changed, or bd12780a is handled as bd12780. +*/ + { "bd12780", bd12780 }, + { "bd12780a", /* driver data unused, see --^ */ },
We don't usually do that. There are various A/B/C variants for many chips, and we just use the base name unless a difference is warranted. Either this is needed, and driver data is needed as well, or it isn't. If it is not needed, it should be dropped.
Quoted 53 lines
drivers/hwmon/pmbus/adm1275.c +9
-6{ "mc09c", sq24905c }, { } };@@ -494,12 +511,13 @@ MODULE_DEVICE_TABLE(i2c, adm1275_id);/* Enable VOUT & TEMP1 if not enabled (disabled by default) */ static int adm1275_enable_vout_temp (struct adm1275_data *data, -struct i2c_client *client,int config) +struct i2c_client *client,int config, +u16 defconfig) {int ret; -if ((config &ADM1278_PMON_DEFCONFIG ) !=ADM1278_PMON_DEFCONFIG ) { - config |=ADM1278_PMON_DEFCONFIG ; +if ((config & defconfig) != defconfig) { + config |= defconfig; ret =adm1275_write_pmon_config (data, client, config);if (ret <0 ) {dev_err (&client->dev,"Failed to enable VOUT/TEMP1 monitoring\n" );@@ -535,7 +553,8 @@ static int adm1275_probe(struct i2c_client *client)return ret; }if ((ret !=3 ||strncmp (block_buffer,"ADI" ,3 )) && - (ret !=2 ||strncmp (block_buffer,"SY" ,2 ))) { + (ret !=2 ||strncmp (block_buffer,"SY" ,2 )) && + (ret !=4 ||strncmp (block_buffer,"ROHM" ,4 ))) {dev_err (&client->dev,"Unsupported Manufacturer ID\n" );return -ENODEV ; }@@ -562,7 +581,7 @@ static int adm1275_probe(struct i2c_client *client)if (mid->driver_data == adm1272 || mid->driver_data == adm1273 || mid->driver_data == adm1278 || mid->driver_data == adm1281 || mid->driver_data == adm1293 || mid->driver_data == adm1294 || - mid->driver_data == sq24905c) + mid->driver_data == bd12780 || mid->driver_data == sq24905c) config_read_fn = i2c_smbus_read_word_data;else config_read_fn = i2c_smbus_read_byte_data;@@ -666,7 +685,8 @@ static int adm1275_probe(struct i2c_client *client)PMBUS_HAVE_VOUT |PMBUS_HAVE_STATUS_VOUT |PMBUS_HAVE_TEMP |PMBUS_HAVE_STATUS_TEMP ; - ret =adm1275_enable_vout_temp (data, client, config); + ret =adm1275_enable_vout_temp (data, client, config, +ADM1278_PMON_DEFCONFIG );if (ret)return ret;@@ -712,7 +732,16 @@ static int adm1275_probe(struct i2c_client *client)break ;case adm1278:case adm1281:
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 744
Quoted 10 lines
drivers/hwmon/pmbus/adm1275.c +9
-0+case bd12780:case sq24905c: + { +u16 defconfig; + +if (data->id == bd12780) + defconfig =BD12780_PMON_DEFCONFIG ; +else + defconfig =ADM1278_PMON_DEFCONFIG ; +
Please add a separate case statement for the new chip and do not overload existing chip data.
Quoted 19 lines
drivers/hwmon/pmbus/adm1275.c +2
-1data->have_vout = true; data->have_pin_max = true; data->have_temp_max = true;@@ -728,13 +757,14 @@ static int adm1275_probe(struct i2c_client *client)PMBUS_HAVE_VOUT |PMBUS_HAVE_STATUS_VOUT |PMBUS_HAVE_TEMP |PMBUS_HAVE_STATUS_TEMP ; - ret =adm1275_enable_vout_temp (data, client, config); + ret =adm1275_enable_vout_temp (data, client, config, defconfig);if (ret)return ret;if (config &ADM1278_VIN_EN ) info->func[0 ] |=PMBUS_HAVE_VIN ;break ; + }case adm1293:case adm1294: data->have_iout_min = true;
Re: [PATCH 3/7] hwmon: adm1275: Support ROHM BD12780
Thanks for taking the time to review this! Feedback is appreciated :)
Quoted 11 lines · Guenter Roeck
Quoted 10 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark
PMON_CONFIGbits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".
Quoted 8 lines
Quoted 8 lines
Support ROHM BD12780 and BD12780A controllers.
Signed-off-by Matti Vaittinen <[email protected]>---
drivers/hwmon/pmbus/Kconfig | 2 +-drivers/hwmon/pmbus/adm1275.c| 46 +++++++++++++++++++++++++++++------2 files changed, 39 insertions(+), 9 deletions(-)
// snip
Quoted 25 lines
Quoted 25 lines
drivers/hwmon/pmbus/adm1275.c +2
-0@@ -487,6 +489,21 @@ static const struct i2c_device_id adm1275_id[] = { { "adm1281", adm1281 }, { "adm1293", adm1293 }, { "adm1294", adm1294 }, + /* +* The BD12780a is functionally identical to BD12780(*). Even the pmbus ID
+ * register contents are same. When instantiated from the DT, it is required
+ * to have the bd12780 as a fall-back. We still need thebd12780aID here,
+ * because thei2c_device_idis created from the first compatible, not from
+ * the fall-back entry.
+ * (*)Until proven to differ. I prefer having own compatible for these
+ * variants for that day. Please note that even though the probe is called
+ * based on the 'bd12780a' -entry, the ID is picked at probe based on the
+ * pmbus register contents and not by DT entry. Thus, if the bd12780 and
+ *bd12780aare found to require different handling, then this needs to be
+ * changed, orbd12780ais handled as bd12780.
Quoted 8 lines
Quoted 3 lines
+ */
+ { "bd12780", bd12780 },
+ { "bd12780a", /* driver data unused, see --^ */ },We don't usually do that. There are various A/B/C variants for many chips, and we just use the base name unless a difference is warranted. Either this is needed, and driver data is needed as well, or it isn't. If it is not needed, it should be dropped.
At the moment the only difference I know is reduced amount of I2C slave addresses. This shouldn't be visible to this driver.
My problem is that I don't know for sure if we later notice something that requires differentiating. Thus I would like to have different DT compatibles (or other source of I2C IDs if those are used to instantiate the driver). If we don't do this, then we have problems if we later find out that these ICs require different handling as users because we can't differentiate these ICs if they are described with same compatible/I2C ID.
The "fun" thing is that both variants have exactly same MFR_MODEL and MFR_REVISION register contents. Thus, these ICs can't be differentiated by reading PMBus registers.
This is also why the driver data entry gets unused. The existing probe mechanism in this driver, scans the names in this ID list and compares it to the PMBus MFR_MODEL, and then picks-up the driver data to use. Now because BD12780 and BD12780A both have same MFR_MODEL, the scan in probe will always pick the same driver data entry, no matter what ID was matched by bus code. That's why I added the comment here.
If I drop the { "bd12780a", /* driver data unused, see --^ */ } -entry from the ID list and add the of_device_ids, then I think this problem is solved for the DT-platforms. As far as I understand, this would still cause any non DT platform to describe both variants as "bd12780", making it impossible to later differentiate ICs in the driver, right? I can do this, but for me it feels a bit like asking for problems...
My thinking was to have different IDs for these variants so hardware description could have different IDs for different ICs. Then, if we later need to differentiate these ICs, we still have an option to change the probe to trust the i2c_get_match_data() when PMBus indicates the bd12780.
This however is some extra complexity, and I would like to add it to the probe only if it really is required.
But yeah, having an ID entry in the list and driver data not used even when the matching IC is found, is unusual and would have caught me off guard. Hence I added the (long) comment.
Quoted 4 lines
Quoted 4 lines
{ "mc09c", sq24905c },
{ }
};
// snip
Quoted 9 lines
Quoted 9 lines
(quoted diff) +0
-0@@ -712,7 +732,16 @@ static int adm1275_probe(struct i2c_client *client) break;case adm1278:
case adm1281:
+ case bd12780:
case sq24905c:
+ {
+ u16 defconfig;
+
Quoted 8 lines
Quoted 5 lines
+ if (
data->id== bd12780)
+ defconfig =BD12780_PMON_DEFCONFIG;
+ else
+ defconfig =ADM1278_PMON_DEFCONFIG;
+Please add a separate case statement for the new chip and do not overload existing chip data.
I originally did just that. I, however, was not happy with this as it resulted quite long own case this IC, which is almost identical to this other (adm1278, adm1281, sq24905c) case. I wanted the code to shout that the DB12780 is indeed (almost) identical to the adm1278. But yeah, I agree, having "if (foo)" in "switch (foo)" case can get confusing.
Yours,
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Guenter Roeck
maintainerRe: [PATCH 3/7] hwmon: adm1275: Support ROHM BD12780
Quoted 58 lines · Matti Vaittinen
Thanks for taking the time to review this! Feedback is appreciated :)
Quoted 11 lines · Guenter Roeck
Quoted 10 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
ROHM BD12780 and BD12780A are hot-swap controllers. They are largely similar to Analog Devices ADM1278. Besides the ID registers and some added functionality, the BD12780 and BD12780A mark
PMON_CONFIGbits [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.The BD12780 has 3 pins usable for configuring the I2C address. The BD12780A lists the ADDR3-pin as "not connect".
Quoted 8 lines
Quoted 8 lines
Support ROHM BD12780 and BD12780A controllers.
Signed-off-by Matti Vaittinen <[email protected]>---
drivers/hwmon/pmbus/Kconfig | 2 +-drivers/hwmon/pmbus/adm1275.c| 46 +++++++++++++++++++++++++++++------2 files changed, 39 insertions(+), 9 deletions(-)// snip
Quoted 16 lines
Quoted 16 lines
drivers/hwmon/pmbus/adm1275.c +12
-0@@ -487,6 +489,21 @@ static const struct i2c_device_id adm1275_id[] = { { "adm1281", adm1281 }, { "adm1293", adm1293 }, { "adm1294", adm1294 }, + /* +* The BD12780a is functionally identical to BD12780(*). Even the pmbus ID +* register contents are same. When instantiated from the DT, it is required +* to have the bd12780 as a fall-back. We still need the bd12780a ID here, +* because the i2c_device_id is created from the first compatible, not from +* the fall-back entry. +* (*)Until proven to differ. I prefer having own compatible for these +* variants for that day. Please note that even though the probe is called +* based on the 'bd12780a' -entry, the ID is picked at probe based on the +* pmbus register contents and not by DT entry. Thus, if the bd12780 and +* bd12780a are found to require different handling, then this needs to be +* changed, or bd12780a is handled as bd12780. Quoted 8 lines
drivers/hwmon/pmbus/adm1275.c static const struct i2c_device_id adm1275_id[] = { · line 506
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +3
-0+*/ + { "bd12780", bd12780 }, + { "bd12780a", /* driver data unused, see --^ */ },We don't usually do that. There are various A/B/C variants for many chips, and we just use the base name unless a difference is warranted. Either this is needed, and driver data is needed as well, or it isn't. If it is not needed, it should be dropped.
At the moment the only difference I know is reduced amount of I2C slave addresses. This shouldn't be visible to this driver.
My problem is that I don't know for sure if we later notice something that requires differentiating. Thus I would like to have different DT compatibles (or other source of I2C IDs if those are used to instantiate the driver). If we don't do this, then we have problems if we later find out that these ICs require different handling as users because we can't differentiate these ICs if they are described with same compatible/I2C ID.
The "fun" thing is that both variants have exactly same
MFR_MODELandMFR_REVISIONregister contents. Thus, these ICs can't be differentiated by reading PMBus registers.This is also why the driver data entry gets unused. The existing probe mechanism in this driver, scans the names in this ID list and compares it to the PMBus
MFR_MODEL, and then picks-up the driver data to use. Now because BD12780 and BD12780A both have sameMFR_MODEL, the scan in probe will always pick the same driver data entry, no matter what ID was matched by bus code. That's why I added the comment here.
Quoted 8 lines
If I drop the { "
bd12780a", /* driver data unused, see --^ */ } -entry from the ID list and add theof_device_ids, then I think this problem is solved for the DT-platforms. As far as I understand, this would still cause any non DT platform to describe both variants as "bd12780", making it impossible to later differentiate ICs in the driver, right? I can do this, but for me it feels a bit like asking for problems...My thinking was to have different IDs for these variants so hardware description could have different IDs for different ICs. Then, if we later need to differentiate these ICs, we still have an option to change the probe to trust the
i2c_get_match_data()when PMBus indicates the bd12780.This however is some extra complexity, and I would like to add it to the probe only if it really is required.
But yeah, having an ID entry in the list and driver data not used even when the matching IC is found, is unusual and would have caught me off guard. Hence I added the (long) comment.
It is unusual and unnecessary. If for whatever reason it turns out to be necessary later to have a separate ID, add it then. Again, there are lots of chips with A/B/C variants. We only add separate entries for them if/when needed just for "in case".
Quoted 25 lines
Quoted 4 lines
drivers/hwmon/pmbus/adm1275.c static const struct i2c_device_id adm1275_id[] = { · line 509
Quoted 4 lines
drivers/hwmon/pmbus/adm1275.c +0
-0{ "mc09c", sq24905c }, { } };// snip
Quoted 9 lines
Quoted 9 lines
(quoted diff) +0
-0@@ -712,7 +732,16 @@ static int adm1275_probe(struct i2c_client *client) break;case adm1278:
case adm1281:
+ case bd12780:
case sq24905c:
+ {
+ u16 defconfig;
+Quoted 8 lines
Quoted 5 lines
+ if (
data->id== bd12780)
+ defconfig =BD12780_PMON_DEFCONFIG;
+ else
+ defconfig =ADM1278_PMON_DEFCONFIG;
+Please add a separate case statement for the new chip and do not overload existing chip data.
Quoted 2 lines
I originally did just that. I, however, was not happy with this as it resulted quite long own case this IC, which is almost identical to this other (adm1278, adm1281, sq24905c) case. I wanted the code to shout that the DB12780 is indeed (almost) identical to the adm1278. But yeah, I agree, having "if (foo)" in "switch (foo)" case can get confusing.
Better that than embedded if statements. At some point it would probably make sense to rework the driver to use per-chip configuration data, similar to other drivers such as lm90. But that would be a separate effort. Until then, please keep per-chip configuration in separate case statements.
Thanks,
Guenter
PATCH 4/7 dt-bindings: adm1275: ROHM BD12790 hot-swap controller
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
Support ROHM BD12790 hot-swap controller which is largely compatible with the Analog Devices adm1272.
--- Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml +5 -0
@@ -33,6 +33,9 @@ description: | https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780muv-lb-e.pdf https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + The BD12790 is a ROHM hot-swap controller, functionally similar to the + ADM1272. + properties: compatible: oneOf:
@@ -48,6 +51,7 @@ properties: - adi,adm1293 - adi,adm1294 - rohm,bd12780 + - rohm,bd12790 - silergy,mc09c # Require BD12780 as a fall-back for BD12780A.
@@ -104,6 +108,7 @@ allOf: enum: - adi,adm1272 - adi,adm1273 + - rohm,bd12790 then: properties: adi,volt-curr-sample-average:
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 4/7] dt-bindings: adm1275: ROHM BD12790 hot-swap controller
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 6 lines
From: Matti Vaittinen <[email protected]>
Support ROHM BD12790 hot-swap controller which is largely compatible with the Analog Devices adm1272.
Signed-off-by Matti Vaittinen <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH 4/7] dt-bindings: adm1275: ROHM BD12790 hot-swap controller
In reply to Matti Vaittinenpw-bot: not-applicable
PATCH 5/7 doc: adm1275: Add ROHM BD12790
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
Add the ROHM BD12790 to the list of the ICs supported by the adm1275 driver.
---
I didn't find public data-sheet yet. I will add a link when one is available.
Documentation/hwmon/adm1275.rst | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/hwmon/adm1275.rst +8 -0
@@ -83,6 +83,14 @@ Supported chips: Datasheet: https://fscdn.rohm.com/en/products/databook/datasheet/ic/power/power_switch/bd12780amuv-lb-e.pdf + * ROHM Semiconductor BD12790 + + Prefix: 'bd12790' + + Addresses scanned: - + + Datasheet: - + * Silergy SQ24905C Prefix: 'mc09c'
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 5/7] doc: adm1275: Add ROHM BD12790
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 6 lines
From: Matti Vaittinen <[email protected]>
Add the ROHM BD12790 to the list of the ICs supported by the adm1275 driver.
Signed-off-by Matti Vaittinen <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH 6/7 hwmon: adm1275: Support ROHM BD12790
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrors adm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)
---
Originally this patch was AI-generated. I did pretty much re-write the probe changes by hand, and also fixed some of the coefficient math afterwards :/ But yeah, this one was AI "assisted". :)
drivers/hwmon/pmbus/Kconfig | 4 +-- drivers/hwmon/pmbus/adm1275.c | 53 +++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 10 deletions(-)
drivers/hwmon/pmbus/Kconfig +2 -2
@@ -52,8 +52,8 @@ config SENSORS_ADM1275 help If you say yes here you get hardware monitoring support for Analog Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281, - ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and - Digital Power Monitors. + ADM1293, ADM1294, ROHM BD12780, ROHM BD12790, and SQ24905C + Hot-Swap Controller and Digital Power Monitors. This driver can also be built as a module. If so, the module will be called adm1275.
drivers/hwmon/pmbus/adm1275.c +45 -8
112 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver compares the device-returned PMBUS_MFR_MODEL string using strncasecmp without verifying the returned length, leading to uninitialized stack memory reads.
- [High] Multiplication of power and current coefficients by shunt overflows a 32-bit unsigned integer for larger shunt resistors.
--
commit 8dff4bf5659b589eabe7fc876665de9339faae1f
Author: Matti Vaittinen <[email protected]>
hwmon: adm1275: Support ROHM BD12790
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +0
-0
[ ... ]
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 612
Quoted 9 lines
drivers/hwmon/pmbus/adm1275.c +2
-1@@ -581,7 +606,8 @@ static int adm1275_probe(struct i2c_client *client)if (mid->driver_data == adm1272 || mid->driver_data == adm1273 || mid->driver_data == adm1278 || mid->driver_data == adm1281 || mid->driver_data == adm1293 || mid->driver_data == adm1294 || - mid->driver_data == bd12780 || mid->driver_data == sq24905c) + mid->driver_data == bd12780 || mid->driver_data == bd12790 || + mid->driver_data == sq24905c) config_read_fn = i2c_smbus_read_word_data;else
[Severity: High]
This is a pre-existing issue, but I noticed that just before this block in adm1275_probe(), there might be an out-of-bounds stack read:
ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
if (ret < 0) { ... }
for (mid = adm1275_id; mid->name[0]; mid++) {
if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
break;
}Does this code read uninitialized stack memory if the device returns a string that matches a valid model name prefix, but is shorter than the valid model name length?
Since i2c_smbus_read_block_data() reads up to 32 bytes into the uninitialized stack array block_buffer without appending a null terminator, strncasecmp() could read past the valid bytes returned in ret.
For example, if the device returns a shorter string like "adm12", checking it against "adm1275" up to the length of "adm1275" will continue reading into uninitialized stack bounds.
[ ... ]
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 726
Quoted 6 lines
drivers/hwmon/pmbus/adm1275.c +2
-0@@ -685,12 +722,23 @@ static int adm1275_probe(struct i2c_client *client)break ;case adm1272:case adm1273: +case bd12790: + {
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but looking further down in adm1275_probe(), could the coefficient calculations for current and power overflow a 32-bit unsigned integer?
info->m[PSC_CURRENT_OUT] =
coefficients[cindex].m * shunt / 1000;
info->m[PSC_POWER] =
coefficients[pindex].m * shunt / 1000;If a larger shunt resistor value is provided via device tree (e.g., 200,000 micro-ohms to measure lower currents), multiplying this by larger coefficients like 30631 (used for ADM1293) evaluates to 6,126,200,000.
Since shunt is a u32 and the coefficient promotes to int, this operation uses 32-bit unsigned arithmetic.
Does this wrap-around the maximum 32-bit value of 4,294,967,295 and result in drastically incorrect telemetry scaling factors?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Guenter Roeck
maintainerRe: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
Quoted 76 lines · On 6/15/26 23:59, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver compares the device-returnedPMBUS_MFR_MODELstring using strncasecmp without verifying the returned length, leading to uninitialized stack memory reads.
- [High] Multiplication of power and current coefficients byshuntoverflows a 32-bit unsigned integer for larger shunt resistors.
--commit
8dff4bf5659b589eabe7fc876665de9339faae1f
Author: Matti Vaittinen <[email protected]>hwmon: adm1275: Support ROHM BD12790
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +0
-0[ ... ]
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 612
Quoted 9 lines
drivers/hwmon/pmbus/adm1275.c +2
-1@@ -581,7 +606,8 @@ static int adm1275_probe(struct i2c_client *client)if (mid->driver_data == adm1272 || mid->driver_data == adm1273 || mid->driver_data == adm1278 || mid->driver_data == adm1281 || mid->driver_data == adm1293 || mid->driver_data == adm1294 || - mid->driver_data == bd12780 || mid->driver_data == sq24905c) + mid->driver_data == bd12780 || mid->driver_data == bd12790 || + mid->driver_data == sq24905c) config_read_fn = i2c_smbus_read_word_data;else [Severity: High]
This is a pre-existing issue, but I noticed that just before this block inadm1275_probe(), there might be an out-of-bounds stack read:ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer); if (ret < 0) { ... } for (mid = adm1275_id; mid->name[0]; mid++) { if (!strncasecmp(mid->name, block_buffer, strlen(mid->name))) break; }Does this code read uninitialized stack memory if the device returns a string that matches a valid model name prefix, but is shorter than the valid model name length?
Since
i2c_smbus_read_block_data()reads up to 32 bytes into the uninitialized stack arrayblock_bufferwithout appending a null terminator,strncasecmp()could read past the valid bytes returned in ret.For example, if the device returns a shorter string like "adm12", checking it against "adm1275" up to the length of "adm1275" will continue reading into uninitialized stack bounds.
[ ... ]
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 726
Quoted 6 lines
drivers/hwmon/pmbus/adm1275.c +2
-0@@ -685,12 +722,23 @@ static int adm1275_probe(struct i2c_client *client)break ;case adm1272:case adm1273: +case bd12790: + {[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but looking further down inadm1275_probe(), could the coefficient calculations for current and power overflow a 32-bit unsigned integer?info->m[PSC_CURRENT_OUT] = coefficients[cindex].m * shunt / 1000; info->m[PSC_POWER] = coefficients[pindex].m * shunt / 1000;If a larger shunt resistor value is provided via device tree (e.g., 200,000 micro-ohms to measure lower currents), multiplying this by larger coefficients like 30631 (used for ADM1293) evaluates to 6,126,200,000.
Quoted 6 lines
Since shunt is a u32 and the coefficient promotes to int, this operation uses 32-bit unsigned arithmetic.
Does this wrap-around the maximum 32-bit value of 4,294,967,295 and result in drastically incorrect telemetry scaling factors?
Feel freew to fix (or not to fix) those problems. They should get fixed, but not necessarily as part of this series.
Thanks,
Guenter
Guenter Roeck
maintainerRe: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
In reply to Matti VaittinenQuoted 115 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in
PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrorsadm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)Signed-off-by Matti Vaittinen <[email protected]>Assisted-by GitHub Copilot:claude-sonnet-4.6---
Originally this patch was AI-generated. I did pretty much re-write the probe changes by hand, and also fixed some of the coefficient math afterwards :/ But yeah, this one was AI "assisted". :)drivers/hwmon/pmbus/Kconfig | 4 +-- drivers/hwmon/pmbus/adm1275.c | 53 +++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 10 deletions(-)drivers/hwmon/pmbus/Kconfig +2
-2@@ -52,8 +52,8 @@ config SENSORS_ADM1275 help If you say yes here you get hardware monitoring support for Analog Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281, - ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and - Digital Power Monitors. + ADM1293, ADM1294, ROHM BD12780, ROHM BD12790, and SQ24905C + Hot-Swap Controller and Digital Power Monitors. This driver can also be built as a module. If so, the module will be called adm1275.drivers/hwmon/pmbus/adm1275.c +30
-4@@ -19,7 +19,7 @@#include "pmbus.h" enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281, - adm1293, adm1294, bd12780, sq24905c }; + adm1293, adm1294, bd12780, bd12790, sq24905c };#define ADM1275_MFR_STATUS_IOUT_WARN2 BIT(0)#define ADM1293_MFR_STATUS_VAUX_UV_WARN BIT(5)@@ -47,8 +47,8 @@ enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281,#define ADM1278_VOUT_EN BIT(1)#define ADM1278_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN | ADM1278_TSFILT) -/* The BD12780 data sheets mark TSFILT bit as reserved. */ -#define BD12780_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) +/* The BD127x0 data sheets mark TSFILT bit as reserved. */ +#define BD127X0_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)#define ADM1293_IRANGE_25 0#define ADM1293_IRANGE_50 BIT(6)@@ -136,6 +136,30 @@ static const struct coefficients adm1272_coefficients[] = { }; +/* +* BD12790 coefficients derived from preliminary datasheet, Table 1 (p.18) +* and the PMBus direct-format relationship X = (Y * 10^(-R) - b) / m. +* +* Voltage: V[V] = 14.77e-3 * code (60V) / 24.62e-3 * code (100V) +* -> m = 6770, R=-2 / m = 4062, R=-2 +* Current: code = I[A] * RS * 132802.1 + 2048 (15mV) / * 66401.06 + 2048 (30mV) +* -> m = 1328, b = 2048 * 10^(-R) = 20480, R=-1 / m = 664, same b and R +* Power: code = k * RS * PIN, k = 35119.94 / 17559.97 / 21071.44 / 10535.72 +* -> m = round(k / 10^(-R)), R=-2 for 60V/15mV, R=-3 for the other three +* Temperature: code = 4.2 * T + 3188 -> m = 42, b = 3188 * 10 = 31880, R=-1 +*/ +static const struct coefficients bd12790_coefficients[] = { + [0 ] = {6770 ,0 ,-2 },/* voltage, vrange 60V */ + [1 ] = {4062 ,0 ,-2 },/* voltage, vrange 100V */ + [2 ] = {1328 ,20480 ,-1 },/* current, vsense range 15mV */ + [3 ] = {664 ,20480 ,-1 },/* current, vsense range 30mV */ + [4 ] = {3512 ,0 ,-2 },/* power, vrange 60V, irange 15mV */ + [5 ] = {21071 ,0 ,-3 },/* power, vrange 100V, irange 15mV */ + [6 ] = {17560 ,0 ,-3 },/* power, vrange 60V, irange 30mV */ + [7 ] = {10536 ,0 ,-3 },/* power, vrange 100V, irange 30mV */ + [8 ] = {42 ,31880 ,-1 },/* temperature */ +}; +static const struct coefficients adm1275_coefficients[] = { [0 ] = {19199 ,0 ,-2 },/* voltage, vrange set */ [1 ] = {6720 ,0 ,-1 },/* voltage, vrange not set */ @@ -504,6 +528,7 @@ static const struct i2c_device_id adm1275_id[] = {*/ { "bd12780", bd12780 }, { "bd12780a", /* driver data unused, see --^ */ }, + { "bd12790", bd12790 }, { "mc09c", sq24905c }, { } };@@ -581,7 +606,8 @@ static int adm1275_probe(struct i2c_client *client)if (mid->driver_data == adm1272 || mid->driver_data == adm1273 || mid->driver_data == adm1278 || mid->driver_data == adm1281 || mid->driver_data == adm1293 || mid->driver_data == adm1294 || - mid->driver_data == bd12780 || mid->driver_data == sq24905c) + mid->driver_data == bd12780 || mid->driver_data == bd12790 || + mid->driver_data == sq24905c) config_read_fn = i2c_smbus_read_word_data;else config_read_fn = i2c_smbus_read_byte_data;
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 684
Quoted 4 lines
drivers/hwmon/pmbus/adm1275.c +1
-0@@ -655,12 +681,23 @@ static int adm1275_probe(struct i2c_client *client)break ;case adm1272:case adm1273: +case bd12790:
Please don't overload the existing case statements. Just add separate case statements for the new chips.
Thanks,
Guenter
Quoted 46 lines
drivers/hwmon/pmbus/adm1275.c +14
-4+ { +u16 defconfig; + data->have_vout = true; data->have_pin_max = true; data->have_temp_max = true; data->have_power_sampling = true; - coefficients = adm1272_coefficients; +if (data->id == bd12790) { + coefficients = bd12790_coefficients; + defconfig =BD127X0_PMON_DEFCONFIG ; + }else { + coefficients = adm1272_coefficients; + defconfig =ADM1278_PMON_DEFCONFIG ; + } + vindex = (config &ADM1275_VRANGE ) ?1 :0 ; cindex = (config &ADM1272_IRANGE ) ?3 :2 ;/* pindex depends on the combination of the above */ @@ -685,14 +722,14 @@ static int adm1275_probe(struct i2c_client *client)PMBUS_HAVE_VOUT |PMBUS_HAVE_STATUS_VOUT |PMBUS_HAVE_TEMP |PMBUS_HAVE_STATUS_TEMP ; - ret =adm1275_enable_vout_temp (data, client, config, -ADM1278_PMON_DEFCONFIG ); + ret =adm1275_enable_vout_temp (data, client, config, defconfig);if (ret)return ret;if (config &ADM1278_VIN_EN ) info->func[0 ] |=PMBUS_HAVE_VIN ;break ; + }case adm1275:if (device_config &ADM1275_IOUT_WARN2_SELECT ) data->have_oc_fault = true;@@ -738,7 +775,7 @@ static int adm1275_probe(struct i2c_client *client)u16 defconfig;if (data->id == bd12780) - defconfig =BD12780_PMON_DEFCONFIG ; + defconfig =BD127X0_PMON_DEFCONFIG ;else defconfig =ADM1278_PMON_DEFCONFIG ;
Re: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
Quoted 18 lines · Guenter Roeck
Quoted 17 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in
PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrorsadm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)Signed-off-by Matti Vaittinen <[email protected]>Assisted-by GitHub Copilot:claude-sonnet-4.6
Quoted 8 lines
Quoted 8 lines
---
Originally this patch was AI-generated. I did pretty much re-write the probe changes by hand, and also fixed some of the coefficient math afterwards :/ But yeah, this one was AI "assisted". :)drivers/hwmon/pmbus/Kconfig | 4 +--
drivers/hwmon/pmbus/adm1275.c| 53 +++++++++++++++++++++++++++++------2 files changed, 47 insertions(+), 10 deletions(-)
// snip
Quoted 9 lines
Quoted 5 lines
(quoted diff) +0
-0@@ -655,12 +681,23 @@ static int adm1275_probe(struct i2c_client *client) break;case adm1272:
case adm1273:
+ case bd12790:Please don't overload the existing case statements. Just add separate case statements for the new chips.
Hmm. Ok, although, here, same as with the BD12780, I would like the code to clearly show that the BD12790 is very very similar to another IC (adm1272). When we have own case for it, this information gets kind of lost as these cases are a tad too long to easily spot the differences. If there are any ideas how to ease spotting this while having own cases - I am keen to hear.
Yours,
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Guenter Roeck
maintainerRe: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
Quoted 40 lines · Matti Vaittinen
Quoted 18 lines · Guenter Roeck
Quoted 17 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in
PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrorsadm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)Signed-off-by Matti Vaittinen <[email protected]>Assisted-by GitHub Copilot:claude-sonnet-4.6Quoted 8 lines
Quoted 8 lines
---
Originally this patch was AI-generated. I did pretty much re-write the probe changes by hand, and also fixed some of the coefficient math afterwards :/ But yeah, this one was AI "assisted". :)drivers/hwmon/pmbus/Kconfig | 4 +--
drivers/hwmon/pmbus/adm1275.c| 53 +++++++++++++++++++++++++++++------2 files changed, 47 insertions(+), 10 deletions(-)// snip
Quoted 9 lines
Quoted 5 lines
(quoted diff) +0
-0@@ -655,12 +681,23 @@ static int adm1275_probe(struct i2c_client *client) break;case adm1272:
case adm1273:
+ case bd12790:Please don't overload the existing case statements. Just add separate case statements for the new chips.
Quoted 2 lines
Hmm. Ok, although, here, same as with the BD12780, I would like the code to clearly show that the BD12790 is very very similar to another IC (adm1272). When we have own case for it, this information gets kind of lost as these cases are a tad too long to easily spot the differences. If there are any ideas how to ease spotting this while having own cases - I am keen to hear.
You could add a comment into the case statement.
Guenter
Guenter Roeck
maintainerRe: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
In reply to Matti VaittinenQuoted 55 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in
PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrorsadm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)Signed-off-by Matti Vaittinen <[email protected]>Assisted-by GitHub Copilot:claude-sonnet-4.6---
Originally this patch was AI-generated. I did pretty much re-write the probe changes by hand, and also fixed some of the coefficient math afterwards :/ But yeah, this one was AI "assisted". :)drivers/hwmon/pmbus/Kconfig | 4 +-- drivers/hwmon/pmbus/adm1275.c | 53 +++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 10 deletions(-)drivers/hwmon/pmbus/Kconfig +2
-2@@ -52,8 +52,8 @@ config SENSORS_ADM1275 help If you say yes here you get hardware monitoring support for Analog Devices ADM1075, ADM1272, ADM1273, ADM1275, ADM1276, ADM1278, ADM1281, - ADM1293, ADM1294, ROHM BD12780, and SQ24905C Hot-Swap Controller and - Digital Power Monitors. + ADM1293, ADM1294, ROHM BD12780, ROHM BD12790, and SQ24905C + Hot-Swap Controller and Digital Power Monitors. This driver can also be built as a module. If so, the module will be called adm1275.drivers/hwmon/pmbus/adm1275.c +1
-1@@ -19,7 +19,7 @@#include "pmbus.h" enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281, - adm1293, adm1294, bd12780, sq24905c }; + adm1293, adm1294, bd12780, bd12790, sq24905c };#define ADM1275_MFR_STATUS_IOUT_WARN2 BIT(0)#define ADM1293_MFR_STATUS_VAUX_UV_WARN BIT(5)
drivers/hwmon/pmbus/adm1275.c enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281, · line 51
Quoted 7 lines
drivers/hwmon/pmbus/adm1275.c +2
-2@@ -47,8 +47,8 @@ enum chips { adm1075, adm1272, adm1273, adm1275, adm1276, adm1278, adm1281,#define ADM1278_VOUT_EN BIT(1)#define ADM1278_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN | ADM1278_TSFILT) -/* The BD12780 data sheets mark TSFILT bit as reserved. */ -#define BD12780_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) +/* The BD127x0 data sheets mark TSFILT bit as reserved. */ +#define BD127X0_PMON_DEFCONFIG (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)
Please don't use such placeholders. Just use BD12780_PMON_DEFCONFIG for both chips, similar to how the defines for all other chips are handled.
Thanks,
Guenter
Re: [PATCH 6/7] hwmon: adm1275: Support ROHM BD12790
Quoted 15 lines · Guenter Roeck
Quoted 14 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Add support for ROHM BD12790 hot-swap controller which is largely similar to Analog Devices adm1272.
The BD12790 uses the same selectable 60V/100V voltage ranges and 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE (bit 5) and IRANGE (bit 0) layout in
PMON_CONFIG. It therefore uses a dedicated coefficient table that mirrorsadm1272_coefficients, with the following differences derived from BD12790 datasheet Table 1 (p.18):
- power 60V/30mV: m=17560 (vs. 17561)
- power 100V/30mV: m=10536 (vs. 10535)
- temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)
Quoted 4 lines
Quoted 4 lines
Signed-off-by Matti Vaittinen <[email protected]>Assisted-by GitHub Copilot:claude-sonnet-4.6---
// snip
Quoted 10 lines
Quoted 6 lines
(quoted diff) +0
-2-/* The BD12780 data sheets mark TSFILT bit as reserved. */ -#define BD12780_PMON_DEFCONFIG (ADM1278_VOUT_EN |
ADM1278_TEMP1_EN)(quoted diff) +2
-0+/* The BD127x0 data sheets mark TSFILT bit as reserved. */ +#define BD127X0_PMON_DEFCONFIG (ADM1278_VOUT_EN |
ADM1278_TEMP1_EN)Please don't use such placeholders. Just use
BD12780_PMON_DEFCONFIGfor both chips, similar to how the defines for all other chips are handled.
Ok, thanks.
Yours,
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
PATCH 7/7 hwmon: adm1275: Support module auto-loading
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
Populating the spi_device_id -table is not enough to make the driver module automatically load when device-tree node for the bd12780 is parsed at boot.
Adding the of_device_id tables causes the driver module to be automatically load at boot. Testing has been done with rather old Debian system.
When inspecting the generated module-aliases with the insmod, following entries seem to be the difference:
alias: of:N*T*Crohm,bd12780C* alias: of:N*T*Crohm,bd12780
I suspect these are required for the module loading to work.
---
I did not add of_device_ids for other supported ICs as I can't verify it doesn't cause side-effects. Please let me know if you think those IDs should be added as well. I would be glad if I got more educated opinion on adding the of-IDs :) (I can squash this to 3/7 and 6/7 in next revision, and add own patch for adding of-IDs for other ICs if required).
--- drivers/hwmon/pmbus/adm1275.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/hwmon/pmbus/adm1275.c +8 -0
@@ -927,9 +927,17 @@ static int adm1275_probe(struct i2c_client *client)return pmbus_do_probe (client, info); } +static const struct of_device_id adm1275_of_match[] = { + { .compatible ="rohm,bd12780" , }, + { .compatible ="rohm,bd12790" , }, + { } +}; +MODULE_DEVICE_TABLE (of, adm1275_of_match); +static struct i2c_driver adm1275_driver = { .driver = { .name ="adm1275" , + .of_match_table = adm1275_of_match, }, .probe = adm1275_probe, .id_table = adm1275_id,
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH 7/7] hwmon: adm1275: Support module auto-loading
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The commit message incorrectly references spi_device_id instead of i2c_device_id.
Pre-existing issues:
- [High] The newly added adm1275_of_match table is incomplete, leaving module auto-loading broken for the majority of supported devices. --
commit ed700404448bb29c6f451edcac6e84d5a0f95644
Author: Matti Vaittinen <[email protected]>
hwmon: adm1275: Support module auto-loading
This commit adds an of_device_id table to the adm1275 driver to enable automatic module loading when devices are parsed from the device tree at boot.
Quoted 3 lines
hwmon: adm1275: Support module auto-loading
Populating the
spi_device_id-table is not enough to make the
[Severity: Low]
This isn't a bug, but there's a typo in the commit message. The driver is an I2C driver and uses an i2c_device_id table, not an spi_device_id table.
[ ... ]
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client) · line 932
Quoted 11 lines
drivers/hwmon/pmbus/adm1275.c +3
-0@@ -927,9 +927,17 @@ static int adm1275_probe(struct i2c_client *client)return pmbus_do_probe (client, info); } +static const struct of_device_id adm1275_of_match[] = { + { .compatible ="rohm,bd12780" , }, + { .compatible ="rohm,bd12790" , },
[Severity: High]
This is a pre-existing issue, but the newly added adm1275_of_match table appears to be incomplete.
Since udev relies on the of_device_id table to export the correct MODALIAS for auto-loading, wouldn't omitting the other devices supported by this driver (such as adm1075, adm1275, adm1276) leave auto-loading broken for those devices on DT systems?
Should we include compatible strings for the rest of the supported hardware here to ensure they also benefit from module auto-loading?
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +3
-0+ { } +}; +MODULE_DEVICE_TABLE (of, adm1275_of_match);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
Guenter Roeck
maintainerRe: [PATCH 7/7] hwmon: adm1275: Support module auto-loading
In reply to Matti VaittinenQuoted 20 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Populating the
spi_device_id-table is not enough to make the driver module automatically load when device-tree node for the bd12780 is parsed at boot.Adding the
of_device_idtables causes the driver module to be automatically load at boot. Testing has been done with rather old Debian system.When inspecting the generated module-aliases with the insmod, following entries seem to be the difference:
alias: of:N*T*Crohm,bd12780C* alias: of:N*T*Crohm,bd12780I suspect these are required for the module loading to work.
Signed-off-by Matti Vaittinen <[email protected]>
Quoted 9 lines
---
I did not add
of_device_idsfor other supported ICs as I can't verify it doesn't cause side-effects. Please let me know if you think those IDs should be added as well. I would be glad if I got more educated opinion on adding the of-IDs :) (I can squash this to 3/7 and 6/7 in next revision, and add own patch for adding of-IDs for other ICs if required).
I don't know what those side effects might be. I am much more concerned about side effects of having some of the devices in adm1275_of_match and some in adm1275_id. So, yes, please add a patch to provide adm1275_of_match for all chips supported by the driver.
Thanks,
Guenter
Quoted 26 lines
--- drivers/hwmon/pmbus/adm1275.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/hwmon/pmbus/adm1275.c +8
-0@@ -927,9 +927,17 @@ static int adm1275_probe(struct i2c_client *client)return pmbus_do_probe (client, info); } +static const struct of_device_id adm1275_of_match[] = { + { .compatible ="rohm,bd12780" , }, + { .compatible ="rohm,bd12790" , }, + { } +}; +MODULE_DEVICE_TABLE (of, adm1275_of_match); +static struct i2c_driver adm1275_driver = { .driver = { .name ="adm1275" , + .of_match_table = adm1275_of_match, }, .probe = adm1275_probe, .id_table = adm1275_id,
Re: [PATCH 7/7] hwmon: adm1275: Support module auto-loading
Quoted 31 lines · Guenter Roeck
Quoted 20 lines · Matti Vaittinen
From: Matti Vaittinen <[email protected]>
Populating the
spi_device_id-table is not enough to make the driver module automatically load when device-tree node for the bd12780 is parsed at boot.Adding the
of_device_idtables causes the driver module to be automatically load at boot. Testing has been done with rather old Debian system.When inspecting the generated module-aliases with the insmod, following entries seem to be the difference:
alias: of:N*T*Crohm,bd12780C*
alias: of:N*T*Crohm,bd12780I suspect these are required for the module loading to work.
Signed-off-by Matti Vaittinen <[email protected]>Quoted 9 lines
---
I did not add
of_device_idsfor other supported ICs as I can't verify it doesn't cause side-effects. Please let me know if you think those IDs should be added as well. I would be glad if I got more educated opinion on adding the of-IDs :) (I can squash this to 3/7 and 6/7 in next revision, and add own patch for adding of-IDs for other ICs if required).
Quoted 4 lines
I don't know what those side effects might be. I am much more concerned about side effects of having some of the devices in
adm1275_of_matchand some inadm1275_id. So, yes, please add a patch to provideadm1275_of_matchfor all chips supported by the driver.
It's nice to have an opinion on this as I was really unsure what is the right way forward. Thanks for all the help this far. I'll do that in v2.
Yours,
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~