2024-03-18 11:36:29

by Radu Sabau

[permalink] [raw]
Subject: [PATCH 0/2] Add ADP1050 support

The ADP1050 is an advanced digital controller with a PMBusā„¢
interface targeting high density, high efficiency dc-to-dc power
conversion which can measure input/output voltages, input currents
and temperature.

Radu Sabau (2):
dt-bindings: hwmon: pmbus: adp1050 : add bindings
hwmon: pmbus: adp1050 : Add driver support

.../bindings/hwmon/pmbus/adi,adp1050.yaml | 65 ++++++++++
Documentation/hwmon/adp1050.rst | 69 +++++++++++
Documentation/hwmon/index.rst | 1 +
MAINTAINERS | 8 ++
drivers/hwmon/pmbus/Kconfig | 10 ++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/adp1050.c | 111 ++++++++++++++++++
7 files changed, 265 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
create mode 100644 Documentation/hwmon/adp1050.rst
create mode 100644 drivers/hwmon/pmbus/adp1050.c

--
2.34.1



2024-03-18 12:24:36

by Radu Sabau

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: hwmon: pmbus: adp1050 : add bindings

Add dt-bindings for adp1050 digital controller for isolated power supply
with pmbus interface voltage, current and temperature monitor.

Signed-off-by: Radu Sabau <[email protected]>
---
.../bindings/hwmon/pmbus/adi,adp1050.yaml | 65 +++++++++++++++++++
MAINTAINERS | 8 +++
2 files changed, 73 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
new file mode 100644
index 000000000000..e3162d0df0e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: htpps://devicetree.org/schemas/hwmon/pmbus/adi,adp1050.yaml#
+$schema: htpps://devicetree.org/meta-schemes/core.yaml#
+
+title: Analog Devices ADP1050 digital controller with PMBus interface
+
+maintainers:
+ - Radu Sabau <[email protected]>
+
+description: |
+ The ADP1050 is used to monitor system voltages, currents and temperatures.
+ Through the PMBus interface, the ADP1050 targets isolated power supplies
+ and has four individual monitors for input/output voltage, input current
+ and temperature.
+ Datasheet:
+ https://www.analog.com/en/products/adp1050.html
+properties:
+ compatbile:
+ const: adi,adp1050
+
+ reg:
+ maxItems: 1
+
+ vcc-supply: true
+
+ adi,vin-scale-monitor:
+ description:
+ The value of the input voltage scale used by the internal ADP1050 ADC in
+ order to read correct voltage values.
+ $ref: /schemas/typees.yaml#/definitions/uint16
+ adi,iin-scale-monitor:
+ description:
+ The value of the input current scale used by the internal ADP1050 ADC in
+ order to read carrect current values.
+ $ref: /schemas/typees.yaml#/definitions/uint16
+
+required:
+ - compatible
+ - reg
+ - vcc-supply
+ - adi,vin-scale-monitor
+ - adi,iin-scale-monitor
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #adress-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <100000>;
+ adp1050@70 {
+ #adress-cells = <1>;
+ #size-cells = <0>;
+ compatible = "adi,adp1050";
+ reg = <0x70>;
+ adi,vin-scale-monitor = <0xB030>;
+ adi,iin-scale-monitor = <0x1>;
+ vcc-supply = <&vcc>;
+ };
+...
+
diff --git a/MAINTAINERS b/MAINTAINERS
index f4d7f7cb7577..c90140859988 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -479,6 +479,14 @@ L: [email protected]
S: Orphan
F: drivers/net/wireless/admtek/adm8211.*

+ADP1050 HARDWARE MONITOR DRIVER
+M: Radu Sabau <[email protected]>
+L: [email protected]
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Dcumentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
+F: drivers/hwmon/pmbus/adp1050.c
+
ADP1653 FLASH CONTROLLER DRIVER
M: Sakari Ailus <[email protected]>
L: [email protected]
--
2.34.1


2024-03-18 13:32:45

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: adp1050 : add bindings


On Mon, 18 Mar 2024 13:21:34 +0200, Radu Sabau wrote:
> Add dt-bindings for adp1050 digital controller for isolated power supply
> with pmbus interface voltage, current and temperature monitor.
>
> Signed-off-by: Radu Sabau <[email protected]>
> ---
> .../bindings/hwmon/pmbus/adi,adp1050.yaml | 65 +++++++++++++++++++
> MAINTAINERS | 8 +++
> 2 files changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.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:
Traceback (most recent call last):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 909, in resolve_from_url
document = self.store[url]
~~~~~~~~~~^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_utils.py", line 28, in __getitem__
return self.store[self.normalize(uri)]
~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
KeyError: 'htpps://devicetree.org/meta-schemes/core.yaml'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 912, in resolve_from_url
document = self.resolve_remote(url)
^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 1018, in resolve_remote
with urlopen(uri) as url:
^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 216, in urlopen
return opener.open(url, data, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 519, in open
response = self._open(req, data)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 541, in _open
return self._call_chain(self.handle_open, 'unknown',
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 496, in _call_chain
result = func(*args)
^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 1419, in unknown_open
raise URLError('unknown url type: %s' % type)
urllib.error.URLError: <urlopen error unknown url type: htpps>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/usr/local/bin/dt-doc-validate", line 8, in <module>
sys.exit(main())
^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/doc_validate.py", line 66, in main
ret |= check_doc(f)
^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/doc_validate.py", line 29, in check_doc
for error in sorted(dtsch.iter_errors(), key=lambda e: e.linecol):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/schema.py", line 120, in iter_errors
meta_schema = self.resolver.resolve_from_url(self['$schema'])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 914, in resolve_from_url
raise exceptions.RefResolutionError(exc)
jsonschema.exceptions.RefResolutionError: <urlopen error unknown url type: htpps>
Error: Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.example.dts:33.3-34.1 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-03-18 15:18:19

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: adp1050 : add bindings

On 3/18/24 04:21, Radu Sabau wrote:
> Add dt-bindings for adp1050 digital controller for isolated power supply
> with pmbus interface voltage, current and temperature monitor.
>
> Signed-off-by: Radu Sabau <[email protected]>
> ---
> .../bindings/hwmon/pmbus/adi,adp1050.yaml | 65 +++++++++++++++++++
> MAINTAINERS | 8 +++
> 2 files changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> new file mode 100644
> index 000000000000..e3162d0df0e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: htpps://devicetree.org/schemas/hwmon/pmbus/adi,adp1050.yaml#
> +$schema: htpps://devicetree.org/meta-schemes/core.yaml#
> +
> +title: Analog Devices ADP1050 digital controller with PMBus interface
> +
> +maintainers:
> + - Radu Sabau <[email protected]>
> +
> +description: |
> + The ADP1050 is used to monitor system voltages, currents and temperatures.
> + Through the PMBus interface, the ADP1050 targets isolated power supplies
> + and has four individual monitors for input/output voltage, input current
> + and temperature.
> + Datasheet:
> + https://www.analog.com/en/products/adp1050.html
> +properties:
> + compatbile:
> + const: adi,adp1050
> +
> + reg:
> + maxItems: 1
> +
> + vcc-supply: true
> +
> + adi,vin-scale-monitor:
> + description:
> + The value of the input voltage scale used by the internal ADP1050 ADC in
> + order to read correct voltage values.
> + $ref: /schemas/typees.yaml#/definitions/uint16
> + adi,iin-scale-monitor:
> + description:
> + The value of the input current scale used by the internal ADP1050 ADC in
> + order to read carrect current values.
> + $ref: /schemas/typees.yaml#/definitions/uint16
> +

I don't see value in "-monitor" in those property names.

Also, I don't see why those properties should be mandatory since the chip
has the ability to store its configuration in eeprom.

The proposed driver code disables vin and iin monitoring if the properties
are set to 0 or not provided. I disagree that this should be possible in
the first place (I don't see its value), but if disabling vin and iin
monitoring is supported it should be documented.

Last but not least, I think those values should be abstracted with some
scale instead of reflecting raw (and unexplained) register values.

Thanks,
Guenter


2024-03-18 16:11:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: adp1050 : add bindings

On 18/03/2024 12:21, Radu Sabau wrote:
> Add dt-bindings for adp1050 digital controller for isolated power supply
> with pmbus interface voltage, current and temperature monitor.
>
> Signed-off-by: Radu Sabau <[email protected]>

Subject: drop space before ':'

> ---
> .../bindings/hwmon/pmbus/adi,adp1050.yaml | 65 +++++++++++++++++++
> MAINTAINERS | 8 +++
> 2 files changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> new file mode 100644
> index 000000000000..e3162d0df0e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +

Drop

> +$id: htpps://devicetree.org/schemas/hwmon/pmbus/adi,adp1050.yaml#
> +$schema: htpps://devicetree.org/meta-schemes/core.yaml#
> +
> +title: Analog Devices ADP1050 digital controller with PMBus interface
> +
> +maintainers:
> + - Radu Sabau <[email protected]>
> +
> +description: |
> + The ADP1050 is used to monitor system voltages, currents and temperatures.
> + Through the PMBus interface, the ADP1050 targets isolated power supplies
> + and has four individual monitors for input/output voltage, input current
> + and temperature.
> + Datasheet:
> + https://www.analog.com/en/products/adp1050.html

Missing blank line

> +properties:
> + compatbile:

Typo. And you did not test it...

> + const: adi,adp1050
> +
> + reg:
> + maxItems: 1
> +
> + vcc-supply: true
> +
> + adi,vin-scale-monitor:
> + description:
> + The value of the input voltage scale used by the internal ADP1050 ADC in
> + order to read correct voltage values.
> + $ref: /schemas/typees.yaml#/definitions/uint16

Missing blank line.

> + adi,iin-scale-monitor:
> + description:
> + The value of the input current scale used by the internal ADP1050 ADC in
> + order to read carrect current values.
> + $ref: /schemas/typees.yaml#/definitions/uint16
> +
> +required:
> + - compatible
> + - reg
> + - vcc-supply
> + - adi,vin-scale-monitor
> + - adi,iin-scale-monitor
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #adress-cells = <1>;

Totally messed indentation.
Use 4 spaces for example indentation.

> + #size-cells = <0>;
> + clock-frequency = <100000>;
> + adp1050@70 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> + #adress-cells = <1>;
> + #size-cells = <0>;
> + compatible = "adi,adp1050";
> + reg = <0x70>;
> + adi,vin-scale-monitor = <0xB030>;
> + adi,iin-scale-monitor = <0x1>;
> + vcc-supply = <&vcc>;
> + };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f4d7f7cb7577..c90140859988 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -479,6 +479,14 @@ L: [email protected]
> S: Orphan
> F: drivers/net/wireless/admtek/adm8211.*
>
> +ADP1050 HARDWARE MONITOR DRIVER
> +M: Radu Sabau <[email protected]>
> +L: [email protected]
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Dcumentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml
> +F: drivers/hwmon/pmbus/adp1050.c

There is no such file...



Best regards,
Krzysztof