diff mbox series

riscv: dts: sifive unmatched: Expose the FU740 core supply regulator.

Message ID 0879c5b0c72b9bf6bf71f880def166f8804f41c7.1637023980.git.plr.vincent@gmail.com (mailing list archive)
State New, archived
Headers show
Series riscv: dts: sifive unmatched: Expose the FU740 core supply regulator. | expand

Commit Message

Vincent Pelletier Nov. 16, 2021, 12:52 a.m. UTC
Provides monitoring of core voltage and current:
tps544b20-i2c-0-1e
Adapter: i2c-ocores
vout1:       906.00 mV
temp1:        -40.0°C  (high = +125.0°C, crit = +150.0°C)
iout1:         5.06 A  (max = +20.00 A, crit max = +26.00 A)

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>

--
Note for review: this patch has one warning from checkpatch.pl:
  WARNING: DT compatible string "tps544b20" appears un-documented -- check ./Documentation/devicetree/bindings/
  #32: FILE: arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts:55:
  +               compatible = "tps544b20";
This chip is handled by the existing pmbus module, and there is indeed no
matching entry in Documentation/devicetree/bindings/hwmon/pmbus. I am not
especially knowledgeable about this chip, I only know it is used by this
board, so I am not sure I can do the best job in putting such a file
together.
If needed I can git it a try.
---
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Heiko Stübner Nov. 16, 2021, 9:53 a.m. UTC | #1
Hi Vincent,

Am Dienstag, 16. November 2021, 01:52:59 CET schrieb Vincent Pelletier:
> Provides monitoring of core voltage and current:
> tps544b20-i2c-0-1e
> Adapter: i2c-ocores
> vout1:       906.00 mV
> temp1:        -40.0°C  (high = +125.0°C, crit = +150.0°C)
> iout1:         5.06 A  (max = +20.00 A, crit max = +26.00 A)
> 
> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
> 
> --
> Note for review: this patch has one warning from checkpatch.pl:
>   WARNING: DT compatible string "tps544b20" appears un-documented -- check ./Documentation/devicetree/bindings/
>   #32: FILE: arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts:55:
>   +               compatible = "tps544b20";
> This chip is handled by the existing pmbus module, and there is indeed no
> matching entry in Documentation/devicetree/bindings/hwmon/pmbus. I am not
> especially knowledgeable about this chip, I only know it is used by this
> board, so I am not sure I can do the best job in putting such a file
> together.
> If needed I can git it a try.

Devicetree bindings are supposed to be stable into the future, so an actually
reviewed binding is quite necessary ;-) .

In the case of your tps544b20 it should also be pretty easy to do, as

	Documentation/devicetree/bindings/hwmon/pmbus/ti,ucd90320.yaml

is probably a pretty good match to what you need in terms of Yaml notation.
Just need to replace the naming in your copy and drop in the correct
description from

	https://www.ti.com/lit/ds/symlink/tps544b20.pdf?ts=1637055780278

and you have a working binding.

Then just add another patch to your series that mimics

	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a36e38d8b0fbb92609e837a67f919202ec7ec51

and include the relevant maintainers that scripts/get_maintainer.pl will
give you, and you're all set :-)


> ---
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index 270360b258b7..e327831d0d48 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -51,6 +51,11 @@ &uart1 {
>  &i2c0 {
>  	status = "okay";
>  
> +	tps544b20@1e {
> +		compatible = "tps544b20";

This should definitly be
		compatible = "ti,tps544b20";

i.e. include the ti vendor-prefix. The i2c-core will automatically
remove this when matching against the i2c devices.

Heiko


> +		reg = <0x1e>;
> +	};
> +
>  	temperature-sensor@4c {
>  		compatible = "ti,tmp451";
>  		reg = <0x4c>;
>
Krzysztof Kozlowski Nov. 16, 2021, 10:44 a.m. UTC | #2
On 16/11/2021 01:52, Vincent Pelletier wrote:
> Provides monitoring of core voltage and current:
> tps544b20-i2c-0-1e
> Adapter: i2c-ocores
> vout1:       906.00 mV
> temp1:        -40.0°C  (high = +125.0°C, crit = +150.0°C)
> iout1:         5.06 A  (max = +20.00 A, crit max = +26.00 A)
> 
> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
> 
> --
> Note for review: this patch has one warning from checkpatch.pl:
>   WARNING: DT compatible string "tps544b20" appears un-documented -- check ./Documentation/devicetree/bindings/
>   #32: FILE: arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts:55:
>   +               compatible = "tps544b20";
> This chip is handled by the existing pmbus module, and there is indeed no
> matching entry in Documentation/devicetree/bindings/hwmon/pmbus. I am not
> especially knowledgeable about this chip, I only know it is used by this
> board, so I am not sure I can do the best job in putting such a file
> together.
> If needed I can git it a try.

It's not required. I can try adding it.

> ---
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index 270360b258b7..e327831d0d48 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -51,6 +51,11 @@ &uart1 {
>  &i2c0 {
>  	status = "okay";
>  
> +	tps544b20@1e {

Node name should be a generic class of a device. This is a DC-DC
converter, so I suppose we should name it "regulator"?

> +		compatible = "tps544b20";
> +		reg = <0x1e>;
> +	};
> +
>  	temperature-sensor@4c {
>  		compatible = "ti,tmp451";
>  		reg = <0x4c>;
> 


Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 16, 2021, 11:08 a.m. UTC | #3
On 16/11/2021 10:53, Heiko Stübner wrote:
> Hi Vincent,
> 
> Am Dienstag, 16. November 2021, 01:52:59 CET schrieb Vincent Pelletier:
>> Provides monitoring of core voltage and current:
>> tps544b20-i2c-0-1e
>> Adapter: i2c-ocores
>> vout1:       906.00 mV
>> temp1:        -40.0°C  (high = +125.0°C, crit = +150.0°C)
>> iout1:         5.06 A  (max = +20.00 A, crit max = +26.00 A)
>>
>> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
>>
>> --
>> Note for review: this patch has one warning from checkpatch.pl:
>>   WARNING: DT compatible string "tps544b20" appears un-documented -- check ./Documentation/devicetree/bindings/
>>   #32: FILE: arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts:55:
>>   +               compatible = "tps544b20";
>> This chip is handled by the existing pmbus module, and there is indeed no
>> matching entry in Documentation/devicetree/bindings/hwmon/pmbus. I am not
>> especially knowledgeable about this chip, I only know it is used by this
>> board, so I am not sure I can do the best job in putting such a file
>> together.
>> If needed I can git it a try.
> 
> Devicetree bindings are supposed to be stable into the future, so an actually
> reviewed binding is quite necessary ;-) .
> 
> In the case of your tps544b20 it should also be pretty easy to do, as
> 
> 	Documentation/devicetree/bindings/hwmon/pmbus/ti,ucd90320.yaml
> 
> is probably a pretty good match to what you need in terms of Yaml notation.
> Just need to replace the naming in your copy and drop in the correct
> description from
> 
> 	https://www.ti.com/lit/ds/symlink/tps544b20.pdf?ts=1637055780278
> 
> and you have a working binding.
> 
> Then just add another patch to your series that mimics
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a36e38d8b0fbb92609e837a67f919202ec7ec51
> 
> and include the relevant maintainers that scripts/get_maintainer.pl will
> give you, and you're all set :-)
> 

Hi Heiko,

In current form the bindings would be close to trivial and we actually
do not know how proper bindings would look like (the device is not
trivial). Therefore based on Rob's recent comments - better to have
trivial schema than nothing - I sent a patch adding them to trivial-devices:

https://lore.kernel.org/linux-devicetree/20211116110207.68494-1-krzysztof.kozlowski@canonical.com/T/#u


Best regards,
Krzysztof
Heiko Stübner Nov. 16, 2021, 11:13 a.m. UTC | #4
Am Dienstag, 16. November 2021, 12:08:01 CET schrieb Krzysztof Kozlowski:
> On 16/11/2021 10:53, Heiko Stübner wrote:
> > Hi Vincent,
> > 
> > Am Dienstag, 16. November 2021, 01:52:59 CET schrieb Vincent Pelletier:
> >> Provides monitoring of core voltage and current:
> >> tps544b20-i2c-0-1e
> >> Adapter: i2c-ocores
> >> vout1:       906.00 mV
> >> temp1:        -40.0°C  (high = +125.0°C, crit = +150.0°C)
> >> iout1:         5.06 A  (max = +20.00 A, crit max = +26.00 A)
> >>
> >> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
> >>
> >> --
> >> Note for review: this patch has one warning from checkpatch.pl:
> >>   WARNING: DT compatible string "tps544b20" appears un-documented -- check ./Documentation/devicetree/bindings/
> >>   #32: FILE: arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts:55:
> >>   +               compatible = "tps544b20";
> >> This chip is handled by the existing pmbus module, and there is indeed no
> >> matching entry in Documentation/devicetree/bindings/hwmon/pmbus. I am not
> >> especially knowledgeable about this chip, I only know it is used by this
> >> board, so I am not sure I can do the best job in putting such a file
> >> together.
> >> If needed I can git it a try.
> > 
> > Devicetree bindings are supposed to be stable into the future, so an actually
> > reviewed binding is quite necessary ;-) .
> > 
> > In the case of your tps544b20 it should also be pretty easy to do, as
> > 
> > 	Documentation/devicetree/bindings/hwmon/pmbus/ti,ucd90320.yaml
> > 
> > is probably a pretty good match to what you need in terms of Yaml notation.
> > Just need to replace the naming in your copy and drop in the correct
> > description from
> > 
> > 	https://www.ti.com/lit/ds/symlink/tps544b20.pdf?ts=1637055780278
> > 
> > and you have a working binding.
> > 
> > Then just add another patch to your series that mimics
> > 
> > 	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a36e38d8b0fbb92609e837a67f919202ec7ec51
> > 
> > and include the relevant maintainers that scripts/get_maintainer.pl will
> > give you, and you're all set :-)
> > 
> 
> Hi Heiko,
> 
> In current form the bindings would be close to trivial and we actually
> do not know how proper bindings would look like (the device is not
> trivial). Therefore based on Rob's recent comments - better to have
> trivial schema than nothing - I sent a patch adding them to trivial-devices:
> 
> https://lore.kernel.org/linux-devicetree/20211116110207.68494-1-krzysztof.kozlowski@canonical.com/T/#u

Though I guess there isn't anything hindering additions to a individual
simpler binding.

But yeah, just adding it to trivial devices will also just work for now, as there
really are no additional properties right now and might make the process
a tad shorter ;-)


Heiko
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index 270360b258b7..e327831d0d48 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -51,6 +51,11 @@  &uart1 {
 &i2c0 {
 	status = "okay";
 
+	tps544b20@1e {
+		compatible = "tps544b20";
+		reg = <0x1e>;
+	};
+
 	temperature-sensor@4c {
 		compatible = "ti,tmp451";
 		reg = <0x4c>;