RE: [PATCH v2 1/4] phy: zynqmp: Enable reference clock correctly

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Sean Anderson <sean.anderson@xxxxxxxxx>
> Sent: Monday, May 6, 2024 10:31 PM
> To: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>; linux-
> phy@xxxxxxxxxxxxxxxxxxx
> Cc: Vinod Koul <vkoul@xxxxxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Michal Simek <michal.simek@xxxxxxx>;
> Kishon Vijay Abraham I <kishon@xxxxxxxxxx>; Sean Anderson
> <sean.anderson@xxxxxxxxx>
> Subject: [PATCH v2 1/4] phy: zynqmp: Enable reference clock correctly
> 
> Lanes can use other lanes' reference clocks, as determined by refclk.
> Use refclk to determine the clock to enable/disable instead of always
> using the lane's own reference clock. This ensures the clock selected in
> xpsgtr_configure_pll is the one enabled.
> 
> For the other half of the equation, always program REF_CLK_SEL even when
> we are selecting the lane's own clock. This ensures that Linux's idea of
> the reference clock matches the hardware. We use the "local" clock mux
> for this instead of going through the ref clock network.
> 
> Fixes: 25d700833513 ("phy: xilinx: phy-zynqmp: dynamic clock support for
> power-save")
> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxxx>
> ---
> 
> Changes in v2:
> - New
> 
>  drivers/phy/xilinx/phy-zynqmp.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-
> zynqmp.c
> index f72c5257d712..5a434382356c 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -80,7 +80,8 @@
> 
>  /* Reference clock selection parameters */
>  #define L0_Ln_REF_CLK_SEL(n)		(0x2860 + (n) * 4)
> -#define L0_REF_CLK_SEL_MASK		0x8f
> +#define L0_REF_CLK_LCL_SEL		BIT(7)
> +#define L0_REF_CLK_SEL_MASK		0x9f
> 
>  /* Calibration digital logic parameters */
>  #define L3_TM_CALIB_DIG19		0xec4c
> @@ -349,11 +350,14 @@ static void xpsgtr_configure_pll(struct xpsgtr_phy
> *gtr_phy)
>  		       PLL_FREQ_MASK, ssc->pll_ref_clk);
> 
>  	/* Enable lane clock sharing, if required */
> -	if (gtr_phy->refclk != gtr_phy->lane) {
> +	if (gtr_phy->refclk == gtr_phy->lane)
> +		/* Lane3 Ref Clock Selection Register */

This is common ref clock selection and not lane 3?

> +		xpsgtr_clr_set(gtr_phy->dev, L0_Ln_REF_CLK_SEL(gtr_phy-
> >lane),
> +			       L0_REF_CLK_SEL_MASK, L0_REF_CLK_LCL_SEL);
> +	else
>  		/* Lane3 Ref Clock Selection Register */
>  		xpsgtr_clr_set(gtr_phy->dev, L0_Ln_REF_CLK_SEL(gtr_phy-
> >lane),
>  			       L0_REF_CLK_SEL_MASK, 1 << gtr_phy->refclk);
> -	}
> 
>  	/* SSC step size [7:0] */
>  	xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_0_LSB,
> @@ -573,7 +577,7 @@ static int xpsgtr_phy_init(struct phy *phy)
>  	mutex_lock(&gtr_dev->gtr_mutex);
> 
>  	/* Configure and enable the clock when peripheral phy_init call */
> -	if (clk_prepare_enable(gtr_dev->clk[gtr_phy->lane]))
> +	if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
>  		goto out;
> 
>  	/* Skip initialization if not required. */
> @@ -625,7 +629,7 @@ static int xpsgtr_phy_exit(struct phy *phy)
>  	gtr_phy->skip_phy_init = false;
> 
>  	/* Ensure that disable clock only, which configure for lane */
> -	clk_disable_unprepare(gtr_dev->clk[gtr_phy->lane]);
> +	clk_disable_unprepare(gtr_dev->clk[gtr_phy->refclk]);
> 
>  	return 0;
>  }
> --
> 2.35.1.1320.gc452695387.dirty