On Mon, May 29, 2023 at 02:56:37PM -0400, Adrien Thierry wrote:
> The AHB2PHY interface clock cfg_ahb_clk is not assigned anywhere in the
> driver. Moreover, it's not used by any device tree, nor is present in
> the qcom,usb-snps-femto-v2 bindings. Remove it.
>
The downstream driver deals with cfg_ahb as well, so I think it would be
more appropriate to ensure that it's actually wired up.
And in that case, I would find it preferable to switch to use the
clk_bulk API for the introduction of the ref clk - to clean up the error
paths if nothing else.
Regards,
Bjorn
> Signed-off-by: Adrien Thierry <athierry@xxxxxxxxxx>
> ---
> I'm not 100% sure if the clock should be removed, or if it should be added
> to bindings and device trees that use this PHY. Better informed opinions
> on the topic are highly welcome.
>
> drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 20 +------------------
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> index 8abf482e81a8..2d9c1105e28c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> @@ -113,7 +113,6 @@ struct phy_override_seq {
> * @phy: generic phy
> * @base: iomapped memory space for snps hs phy
> *
> - * @cfg_ahb_clk: AHB2PHY interface clock
> * @ref_clk: phy reference clock
> * @phy_reset: phy reset control
> * @vregs: regulator supplies bulk data
> @@ -125,7 +124,6 @@ struct qcom_snps_hsphy {
> struct phy *phy;
> void __iomem *base;
>
> - struct clk *cfg_ahb_clk;
> struct clk *ref_clk;
> struct reset_control *phy_reset;
> struct regulator_bulk_data vregs[SNPS_HS_NUM_VREGS];
> @@ -165,7 +163,6 @@ static int qcom_snps_hsphy_suspend(struct qcom_snps_hsphy *hsphy)
> 0, USB2_AUTO_RESUME);
> }
>
> - clk_disable_unprepare(hsphy->cfg_ahb_clk);
> clk_disable_unprepare(hsphy->ref_clk);
> return 0;
> }
> @@ -176,12 +173,6 @@ static int qcom_snps_hsphy_resume(struct qcom_snps_hsphy *hsphy)
>
> dev_dbg(&hsphy->phy->dev, "Resume QCOM SNPS PHY, mode\n");
>
> - ret = clk_prepare_enable(hsphy->cfg_ahb_clk);
> - if (ret) {
> - dev_err(&hsphy->phy->dev, "failed to enable cfg ahb clock\n");
> - return ret;
> - }
> -
> ret = clk_prepare_enable(hsphy->ref_clk);
> if (ret) {
> dev_err(&hsphy->phy->dev, "failed to enable ref clock\n");
> @@ -381,16 +372,10 @@ static int qcom_snps_hsphy_init(struct phy *phy)
> if (ret)
> return ret;
>
> - ret = clk_prepare_enable(hsphy->cfg_ahb_clk);
> - if (ret) {
> - dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
> - goto poweroff_phy;
> - }
> -
> ret = clk_prepare_enable(hsphy->ref_clk);
> if (ret) {
> dev_err(&phy->dev, "failed to enable ref clock, %d\n", ret);
> - goto disable_ahb_clk;
> + goto poweroff_phy;
> }
>
> ret = reset_control_assert(hsphy->phy_reset);
> @@ -463,8 +448,6 @@ static int qcom_snps_hsphy_init(struct phy *phy)
>
> disable_ref_clk:
> clk_disable_unprepare(hsphy->ref_clk);
> -disable_ahb_clk:
> - clk_disable_unprepare(hsphy->cfg_ahb_clk);
> poweroff_phy:
> regulator_bulk_disable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);
>
> @@ -476,7 +459,6 @@ static int qcom_snps_hsphy_exit(struct phy *phy)
> struct qcom_snps_hsphy *hsphy = phy_get_drvdata(phy);
>
> reset_control_assert(hsphy->phy_reset);
> - clk_disable_unprepare(hsphy->cfg_ahb_clk);
> clk_disable_unprepare(hsphy->ref_clk);
> regulator_bulk_disable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);
> hsphy->phy_initialized = false;
> --
> 2.40.1
>
