[PATCH v1 0/4] Support bcm6123 Bus Converter

[PATCH v1 0/4] Support bcm6123 Bus Converter

[Marcello Sylvester Bauer]

Hi,

This patchset adds support for BCM6123 Bus Converter from Vicor
Corporation.

Marcello Sylvester Bauer (3):
  dt-bindings: vendor-prefixes: add Vicor Corporation
  dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter
  pmbus: remove trailing whitespaces

Patrick Rudolph (1):
  pmbus: Add support for bcm6123 Bus Converter

 .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 drivers/hwmon/pmbus/Kconfig                   | 13 ++-
 drivers/hwmon/pmbus/Makefile                  |  1 +
 drivers/hwmon/pmbus/bcm6123.c                 | 90 +++++++++++++++++++
 5 files changed, 145 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
 create mode 100644 drivers/hwmon/pmbus/bcm6123.c

-- 
2.33.1

[PATCH v1 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation

[Marcello Sylvester Bauer]

Add vendor prefix for Vicor Corporation.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 66d6432fd781..8a2a205d6d34 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1273,6 +1273,8 @@ patternProperties:
   "^vdl,.*":
     description: Van der Laan b.v.
   "^via,.*":
+    description: Vicor Corporation
+  "^vicor,.*":
     description: VIA Technologies, Inc.
   "^videostrong,.*":
     description: Videostrong Technology Co., Ltd.
-- 
2.33.1

[PATCH v1 2/4] dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter

[Marcello Sylvester Bauer]

Add bindings for BCM6123 Bus Converter from Vicor Corporation.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
new file mode 100644
index 000000000000..5559d22e00f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/pmbus/vicor,bcm6123.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Vicor Corporation BCM6123 Bus Converter
+
+description: |
+  The BCM6123 is an isolated Fixed-Ratio DC-DC Converter,
+  operating from a 260V to 410V primary bus to deliver an unregulated
+  ratiometric secondary voltage.
+
+  Datasheet: https://www.vicorpower.com/documents/datasheets/ds_BCM6123xD1E5135yzz.pdf
+
+properties:
+  compatible:
+    enum:
+      - vicor,bcm6123
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        bcm6123@5f {
+            compatible = "vicor,bcm6123";
+            reg = <0x5f>;
+        };
+    };
-- 
2.33.1

[PATCH v1 3/4] pmbus: remove trailing whitespaces

[Marcello Sylvester Bauer]

Fix checkpatch issues by removing trailing whitespaces in Kconfig.

Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 drivers/hwmon/pmbus/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 41f6cbf96d3b..c96f7b7338bd 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -189,8 +189,8 @@ config SENSORS_LTC2978_REGULATOR
 	depends on SENSORS_LTC2978 && REGULATOR
 	help
 	  If you say yes here you get regulator support for Linear Technology
-	  LTC3880, LTC3883, LTC3884, LTC3886, LTC3887, LTC3889, LTC7880, 
-	  LTM4644, LTM4675, LTM4676, LTM4677, LTM4678, LTM4680, LTM4686, 
+	  LTC3880, LTC3883, LTC3884, LTC3886, LTC3887, LTC3889, LTC7880,
+	  LTM4644, LTM4675, LTM4676, LTM4677, LTM4678, LTM4680, LTM4686,
 	  and LTM4700.
 
 config SENSORS_LTC3815
-- 
2.33.1

[PATCH v1 4/4] pmbus: Add support for bcm6123 Bus Converter

[Marcello Sylvester Bauer]

From: Patrick Rudolph <patrick.rudolph@9elements.com>

BCM6123 is an Fixed-Ratio DC-DC Converter.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
---
 drivers/hwmon/pmbus/Kconfig   |  9 ++++
 drivers/hwmon/pmbus/Makefile  |  1 +
 drivers/hwmon/pmbus/bcm6123.c | 90 +++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/bcm6123.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index c96f7b7338bd..62dac90631c5 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -48,6 +48,15 @@ config SENSORS_ADM1275
 	  This driver can also be built as a module. If so, the module will
 	  be called adm1275.
 
+config SENSORS_BCM6123
+	tristate "Vicor BCM6123 Compatible Power Supplies"
+	help
+	  If you say yes here you get hardware monitoring support for Vicor
+	  BCM6123 Power Supplies.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called bcm6123.
+
 config SENSORS_BEL_PFE
 	tristate "Bel PFE Compatible Power Supplies"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index e5935f70c9e0..2918c2ea7bc5 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)		+= pmbus_core.o
 obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
 obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
 obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
+obj-$(CONFIG_SENSORS_BCM6123)	+= bcm6123.o
 obj-$(CONFIG_SENSORS_BEL_PFE)	+= bel-pfe.o
 obj-$(CONFIG_SENSORS_BPA_RS600)	+= bpa-rs600.o
 obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
diff --git a/drivers/hwmon/pmbus/bcm6123.c b/drivers/hwmon/pmbus/bcm6123.c
new file mode 100644
index 000000000000..78fc259bc40f
--- /dev/null
+++ b/drivers/hwmon/pmbus/bcm6123.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for Infineon bcm6123
+ *
+ * Copyright (c) 2021 9elements GmbH
+ *
+ * VOUT_MODE is not supported by the device. The driver fakes VOUT linear16
+ * mode with exponent value -8 as direct mode with m=256/b=0/R=0.
+ *
+ * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
+ * this driver does not currently support them.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pmbus.h>
+#include "pmbus.h"
+
+static struct pmbus_platform_data bcm6123_plat_data = {
+	.flags = PMBUS_NO_CAPABILITY,
+};
+
+static struct pmbus_driver_info bcm6123_info = {
+	.pages = 2,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_VOLTAGE_OUT] = direct,
+	.format[PSC_CURRENT_IN] = direct,
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_POWER] = linear,
+	.format[PSC_TEMPERATURE] = linear,
+	.m[PSC_VOLTAGE_IN] = 1,
+	.b[PSC_VOLTAGE_IN] = 0,
+	.R[PSC_VOLTAGE_IN] = 1,
+	.m[PSC_VOLTAGE_OUT] = 1,
+	.b[PSC_VOLTAGE_OUT] = 0,
+	.R[PSC_VOLTAGE_OUT] = 1,
+	.m[PSC_CURRENT_IN] = 1,
+	.b[PSC_CURRENT_IN] = 0,
+	.R[PSC_CURRENT_IN] = 3,
+	.m[PSC_CURRENT_OUT] = 1,
+	.b[PSC_CURRENT_OUT] = 0,
+	.R[PSC_CURRENT_OUT] = 2,
+	.func[0] = 0, /* Summing page without voltage readings */
+	.func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
+	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
+	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
+	    | PMBUS_HAVE_IIN | PMBUS_HAVE_POUT,
+};
+
+static int bcm6123_probe(struct i2c_client *client)
+{
+	client->dev.platform_data = &bcm6123_plat_data;
+
+	return pmbus_do_probe(client, &bcm6123_info);
+}
+
+static const struct i2c_device_id bcm6123_id[] = {
+	{"bcm6123", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, bcm6123_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id bcm6123_of_match[] = {
+	{ .compatible = "vicor,bcm6123" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm6123_of_match);
+#endif
+
+/* This is the driver that will be inserted */
+static struct i2c_driver bcm6123_driver = {
+	.driver = {
+		   .name = "bcm6123",
+		   .of_match_table = of_match_ptr(bcm6123_of_match),
+		   },
+	.probe_new = bcm6123_probe,
+	.id_table = bcm6123_id,
+};
+
+module_i2c_driver(bcm6123_driver);
+
+MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
+MODULE_DESCRIPTION("PMBus driver for Vicor bcm6123");
+MODULE_LICENSE("GPL");
-- 
2.33.1

Re: [PATCH v1 4/4] pmbus: Add support for bcm6123 Bus Converter

[Guenter Roeck]

On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> 
> BCM6123 is an Fixed-Ratio DC-DC Converter.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> ---
>   drivers/hwmon/pmbus/Kconfig   |  9 ++++
>   drivers/hwmon/pmbus/Makefile  |  1 +
>   drivers/hwmon/pmbus/bcm6123.c | 90 +++++++++++++++++++++++++++++++++++

Documentation/hwmon/bcm6123 is missing.

>   3 files changed, 100 insertions(+)
>   create mode 100644 drivers/hwmon/pmbus/bcm6123.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index c96f7b7338bd..62dac90631c5 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -48,6 +48,15 @@ config SENSORS_ADM1275
>   	  This driver can also be built as a module. If so, the module will
>   	  be called adm1275.
>   
> +config SENSORS_BCM6123
> +	tristate "Vicor BCM6123 Compatible Power Supplies"

Is this a power supply or a chip ? It can't be both.

> +	help
> +	  If you say yes here you get hardware monitoring support for Vicor
> +	  BCM6123 Power Supplies.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called bcm6123.
> +
>   config SENSORS_BEL_PFE
>   	tristate "Bel PFE Compatible Power Supplies"
>   	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index e5935f70c9e0..2918c2ea7bc5 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)		+= pmbus_core.o
>   obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
>   obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
>   obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
> +obj-$(CONFIG_SENSORS_BCM6123)	+= bcm6123.o
>   obj-$(CONFIG_SENSORS_BEL_PFE)	+= bel-pfe.o
>   obj-$(CONFIG_SENSORS_BPA_RS600)	+= bpa-rs600.o
>   obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> diff --git a/drivers/hwmon/pmbus/bcm6123.c b/drivers/hwmon/pmbus/bcm6123.c
> new file mode 100644
> index 000000000000..78fc259bc40f
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/bcm6123.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon bcm6123
> + *
Infineon ?

> + * Copyright (c) 2021 9elements GmbH
> + *
> + * VOUT_MODE is not supported by the device. The driver fakes VOUT linear16
> + * mode with exponent value -8 as direct mode with m=256/b=0/R=0.
> + *

Does it not ? The datasheet doesn't say, and the code below doesn't match
this description.

> + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
> + * this driver does not currently support them.

Does it ? There is no reference for this in the datasheet.

Overall it seems like there is a lot of cut-and-paste. Please clean this up.

> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include "pmbus.h"
> +
> +static struct pmbus_platform_data bcm6123_plat_data = {
> +	.flags = PMBUS_NO_CAPABILITY,
> +};

We should only set this flag if it is really needed.

> +
> +static struct pmbus_driver_info bcm6123_info = {
> +	.pages = 2,
> +	.format[PSC_VOLTAGE_IN] = direct,
> +	.format[PSC_VOLTAGE_OUT] = direct,
> +	.format[PSC_CURRENT_IN] = direct,
> +	.format[PSC_CURRENT_OUT] = direct,
> +	.format[PSC_POWER] = linear,
> +	.format[PSC_TEMPERATURE] = linear,
> +	.m[PSC_VOLTAGE_IN] = 1,
> +	.b[PSC_VOLTAGE_IN] = 0,
> +	.R[PSC_VOLTAGE_IN] = 1,
> +	.m[PSC_VOLTAGE_OUT] = 1,
> +	.b[PSC_VOLTAGE_OUT] = 0,
> +	.R[PSC_VOLTAGE_OUT] = 1,
> +	.m[PSC_CURRENT_IN] = 1,
> +	.b[PSC_CURRENT_IN] = 0,
> +	.R[PSC_CURRENT_IN] = 3,
> +	.m[PSC_CURRENT_OUT] = 1,
> +	.b[PSC_CURRENT_OUT] = 0,
> +	.R[PSC_CURRENT_OUT] = 2,
> +	.func[0] = 0, /* Summing page without voltage readings */

This needs further explanation. The public datasheet doesn't say anything
about multiple pages, and it doesn't really make much sense to have an
"empty" page with no information in it.

> +	.func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> +	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> +	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> +	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> +	    | PMBUS_HAVE_IIN | PMBUS_HAVE_POUT,
> +};
> +
> +static int bcm6123_probe(struct i2c_client *client)
> +{
> +	client->dev.platform_data = &bcm6123_plat_data;
> +
> +	return pmbus_do_probe(client, &bcm6123_info);
> +}
> +
> +static const struct i2c_device_id bcm6123_id[] = {
> +	{"bcm6123", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, bcm6123_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id bcm6123_of_match[] = {
> +	{ .compatible = "vicor,bcm6123" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, bcm6123_of_match);
> +#endif
> +
> +/* This is the driver that will be inserted */
> +static struct i2c_driver bcm6123_driver = {
> +	.driver = {
> +		   .name = "bcm6123",
> +		   .of_match_table = of_match_ptr(bcm6123_of_match),
> +		   },
> +	.probe_new = bcm6123_probe,
> +	.id_table = bcm6123_id,
> +};
> +
> +module_i2c_driver(bcm6123_driver);
> +
> +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
> +MODULE_DESCRIPTION("PMBus driver for Vicor bcm6123");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH v1 2/4] dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter

[Guenter Roeck]

On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
> Add bindings for BCM6123 Bus Converter from Vicor Corporation.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>

Can this be added to trivial devices instead ?

Guenter

> ---
>   .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++++++++++++
>   1 file changed, 41 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> new file mode 100644
> index 000000000000..5559d22e00f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/pmbus/vicor,bcm6123.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Vicor Corporation BCM6123 Bus Converter
> +
> +description: |
> +  The BCM6123 is an isolated Fixed-Ratio DC-DC Converter,
> +  operating from a 260V to 410V primary bus to deliver an unregulated
> +  ratiometric secondary voltage.
> +
> +  Datasheet: https://www.vicorpower.com/documents/datasheets/ds_BCM6123xD1E5135yzz.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - vicor,bcm6123
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        bcm6123@5f {
> +            compatible = "vicor,bcm6123";
> +            reg = <0x5f>;
> +        };
> +    };
> 

Re: [PATCH v1 4/4] pmbus: Add support for bcm6123 Bus Converter

[Guenter Roeck]

On 1/17/22 9:01 AM, Guenter Roeck wrote:
> On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>
>> BCM6123 is an Fixed-Ratio DC-DC Converter.
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>> ---
>>   drivers/hwmon/pmbus/Kconfig   |  9 ++++
>>   drivers/hwmon/pmbus/Makefile  |  1 +
>>   drivers/hwmon/pmbus/bcm6123.c | 90 +++++++++++++++++++++++++++++++++++
> 
> Documentation/hwmon/bcm6123 is missing.
> 
>>   3 files changed, 100 insertions(+)
>>   create mode 100644 drivers/hwmon/pmbus/bcm6123.c
>>
>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>> index c96f7b7338bd..62dac90631c5 100644
>> --- a/drivers/hwmon/pmbus/Kconfig
>> +++ b/drivers/hwmon/pmbus/Kconfig
>> @@ -48,6 +48,15 @@ config SENSORS_ADM1275
>>         This driver can also be built as a module. If so, the module will
>>         be called adm1275.
>> +config SENSORS_BCM6123
>> +    tristate "Vicor BCM6123 Compatible Power Supplies"
> 
> Is this a power supply or a chip ? It can't be both.
> 
>> +    help
>> +      If you say yes here you get hardware monitoring support for Vicor
>> +      BCM6123 Power Supplies.
>> +
>> +      This driver can also be built as a module. If so, the module will
>> +      be called bcm6123.
>> +
>>   config SENSORS_BEL_PFE
>>       tristate "Bel PFE Compatible Power Supplies"
>>       help
>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>> index e5935f70c9e0..2918c2ea7bc5 100644
>> --- a/drivers/hwmon/pmbus/Makefile
>> +++ b/drivers/hwmon/pmbus/Makefile
>> @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)        += pmbus_core.o
>>   obj-$(CONFIG_SENSORS_PMBUS)    += pmbus.o
>>   obj-$(CONFIG_SENSORS_ADM1266)    += adm1266.o
>>   obj-$(CONFIG_SENSORS_ADM1275)    += adm1275.o
>> +obj-$(CONFIG_SENSORS_BCM6123)    += bcm6123.o
>>   obj-$(CONFIG_SENSORS_BEL_PFE)    += bel-pfe.o
>>   obj-$(CONFIG_SENSORS_BPA_RS600)    += bpa-rs600.o
>>   obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
>> diff --git a/drivers/hwmon/pmbus/bcm6123.c b/drivers/hwmon/pmbus/bcm6123.c
>> new file mode 100644
>> index 000000000000..78fc259bc40f
>> --- /dev/null
>> +++ b/drivers/hwmon/pmbus/bcm6123.c
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Hardware monitoring driver for Infineon bcm6123
>> + *
> Infineon ?
> 
>> + * Copyright (c) 2021 9elements GmbH
>> + *
>> + * VOUT_MODE is not supported by the device. The driver fakes VOUT linear16
>> + * mode with exponent value -8 as direct mode with m=256/b=0/R=0.
>> + *
> 
> Does it not ? The datasheet doesn't say, and the code below doesn't match
> this description.
> 
>> + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
>> + * this driver does not currently support them.
> 
> Does it ? There is no reference for this in the datasheet.
> 
> Overall it seems like there is a lot of cut-and-paste. Please clean this up.
> 
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pmbus.h>
>> +#include "pmbus.h"
>> +
>> +static struct pmbus_platform_data bcm6123_plat_data = {
>> +    .flags = PMBUS_NO_CAPABILITY,
>> +};
> 
> We should only set this flag if it is really needed.
> 
>> +
>> +static struct pmbus_driver_info bcm6123_info = {
>> +    .pages = 2,
>> +    .format[PSC_VOLTAGE_IN] = direct,
>> +    .format[PSC_VOLTAGE_OUT] = direct,
>> +    .format[PSC_CURRENT_IN] = direct,
>> +    .format[PSC_CURRENT_OUT] = direct,
>> +    .format[PSC_POWER] = linear,
>> +    .format[PSC_TEMPERATURE] = linear,
>> +    .m[PSC_VOLTAGE_IN] = 1,
>> +    .b[PSC_VOLTAGE_IN] = 0,
>> +    .R[PSC_VOLTAGE_IN] = 1,
>> +    .m[PSC_VOLTAGE_OUT] = 1,
>> +    .b[PSC_VOLTAGE_OUT] = 0,
>> +    .R[PSC_VOLTAGE_OUT] = 1,
>> +    .m[PSC_CURRENT_IN] = 1,
>> +    .b[PSC_CURRENT_IN] = 0,
>> +    .R[PSC_CURRENT_IN] = 3,
>> +    .m[PSC_CURRENT_OUT] = 1,
>> +    .b[PSC_CURRENT_OUT] = 0,
>> +    .R[PSC_CURRENT_OUT] = 2,
>> +    .func[0] = 0, /* Summing page without voltage readings */
> 
> This needs further explanation. The public datasheet doesn't say anything
> about multiple pages, and it doesn't really make much sense to have an
> "empty" page with no information in it.
> 

Digging further, there is actually a digital supervisor (D44TL1A0) which is used
to access BCM6123, and there can be up to 4 BCM6123 connected to D44TL1A0.
Page 0 is the supervisor, and pages 1..4 provide data for the connected BCM6123s.
Page 0 does report various telemetry values, which should be supported. Pages
2..4 should be supported as well.

Question is if the driver should be named after the supervisor or after
the voltage converter; I'll leave that up to you. Either case it needs
to support all five pages.

Guenter

>> +    .func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
>> +        | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
>> +        | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
>> +        | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
>> +        | PMBUS_HAVE_IIN | PMBUS_HAVE_POUT,
>> +};
>> +
>> +static int bcm6123_probe(struct i2c_client *client)
>> +{
>> +    client->dev.platform_data = &bcm6123_plat_data;
>> +
>> +    return pmbus_do_probe(client, &bcm6123_info);
>> +}
>> +
>> +static const struct i2c_device_id bcm6123_id[] = {
>> +    {"bcm6123", 0},
>> +    {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, bcm6123_id);
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id bcm6123_of_match[] = {
>> +    { .compatible = "vicor,bcm6123" },
>> +    { },
>> +};
>> +MODULE_DEVICE_TABLE(of, bcm6123_of_match);
>> +#endif
>> +
>> +/* This is the driver that will be inserted */
>> +static struct i2c_driver bcm6123_driver = {
>> +    .driver = {
>> +           .name = "bcm6123",
>> +           .of_match_table = of_match_ptr(bcm6123_of_match),
>> +           },
>> +    .probe_new = bcm6123_probe,
>> +    .id_table = bcm6123_id,
>> +};
>> +
>> +module_i2c_driver(bcm6123_driver);
>> +
>> +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
>> +MODULE_DESCRIPTION("PMBus driver for Vicor bcm6123");
>> +MODULE_LICENSE("GPL");
>>
> 

Re: [PATCH v1 2/4] dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter

[Rob Herring]

On Mon, 17 Jan 2022 17:12:48 +0100, Marcello Sylvester Bauer wrote:
> Add bindings for BCM6123 Bus Converter from Vicor Corporation.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> ---
>  .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml: ignoring, error in schema: 
Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.example.dt.yaml:0:0: /example-0/i2c/bcm6123@5f: failed to match any schema with compatible: ['vicor,bcm6123']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1580909

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v1 2/4] dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter

[sylv]

On Mon, 2022-01-17 at 09:06 -0800, Guenter Roeck wrote:
> On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
> > Add bindings for BCM6123 Bus Converter from Vicor Corporation.
> > 
> > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> 
> Can this be added to trivial devices instead ?
> 
> Guenter

Indeed. Thanks.

Marcello

> 
> > ---
> >   .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++++++++++++
> >   1 file changed, 41 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> > new file mode 100644
> > index 000000000000..5559d22e00f1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/hwmon/pmbus/vicor,bcm6123.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Vicor Corporation BCM6123 Bus Converter
> > +
> > +description: |
> > +  The BCM6123 is an isolated Fixed-Ratio DC-DC Converter,
> > +  operating from a 260V to 410V primary bus to deliver an unregulated
> > +  ratiometric secondary voltage.
> > +
> > +  Datasheet: https://www.vicorpower.com/documents/datasheets/ds_BCM6123xD1E5135yzz.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - vicor,bcm6123
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        bcm6123@5f {
> > +            compatible = "vicor,bcm6123";
> > +            reg = <0x5f>;
> > +        };
> > +    };
> > 
> 

Re: [PATCH v1 3/4] pmbus: remove trailing whitespaces

[Guenter Roeck]

On Mon, Jan 17, 2022 at 05:12:49PM +0100, Marcello Sylvester Bauer wrote:
> Fix checkpatch issues by removing trailing whitespaces in Kconfig.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>

Applied, after updating Kconfig file. No need to resend.

> ---
>  drivers/hwmon/pmbus/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 41f6cbf96d3b..c96f7b7338bd 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -189,8 +189,8 @@ config SENSORS_LTC2978_REGULATOR
>  	depends on SENSORS_LTC2978 && REGULATOR
>  	help
>  	  If you say yes here you get regulator support for Linear Technology
> -	  LTC3880, LTC3883, LTC3884, LTC3886, LTC3887, LTC3889, LTC7880, 
> -	  LTM4644, LTM4675, LTM4676, LTM4677, LTM4678, LTM4680, LTM4686, 
> +	  LTC3880, LTC3883, LTC3884, LTC3886, LTC3887, LTC3889, LTC7880,
> +	  LTM4644, LTM4675, LTM4676, LTM4677, LTM4678, LTM4680, LTM4686,
>  	  and LTM4700.
>  
>  config SENSORS_LTC3815

Re: [PATCH v1 4/4] pmbus: Add support for bcm6123 Bus Converter

[sylv]

On Mon, 2022-01-17 at 09:21 -0800, Guenter Roeck wrote:
> On 1/17/22 9:01 AM, Guenter Roeck wrote:
> > On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
> > > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> > > 
> > > BCM6123 is an Fixed-Ratio DC-DC Converter.
> > > 
> > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> > > ---
> > >   drivers/hwmon/pmbus/Kconfig   |  9 ++++
> > >   drivers/hwmon/pmbus/Makefile  |  1 +
> > >   drivers/hwmon/pmbus/bcm6123.c | 90 +++++++++++++++++++++++++++++++++++
> > 
> > Documentation/hwmon/bcm6123 is missing.
> > 
> > >   3 files changed, 100 insertions(+)
> > >   create mode 100644 drivers/hwmon/pmbus/bcm6123.c
> > > 
> > > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > > index c96f7b7338bd..62dac90631c5 100644
> > > --- a/drivers/hwmon/pmbus/Kconfig
> > > +++ b/drivers/hwmon/pmbus/Kconfig
> > > @@ -48,6 +48,15 @@ config SENSORS_ADM1275
> > >         This driver can also be built as a module. If so, the module will
> > >         be called adm1275.
> > > +config SENSORS_BCM6123
> > > +    tristate "Vicor BCM6123 Compatible Power Supplies"
> > 
> > Is this a power supply or a chip ? It can't be both.
> > 
> > > +    help
> > > +      If you say yes here you get hardware monitoring support for Vicor
> > > +      BCM6123 Power Supplies.
> > > +
> > > +      This driver can also be built as a module. If so, the module will
> > > +      be called bcm6123.
> > > +
> > >   config SENSORS_BEL_PFE
> > >       tristate "Bel PFE Compatible Power Supplies"
> > >       help
> > > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> > > index e5935f70c9e0..2918c2ea7bc5 100644
> > > --- a/drivers/hwmon/pmbus/Makefile
> > > +++ b/drivers/hwmon/pmbus/Makefile
> > > @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)        += pmbus_core.o
> > >   obj-$(CONFIG_SENSORS_PMBUS)    += pmbus.o
> > >   obj-$(CONFIG_SENSORS_ADM1266)    += adm1266.o
> > >   obj-$(CONFIG_SENSORS_ADM1275)    += adm1275.o
> > > +obj-$(CONFIG_SENSORS_BCM6123)    += bcm6123.o
> > >   obj-$(CONFIG_SENSORS_BEL_PFE)    += bel-pfe.o
> > >   obj-$(CONFIG_SENSORS_BPA_RS600)    += bpa-rs600.o
> > >   obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> > > diff --git a/drivers/hwmon/pmbus/bcm6123.c b/drivers/hwmon/pmbus/bcm6123.c
> > > new file mode 100644
> > > index 000000000000..78fc259bc40f
> > > --- /dev/null
> > > +++ b/drivers/hwmon/pmbus/bcm6123.c
> > > @@ -0,0 +1,90 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Hardware monitoring driver for Infineon bcm6123
> > > + *
> > Infineon ?
> > 
> > > + * Copyright (c) 2021 9elements GmbH
> > > + *
> > > + * VOUT_MODE is not supported by the device. The driver fakes VOUT linear16
> > > + * mode with exponent value -8 as direct mode with m=256/b=0/R=0.
> > > + *
> > 
> > Does it not ? The datasheet doesn't say, and the code below doesn't match
> > this description.
> > 
> > > + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
> > > + * this driver does not currently support them.
> > 
> > Does it ? There is no reference for this in the datasheet.
> > 
> > Overall it seems like there is a lot of cut-and-paste. Please clean this up.

Sorry. Looks like it is not related to this driver and should be
removed.

> > 
> > > + */
> > > +
> > > +#include <linux/err.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/init.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pmbus.h>
> > > +#include "pmbus.h"
> > > +
> > > +static struct pmbus_platform_data bcm6123_plat_data = {
> > > +    .flags = PMBUS_NO_CAPABILITY,
> > > +};
> > 
> > We should only set this flag if it is really needed.

The capability command did not return valid information during testing.
e.g. it claimed PEC is supported and caused therefore unexpected
issues. I'll test it again to see if it's necessary for the driver to
work.

> > 
> > > +
> > > +static struct pmbus_driver_info bcm6123_info = {
> > > +    .pages = 2,
> > > +    .format[PSC_VOLTAGE_IN] = direct,
> > > +    .format[PSC_VOLTAGE_OUT] = direct,
> > > +    .format[PSC_CURRENT_IN] = direct,
> > > +    .format[PSC_CURRENT_OUT] = direct,
> > > +    .format[PSC_POWER] = linear,
> > > +    .format[PSC_TEMPERATURE] = linear,
> > > +    .m[PSC_VOLTAGE_IN] = 1,
> > > +    .b[PSC_VOLTAGE_IN] = 0,
> > > +    .R[PSC_VOLTAGE_IN] = 1,
> > > +    .m[PSC_VOLTAGE_OUT] = 1,
> > > +    .b[PSC_VOLTAGE_OUT] = 0,
> > > +    .R[PSC_VOLTAGE_OUT] = 1,
> > > +    .m[PSC_CURRENT_IN] = 1,
> > > +    .b[PSC_CURRENT_IN] = 0,
> > > +    .R[PSC_CURRENT_IN] = 3,
> > > +    .m[PSC_CURRENT_OUT] = 1,
> > > +    .b[PSC_CURRENT_OUT] = 0,
> > > +    .R[PSC_CURRENT_OUT] = 2,
> > > +    .func[0] = 0, /* Summing page without voltage readings */
> > 
> > This needs further explanation. The public datasheet doesn't say anything
> > about multiple pages, and it doesn't really make much sense to have an
> > "empty" page with no information in it.
> > 
> 
> Digging further, there is actually a digital supervisor (D44TL1A0) which is used
> to access BCM6123, and there can be up to 4 BCM6123 connected to D44TL1A0.
> Page 0 is the supervisor, and pages 1..4 provide data for the connected BCM6123s.
> Page 0 does report various telemetry values, which should be supported. Pages
> 2..4 should be supported as well.
> 
> Question is if the driver should be named after the supervisor or after
> the voltage converter; I'll leave that up to you. Either case it needs
> to support all five pages.
> 
> Guenter

After Digging further ourselves, it looks like our hardware also
contains a digital supervisor IC (PLI1209BC), which is connected to one
BCM6123 only. Unfortunately, we were not able to find the data sheet
before:

https://www.vicorpower.com/documents/datasheets/ds-PLI1209BCxyzz-VICOR.pdf
This explains why there are two pages in our case. Since there are
different hardware configurations it would make more sense to name this
driver after the supervisor.

I'll rework this patch set and implement the PLI1209BC properly.

Marcello

> 
> > > +    .func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> > > +        | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> > > +        | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> > > +        | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> > > +        | PMBUS_HAVE_IIN | PMBUS_HAVE_POUT,
> > > +};
> > > +
> > > +static int bcm6123_probe(struct i2c_client *client)
> > > +{
> > > +    client->dev.platform_data = &bcm6123_plat_data;
> > > +
> > > +    return pmbus_do_probe(client, &bcm6123_info);
> > > +}
> > > +
> > > +static const struct i2c_device_id bcm6123_id[] = {
> > > +    {"bcm6123", 0},
> > > +    {}
> > > +};
> > > +
> > > +MODULE_DEVICE_TABLE(i2c, bcm6123_id);
> > > +
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id bcm6123_of_match[] = {
> > > +    { .compatible = "vicor,bcm6123" },
> > > +    { },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, bcm6123_of_match);
> > > +#endif
> > > +
> > > +/* This is the driver that will be inserted */
> > > +static struct i2c_driver bcm6123_driver = {
> > > +    .driver = {
> > > +           .name = "bcm6123",
> > > +           .of_match_table = of_match_ptr(bcm6123_of_match),
> > > +           },
> > > +    .probe_new = bcm6123_probe,
> > > +    .id_table = bcm6123_id,
> > > +};
> > > +
> > > +module_i2c_driver(bcm6123_driver);
> > > +
> > > +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
> > > +MODULE_DESCRIPTION("PMBus driver for Vicor bcm6123");
> > > +MODULE_LICENSE("GPL");
> > > 
> > 
> 

Re: [PATCH v1 4/4] pmbus: Add support for bcm6123 Bus Converter

[Guenter Roeck]

On 1/18/22 8:47 AM, sylv wrote:
> On Mon, 2022-01-17 at 09:21 -0800, Guenter Roeck wrote:
>> On 1/17/22 9:01 AM, Guenter Roeck wrote:
>>> On 1/17/22 8:12 AM, Marcello Sylvester Bauer wrote:
>>>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>>
>>>> BCM6123 is an Fixed-Ratio DC-DC Converter.
>>>>
>>>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>>>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>>>> ---
>>>>    drivers/hwmon/pmbus/Kconfig   |  9 ++++
>>>>    drivers/hwmon/pmbus/Makefile  |  1 +
>>>>    drivers/hwmon/pmbus/bcm6123.c | 90 +++++++++++++++++++++++++++++++++++
>>>
>>> Documentation/hwmon/bcm6123 is missing.
>>>
>>>>    3 files changed, 100 insertions(+)
>>>>    create mode 100644 drivers/hwmon/pmbus/bcm6123.c
>>>>
>>>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
>>>> index c96f7b7338bd..62dac90631c5 100644
>>>> --- a/drivers/hwmon/pmbus/Kconfig
>>>> +++ b/drivers/hwmon/pmbus/Kconfig
>>>> @@ -48,6 +48,15 @@ config SENSORS_ADM1275
>>>>          This driver can also be built as a module. If so, the module will
>>>>          be called adm1275.
>>>> +config SENSORS_BCM6123
>>>> +    tristate "Vicor BCM6123 Compatible Power Supplies"
>>>
>>> Is this a power supply or a chip ? It can't be both.
>>>
>>>> +    help
>>>> +      If you say yes here you get hardware monitoring support for Vicor
>>>> +      BCM6123 Power Supplies.
>>>> +
>>>> +      This driver can also be built as a module. If so, the module will
>>>> +      be called bcm6123.
>>>> +
>>>>    config SENSORS_BEL_PFE
>>>>        tristate "Bel PFE Compatible Power Supplies"
>>>>        help
>>>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
>>>> index e5935f70c9e0..2918c2ea7bc5 100644
>>>> --- a/drivers/hwmon/pmbus/Makefile
>>>> +++ b/drivers/hwmon/pmbus/Makefile
>>>> @@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS)        += pmbus_core.o
>>>>    obj-$(CONFIG_SENSORS_PMBUS)    += pmbus.o
>>>>    obj-$(CONFIG_SENSORS_ADM1266)    += adm1266.o
>>>>    obj-$(CONFIG_SENSORS_ADM1275)    += adm1275.o
>>>> +obj-$(CONFIG_SENSORS_BCM6123)    += bcm6123.o
>>>>    obj-$(CONFIG_SENSORS_BEL_PFE)    += bel-pfe.o
>>>>    obj-$(CONFIG_SENSORS_BPA_RS600)    += bpa-rs600.o
>>>>    obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
>>>> diff --git a/drivers/hwmon/pmbus/bcm6123.c b/drivers/hwmon/pmbus/bcm6123.c
>>>> new file mode 100644
>>>> index 000000000000..78fc259bc40f
>>>> --- /dev/null
>>>> +++ b/drivers/hwmon/pmbus/bcm6123.c
>>>> @@ -0,0 +1,90 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Hardware monitoring driver for Infineon bcm6123
>>>> + *
>>> Infineon ?
>>>
>>>> + * Copyright (c) 2021 9elements GmbH
>>>> + *
>>>> + * VOUT_MODE is not supported by the device. The driver fakes VOUT linear16
>>>> + * mode with exponent value -8 as direct mode with m=256/b=0/R=0.
>>>> + *
>>>
>>> Does it not ? The datasheet doesn't say, and the code below doesn't match
>>> this description.
>>>
>>>> + * The device supports VOUT_PEAK, IOUT_PEAK, and TEMPERATURE_PEAK, however
>>>> + * this driver does not currently support them.
>>>
>>> Does it ? There is no reference for this in the datasheet.
>>>
>>> Overall it seems like there is a lot of cut-and-paste. Please clean this up.
> 
> Sorry. Looks like it is not related to this driver and should be
> removed.
> 
>>>
>>>> + */
>>>> +
>>>> +#include <linux/err.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/pmbus.h>
>>>> +#include "pmbus.h"
>>>> +
>>>> +static struct pmbus_platform_data bcm6123_plat_data = {
>>>> +    .flags = PMBUS_NO_CAPABILITY,
>>>> +};
>>>
>>> We should only set this flag if it is really needed.
> 
> The capability command did not return valid information during testing.
> e.g. it claimed PEC is supported and caused therefore unexpected
> issues. I'll test it again to see if it's necessary for the driver to
> work.
> 
>>>
>>>> +
>>>> +static struct pmbus_driver_info bcm6123_info = {
>>>> +    .pages = 2,
>>>> +    .format[PSC_VOLTAGE_IN] = direct,
>>>> +    .format[PSC_VOLTAGE_OUT] = direct,
>>>> +    .format[PSC_CURRENT_IN] = direct,
>>>> +    .format[PSC_CURRENT_OUT] = direct,
>>>> +    .format[PSC_POWER] = linear,
>>>> +    .format[PSC_TEMPERATURE] = linear,
>>>> +    .m[PSC_VOLTAGE_IN] = 1,
>>>> +    .b[PSC_VOLTAGE_IN] = 0,
>>>> +    .R[PSC_VOLTAGE_IN] = 1,
>>>> +    .m[PSC_VOLTAGE_OUT] = 1,
>>>> +    .b[PSC_VOLTAGE_OUT] = 0,
>>>> +    .R[PSC_VOLTAGE_OUT] = 1,
>>>> +    .m[PSC_CURRENT_IN] = 1,
>>>> +    .b[PSC_CURRENT_IN] = 0,
>>>> +    .R[PSC_CURRENT_IN] = 3,
>>>> +    .m[PSC_CURRENT_OUT] = 1,
>>>> +    .b[PSC_CURRENT_OUT] = 0,
>>>> +    .R[PSC_CURRENT_OUT] = 2,
>>>> +    .func[0] = 0, /* Summing page without voltage readings */
>>>
>>> This needs further explanation. The public datasheet doesn't say anything
>>> about multiple pages, and it doesn't really make much sense to have an
>>> "empty" page with no information in it.
>>>
>>
>> Digging further, there is actually a digital supervisor (D44TL1A0) which is used
>> to access BCM6123, and there can be up to 4 BCM6123 connected to D44TL1A0.
>> Page 0 is the supervisor, and pages 1..4 provide data for the connected BCM6123s.
>> Page 0 does report various telemetry values, which should be supported. Pages
>> 2..4 should be supported as well.
>>
>> Question is if the driver should be named after the supervisor or after
>> the voltage converter; I'll leave that up to you. Either case it needs
>> to support all five pages.
>>
>> Guenter
> 
> After Digging further ourselves, it looks like our hardware also
> contains a digital supervisor IC (PLI1209BC), which is connected to one
> BCM6123 only. Unfortunately, we were not able to find the data sheet
> before:
> 
> https://www.vicorpower.com/documents/datasheets/ds-PLI1209BCxyzz-VICOR.pdf
> This explains why there are two pages in our case. Since there are
> different hardware configurations it would make more sense to name this
> driver after the supervisor.
> 
> I'll rework this patch set and implement the PLI1209BC properly.
> 

Apparently there are (at least) two different supervisor chips,
PLI1209BC and D44TL1A0. Given that, it is ok if you only implement
support for PLI1209BC is that is what you use, but the driver name
should indeed reflect the supervisor chip to ensure that support
for the other supervisor chip can be added at a later point.

Thanks,
Guenter

> Marcello
> 
>>
>>>> +    .func[1] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
>>>> +        | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
>>>> +        | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
>>>> +        | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
>>>> +        | PMBUS_HAVE_IIN | PMBUS_HAVE_POUT,
>>>> +};
>>>> +
>>>> +static int bcm6123_probe(struct i2c_client *client)
>>>> +{
>>>> +    client->dev.platform_data = &bcm6123_plat_data;
>>>> +
>>>> +    return pmbus_do_probe(client, &bcm6123_info);
>>>> +}
>>>> +
>>>> +static const struct i2c_device_id bcm6123_id[] = {
>>>> +    {"bcm6123", 0},
>>>> +    {}
>>>> +};
>>>> +
>>>> +MODULE_DEVICE_TABLE(i2c, bcm6123_id);
>>>> +
>>>> +#ifdef CONFIG_OF
>>>> +static const struct of_device_id bcm6123_of_match[] = {
>>>> +    { .compatible = "vicor,bcm6123" },
>>>> +    { },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, bcm6123_of_match);
>>>> +#endif
>>>> +
>>>> +/* This is the driver that will be inserted */
>>>> +static struct i2c_driver bcm6123_driver = {
>>>> +    .driver = {
>>>> +           .name = "bcm6123",
>>>> +           .of_match_table = of_match_ptr(bcm6123_of_match),
>>>> +           },
>>>> +    .probe_new = bcm6123_probe,
>>>> +    .id_table = bcm6123_id,
>>>> +};
>>>> +
>>>> +module_i2c_driver(bcm6123_driver);
>>>> +
>>>> +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
>>>> +MODULE_DESCRIPTION("PMBus driver for Vicor bcm6123");
>>>> +MODULE_LICENSE("GPL");
>>>>
>>>
>>
> 

Re: [PATCH v1 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation

[Rob Herring]

On Mon, Jan 17, 2022 at 05:12:47PM +0100, Marcello Sylvester Bauer wrote:
> Add vendor prefix for Vicor Corporation.
> 
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 66d6432fd781..8a2a205d6d34 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1273,6 +1273,8 @@ patternProperties:
>    "^vdl,.*":
>      description: Van der Laan b.v.
>    "^via,.*":
> +    description: Vicor Corporation

You just changed the description for VIA.

> +  "^vicor,.*":
>      description: VIA Technologies, Inc.
>    "^videostrong,.*":
>      description: Videostrong Technology Co., Ltd.
> -- 
> 2.33.1
> 
> 

Re: [PATCH v1 1/4] dt-bindings: vendor-prefixes: add Vicor Corporation

[sylv]

On Tue, 2022-02-08 at 20:50 -0600, Rob Herring wrote:
> On Mon, Jan 17, 2022 at 05:12:47PM +0100, Marcello Sylvester Bauer
> wrote:
> > Add vendor prefix for Vicor Corporation.
> > 
> > Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> > ---
> >  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index 66d6432fd781..8a2a205d6d34 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -1273,6 +1273,8 @@ patternProperties:
> >    "^vdl,.*":
> >      description: Van der Laan b.v.
> >    "^via,.*":
> > +    description: Vicor Corporation
> 
> You just changed the description for VIA.

Indeed. My bad.

> 
> > +  "^vicor,.*":
> >      description: VIA Technologies, Inc.
> >    "^videostrong,.*":
> >      description: Videostrong Technology Co., Ltd.
> > -- 
> > 2.33.1
> > 
> > 

Re: [PATCH v1 0/4] Support bcm6123 Bus Converter

[sylv]

Hi,

since this driver actually implements the supervisor pli1209bc,
see this patch set as discontinued. A new patch set will be uploaded
soon.

Thanks,
Marcello

On Mon, 2022-01-17 at 17:12 +0100, Marcello Sylvester Bauer wrote:
> Hi,
> 
> This patchset adds support for BCM6123 Bus Converter from Vicor
> Corporation.
> 
> Marcello Sylvester Bauer (3):
>   dt-bindings: vendor-prefixes: add Vicor Corporation
>   dt-bindings: hwmon/pmbus: Add vicor,bcm6123 Bus Converter
>   pmbus: remove trailing whitespaces
> 
> Patrick Rudolph (1):
>   pmbus: Add support for bcm6123 Bus Converter
> 
>  .../bindings/hwmon/pmbus/vicor,bcm6123.yaml   | 41 +++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  drivers/hwmon/pmbus/Kconfig                   | 13 ++-
>  drivers/hwmon/pmbus/Makefile                  |  1 +
>  drivers/hwmon/pmbus/bcm6123.c                 | 90
> +++++++++++++++++++
>  5 files changed, 145 insertions(+), 2 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/hwmon/pmbus/vicor,bcm6123.yaml
>  create mode 100644 drivers/hwmon/pmbus/bcm6123.c
>