[PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[Vincent Pelletier]

Follow the pin descriptions given in the version 3 of the board schematics.

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index 4f66919215f6..305a086e5207 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -245,4 +245,8 @@ &pwm1 {
 
 &gpio {
 	status = "okay";
+	gpio-line-names = "J29.1", "PMICNTB", "PMICSHDN", "J8.1", "J8.3",
+		"PCIe_PWREN", "THERM", "UBRDG_RSTN", "PCIe_PERSTN",
+		"ULPI_RSTN", "J8.2", "UHUB_RSTN", "GEMGXL_RST", "J8.4",
+		"EN_VDD_SD", "SD_CD";
 };
-- 
2.33.1

[PATCH] riscv: dts: sifive unmatched: Expose the board ID eeprom.

[Vincent Pelletier]

Mark it as read-only as it is factory-programmed with identifying
information, and no executable nor configuration:
- eth MAC address
- board model (PCB version, BoM version)
- board serial number
Accidental modification would cause misidentification which could brick
the board, so marking read-only seem like both a safe and non-constraining
choice.

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index 305a086e5207..cf8937708829 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -58,6 +58,16 @@ temperature-sensor@4c {
 		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
 	};
 
+	eeprom@54 {
+		compatible = "microchip,24c02", "atmel,24c02";
+		reg = <0x54>;
+		vcc-supply = <&vdd_bpro>;
+		label = "board-id";
+		pagesize = <16>;
+		read-only;
+		size = <256>;
+	};
+
 	pmic@58 {
 		compatible = "dlg,da9063";
 		reg = <0x58>;
-- 
2.33.1

[PATCH] riscv: dts: sifive unmatched: Expose the PMIC sub-functions.

[Vincent Pelletier]

These sub-functions are available in the chip revision on this board, so
expose them.

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index cf8937708829..270360b258b7 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -75,6 +75,18 @@ pmic@58 {
 		interrupts = <1 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
 
+		onkey {
+			compatible = "dlg,da9063-onkey";
+		};
+
+		rtc {
+			compatible = "dlg,da9063-rtc";
+		};
+
+		wdt {
+			compatible = "dlg,da9063-watchdog";
+		};
+
 		regulators {
 			vdd_bcore1: bcore1 {
 				regulator-min-microvolt = <900000>;
-- 
2.33.1

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

[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.
---
 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";
+		reg = <0x1e>;
+	};
+
 	temperature-sensor@4c {
 		compatible = "ti,tmp451";
 		reg = <0x4c>;
-- 
2.33.1

[PATCH] riscv: dts: sifive unmatched: Fix regulator for board rev3.

[Vincent Pelletier]

The existing values are rejected by the da9063 regulator driver, as they
are unachievable with the declared chip setup (non-merged vcore and bmem
are unable to provide the declared curent).

Fix voltages to match rev3 schematics, which also matches their boot-up
configuration within the chip's available precision.
Declare bcore1/bcore2 and bmem/bio as merged.
Set ldo09 and ldo10 as always-on as their consumers are not declared but
exist.
Drop ldo current limits as there is no current limit feature for these
regulators in the DA9063. Fixes warnings like:
  DA9063_LDO3: Operation of current configuration missing

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 .../boot/dts/sifive/hifive-unmatched-a00.dts  | 84 ++++++-------------
 1 file changed, 24 insertions(+), 60 deletions(-)

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index e327831d0d48..bd6e90288c8a 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -93,47 +93,31 @@ wdt {
 		};
 
 		regulators {
-			vdd_bcore1: bcore1 {
-				regulator-min-microvolt = <900000>;
-				regulator-max-microvolt = <900000>;
-				regulator-min-microamp = <5000000>;
-				regulator-max-microamp = <5000000>;
-				regulator-always-on;
-			};
-
-			vdd_bcore2: bcore2 {
-				regulator-min-microvolt = <900000>;
-				regulator-max-microvolt = <900000>;
-				regulator-min-microamp = <5000000>;
-				regulator-max-microamp = <5000000>;
+			vdd_bcore: bcores-merged {
+				regulator-min-microvolt = <1050000>;
+				regulator-max-microvolt = <1050000>;
+				regulator-min-microamp = <4800000>;
+				regulator-max-microamp = <4800000>;
 				regulator-always-on;
 			};
 
 			vdd_bpro: bpro {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <2500000>;
-				regulator-max-microamp = <2500000>;
+				regulator-min-microamp = <2400000>;
+				regulator-max-microamp = <2400000>;
 				regulator-always-on;
 			};
 
 			vdd_bperi: bperi {
-				regulator-min-microvolt = <1050000>;
-				regulator-max-microvolt = <1050000>;
+				regulator-min-microvolt = <1060000>;
+				regulator-max-microvolt = <1060000>;
 				regulator-min-microamp = <1500000>;
 				regulator-max-microamp = <1500000>;
 				regulator-always-on;
 			};
 
-			vdd_bmem: bmem {
-				regulator-min-microvolt = <1200000>;
-				regulator-max-microvolt = <1200000>;
-				regulator-min-microamp = <3000000>;
-				regulator-max-microamp = <3000000>;
-				regulator-always-on;
-			};
-
-			vdd_bio: bio {
+			vdd_bmem_bio: bmem-bio-merged {
 				regulator-min-microvolt = <1200000>;
 				regulator-max-microvolt = <1200000>;
 				regulator-min-microamp = <3000000>;
@@ -144,86 +128,66 @@ vdd_bio: bio {
 			vdd_ldo1: ldo1 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <100000>;
-				regulator-max-microamp = <100000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo2: ldo2 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo3: ldo3 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo4: ldo4 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo5: ldo5 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <100000>;
-				regulator-max-microamp = <100000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo6: ldo6 {
-				regulator-min-microvolt = <3300000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo7: ldo7 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
 			vdd_ldo8: ldo8 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
 			vdd_ld09: ldo9 {
 				regulator-min-microvolt = <1050000>;
 				regulator-max-microvolt = <1050000>;
-				regulator-min-microamp = <200000>;
-				regulator-max-microamp = <200000>;
+				regulator-always-on;
 			};
 
 			vdd_ldo10: ldo10 {
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
-				regulator-min-microamp = <300000>;
-				regulator-max-microamp = <300000>;
+				regulator-always-on;
 			};
 
 			vdd_ldo11: ldo11 {
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
-				regulator-min-microamp = <300000>;
-				regulator-max-microamp = <300000>;
 				regulator-always-on;
 			};
 		};
-- 
2.33.1

[PATCH] riscv: dts: sifive unmatched: Link the tmp451 with its power supply.

[Vincent Pelletier]

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index bd6e90288c8a..73c1e4adf650 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -59,6 +59,7 @@ tps544b20@1e {
 	temperature-sensor@4c {
 		compatible = "ti,tmp451";
 		reg = <0x4c>;
+		vcc-supply = <&vdd_bpro>;
 		interrupt-parent = <&gpio>;
 		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
 	};
-- 
2.33.1

Re: [PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[Heiko Stübner]

Hi Vincent,

Am Dienstag, 16. November 2021, 01:52:56 CET schrieb Vincent Pelletier:
> Follow the pin descriptions given in the version 3 of the board schematics.
> 
> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>

when sending a patch series with "git format-patch -6" and friends will
automcatically generate x/y additions like "[PATCH 1/6]" and so on.

Please try to keep them around when sending, as automated tools for patch
handling like "b4", stumble when they encounter a patch series without them.

In this case a

	b4 am f6512cc50dc31a086e00ed59c63ea60d8c148fc4.1637023980.git.plr.vincent@gmail.com

[first patch in the series]
will actually only retrieve the last patch

	"[PATCH] riscv: dts: sifive unmatched: Link the tmp451 with its power supply."

as it thinks it's a new version of the first one.


Thanks
Heiko

> ---
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index 4f66919215f6..305a086e5207 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -245,4 +245,8 @@ &pwm1 {
>  
>  &gpio {
>  	status = "okay";
> +	gpio-line-names = "J29.1", "PMICNTB", "PMICSHDN", "J8.1", "J8.3",
> +		"PCIe_PWREN", "THERM", "UBRDG_RSTN", "PCIe_PERSTN",
> +		"ULPI_RSTN", "J8.2", "UHUB_RSTN", "GEMGXL_RST", "J8.4",
> +		"EN_VDD_SD", "SD_CD";
>  };
> 

Re: [PATCH] riscv: dts: sifive unmatched: Expose the FU740 core supply regulator.

[Heiko Stübner]

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>;
> 

Re: [PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[Vincent Pelletier]

Hello,

On Tue, 16 Nov 2021 10:39:04 +0100, Heiko Stübner <heiko@sntech.de> wrote:
> Hi Vincent,
> 
> Am Dienstag, 16. November 2021, 01:52:56 CET schrieb Vincent Pelletier:
> > Follow the pin descriptions given in the version 3 of the board schematics.
> > 
> > Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>  
> 
> when sending a patch series with "git format-patch -6" and friends will
> automcatically generate x/y additions like "[PATCH 1/6]" and so on.
> 
> Please try to keep them around when sending, as automated tools for patch
> handling like "b4", stumble when they encounter a patch series without them.

Actually my intent was to not make this a series, as in my understanding
independent patches should/may be submitted separately from each other.
They just happen to be stacked, and in this order, in my working copy,
but should not functionally have any dependency on one another (I
believe they should even apply cleanly in any order).

It is only after sending that I realised that
  git send-email --no-thread [...] v5.16-rc1..riscv_for_upstream
still decided to thread the emails...
Which indeed makes my use of "--no-numbered" a mistake. Sorry for not
checking --dry-run output more closely.

> In this case a
> 
> 	b4 am f6512cc50dc31a086e00ed59c63ea60d8c148fc4.1637023980.git.plr.vincent@gmail.com
> 
> [first patch in the series]
> will actually only retrieve the last patch
> 
> 	"[PATCH] riscv: dts: sifive unmatched: Link the tmp451 with its power supply."
> 
> as it thinks it's a new version of the first one.
> 
> 
> Thanks
> Heiko
> 
> > ---
> >  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > index 4f66919215f6..305a086e5207 100644
> > --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > @@ -245,4 +245,8 @@ &pwm1 {
> >  
> >  &gpio {
> >  	status = "okay";
> > +	gpio-line-names = "J29.1", "PMICNTB", "PMICSHDN", "J8.1", "J8.3",
> > +		"PCIe_PWREN", "THERM", "UBRDG_RSTN", "PCIe_PERSTN",
> > +		"ULPI_RSTN", "J8.2", "UHUB_RSTN", "GEMGXL_RST", "J8.4",
> > +		"EN_VDD_SD", "SD_CD";
> >  };
> >   
> 
> 
> 
> 




-- 
Vincent Pelletier
GPG fingerprint 983A E8B7 3B91 1598 7A92 3845 CAC9 3691 4257 B0C1

Re: [PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[Krzysztof Kozlowski]

On 16/11/2021 01:52, Vincent Pelletier wrote:
> Follow the pin descriptions given in the version 3 of the board schematics.
> 
> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
> ---
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Please do not end the subject line with a dot.


Best regards,
Krzysztof

Re: [PATCH] riscv: dts: sifive unmatched: Expose the FU740 core supply regulator.

[Krzysztof Kozlowski]

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

Re: [PATCH] riscv: dts: sifive unmatched: Link the tmp451 with its power supply.

[Krzysztof Kozlowski]

On 16/11/2021 01:53, Vincent Pelletier wrote:
> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>

This needs commit description, explaining what are you doing and why.

> ---
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index bd6e90288c8a..73c1e4adf650 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -59,6 +59,7 @@ tps544b20@1e {
>  	temperature-sensor@4c {
>  		compatible = "ti,tmp451";
>  		reg = <0x4c>;
> +		vcc-supply = <&vdd_bpro>;
>  		interrupt-parent = <&gpio>;
>  		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
>  	};
> 


Best regards,
Krzysztof

Re: [PATCH] riscv: dts: sifive unmatched: Name gpio lines.

[Krzysztof Kozlowski]

On 16/11/2021 11:17, Vincent Pelletier wrote:
> Hello,
> 
> On Tue, 16 Nov 2021 10:39:04 +0100, Heiko Stübner <heiko@sntech.de> wrote:
>> Hi Vincent,
>>
>> Am Dienstag, 16. November 2021, 01:52:56 CET schrieb Vincent Pelletier:
>>> Follow the pin descriptions given in the version 3 of the board schematics.
>>>
>>> Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>  
>>
>> when sending a patch series with "git format-patch -6" and friends will
>> automcatically generate x/y additions like "[PATCH 1/6]" and so on.
>>
>> Please try to keep them around when sending, as automated tools for patch
>> handling like "b4", stumble when they encounter a patch series without them.
> 
> Actually my intent was to not make this a series, as in my understanding
> independent patches should/may be submitted separately from each other.
> They just happen to be stacked, and in this order, in my working copy,
> but should not functionally have any dependency on one another (I
> believe they should even apply cleanly in any order).
> 

This should be a series because the patches, even though independent,
are logically connected/grouped as one work.

Best regards,
Krzysztof

Re: [PATCH] riscv: dts: sifive unmatched: Expose the FU740 core supply regulator.

[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


Best regards,
Krzysztof

Re: [PATCH] riscv: dts: sifive unmatched: Expose the FU740 core supply regulator.

[Heiko Stübner]

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