[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c91c1a5a-44cf-f2d5-4d1b-f864912fcc90@gmail.com>
Date: Thu, 16 Dec 2021 13:19:03 +0100
From: Johan Jonker <jbx6244@...il.com>
To: Yifeng Zhao <yifeng.zhao@...k-chips.com>, heiko@...ech.de
Cc: robh+dt@...nel.org, devicetree@...r.kernel.org, vkoul@...nel.org,
michael.riesch@...fvision.net, linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, kishon@...com,
p.zabel@...gutronix.de, cl@...k-chips.com,
kever.yang@...k-chips.com
Subject: Re: [PATCH v5 4/4] arm64: dts: rockchip: add naneng combo phy nodes
for rk3568
Hi Heiko, Yifeng,
Could you advise whether we should change the DT layout and driver a bit.
Some properties don't really fit to one particular phy node, so move
them to a parent node.
Maybe replace rockchip,sgmii-mac-sel by a node in phy 1 and 2 ??
Also there are two USB3.0 Controllers, one can use as USB3.0 OTG
Controller, another one can use as USB3.0 Host Controller only.
Yet properties are called: rockchip,dis-u3otg0-port,
rockchip,dis-u3otg1-port.
Maybe replace them by a node ??
Unknown capabilities of future (rk3588) Naneng versions (combined host
and otg ??)
Maybe follow more in line with the phy-rockchip-inno-usb2.c and
phy-rockchip-usb.c drivers.
Could you confirm combo PHY on the RK3566 lacks the QSGMII/SGMII stuff?
(Lack of rk3566 TRM)
With so many differences maybe use separate compatible string and keep
the DT nodes completely apart.
(rockchip,rk3566-naneng-combphy, rockchip,rk3568-naneng-combphy)
Johan
================================
combphy: combphy {
compatible = "rockchip,rk3568-naneng-combphy";
========
rockchip,ext-refclk;
rockchip,enable-ssc;
rockchip,pipe-grf = <&pipegrf>;
========
#address-cells = <2>;
#size-cells = <2>;
ranges;
status = "disabled";
combphy0: combi-phy@...20000 {
reg = <0x0 0xfe820000 0x0 0x100>;
clocks = <&pmucru CLK_PCIEPHY0_REF>,
<&cru PCLK_PIPEPHY0>,
<&cru PCLK_PIPE>;
clock-names = "ref", "apb", "pipe";
assigned-clocks = <&pmucru CLK_PCIEPHY0_REF>;
assigned-clock-rates = <100000000>;
resets = <&cru SRST_PIPEPHY0>;
rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
#phy-cells = <1>;
status = "disabled";
u3phy0_otg: otg-port {
===
/* ??? */
#phy-cells = <0>;
status = "disabled";
===
};
};
combphy1: combi-phy@...30000 {
reg = <0x0 0xfe830000 0x0 0x100>;
clocks = <&pmucru CLK_PCIEPHY1_REF>,
<&cru PCLK_PIPEPHY1>,
<&cru PCLK_PIPE>;
clock-names = "ref", "apb", "pipe";
assigned-clocks = <&pmucru CLK_PCIEPHY1_REF>;
assigned-clock-rates = <100000000>;
resets = <&cru SRST_PIPEPHY1>;
rockchip,pipe-phy-grf = <&pipe_phy_grf1>;
#phy-cells = <1>;
status = "disabled";
u3phy0_host: host-port {
===
/* ??? */
#phy-cells = <0>;
status = "disabled";
===
}
sgmii_port: sgmii-port {
/* ??? */
status = "disabled";
}
};
combphy2: combi-phy@...40000 {
reg = <0x0 0xfe840000 0x0 0x100>;
clocks = <&pmucru CLK_PCIEPHY2_REF>,
<&cru PCLK_PIPEPHY2>,
<&cru PCLK_PIPE>;
clock-names = "ref", "apb", "pipe";
assigned-clocks = <&pmucru CLK_PCIEPHY2_REF>;
assigned-clock-rates = <100000000>;
resets = <&cru SRST_PIPEPHY2>;
rockchip,pipe-phy-grf = <&pipe_phy_grf2>;
#phy-cells = <1>;
status = "disabled";
sgmii_port: sgmii-port {
/* ??? */
status = "disabled";
}
};
}
==============
Example from phy-rockchip-usb.c:
for_each_available_child_of_node(dev->of_node, child) {
err = rockchip_combphy_init(phy_base, child);
if (err) {
of_node_put(child);
return err;
}
}
==============
Example from phy-rockchip-inno-usb2.c:
/* initialize otg/host port separately */
if (of_node_name_eq(child_np, "host-port")) {
ret = rockchip_usb3phy_host_port_init(rphy, rport,
child_np);
if (ret)
goto put_child;
} else {
ret = rockchip_usb3phy_otg_port_init(rphy, rport,
child_np);
if (ret)
goto put_child;
}
===============
Example from dwmac-rk.c:
/* Multiple Naneng phy controllers need
* to be distinguished.
*/
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res) {
int i = 0;
while (ops->regs[i]) {
if (ops->regs[i] == res->start) {
bsp_priv->id = i;
break;
}
i++;
}
}
================
On 12/15/21 10:56 AM, Yifeng Zhao wrote:
> Add the core dt-node for the rk3568's naneng combo phys.
>
> Signed-off-by: Yifeng Zhao <yifeng.zhao@...k-chips.com>
> Signed-off-by: Johan Jonker <jbx6244@...il.com>
> ---
>
> Changes in v5:
> - remove apb reset
>
> Changes in v4:
> - rename node name
> - remove reset-names
> - move #phy-cells
> - add rockchip,rk3568-pipe-grf
> - add rockchip,rk3568-pipe-phy-grf
>
> Changes in v3:
> - Move pipe_phy_grf0 to rk3568.dtsi
>
> Changes in v2:
> - Move phy0 to rk3568.dtsi
>
> arch/arm64/boot/dts/rockchip/rk3568.dtsi | 21 +++++++++++
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 47 ++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 2fd313a295f8..91a0b798b857 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -8,6 +8,11 @@
> / {
> compatible = "rockchip,rk3568";
>
> + pipe_phy_grf0: syscon@...70000 {
> + compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
> + reg = <0x0 0xfdc70000 0x0 0x1000>;
> + };
> +
> qos_pcie3x1: qos@...90080 {
> compatible = "rockchip,rk3568-qos", "syscon";
> reg = <0x0 0xfe190080 0x0 0x20>;
> @@ -71,6 +76,22 @@
> queue0 {};
> };
> };
> +
> + combphy0: phy@...20000 {
> + compatible = "rockchip,rk3568-naneng-combphy";
> + reg = <0x0 0xfe820000 0x0 0x100>;
> + clocks = <&pmucru CLK_PCIEPHY0_REF>,
> + <&cru PCLK_PIPEPHY0>,
> + <&cru PCLK_PIPE>;
> + clock-names = "ref", "apb", "pipe";
> + assigned-clocks = <&pmucru CLK_PCIEPHY0_REF>;
> + assigned-clock-rates = <100000000>;
> + resets = <&cru SRST_PIPEPHY0>;
> + rockchip,pipe-grf = <&pipegrf>;
> + rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
> + #phy-cells = <1>;
> + status = "disabled";
> + };
> };
>
> &cpu0_opp_table {
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index a68033a23975..93f230f799f1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -218,11 +218,26 @@
> };
> };
>
> + pipegrf: syscon@...50000 {
> + compatible = "rockchip,rk3568-pipe-grf", "syscon";
> + reg = <0x0 0xfdc50000 0x0 0x1000>;
> + };
> +
> grf: syscon@...60000 {
> compatible = "rockchip,rk3568-grf", "syscon", "simple-mfd";
> reg = <0x0 0xfdc60000 0x0 0x10000>;
> };
>
> + pipe_phy_grf1: syscon@...80000 {
> + compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
> + reg = <0x0 0xfdc80000 0x0 0x1000>;
> + };
> +
> + pipe_phy_grf2: syscon@...90000 {
> + compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
> + reg = <0x0 0xfdc90000 0x0 0x1000>;
> + };
> +
> pmucru: clock-controller@...00000 {
> compatible = "rockchip,rk3568-pmucru";
> reg = <0x0 0xfdd00000 0x0 0x1000>;
> @@ -1141,6 +1156,38 @@
> status = "disabled";
> };
>
> + combphy1: phy@...30000 {
> + compatible = "rockchip,rk3568-naneng-combphy";
> + reg = <0x0 0xfe830000 0x0 0x100>;
> + clocks = <&pmucru CLK_PCIEPHY1_REF>,
> + <&cru PCLK_PIPEPHY1>,
> + <&cru PCLK_PIPE>;
> + clock-names = "ref", "apb", "pipe";
> + assigned-clocks = <&pmucru CLK_PCIEPHY1_REF>;
> + assigned-clock-rates = <100000000>;
> + resets = <&cru SRST_PIPEPHY1>;
> + rockchip,pipe-grf = <&pipegrf>;
> + rockchip,pipe-phy-grf = <&pipe_phy_grf1>;
> + #phy-cells = <1>;
> + status = "disabled";
> + };
> +
> + combphy2: phy@...40000 {
> + compatible = "rockchip,rk3568-naneng-combphy";
> + reg = <0x0 0xfe840000 0x0 0x100>;
> + clocks = <&pmucru CLK_PCIEPHY2_REF>,
> + <&cru PCLK_PIPEPHY2>,
> + <&cru PCLK_PIPE>;
> + clock-names = "ref", "apb", "pipe";
> + assigned-clocks = <&pmucru CLK_PCIEPHY2_REF>;
> + assigned-clock-rates = <100000000>;
> + resets = <&cru SRST_PIPEPHY2>;
> + rockchip,pipe-grf = <&pipegrf>;
> + rockchip,pipe-phy-grf = <&pipe_phy_grf2>;
> + #phy-cells = <1>;
> + status = "disabled";
> + };
> +
> pinctrl: pinctrl {
> compatible = "rockchip,rk3568-pinctrl";
> rockchip,grf = <&grf>;
>
Powered by blists - more mailing lists