Hi Frank, Am Freitag, 27. September 2024, 08:16:42 CEST schrieb Frank Wang: > On 2024/9/26 18:48, Heiko Stuebner wrote: > > Am Donnerstag, 26. September 2024, 12:32:23 CEST schrieb Frank Wang: > >> From: William Wu <william.wu@xxxxxxxxxxxxxx> > >> > >> The RK3576 SoC has two independent USB2.0 PHYs, and each PHY has > >> one port. > >> > >> This change also converts the clock management from single to bulk > >> as some Rockchip SoCs (e.g RK3576) have more than one clock. > >> > >> Signed-off-by: William Wu <william.wu@xxxxxxxxxxxxxx> > >> Signed-off-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx> > >> --- > >> Changelog: > >> v3: > >> - amend the commit log adds clocks converting. > >> - retrieve the clock by "clks.id" in *_clk480m_register() function. > >> > >> v2: > >> - no changes. > >> v1: > >> - https://patchwork.kernel.org/project/linux-phy/patch/20240923025326.10467-2-frank.wang@xxxxxxxxxxxxxx/ > >> > >> drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 139 +++++++++++++++++- > >> 1 file changed, 131 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > >> index 4f71373ae6e1..642c7857c5ae 100644 > >> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > >> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > >> @@ -229,9 +229,10 @@ struct rockchip_usb2phy_port { > >> * @dev: pointer to device. > >> * @grf: General Register Files regmap. > >> * @usbgrf: USB General Register Files regmap. > >> - * @clk: clock struct of phy input clk. > >> + * @clks: array of phy input clocks. > >> * @clk480m: clock struct of phy output clk. > >> * @clk480m_hw: clock struct of phy output clk management. > >> + * @num_clks: number of phy input clocks. > >> * @phy_reset: phy reset control. > >> * @chg_state: states involved in USB charger detection. > >> * @chg_type: USB charger types. > >> @@ -246,9 +247,10 @@ struct rockchip_usb2phy { > >> struct device *dev; > >> struct regmap *grf; > >> struct regmap *usbgrf; > >> - struct clk *clk; > >> + struct clk_bulk_data *clks; > >> struct clk *clk480m; > >> struct clk_hw clk480m_hw; > >> + int num_clks; > >> struct reset_control *phy_reset; > >> enum usb_chg_state chg_state; > >> enum power_supply_type chg_type; > >> @@ -376,7 +378,9 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy) > >> { > >> struct device_node *node = rphy->dev->of_node; > >> struct clk_init_data init; > >> + struct clk *refclk = NULL; > >> const char *clk_name; > >> + int i; > >> int ret = 0; > >> > >> init.flags = 0; > >> @@ -386,8 +390,15 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy) > >> /* optional override of the clockname */ > >> of_property_read_string(node, "clock-output-names", &init.name); > >> > >> - if (rphy->clk) { > >> - clk_name = __clk_get_name(rphy->clk); > >> + for (i = 0; i < rphy->num_clks; i++) { > >> + if (!strncmp(rphy->clks[i].id, "phyclk", 6)) { > >> + refclk = rphy->clks[i].clk; > >> + break; > >> + } > >> + } > >> + > >> + if (!IS_ERR(refclk)) { > >> + clk_name = __clk_get_name(refclk); > >> init.parent_names = &clk_name; > >> init.num_parents = 1; > >> } else { > >> @@ -1406,22 +1417,29 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev) > >> if (IS_ERR(rphy->phy_reset)) > >> return PTR_ERR(rphy->phy_reset); > >> > >> - rphy->clk = devm_clk_get_optional_enabled(dev, "phyclk"); > >> - if (IS_ERR(rphy->clk)) { > >> - return dev_err_probe(&pdev->dev, PTR_ERR(rphy->clk), > >> + ret = devm_clk_bulk_get_all(dev, &rphy->clks); > >> + if (ret == -EPROBE_DEFER) { > >> + return dev_err_probe(&pdev->dev, -EPROBE_DEFER, > >> "failed to get phyclk\n"); > >> } > >> > >> + /* Clocks are optional */ > >> + rphy->num_clks = ret < 0 ? 0 : ret; > >> + > >> ret = rockchip_usb2phy_clk480m_register(rphy); > >> if (ret) { > >> dev_err(dev, "failed to register 480m output clock\n"); > >> return ret; > >> } > >> > >> + ret = clk_bulk_prepare_enable(rphy->num_clks, rphy->clks); > >> + if (ret) > >> + return ret; > >> + > > Do you actually need that separate enable step? > > There exists devm_clk_bulk_get_all_enable() > > > > https://elixir.bootlin.com/linux/v6.11/source/include/linux/clk.h#L511 > > > > which you could use above. Especially as otherwise you'd need a remove > > callback to disable the clock on driver unbind? > > > > Using devm_clk_bulk_get_all() just can get clocknumbers, > and devm_clk_bulk_get_all_enable() is not. > It seems that there is no other API can get the number of clocks separately. Hmm, didn't see that yesterday, but you're right.
