Re: [PATCH RFC 2/2] phy: qcom-snps-femto-v2: Remove AHB2PHY interface clock

[Bjorn Andersson <andersson@kernel.org>]

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@redhat.com>
> ---
> 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
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy