On Thu, Sep 11, 2025 at 10:55:05PM +0800, Xiangxu Yin wrote: > Add USB/DP switchable PHY clock registration and DT parsing for DP offsets. > Extend qmp_usbc_register_clocks and clock provider logic to support both > USB and DP instances. > > Signed-off-by: Xiangxu Yin <xiangxu....@oss.qualcomm.com> > --- > drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 208 > +++++++++++++++++++++++++++++-- > 1 file changed, 195 insertions(+), 13 deletions(-) > > @@ -1276,8 +1291,11 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, > struct device_node *np) > > ret = of_property_read_string(np, "clock-output-names", &init.name); > if (ret) { > - dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np); > - return ret; > + char name[64]; > + > + /* Clock name is not mandatory. */ > + snprintf(name, sizeof(name), "%s::pipe_clk", > dev_name(qmp->dev)); > + init.name = name; > }
Do we have any guarantees that memory for 'name' exists beyond this point? > > init.ops = &clk_fixed_rate_ops; > @@ -1286,19 +1304,176 @@ static int phy_pipe_clk_register(struct qmp_usbc > *qmp, struct device_node *np) > fixed->fixed_rate = 125000000; > fixed->hw.init = &init; > > - ret = devm_clk_hw_register(qmp->dev, &fixed->hw); > - if (ret) > + return devm_clk_hw_register(qmp->dev, &fixed->hw); > +} > + > + > +/* > + * Display Port PLL driver block diagram for branch clocks > + * > + * +------------------------------+ > + * | DP_VCO_CLK | > + * | | > + * | +-------------------+ | > + * | | (DP PLL/VCO) | | > + * | +---------+---------+ | > + * | v | > + * | +----------+-----------+ | > + * | | hsclk_divsel_clk_src | | > + * | +----------+-----------+ | > + * +------------------------------+ > + * | > + * +---------<---------v------------>----------+ > + * | | > + * +--------v----------------+ | > + * | dp_phy_pll_link_clk | | > + * | link_clk | | > + * +--------+----------------+ | > + * | | > + * | | > + * v v > + * Input to DISPCC block | > + * for link clk, crypto clk | > + * and interface clock | > + * | > + * | > + * +--------<------------+-----------------+---<---+ > + * | | | > + * +----v---------+ +--------v-----+ +--------v------+ > + * | vco_divided | | vco_divided | | vco_divided | > + * | _clk_src | | _clk_src | | _clk_src | > + * | | | | | | > + * |divsel_six | | divsel_two | | divsel_four | > + * +-------+------+ +-----+--------+ +--------+------+ > + * | | | > + * v---->----------v-------------<------v > + * | > + * +----------+-----------------+ > + * | dp_phy_pll_vco_div_clk | > + * +---------+------------------+ > + * | > + * v > + * Input to DISPCC block > + * for DP pixel clock > + * > + */ > +static int qmp_dp_pixel_clk_determine_rate(struct clk_hw *hw, struct > clk_rate_request *req) > +{ > + switch (req->rate) { > + case 1620000000UL / 2: > + case 2700000000UL / 2: > + /* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */ > + return 0; > + default: > + return -EINVAL; > + } > +} > + > +static unsigned long qmp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, > unsigned long parent_rate) > +{ > + const struct qmp_usbc *qmp; > + const struct phy_configure_opts_dp *dp_opts; > + > + qmp = container_of(hw, struct qmp_usbc, dp_pixel_hw); > + > + dp_opts = &qmp->dp_opts; > + > + switch (dp_opts->link_rate) { > + case 1620: > + return 1620000000UL / 2; > + case 2700: > + return 2700000000UL / 2; > + case 5400: > + return 5400000000UL / 4; No HBR3 support? Then why was it mentioned few lines above? > + default: > + return 0; > + } > +} > + > +static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node > *np) > +{ > + int ret; > > - ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw); > + ret = phy_pipe_clk_register(qmp, np); > if (ret) > return ret; > > - /* > - * Roll a devm action because the clock provider is the child node, but > - * the child node is not actually a device. > - */ > - return devm_add_action_or_reset(qmp->dev, phy_clk_release_provider, np); > + if (qmp->dp_serdes != 0) { > + ret = phy_dp_clks_register(qmp, np); > + if (ret) > + return ret; > + } > + > + return devm_of_clk_add_hw_provider(qmp->dev, qmp_usbc_clks_hw_get, qmp); Do you understand what did the comment (that you've removed) say? And why? > } > > #if IS_ENABLED(CONFIG_TYPEC) > @@ -1429,6 +1604,13 @@ static int qmp_usbc_parse_dt(struct qmp_usbc *qmp) > if (IS_ERR(base)) > return PTR_ERR(base); > > + if (offs->dp_serdes != 0) { > + qmp->dp_serdes = base + offs->dp_serdes; > + qmp->dp_tx = base + offs->dp_txa; > + qmp->dp_tx2 = base + offs->dp_txb; > + qmp->dp_dp_phy = base + offs->dp_dp_phy; > + } > + > qmp->serdes = base + offs->serdes; > qmp->pcs = base + offs->pcs; > if (offs->pcs_misc) > @@ -1537,7 +1719,7 @@ static int qmp_usbc_probe(struct platform_device *pdev) > */ > pm_runtime_forbid(dev); > > - ret = phy_pipe_clk_register(qmp, np); > + ret = qmp_usbc_register_clocks(qmp, np); > if (ret) > goto err_node_put; > > > -- > 2.34.1 > -- With best wishes Dmitry