* [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 7:47 ` Coia Prant
0 siblings, 0 replies; 18+ messages in thread
From: Coia Prant @ 2026-10-06 7:47 UTC (permalink / raw)
To: Vinod Koul
Cc: Neil Armstrong, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Coia Prant
On RK3568, the SGMII interface can be routed to either GMAC0 or
GMAC1 via the GRF register pipe_sgmii_mac_sel.
Add support for this selection by introducing
the "rockchip,sgmii-mac-sel" DT property.
From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
bit 1 (pipe_sgmii_mac_sel) is defined as:
0: SGMII routed to GMAC0
1: SGMII routed to GMAC1
The hardware reset value is 1 (GMAC1). If the property is set to 0,
the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
remains at GMAC1.
This is necessary for boards such as the Ariaboard Photonicat, which
uses the SGMII interface connected to GMAC0.
Out-of-range values are rejected by dtschema, so the driver does not
duplicate the range check.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 7843356a4dd47..7b867e7520064 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
struct combphy_reg pipe_xpcs_phy_ready;
struct combphy_reg pipe_pcie1l0_sel;
struct combphy_reg pipe_pcie1l1_sel;
+ struct combphy_reg pipe_sgmii_mac_sel;
struct combphy_reg u3otg0_port_en;
struct combphy_reg u3otg1_port_en;
};
@@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
bool enable_ssc;
bool ext_refclk;
struct clk *refclk;
+ u32 sgmii_mac_sel;
};
static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
@@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
+ priv->sgmii_mac_sel = 1;
+ device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
+
priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
/* fallback to old behaviour */
if (PTR_ERR(priv->phy_rst) == -ENOENT)
@@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
break;
case PHY_TYPE_SGMII:
+ rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
+ priv->sgmii_mac_sel > 0);
rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
@@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
.con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
/* pipe-grf */
.pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
+ .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
.pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
.u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
.u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
--
2.47.3
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 7:47 ` Coia Prant
0 siblings, 0 replies; 18+ messages in thread
From: Coia Prant @ 2026-10-06 7:47 UTC (permalink / raw)
To: Vinod Koul
Cc: Neil Armstrong, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Coia Prant
On RK3568, the SGMII interface can be routed to either GMAC0 or
GMAC1 via the GRF register pipe_sgmii_mac_sel.
Add support for this selection by introducing
the "rockchip,sgmii-mac-sel" DT property.
From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
bit 1 (pipe_sgmii_mac_sel) is defined as:
0: SGMII routed to GMAC0
1: SGMII routed to GMAC1
The hardware reset value is 1 (GMAC1). If the property is set to 0,
the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
remains at GMAC1.
This is necessary for boards such as the Ariaboard Photonicat, which
uses the SGMII interface connected to GMAC0.
Out-of-range values are rejected by dtschema, so the driver does not
duplicate the range check.
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 7843356a4dd47..7b867e7520064 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
struct combphy_reg pipe_xpcs_phy_ready;
struct combphy_reg pipe_pcie1l0_sel;
struct combphy_reg pipe_pcie1l1_sel;
+ struct combphy_reg pipe_sgmii_mac_sel;
struct combphy_reg u3otg0_port_en;
struct combphy_reg u3otg1_port_en;
};
@@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
bool enable_ssc;
bool ext_refclk;
struct clk *refclk;
+ u32 sgmii_mac_sel;
};
static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
@@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
+ priv->sgmii_mac_sel = 1;
+ device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
+
priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
/* fallback to old behaviour */
if (PTR_ERR(priv->phy_rst) == -ENOENT)
@@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
break;
case PHY_TYPE_SGMII:
+ rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
+ priv->sgmii_mac_sel > 0);
rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
@@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
.con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
/* pipe-grf */
.pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
+ .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
.pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
.u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
.u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
2026-10-06 7:47 ` Coia Prant
(?)
@ 2026-10-06 14:53 ` Shawn Lin
-1 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2026-10-06 14:53 UTC (permalink / raw)
To: Coia Prant
Cc: shawn.lin, Neil Armstrong, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
Hi Coia
在 2026/10/06 星期二 15:47, Coia Prant 写道:
> On RK3568, the SGMII interface can be routed to either GMAC0 or
> GMAC1 via the GRF register pipe_sgmii_mac_sel.
>
> Add support for this selection by introducing
> the "rockchip,sgmii-mac-sel" DT property.
>
> From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> bit 1 (pipe_sgmii_mac_sel) is defined as:
>
> 0: SGMII routed to GMAC0
> 1: SGMII routed to GMAC1
>
> The hardware reset value is 1 (GMAC1). If the property is set to 0,
> the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> remains at GMAC1.
>
Thanks for your patch. I have a question below, please have a look.
> This is necessary for boards such as the Ariaboard Photonicat, which
> uses the SGMII interface connected to GMAC0.
>
> Out-of-range values are rejected by dtschema, so the driver does not
> duplicate the range check.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 7843356a4dd47..7b867e7520064 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> struct combphy_reg pipe_xpcs_phy_ready;
> struct combphy_reg pipe_pcie1l0_sel;
> struct combphy_reg pipe_pcie1l1_sel;
> + struct combphy_reg pipe_sgmii_mac_sel;
> struct combphy_reg u3otg0_port_en;
> struct combphy_reg u3otg1_port_en;
> };
> @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> bool enable_ssc;
> bool ext_refclk;
> struct clk *refclk;
> + u32 sgmii_mac_sel;
> };
>
> static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
>
> priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
>
> + priv->sgmii_mac_sel = 1;
Before this patch, SGMMI was routed to GMACX depends on bootloader(if
setting), or routed to GMAC1 by chip default. Without knowing any
potential candidates setting it via bootloader, you might not force any
DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
Maybe you could just keep the SGMII route as-it if the property is not
present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
value passing from DT.
> + device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
> +
> priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
> /* fallback to old behaviour */
> if (PTR_ERR(priv->phy_rst) == -ENOENT)
> @@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
> break;
>
> case PHY_TYPE_SGMII:
> + rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
> + priv->sgmii_mac_sel > 0);
> rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
> @@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
> .con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
> /* pipe-grf */
> .pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
> + .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
> .pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
> .u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
> .u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 14:53 ` Shawn Lin
0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2026-10-06 14:53 UTC (permalink / raw)
To: Coia Prant
Cc: shawn.lin, Neil Armstrong, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
Hi Coia
在 2026/10/06 星期二 15:47, Coia Prant 写道:
> On RK3568, the SGMII interface can be routed to either GMAC0 or
> GMAC1 via the GRF register pipe_sgmii_mac_sel.
>
> Add support for this selection by introducing
> the "rockchip,sgmii-mac-sel" DT property.
>
> From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> bit 1 (pipe_sgmii_mac_sel) is defined as:
>
> 0: SGMII routed to GMAC0
> 1: SGMII routed to GMAC1
>
> The hardware reset value is 1 (GMAC1). If the property is set to 0,
> the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> remains at GMAC1.
>
Thanks for your patch. I have a question below, please have a look.
> This is necessary for boards such as the Ariaboard Photonicat, which
> uses the SGMII interface connected to GMAC0.
>
> Out-of-range values are rejected by dtschema, so the driver does not
> duplicate the range check.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 7843356a4dd47..7b867e7520064 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> struct combphy_reg pipe_xpcs_phy_ready;
> struct combphy_reg pipe_pcie1l0_sel;
> struct combphy_reg pipe_pcie1l1_sel;
> + struct combphy_reg pipe_sgmii_mac_sel;
> struct combphy_reg u3otg0_port_en;
> struct combphy_reg u3otg1_port_en;
> };
> @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> bool enable_ssc;
> bool ext_refclk;
> struct clk *refclk;
> + u32 sgmii_mac_sel;
> };
>
> static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
>
> priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
>
> + priv->sgmii_mac_sel = 1;
Before this patch, SGMMI was routed to GMACX depends on bootloader(if
setting), or routed to GMAC1 by chip default. Without knowing any
potential candidates setting it via bootloader, you might not force any
DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
Maybe you could just keep the SGMII route as-it if the property is not
present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
value passing from DT.
> + device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
> +
> priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
> /* fallback to old behaviour */
> if (PTR_ERR(priv->phy_rst) == -ENOENT)
> @@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
> break;
>
> case PHY_TYPE_SGMII:
> + rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
> + priv->sgmii_mac_sel > 0);
> rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
> @@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
> .con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
> /* pipe-grf */
> .pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
> + .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
> .pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
> .u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
> .u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 14:53 ` Shawn Lin
0 siblings, 0 replies; 18+ messages in thread
From: Shawn Lin @ 2026-10-06 14:53 UTC (permalink / raw)
To: Coia Prant
Cc: shawn.lin, Neil Armstrong, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
Hi Coia
在 2026/10/06 星期二 15:47, Coia Prant 写道:
> On RK3568, the SGMII interface can be routed to either GMAC0 or
> GMAC1 via the GRF register pipe_sgmii_mac_sel.
>
> Add support for this selection by introducing
> the "rockchip,sgmii-mac-sel" DT property.
>
> From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> bit 1 (pipe_sgmii_mac_sel) is defined as:
>
> 0: SGMII routed to GMAC0
> 1: SGMII routed to GMAC1
>
> The hardware reset value is 1 (GMAC1). If the property is set to 0,
> the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> remains at GMAC1.
>
Thanks for your patch. I have a question below, please have a look.
> This is necessary for boards such as the Ariaboard Photonicat, which
> uses the SGMII interface connected to GMAC0.
>
> Out-of-range values are rejected by dtschema, so the driver does not
> duplicate the range check.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 7843356a4dd47..7b867e7520064 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> struct combphy_reg pipe_xpcs_phy_ready;
> struct combphy_reg pipe_pcie1l0_sel;
> struct combphy_reg pipe_pcie1l1_sel;
> + struct combphy_reg pipe_sgmii_mac_sel;
> struct combphy_reg u3otg0_port_en;
> struct combphy_reg u3otg1_port_en;
> };
> @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> bool enable_ssc;
> bool ext_refclk;
> struct clk *refclk;
> + u32 sgmii_mac_sel;
> };
>
> static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
>
> priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
>
> + priv->sgmii_mac_sel = 1;
Before this patch, SGMMI was routed to GMACX depends on bootloader(if
setting), or routed to GMAC1 by chip default. Without knowing any
potential candidates setting it via bootloader, you might not force any
DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
Maybe you could just keep the SGMII route as-it if the property is not
present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
value passing from DT.
> + device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
> +
> priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy");
> /* fallback to old behaviour */
> if (PTR_ERR(priv->phy_rst) == -ENOENT)
> @@ -873,6 +878,8 @@ static int rk3568_combphy_cfg(struct rockchip_combphy_priv *priv)
> break;
>
> case PHY_TYPE_SGMII:
> + rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_sgmii_mac_sel,
> + priv->sgmii_mac_sel > 0);
> rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_xpcs_phy_ready, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_phymode_sel, true);
> rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_sel_qsgmii, true);
> @@ -984,6 +991,7 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
> .con3_for_sata = { 0x000c, 15, 0, 0x00, 0x4407 },
> /* pipe-grf */
> .pipe_con0_for_sata = { 0x0000, 15, 0, 0x00, 0x2220 },
> + .pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
> .pipe_xpcs_phy_ready = { 0x0040, 2, 2, 0x00, 0x01 },
> .u3otg0_port_en = { 0x0104, 15, 0, 0x0181, 0x1100 },
> .u3otg1_port_en = { 0x0144, 15, 0, 0x0181, 0x1100 },
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
2026-10-06 14:53 ` Shawn Lin
(?)
@ 2026-10-06 16:14 ` Rob Herring
-1 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2026-10-06 16:14 UTC (permalink / raw)
To: Shawn Lin
Cc: Coia Prant, Neil Armstrong, Manivannan Sadhasivam,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> Hi Coia
>
> 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> >
> > Add support for this selection by introducing
> > the "rockchip,sgmii-mac-sel" DT property.
> >
> > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > bit 1 (pipe_sgmii_mac_sel) is defined as:
> >
> > 0: SGMII routed to GMAC0
> > 1: SGMII routed to GMAC1
> >
> > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > remains at GMAC1.
> >
>
> Thanks for your patch. I have a question below, please have a look.
>
> > This is necessary for boards such as the Ariaboard Photonicat, which
> > uses the SGMII interface connected to GMAC0.
> >
> > Out-of-range values are rejected by dtschema, so the driver does not
> > duplicate the range check.
> >
> > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > ---
> > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > index 7843356a4dd47..7b867e7520064 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > struct combphy_reg pipe_xpcs_phy_ready;
> > struct combphy_reg pipe_pcie1l0_sel;
> > struct combphy_reg pipe_pcie1l1_sel;
> > + struct combphy_reg pipe_sgmii_mac_sel;
> > struct combphy_reg u3otg0_port_en;
> > struct combphy_reg u3otg1_port_en;
> > };
> > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > bool enable_ssc;
> > bool ext_refclk;
> > struct clk *refclk;
> > + u32 sgmii_mac_sel;
> > };
> > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > + priv->sgmii_mac_sel = 1;
>
> Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> setting), or routed to GMAC1 by chip default. Without knowing any
> potential candidates setting it via bootloader, you might not force any
> DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
>
> Maybe you could just keep the SGMII route as-it if the property is not
> present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> value passing from DT.
That is the typical way tri-state properties work (as opposed to using a
boolean). In that case, no 'default' in the schema would then be
appropriate.
Rob
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 16:14 ` Rob Herring
0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2026-10-06 16:14 UTC (permalink / raw)
To: Shawn Lin
Cc: Coia Prant, Neil Armstrong, Manivannan Sadhasivam,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> Hi Coia
>
> 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> >
> > Add support for this selection by introducing
> > the "rockchip,sgmii-mac-sel" DT property.
> >
> > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > bit 1 (pipe_sgmii_mac_sel) is defined as:
> >
> > 0: SGMII routed to GMAC0
> > 1: SGMII routed to GMAC1
> >
> > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > remains at GMAC1.
> >
>
> Thanks for your patch. I have a question below, please have a look.
>
> > This is necessary for boards such as the Ariaboard Photonicat, which
> > uses the SGMII interface connected to GMAC0.
> >
> > Out-of-range values are rejected by dtschema, so the driver does not
> > duplicate the range check.
> >
> > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > ---
> > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > index 7843356a4dd47..7b867e7520064 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > struct combphy_reg pipe_xpcs_phy_ready;
> > struct combphy_reg pipe_pcie1l0_sel;
> > struct combphy_reg pipe_pcie1l1_sel;
> > + struct combphy_reg pipe_sgmii_mac_sel;
> > struct combphy_reg u3otg0_port_en;
> > struct combphy_reg u3otg1_port_en;
> > };
> > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > bool enable_ssc;
> > bool ext_refclk;
> > struct clk *refclk;
> > + u32 sgmii_mac_sel;
> > };
> > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > + priv->sgmii_mac_sel = 1;
>
> Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> setting), or routed to GMAC1 by chip default. Without knowing any
> potential candidates setting it via bootloader, you might not force any
> DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
>
> Maybe you could just keep the SGMII route as-it if the property is not
> present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> value passing from DT.
That is the typical way tri-state properties work (as opposed to using a
boolean). In that case, no 'default' in the schema would then be
appropriate.
Rob
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 16:14 ` Rob Herring
0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2026-10-06 16:14 UTC (permalink / raw)
To: Shawn Lin
Cc: Coia Prant, Neil Armstrong, Manivannan Sadhasivam,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, linux-phy,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Vinod Koul
On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> Hi Coia
>
> 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> >
> > Add support for this selection by introducing
> > the "rockchip,sgmii-mac-sel" DT property.
> >
> > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > bit 1 (pipe_sgmii_mac_sel) is defined as:
> >
> > 0: SGMII routed to GMAC0
> > 1: SGMII routed to GMAC1
> >
> > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > remains at GMAC1.
> >
>
> Thanks for your patch. I have a question below, please have a look.
>
> > This is necessary for boards such as the Ariaboard Photonicat, which
> > uses the SGMII interface connected to GMAC0.
> >
> > Out-of-range values are rejected by dtschema, so the driver does not
> > duplicate the range check.
> >
> > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > ---
> > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > index 7843356a4dd47..7b867e7520064 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > struct combphy_reg pipe_xpcs_phy_ready;
> > struct combphy_reg pipe_pcie1l0_sel;
> > struct combphy_reg pipe_pcie1l1_sel;
> > + struct combphy_reg pipe_sgmii_mac_sel;
> > struct combphy_reg u3otg0_port_en;
> > struct combphy_reg u3otg1_port_en;
> > };
> > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > bool enable_ssc;
> > bool ext_refclk;
> > struct clk *refclk;
> > + u32 sgmii_mac_sel;
> > };
> > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > + priv->sgmii_mac_sel = 1;
>
> Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> setting), or routed to GMAC1 by chip default. Without knowing any
> potential candidates setting it via bootloader, you might not force any
> DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
>
> Maybe you could just keep the SGMII route as-it if the property is not
> present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> value passing from DT.
That is the typical way tri-state properties work (as opposed to using a
boolean). In that case, no 'default' in the schema would then be
appropriate.
Rob
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
2026-10-06 16:14 ` Rob Herring
(?)
@ 2026-10-06 16:30 ` Coia Prant
-1 siblings, 0 replies; 18+ messages in thread
From: Coia Prant @ 2026-10-06 16:30 UTC (permalink / raw)
To: Rob Herring, Shawn Lin
Cc: Neil Armstrong, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, linux-phy, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Vinod Koul
Rob Herring <robh@kernel.org> 于2026年10月7日周三 00:14写道:
>
> On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> > Hi Coia
> >
> > 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> > >
> > > Add support for this selection by introducing
> > > the "rockchip,sgmii-mac-sel" DT property.
> > >
> > > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > > bit 1 (pipe_sgmii_mac_sel) is defined as:
> > >
> > > 0: SGMII routed to GMAC0
> > > 1: SGMII routed to GMAC1
> > >
> > > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > > remains at GMAC1.
> > >
> >
> > Thanks for your patch. I have a question below, please have a look.
> >
> > > This is necessary for boards such as the Ariaboard Photonicat, which
> > > uses the SGMII interface connected to GMAC0.
> > >
> > > Out-of-range values are rejected by dtschema, so the driver does not
> > > duplicate the range check.
> > >
> > > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > > ---
> > > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index 7843356a4dd47..7b867e7520064 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > > struct combphy_reg pipe_xpcs_phy_ready;
> > > struct combphy_reg pipe_pcie1l0_sel;
> > > struct combphy_reg pipe_pcie1l1_sel;
> > > + struct combphy_reg pipe_sgmii_mac_sel;
> > > struct combphy_reg u3otg0_port_en;
> > > struct combphy_reg u3otg1_port_en;
> > > };
> > > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > > bool enable_ssc;
> > > bool ext_refclk;
> > > struct clk *refclk;
> > > + u32 sgmii_mac_sel;
> > > };
> > > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > > + priv->sgmii_mac_sel = 1;
> >
> > Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> > setting), or routed to GMAC1 by chip default. Without knowing any
> > potential candidates setting it via bootloader, you might not force any
> > DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
> >
> > Maybe you could just keep the SGMII route as-it if the property is not
> > present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> > value passing from DT.
>
> That is the typical way tri-state properties work (as opposed to using a
> boolean). In that case, no 'default' in the schema would then be
> appropriate.
>
> Rob
Hi Shawn, Rob,
Thanks for the review and feedback. I've just posted v13, which addresses
both points:
- The driver no longer forces GMAC1 when the property is absent; it
leaves the routing as-is.
- The driver validates the value and rejects anything other than 0 or 1.
- The binding drops 'default', since the property is tri-state.
I'll avoid replying to each comment individually to keep the thread
clean, but please let me know if anything is still off.
Thanks,
Coia
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 16:30 ` Coia Prant
0 siblings, 0 replies; 18+ messages in thread
From: Coia Prant @ 2026-10-06 16:30 UTC (permalink / raw)
To: Rob Herring, Shawn Lin
Cc: Neil Armstrong, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, linux-phy, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Vinod Koul
Rob Herring <robh@kernel.org> 于2026年10月7日周三 00:14写道:
>
> On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> > Hi Coia
> >
> > 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> > >
> > > Add support for this selection by introducing
> > > the "rockchip,sgmii-mac-sel" DT property.
> > >
> > > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > > bit 1 (pipe_sgmii_mac_sel) is defined as:
> > >
> > > 0: SGMII routed to GMAC0
> > > 1: SGMII routed to GMAC1
> > >
> > > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > > remains at GMAC1.
> > >
> >
> > Thanks for your patch. I have a question below, please have a look.
> >
> > > This is necessary for boards such as the Ariaboard Photonicat, which
> > > uses the SGMII interface connected to GMAC0.
> > >
> > > Out-of-range values are rejected by dtschema, so the driver does not
> > > duplicate the range check.
> > >
> > > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > > ---
> > > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index 7843356a4dd47..7b867e7520064 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > > struct combphy_reg pipe_xpcs_phy_ready;
> > > struct combphy_reg pipe_pcie1l0_sel;
> > > struct combphy_reg pipe_pcie1l1_sel;
> > > + struct combphy_reg pipe_sgmii_mac_sel;
> > > struct combphy_reg u3otg0_port_en;
> > > struct combphy_reg u3otg1_port_en;
> > > };
> > > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > > bool enable_ssc;
> > > bool ext_refclk;
> > > struct clk *refclk;
> > > + u32 sgmii_mac_sel;
> > > };
> > > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > > + priv->sgmii_mac_sel = 1;
> >
> > Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> > setting), or routed to GMAC1 by chip default. Without knowing any
> > potential candidates setting it via bootloader, you might not force any
> > DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
> >
> > Maybe you could just keep the SGMII route as-it if the property is not
> > present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> > value passing from DT.
>
> That is the typical way tri-state properties work (as opposed to using a
> boolean). In that case, no 'default' in the schema would then be
> appropriate.
>
> Rob
Hi Shawn, Rob,
Thanks for the review and feedback. I've just posted v13, which addresses
both points:
- The driver no longer forces GMAC1 when the property is absent; it
leaves the routing as-is.
- The driver validates the value and rejects anything other than 0 or 1.
- The binding drops 'default', since the property is tri-state.
I'll avoid replying to each comment individually to keep the thread
clean, but please let me know if anything is still off.
Thanks,
Coia
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v12 2/2] phy: rockchip: naneng-combphy: add SGMII MAC selection for RK3568
@ 2026-10-06 16:30 ` Coia Prant
0 siblings, 0 replies; 18+ messages in thread
From: Coia Prant @ 2026-10-06 16:30 UTC (permalink / raw)
To: Rob Herring, Shawn Lin
Cc: Neil Armstrong, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, linux-phy, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel, Vinod Koul
Rob Herring <robh@kernel.org> 于2026年10月7日周三 00:14写道:
>
> On Tue, Oct 06, 2026 at 10:53:22PM +0800, Shawn Lin wrote:
> > Hi Coia
> >
> > 在 2026/10/06 星期二 15:47, Coia Prant 写道:
> > > On RK3568, the SGMII interface can be routed to either GMAC0 or
> > > GMAC1 via the GRF register pipe_sgmii_mac_sel.
> > >
> > > Add support for this selection by introducing
> > > the "rockchip,sgmii-mac-sel" DT property.
> > >
> > > From the RK3568 TRM (Part1, Page 229), the PIPE_GRF_XPCS_CON0
> > > bit 1 (pipe_sgmii_mac_sel) is defined as:
> > >
> > > 0: SGMII routed to GMAC0
> > > 1: SGMII routed to GMAC1
> > >
> > > The hardware reset value is 1 (GMAC1). If the property is set to 0,
> > > the driver routes SGMII to GMAC0; if set to 1 (or omitted), it
> > > remains at GMAC1.
> > >
> >
> > Thanks for your patch. I have a question below, please have a look.
> >
> > > This is necessary for boards such as the Ariaboard Photonicat, which
> > > uses the SGMII interface connected to GMAC0.
> > >
> > > Out-of-range values are rejected by dtschema, so the driver does not
> > > duplicate the range check.
> > >
> > > Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 229)
> > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > > ---
> > > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index 7843356a4dd47..7b867e7520064 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > @@ -186,6 +186,7 @@ struct rockchip_combphy_grfcfg {
> > > struct combphy_reg pipe_xpcs_phy_ready;
> > > struct combphy_reg pipe_pcie1l0_sel;
> > > struct combphy_reg pipe_pcie1l1_sel;
> > > + struct combphy_reg pipe_sgmii_mac_sel;
> > > struct combphy_reg u3otg0_port_en;
> > > struct combphy_reg u3otg1_port_en;
> > > };
> > > @@ -212,6 +213,7 @@ struct rockchip_combphy_priv {
> > > bool enable_ssc;
> > > bool ext_refclk;
> > > struct clk *refclk;
> > > + u32 sgmii_mac_sel;
> > > };
> > > static void rockchip_combphy_updatel(struct rockchip_combphy_priv *priv,
> > > @@ -375,6 +377,9 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > > + priv->sgmii_mac_sel = 1;
> >
> > Before this patch, SGMMI was routed to GMACX depends on bootloader(if
> > setting), or routed to GMAC1 by chip default. Without knowing any
> > potential candidates setting it via bootloader, you might not force any
> > DTS files without "rockchip,sgmii-mac-sel" to pick GMAC1 as fallback.
> >
> > Maybe you could just keep the SGMII route as-it if the property is not
> > present. Meanwhile, should validate rockchip,sgmii-mac-sel is a valid
> > value passing from DT.
>
> That is the typical way tri-state properties work (as opposed to using a
> boolean). In that case, no 'default' in the schema would then be
> appropriate.
>
> Rob
Hi Shawn, Rob,
Thanks for the review and feedback. I've just posted v13, which addresses
both points:
- The driver no longer forces GMAC1 when the property is absent; it
leaves the routing as-is.
- The driver validates the value and rejects anything other than 0 or 1.
- The binding drops 'default', since the property is tri-state.
I'll avoid replying to each comment individually to keep the thread
clean, but please let me know if anything is still off.
Thanks,
Coia
^ permalink raw reply [flat|nested] 18+ messages in thread