The PCIe Gen4x2 PHY found in the SM8[456]50 SoCs have a second clock,
in order to expose it, split the current clock registering in two parts:
- CCF clock registering
- DT clock registering
Also switch to devm_of_clk_add_hw_provider().
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 8836bb1ff0cc..079b3e306489 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -3635,11 +3635,6 @@ static int qmp_pcie_clk_init(struct qmp_pcie *qmp)
return devm_clk_bulk_get_optional(dev, num, qmp->clks);
}
-static void phy_clk_release_provider(void *res)
-{
- of_clk_del_provider(res);
-}
-
/*
* Register a fixed rate pipe clock.
*
@@ -3664,7 +3659,7 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
struct clk_init_data init = { };
int ret;
- ret = of_property_read_string(np, "clock-output-names", &init.name);
+ ret = of_property_read_string_index(np, "clock-output-names", 0, &init.name);
if (ret) {
dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
return ret;
@@ -3683,19 +3678,19 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
fixed->hw.init = &init;
- ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
- if (ret)
- return ret;
+ return devm_clk_hw_register(qmp->dev, &fixed->hw);
+}
- ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
+static int qmp_pcie_register_clocks(struct qmp_pcie *qmp, struct device_node *np)
+{
+ int ret;
+
+ 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);
+ return devm_of_clk_add_hw_provider(qmp->dev, of_clk_hw_simple_get,
+ &qmp->pipe_clk_fixed.hw);
}
static int qmp_pcie_parse_dt_legacy(struct qmp_pcie *qmp, struct device_node *np)
@@ -3899,7 +3894,7 @@ static int qmp_pcie_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
- ret = phy_pipe_clk_register(qmp, np);
+ ret = qmp_pcie_register_clocks(qmp, np);
if (ret)
goto err_node_put;
--
2.34.1On Tue, 19 Mar 2024 at 12:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> The PCIe Gen4x2 PHY found in the SM8[456]50 SoCs have a second clock,
> in order to expose it, split the current clock registering in two parts:
> - CCF clock registering
> - DT clock registering
>
> Also switch to devm_of_clk_add_hw_provider().
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 27 +++++++++++----------------
> 1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 8836bb1ff0cc..079b3e306489 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -3635,11 +3635,6 @@ static int qmp_pcie_clk_init(struct qmp_pcie *qmp)
> return devm_clk_bulk_get_optional(dev, num, qmp->clks);
> }
>
> -static void phy_clk_release_provider(void *res)
> -{
> - of_clk_del_provider(res);
> -}
> -
> /*
> * Register a fixed rate pipe clock.
> *
> @@ -3664,7 +3659,7 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
> struct clk_init_data init = { };
> int ret;
>
> - ret = of_property_read_string(np, "clock-output-names", &init.name);
> + ret = of_property_read_string_index(np, "clock-output-names", 0, &init.name);
> if (ret) {
> dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
> return ret;
> @@ -3683,19 +3678,19 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
>
> fixed->hw.init = &init;
>
> - ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
> - if (ret)
> - return ret;
> + return devm_clk_hw_register(qmp->dev, &fixed->hw);
> +}
>
> - ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> +static int qmp_pcie_register_clocks(struct qmp_pcie *qmp, struct device_node *np)
> +{
> + int ret;
> +
> + 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);
> + return devm_of_clk_add_hw_provider(qmp->dev, of_clk_hw_simple_get,
> + &qmp->pipe_clk_fixed.hw);
No. The driver has to register a clock provider at the np rather than
at dev->of_node. Otherwise legacy DT will be broken.
> }
>
> static int qmp_pcie_parse_dt_legacy(struct qmp_pcie *qmp, struct device_node *np)
> @@ -3899,7 +3894,7 @@ static int qmp_pcie_probe(struct platform_device *pdev)
> if (ret)
> goto err_node_put;
>
> - ret = phy_pipe_clk_register(qmp, np);
> + ret = qmp_pcie_register_clocks(qmp, np);
> if (ret)
> goto err_node_put;
>
>
> --
> 2.34.1
>
>
--
With best wishes
Dmitry
On 19/03/2024 11:50, Dmitry Baryshkov wrote:
> On Tue, 19 Mar 2024 at 12:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> The PCIe Gen4x2 PHY found in the SM8[456]50 SoCs have a second clock,
>> in order to expose it, split the current clock registering in two parts:
>> - CCF clock registering
>> - DT clock registering
>>
>> Also switch to devm_of_clk_add_hw_provider().
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 27 +++++++++++----------------
>> 1 file changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> index 8836bb1ff0cc..079b3e306489 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> @@ -3635,11 +3635,6 @@ static int qmp_pcie_clk_init(struct qmp_pcie *qmp)
>> return devm_clk_bulk_get_optional(dev, num, qmp->clks);
>> }
>>
>> -static void phy_clk_release_provider(void *res)
>> -{
>> - of_clk_del_provider(res);
>> -}
>> -
>> /*
>> * Register a fixed rate pipe clock.
>> *
>> @@ -3664,7 +3659,7 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
>> struct clk_init_data init = { };
>> int ret;
>>
>> - ret = of_property_read_string(np, "clock-output-names", &init.name);
>> + ret = of_property_read_string_index(np, "clock-output-names", 0, &init.name);
>> if (ret) {
>> dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
>> return ret;
>> @@ -3683,19 +3678,19 @@ static int phy_pipe_clk_register(struct qmp_pcie *qmp, struct device_node *np)
>>
>> fixed->hw.init = &init;
>>
>> - ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
>> - if (ret)
>> - return ret;
>> + return devm_clk_hw_register(qmp->dev, &fixed->hw);
>> +}
>>
>> - ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
>> +static int qmp_pcie_register_clocks(struct qmp_pcie *qmp, struct device_node *np)
>> +{
>> + int ret;
>> +
>> + 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);
>> + return devm_of_clk_add_hw_provider(qmp->dev, of_clk_hw_simple_get,
>> + &qmp->pipe_clk_fixed.hw);
>
> No. The driver has to register a clock provider at the np rather than
> at dev->of_node. Otherwise legacy DT will be broken.
Indeed
Thx
Neil
>
>> }
>>
>> static int qmp_pcie_parse_dt_legacy(struct qmp_pcie *qmp, struct device_node *np)
>> @@ -3899,7 +3894,7 @@ static int qmp_pcie_probe(struct platform_device *pdev)
>> if (ret)
>> goto err_node_put;
>>
>> - ret = phy_pipe_clk_register(qmp, np);
>> + ret = qmp_pcie_register_clocks(qmp, np);
>> if (ret)
>> goto err_node_put;
>>
>>
>> --
>> 2.34.1
>>
>>
>
>
© 2016 - 2026 Red Hat, Inc.