| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <jjsijdmh4hdbgd2boebtrmzvblvhz2hnl7mtv5ga76ine2fnsb@i72dz3r4lbjp>
Date: Wed, 20 Aug 2025 14:42:31 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Xiangxu Yin <xiangxu.yin@....qualcomm.com>
Cc: Rob Clark <robin.clark@....qualcomm.com>,
Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>,
Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, fange.zhang@....qualcomm.com,
yongxing.mou@....qualcomm.com, tingwei.zhang@....qualcomm.com,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, quic_lliu6@...cinc.com
Subject: Re: [PATCH v3 11/14] phy: qcom: qmp-usbc: Finalize USB/DP switchable
PHY support
On Wed, Aug 20, 2025 at 05:34:53PM +0800, Xiangxu Yin wrote:
> Complete USB/DP switchable PHY integration by adding DP clock
> registration, aux bridge setup, and DT parsing. Implement clock
> provider logic for USB and DP branches, and extend PHY translation
> to support both USB and DP instances.
>
> Signed-off-by: Xiangxu Yin <xiangxu.yin@....qualcomm.com>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
> 1 file changed, 299 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index 821398653bef23e1915d9d3a3a2950b0bfbefb9a..74b9f75c8864efe270f394bfbfd748793dada1f5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -995,6 +995,11 @@ static int qmp_usbc_usb_power_on(struct phy *phy)
> qmp_configure(qmp->dev, qmp->serdes, cfg->serdes_tbl,
> cfg->serdes_tbl_num);
>
> + if (IS_ERR(qmp->pipe_clk)) {
> + return dev_err_probe(qmp->dev, PTR_ERR(qmp->pipe_clk),
> + "pipe clock not defined\n");
> + }
No, this should not be allowed.
> +
> ret = clk_prepare_enable(qmp->pipe_clk);
> if (ret) {
> dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
> @@ -1365,11 +1370,13 @@ static int __maybe_unused qmp_usbc_runtime_resume(struct device *dev)
> if (ret)
> return ret;
>
> - ret = clk_prepare_enable(qmp->pipe_clk);
> - if (ret) {
> - dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
> - clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
> - return ret;
> + if (!IS_ERR(qmp->pipe_clk)) {
Similarly.
> + ret = clk_prepare_enable(qmp->pipe_clk);
> + if (ret) {
> + dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
> + clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
> + return ret;
> + }
> }
>
> qmp_usbc_disable_autonomous_mode(qmp);
> @@ -1422,9 +1429,23 @@ static int qmp_usbc_clk_init(struct qmp_usbc *qmp)
> return devm_clk_bulk_get_optional(dev, num, qmp->clks);
> }
>
> -static void phy_clk_release_provider(void *res)
> +static struct clk_hw *qmp_usbc_clks_hw_get(struct of_phandle_args *clkspec, void *data)
> {
> - of_clk_del_provider(res);
> + struct qmp_usbc *qmp = data;
> +
> + if (clkspec->args_count == 0)
> + return &qmp->pipe_clk_fixed.hw;
> +
> + switch (clkspec->args[0]) {
> + case QMP_USB43DP_USB3_PIPE_CLK:
> + return &qmp->pipe_clk_fixed.hw;
> + case QMP_USB43DP_DP_LINK_CLK:
> + return &qmp->dp_link_hw;
> + case QMP_USB43DP_DP_VCO_DIV_CLK:
> + return &qmp->dp_pixel_hw;
> + }
> +
> + return ERR_PTR(-EINVAL);
> }
>
> /*
> @@ -1453,8 +1474,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;
> }
>
> init.ops = &clk_fixed_rate_ops;
> @@ -1463,19 +1487,7 @@ 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 ret;
> -
> - ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> - 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);
> + return devm_clk_hw_register(qmp->dev, &fixed->hw);
> }
>
> #if IS_ENABLED(CONFIG_TYPEC)
> @@ -1660,6 +1672,235 @@ static int qmp_usbc_parse_tcsr(struct qmp_usbc *qmp)
> return 0;
> }
>
> +static int qmp_usbc_parse_usb3dp_dt(struct qmp_usbc *qmp)
> +{
> + struct platform_device *pdev = to_platform_device(qmp->dev);
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + const struct qmp_usbc_offsets *offs = cfg->offsets;
> + struct device *dev = qmp->dev;
> + void __iomem *base;
> + int ret;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + 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;
Squash this into qmp_usbc_parse_dt(). Set these fields if
dp_serdes != 0.
> + qmp->serdes = base + offs->serdes;
> + qmp->pcs = base + offs->pcs;
> + if (offs->pcs_misc)
> + qmp->pcs_misc = base + offs->pcs_misc;
> + qmp->tx = base + offs->tx;
> + qmp->rx = base + offs->rx;
> +
> + qmp->tx2 = base + offs->tx2;
> + qmp->rx2 = base + offs->rx2;
> +
> + ret = qmp_usbc_clk_init(qmp);
> + if (ret)
> + return ret;
> +
> + qmp->pipe_clk = devm_clk_get(dev, "pipe");
> + if (IS_ERR(qmp->pipe_clk)) {
> + /* usb3dp allow no pipe clk define */
> + if (cfg->type == QMP_PHY_USBC_USB3_ONLY)
> + return dev_err_probe(dev, PTR_ERR(qmp->pipe_clk),
> + "failed to get pipe clock\n");
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * 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;
> + default:
> + return 0;
> + }
> +}
> +
> +static const struct clk_ops qmp_dp_pixel_clk_ops = {
> + .determine_rate = qmp_dp_pixel_clk_determine_rate,
> + .recalc_rate = qmp_dp_pixel_clk_recalc_rate,
> +};
> +
> +static int qmp_dp_link_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
> +{
> + switch (req->rate) {
> + case 162000000:
> + case 270000000:
> + case 540000000:
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static unsigned long qmp_dp_link_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_link_hw);
> + dp_opts = &qmp->dp_opts;
> +
> + switch (dp_opts->link_rate) {
> + case 1620:
> + case 2700:
> + case 5400:
> + return dp_opts->link_rate * 100000;
> + default:
> + return 0;
> + }
> +}
> +
> +static const struct clk_ops qmp_dp_link_clk_ops = {
> + .determine_rate = qmp_dp_link_clk_determine_rate,
> + .recalc_rate = qmp_dp_link_clk_recalc_rate,
> +};
> +
> +static int phy_dp_clks_register(struct qmp_usbc *qmp, struct device_node *np)
> +{
> + struct clk_init_data init = { };
> + char name[64];
> + int ret;
> +
> + snprintf(name, sizeof(name), "%s::link_clk", dev_name(qmp->dev));
> + init.ops = &qmp_dp_link_clk_ops;
> + init.name = name;
> + qmp->dp_link_hw.init = &init;
> + ret = devm_clk_hw_register(qmp->dev, &qmp->dp_link_hw);
> + if (ret < 0) {
> + dev_err(qmp->dev, "link clk reg fail ret=%d\n", ret);
> + return ret;
> + }
> +
> + snprintf(name, sizeof(name), "%s::vco_div_clk", dev_name(qmp->dev));
> + init.ops = &qmp_dp_pixel_clk_ops;
> + init.name = name;
> + qmp->dp_pixel_hw.init = &init;
> + ret = devm_clk_hw_register(qmp->dev, &qmp->dp_pixel_hw);
> + if (ret) {
> + dev_err(qmp->dev, "pxl clk reg fail ret=%d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node *np)
> +{
> + int ret;
> +
> + if (!IS_ERR(qmp->pipe_clk)) {
> + ret = phy_pipe_clk_register(qmp, np);
> + if (ret)
> + return ret;
> + }
> +
> + if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
if 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);
> +}
> +
> +static struct phy *qmp_usbc_phy_xlate(struct device *dev, const struct of_phandle_args *args)
> +{
> + struct qmp_usbc *qmp = dev_get_drvdata(dev);
> +
> + if (args->args_count == 0)
> + return qmp->usb_phy;
> +
> + switch (args->args[0]) {
> + case QMP_USB43DP_USB3_PHY:
> + return qmp->usb_phy;
> + case QMP_USB43DP_DP_PHY:
> + return qmp->dp_phy;
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> static int qmp_usbc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - /* Check for legacy binding with child node. */
> - np = of_get_child_by_name(dev->of_node, "phy");
> - if (np) {
> - ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
> - } else {
> + if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
Should not be necessary.
> np = of_node_get(dev->of_node);
> - ret = qmp_usbc_parse_usb_dt(qmp);
> +
> + ret = qmp_usbc_parse_usb3dp_dt(qmp);
> + if (ret) {
> + dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
> + goto err_node_put;
> + }
> +
> + ret = drm_aux_bridge_register(dev);
> + if (ret) {
> + dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
> + goto err_node_put;
> + }
> + } else {
> + /* Check for legacy binding with child node. */
> + np = of_get_child_by_name(dev->of_node, "phy");
> + if (np) {
> + ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
> + } else {
> + np = of_node_get(dev->of_node);
> + ret = qmp_usbc_parse_usb_dt(qmp);
> + }
> + if (ret)
> + goto err_node_put;
> }
> - if (ret)
> - goto err_node_put;
>
> pm_runtime_set_active(dev);
> ret = devm_pm_runtime_enable(dev);
> @@ -1724,7 +1981,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;
>
> @@ -1737,9 +1994,19 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>
> phy_set_drvdata(qmp->usb_phy, qmp);
>
> + if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
if dp_serdes != 0
> + qmp->dp_phy = devm_phy_create(dev, np, &qmp_usbc_dp_phy_ops);
> + if (IS_ERR(qmp->dp_phy)) {
> + ret = PTR_ERR(qmp->dp_phy);
> + dev_err(dev, "failed to create PHY: %d\n", ret);
> + goto err_node_put;
> + }
> + phy_set_drvdata(qmp->dp_phy, qmp);
> + }
> +
> of_node_put(np);
>
> - phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>
> return PTR_ERR_OR_ZERO(phy_provider);
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists