On 2024-09-21 15:27, Junhao Xie wrote:
> On 2024/9/19 22:52, Jonas Karlman wrote:
>> Hi Junhao,
>>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-orangepi-5-plus.dtb
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-photonicat.dtb
>>
>> This should probably be added where the rest of rk3568 dtb is located,
>> not with the rk3588 ones.
>
> Thanks for your reminder, I made a mistake in the order when renaming dts, I will fix it
>
>>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-quartzpro64.dtb
>>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5-itx.dtb
>>> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb
> [...]
>>> +
>>> +&gmac0 {
>>> + status = "disabled";
>>> + /* Motorcomm YT8521SC LAN port (require SGMII) */
>>
>> nit: Please be consistent where comments is added, for recgulators above
>> the comments is above the node, suggest you move the comment above the
>> "&gmac0 {" line, and same for similar comments.
>
> I will edit it. This looks better:
>
> /* Motorcomm YT8521SC LAN port (require SGMII) */
> &gmac0 {
> status = "disabled";
> };
>
>>> +};
>>> +
>>> +&gmac1 {
>>> + assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
>>> + assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
>>> + assigned-clock-rates = <0>, <125000000>;
>>> + clock_in_out = "output";
>>> + phy-handle = <&rgmii_phy1>;
>>> + phy-mode = "rgmii-id";
>>> + phy-supply = <&vcc_3v3>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&gmac1m1_miim
>>> + &gmac1m1_tx_bus2
>>> + &gmac1m1_rx_bus2
>>> + &gmac1m1_rgmii_clk
>>> + &gmac1m1_rgmii_bus>;
>>> + snps,reset-gpio = <&gpio4 RK_PC0 GPIO_ACTIVE_LOW>;
>>> + snps,reset-active-low;
>>> + snps,reset-delays-us = <0 20000 100000>;
>>
>> The snps,reset props is deprecated use resets props in phy node instad.
>
> I edited it and it works fine, is the following correct to write?
Looks like below props in phy node should match the deprecated
snps,reset props.
>
> /* Motorcomm YT8521SC WAN port */
> &gmac1 {
> assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>;
> assigned-clock-rates = <0>, <125000000>;
> clock_in_out = "output";
> phy-handle = <&rgmii_phy1>;
> phy-mode = "rgmii-id";
> phy-supply = <&vcc_3v3>;
> pinctrl-names = "default";
> pinctrl-0 = <&gmac1m1_miim
> &gmac1m1_tx_bus2
> &gmac1m1_rx_bus2
> &gmac1m1_rgmii_clk
> &gmac1m1_rgmii_bus>;
> tx_delay = <0>;
> rx_delay = <0>;
> status = "okay";
> };
>
> &mdio1 {
> rgmii_phy1: ethernet-phy@0 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <0x0>;
> pinctrl-names = "default";
> pinctrl-0 = <ð1_phy_rst>;
> reset-assert-us = <20000>;
> reset-deassert-us = <100000>;
> reset-gpios = <&gpio4 RK_PC0 GPIO_ACTIVE_LOW>;
> rx-internal-delay-ps = <1500>;
> tx-internal-delay-ps = <1500>;
> };
> };
>
> &pinctrl {
> [...]
> ethernet {
> eth1_phy_rst: eth1_phy_rst {
> rockchip,pins = <4 RK_PC0 RK_FUNC_GPIO &pcfg_pull_none>;
> };
> };
> [...]
> };
>
>>
>>> + status = "okay";
>>> + /* Motorcomm YT8521SC WAN port */
>>> +};
> [...]
>>> +&sdhci {
>>> + bus-width = <8>;
>>> + max-frequency = <200000000>;
>>
>> Can be dropped, already in base dtsi.
>
> max-frequency is not in sdhci node of rk356x.dtsi and rk3568.dtsi.
> I simply removed the max-frequency and it worked fine.
> Can it really be removed?
Ahh, sorry, I looked in rk3588 sdhci node, please ignore my comment
regarding max-frequency prop :-)
Regards,
Jonas
>
>>> + mmc-hs200-1_8v;
>>> + non-removable;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd>;
>>> + vmmc-supply = <&vcc_3v3>;
>>> + vqmmc-supply = <&vcc_1v8>;
>>> + status = "okay";
>>> + /* eMMC */
>>> +};
>
> Thanks for your review, I will fix all problems in next version!
>
> Best regards,
> Junhao
>
