[1/2] clk: fixed-rate: add devm_clk_hw_register_fixed_rate

Add devm_clk_hw_register_fixed_rate(), devres-managed helper to register
fixed-rate clock.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/clk-asm9260.c    |  2 +-
 drivers/clk/clk-fixed-rate.c | 28 ++++++++++++++++++++++++----
 include/linux/clk-provider.h | 27 ++++++++++++++++++++-------
 3 files changed, 45 insertions(+), 12 deletions(-)

Quoting Dmitry Baryshkov (2022-06-20 08:39:56)
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index a2526068232b..0f31d3255897 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -1167,6 +1167,54 @@ static int qcom_qmp_phy_ufs_clk_init(struct device *dev, const struct qmp_phy_cf
>         return devm_clk_bulk_get(dev, num, qmp->clks);
>  }
>
> +static void phy_clk_release_provider(void *res)
> +{
> +       of_clk_del_provider(res);
> +}
> +
> +#define UFS_SYMBOL_CLOCKS 3
> +
> +static int phy_symbols_clk_register(struct qcom_qmp *qmp, struct device_node *np)
> +{
> +       struct clk_hw_onecell_data *clk_data;
> +       struct clk_hw *hw;
> +       int ret;
> +
> +       clk_data = devm_kzalloc(qmp->dev, struct_size(clk_data, hws, UFS_SYMBOL_CLOCKS), GFP_KERNEL);
> +       clk_data->num = UFS_SYMBOL_CLOCKS;
> +
> +       hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_rx_symbol_0_clk_src",
> +                                                          NULL, 0, 0);
> +       if (IS_ERR(hw))
> +               return PTR_ERR(hw);
> +
> +       clk_data->hws[0] = hw;
> +
> +       hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_rx_symbol_1_clk_src",
> +                                                          NULL, 0, 0);
> +       if (IS_ERR(hw))
> +               return PTR_ERR(hw);
> +
> +       clk_data->hws[1] = hw;
> +
> +       hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_tx_symbol_0_clk_src",
> +                                                          NULL, 0, 0);

This line should be aligned with the opening parenthesis. Same for the
above other two.

> +       if (IS_ERR(hw))
> +               return PTR_ERR(hw);
> +
> +       clk_data->hws[2] = hw;
> +
> +       ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
> +       if (ret)
> +               return ret;
> +
> +        /*

This is tabbed weird. It doesn't align with the if above.


> +         * 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);
> +}
> +
>  static const struct phy_ops qcom_qmp_ufs_ops = {
>         .power_on       = qcom_qmp_phy_ufs_enable,
>         .power_off      = qcom_qmp_phy_ufs_disable,

Quoting Dmitry Baryshkov (2022-06-20 08:39:55)
> Add devm_clk_hw_register_fixed_rate(), devres-managed helper to register
> fixed-rate clock.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/clk-asm9260.c    |  2 +-
>  drivers/clk/clk-fixed-rate.c | 28 ++++++++++++++++++++++++----
>  include/linux/clk-provider.h | 27 ++++++++++++++++++++-------
>  3 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/clk-asm9260.c b/drivers/clk/clk-asm9260.c
> index bacebd457e6f..4da5f38249bf 100644
> --- a/drivers/clk/clk-asm9260.c
> +++ b/drivers/clk/clk-asm9260.c
> @@ -278,7 +278,7 @@ static void __init asm9260_acc_init(struct device_node *np)
>         ref_clk = of_clk_get_parent_name(np, 0);
>         hw = __clk_hw_register_fixed_rate(NULL, NULL, pll_clk,
>                         ref_clk, NULL, NULL, 0, rate, 0,
> -                       CLK_FIXED_RATE_PARENT_ACCURACY);
> +                       CLK_FIXED_RATE_PARENT_ACCURACY, false);

Can you also make this usage into a macro so that it doesn't change in
the future when a new argument is added?

>  
>         if (IS_ERR(hw))
>                 panic("%pOFn: can't register REFCLK. Check DT!", np);

On Mon, Jun 20, 2022 at 06:39:56PM +0300, Dmitry Baryshkov wrote:
> Register three UFS symbol clocks (ufs_rx_symbol_0_clk_src,
> ufs_rx_symbol_1_clk_src ufs_tx_symbol_0_clk_src). Register OF clock
> provider to let other devices link these clocks through the DT.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 55 +++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index a2526068232b..0f31d3255897 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -1167,6 +1167,54 @@ static int qcom_qmp_phy_ufs_clk_init(struct device *dev, const struct qmp_phy_cf
>  	return devm_clk_bulk_get(dev, num, qmp->clks);
>  }
>  
> +static void phy_clk_release_provider(void *res)
> +{
> +	of_clk_del_provider(res);
> +}
> +
> +#define UFS_SYMBOL_CLOCKS 3
> +
> +static int phy_symbols_clk_register(struct qcom_qmp *qmp, struct device_node *np)
> +{
> +	struct clk_hw_onecell_data *clk_data;
> +	struct clk_hw *hw;
> +	int ret;
> +
> +	clk_data = devm_kzalloc(qmp->dev, struct_size(clk_data, hws, UFS_SYMBOL_CLOCKS), GFP_KERNEL);

Missing error handling.

> +	clk_data->num = UFS_SYMBOL_CLOCKS;
> +
> +	hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_rx_symbol_0_clk_src",

Don't the clock names need to be globally unique and hence either come
from the devicetree or encode the device topology some other way?

We have two UFS PHYs on sc8280xp for example.

> +							   NULL, 0, 0);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	clk_data->hws[0] = hw;
> +
> +	hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_rx_symbol_1_clk_src",
> +							   NULL, 0, 0);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	clk_data->hws[1] = hw;
> +
> +	hw = devm_clk_hw_register_fixed_rate(qmp->dev, "ufs_tx_symbol_0_clk_src",
> +							   NULL, 0, 0);
> +	if (IS_ERR(hw))
> +		return PTR_ERR(hw);
> +
> +	clk_data->hws[2] = hw;
> +
> +	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
> +	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);
> +}

Johan