linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Patrick Wildt <patrick@blueri.se>, linux-rockchip@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	Kever Yang <kever.yang@rock-chips.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add MNT Reform 2
Date: Thu, 30 Jan 2025 13:05:45 +0100	[thread overview]
Message-ID: <cd6edfed-08a8-44de-9bfb-5199fd2813fd@kernel.org> (raw)
In-Reply-To: <Z5tobRnjXmr4dl7j@windev.fritz.box>

On 30/01/2025 12:54, Patrick Wildt wrote:
> +
> +&hdptxphy_hdmi0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c0m2_xfer>;
> +	status = "okay";
> +
> +	vdd_cpu_big0_s0: vdd_cpu_big0_mem_s0: rk8602@42 {

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



> +		compatible = "rockchip,rk8602";
> +		reg = <0x42>;
> +
> +		vin-supply = <&vcc5v0_sys>;
> +		fcs,suspend-voltage-selector = <1>;
> +		rockchip,suspend-voltage-selector = <1>;
> +
> +		regulator-compatible = "rk860x-reg";
> +		regulator-name = "vdd_cpu_big0_s0";
> +		regulator-min-microvolt = <0x86470>;
> +		regulator-max-microvolt = <0x100590>;
> +		regulator-ramp-delay = <0x8fc>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +
> +	vdd_cpu_big1_s0: vdd_cpu_big1_mem_s0: rk8603@43 {

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



> +		compatible = "rockchip,rk8602", "rockchip,rk8603";
> +		reg = <0x43>;
> +


...

> +&spi2 {
> +	status = "okay";
> +	assigned-clocks = <&cru CLK_SPI2>;
> +	assigned-clock-rates = <200000000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
> +	num-cs = <1>;
> +
> +	rk806single: pmic@0 {
> +		compatible = "rockchip,rk806";
> +		spi-max-frequency = <1000000>;
> +		reg = <0x0>;

reg is always theh second property.

> +
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +
> +		pinctrl-names = "default", "pmic-power-off";
> +		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
> +			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
> +		pinctrl-1 = <&rk806_dvs1_pwrdn>;
> +
> +		pmic-reset-func = <1>;
> +
> +		vcc1-supply = <&vcc5v0_sys>;
> +		vcc2-supply = <&vcc5v0_sys>;
> +		vcc3-supply = <&vcc5v0_sys>;
> +		vcc4-supply = <&vcc5v0_sys>;
> +		vcc5-supply = <&vcc5v0_sys>;
> +		vcc6-supply = <&vcc5v0_sys>;
> +		vcc7-supply = <&vcc5v0_sys>;
> +		vcc8-supply = <&vcc5v0_sys>;
> +		vcc9-supply = <&vcc5v0_sys>;
> +		vcc10-supply = <&vcc5v0_sys>;
> +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> +		vcc12-supply = <&vcc5v0_sys>;
> +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> +		vcca-supply = <&vcc5v0_sys>;
> +
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +
> +		rk806_dvs1_null: dvs1-null-pins {
> +			pins = "gpio_pwrctrl2";
> +			function = "pin_fun0";
> +		};
> +
> +		rk806_dvs1_slp: rk806_dvs1_slp {

No underscores in node names. Missing suffix also suggests this fails
tests, so standard disclaimer:

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.


> +			pins = "gpio_pwrctrl1";
> +			function = "pin_fun1";
> +		};

Best regards,
Krzysztof


  reply	other threads:[~2025-01-30 12:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-30 11:53 [PATCH 1/2] dt-bindings: arm: rockchip: Add MNT Reform 2 (RCORE) Patrick Wildt
2025-01-30 11:54 ` [PATCH 2/2] arm64: dts: rockchip: add MNT Reform 2 Patrick Wildt
2025-01-30 12:05   ` Krzysztof Kozlowski [this message]
2025-02-10 22:14     ` Patrick Wildt
2025-01-30 23:38 ` [PATCH 1/2] dt-bindings: arm: rockchip: Add MNT Reform 2 (RCORE) Rob Herring (Arm)
2025-01-31  7:42 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd6edfed-08a8-44de-9bfb-5199fd2813fd@kernel.org \
    --to=krzk@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=kever.yang@rock-chips.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=patrick@blueri.se \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).