| Message ID | 20250106100001.1344418-2-amadeus@jmu.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | phy: rockchip: naneng-combphy: compatible reset with old DT | expand |
Hi Vinod, On 2025-01-06 11:00, Chukun Pan wrote: > The device tree of RK3568 did not specify reset-names before. > So add fallback to old behaviour to be compatible with old DT. > > Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") The commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") broke backwards compatibly with old DTs and has already been backported breaking i.e. PCIe and USB on Rockchip RK356x boards on stable and longterm kernels. E.g with v6.12.9 on a Radxa ROCK 3A (RK3568) board: rockchip-naneng-combphy fe830000.phy: error -ENOENT: failed to get phy reset rockchip-naneng-combphy fe830000.phy: probe with driver rockchip-naneng-combphy failed with error -2 rockchip-naneng-combphy fe840000.phy: error -ENOENT: failed to get phy reset rockchip-naneng-combphy fe840000.phy: probe with driver rockchip-naneng-combphy failed with error -2 rockchip-naneng-combphy fe820000.phy: error -ENOENT: failed to get phy reset rockchip-naneng-combphy fe820000.phy: probe with driver rockchip-naneng-combphy failed with error -2 # cat /sys/kernel/debug/devices_deferred 3c0000000.pcie platform: supplier fe840000.phy not ready fcc00000.usb platform: supplier fe820000.phy not ready fd000000.usb platform: supplier fe830000.phy not ready Please revert the commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") or merge this patch to restore PCIe and USB RK356x. > Cc: Jianfeng Liu <liujianfeng1994@gmail.com> > Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn> With this patch PCIe and USB start working again on same board. Reviewed-by: Jonas Karlman <jonas@kwiboo.se> Regards, Jonas > --- > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > index a1532ef8bbe9..8c3ce57f8915 100644 > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > @@ -324,7 +324,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy > > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk"); > > - priv->phy_rst = devm_reset_control_get(dev, "phy"); > + priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy"); > + /* fallback to old behaviour */ > + if (PTR_ERR(priv->phy_rst) == -ENOENT) > + priv->phy_rst = devm_reset_control_array_get_exclusive(dev); > if (IS_ERR(priv->phy_rst)) > return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n"); >
Hi, > The commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") > broke backwards compatibly with old DTs and has already been backported > breaking i.e. PCIe and USB on Rockchip RK356x boards on stable and > longterm kernels. The dts changes have been merged into v6.13-rc7, maybe it can be sent to linux-stable. Thanks, Chukun
Hi Chukun, On 2025-01-15 05:00, Chukun Pan wrote: > Hi, >> The commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") >> broke backwards compatibly with old DTs and has already been backported >> breaking i.e. PCIe and USB on Rockchip RK356x boards on stable and >> longterm kernels. > > The dts changes have been merged into v6.13-rc7, > maybe it can be sent to linux-stable. The original commit broke ABI and does not allow upgrading Linux using an existing device tree, even if the DT patch adding reset-names props also is backported you still have uses in the wild e.g. U-Boot v2024.10 and newer that comes bundled with an unpatched DT. Trying to boot into Linux v6.1.123+, v6.6.69+ and v6.12.8+ using the U-Boot bundled DT for RK356x boards now result in broken PCIe, SATA and USB thanks to the offending commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset"). Regards, Jonas > > Thanks, > Chukun >
On 06-01-25, 18:00, Chukun Pan wrote: > The device tree of RK3568 did not specify reset-names before. > So add fallback to old behaviour to be compatible with old DT. > > Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") > Cc: Jianfeng Liu <liujianfeng1994@gmail.com> > Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn> > --- > drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > index a1532ef8bbe9..8c3ce57f8915 100644 > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c > @@ -324,7 +324,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy > > priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk"); > > - priv->phy_rst = devm_reset_control_get(dev, "phy"); > + priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy"); > + /* fallback to old behaviour */ > + if (PTR_ERR(priv->phy_rst) == -ENOENT) > + priv->phy_rst = devm_reset_control_array_get_exclusive(dev); That is not *same* behaviour on fallback, why _exclusive() for the fallback? > if (IS_ERR(priv->phy_rst)) > return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n"); > > -- > 2.25.1
Hi Vinod, On 2025-01-30 06:57, Vinod Koul wrote: > On 06-01-25, 18:00, Chukun Pan wrote: >> The device tree of RK3568 did not specify reset-names before. >> So add fallback to old behaviour to be compatible with old DT. >> >> Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") >> Cc: Jianfeng Liu <liujianfeng1994@gmail.com> >> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn> >> --- >> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c >> index a1532ef8bbe9..8c3ce57f8915 100644 >> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c >> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c >> @@ -324,7 +324,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy >> >> priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk"); >> >> - priv->phy_rst = devm_reset_control_get(dev, "phy"); >> + priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy"); >> + /* fallback to old behaviour */ >> + if (PTR_ERR(priv->phy_rst) == -ENOENT) >> + priv->phy_rst = devm_reset_control_array_get_exclusive(dev); > > That is not *same* behaviour on fallback, why _exclusive() for the > fallback? Prior to the commit fd3ac6e80497 ("dt-bindings: phy: rockchip: rk3588 has two reset lines") the dt-binding did not require reset-names and it still does not for the rockchip,rk3568-naneng-combphy compatible. The binding documentation also mentioned "exclusive PHY reset line" for the resets prop. The commit fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") broke backward compatibility with the rockchip,rk3568-naneng-combphy compatible by requiring existence of a "phy" in reset-names. This patch restores the old behavior prior to fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") when devm_reset_control_array_get_exclusive() was used to restore compatibility with the rockchip,rk3568-naneng-combphy dt-binding. Regards, Jonas > >> if (IS_ERR(priv->phy_rst)) >> return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n"); >> >> -- >> 2.25.1 >
diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c index a1532ef8bbe9..8c3ce57f8915 100644 --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c @@ -324,7 +324,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk"); - priv->phy_rst = devm_reset_control_get(dev, "phy"); + priv->phy_rst = devm_reset_control_get_exclusive(dev, "phy"); + /* fallback to old behaviour */ + if (PTR_ERR(priv->phy_rst) == -ENOENT) + priv->phy_rst = devm_reset_control_array_get_exclusive(dev); if (IS_ERR(priv->phy_rst)) return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n");
The device tree of RK3568 did not specify reset-names before. So add fallback to old behaviour to be compatible with old DT. Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset") Cc: Jianfeng Liu <liujianfeng1994@gmail.com> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn> --- drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)