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!
