linux-kernel - Re: [PATCH 3/3] arm64: dts: rockchip: add dts for LCKFB Taishan Pi RK3566

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6edb8443-6bb4-4174-8fe0-906208757cef@classfun.cn>
Date: Mon, 26 Aug 2024 16:03:01 +0800
From: Junhao Xie <bigfoot@...ssfun.cn>
To: Chukun Pan <amadeus@....edu.cn>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Jonas Karlman <jonas@...boo.se>, Chukun Pan <amadeus@....edu.cn>,
 FUKAUMI Naoki <naoki@...xa.com>, Dragan Simic <dsimic@...jaro.org>,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Junhao Xie <bigfoot@...ssfun.cn>
Subject: Re: [PATCH 3/3] arm64: dts: rockchip: add dts for LCKFB Taishan Pi
 RK3566

On 2024/8/26 15:01, Chukun Pan wrote:
> Hi Junhao,
> 
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-lckfb-tspi.dts
>> ...
>> +	aliases {
>> +		mmc0 = &sdmmc0;
>> +		mmc1 = &sdhci;
>> +		mmc2 = &sdmmc1;
>> +		wifi = &brcmf;
>> +		bluetooth = &bluetooth;
>> +	};
> 
> WiFi and Bluetooth do not need aliases.
> 
>> ...
>> +	vcc5v0_host: vcc5v0-host-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vcc5v0_host";
>> +		regulator-boot-on;
>> +		regulator-always-on;
> 
> This regulator does not need always-on and boot-on.
> 

I will remove them.

> 
>> ...
>> +	vccio_flash: vccio-flash {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vccio_flash";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		vin-supply = <&vcc_1v8>;
>> +	};
> 
> Generally speaking, vccio_flash is not a DCDC regulator,
> it is directly connected to vcc_1v8. Maybe you need to
> confirm the schematics.
> 

Yes, vccio_flash and vccio_wl are not DCDC regulator, they are connected to vcc_1v8 and vcc_3v3 through voltage selection resistors.

> 
>> +
>> +	vccio_wl: vccio-wl {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vccio_wl";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		vin-supply = <&vcc_3v3>;
>> +	};
> 
> Same as above, usually like this:
> VCC_1V8 or VCCA1V8_PMU - 0R - VCCIO_WL
> `sd-uhs-sdr104` requires 1.8v io voltage,
> you need to confirm the schematics.
> 

It is actually connected to vcc_1v8, I will fix it.

> 
>> ...
>> +&pinctrl {
>> ...
>> pmic {
>> +	pmic {
>> +		pmic_int: pmic_int {
> 
> Only the pmic_int is needed.
> Also `pmic_int: pmic-int {`
> 
>> ...
>> +&sdhci {
>> +	bus-width = <8>;
>> +	max-frequency = <200000000>;
>> +	non-removable;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
>> +	status = "okay";
>> +	vmmc-supply = <&vccio_flash>;
>> +	vqmmc-supply = <&vccio_flash>;
>> +};
> 
> The vmmc requires 3.3V supply (on rk356x).
> 

Yes, the vmmc of eMMC is actually connected to vcc_3v3, I will fix it.

> 
>> ...
>> +&sdmmc1 {
>> ...
>> +	sd-uhs-sdr104;
>> +	vmmc-supply = <&vccio_wl>;
>> +	vqmmc-supply = <&vccio_wl>;
> 
> Same as above.
> 
>> ...
> +&uart1 {
>> ...
> +		vbat-supply = <&vcc3v3_sys>;
> +		vddio-supply = <&vccio_wl>;
> 
> Here corresponds to sdmmc1.
> 
> Thanks,
> Chukun
> 

Thanks for your review, I will fix all problems and post patch v2!