Re: [PATCH RFC 3/4] clk: qcom: tcsrcc-glymur: Migrate tcsr_pcie_N_clkref_en to clk_ref common helper

[Qiang Yu]

On Wed, Apr 01, 2026 at 10:05:12PM +0530, Taniya Das wrote:
> 
> 
> On 4/1/2026 12:05 PM, Qiang Yu wrote:
> > Replace local clk_branch-based clkref definitions with descriptor-based
> > registration via qcom_clk_ref_probe().
> > 
> > This keeps the glymur driver focused on clock metadata and reuses common
> > runtime logic for regulator handling, enable/disable sequencing, and OF
> > provider wiring.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/clk/qcom/tcsrcc-glymur.c | 340 +++++++++++----------------------------
> >  1 file changed, 93 insertions(+), 247 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/tcsrcc-glymur.c b/drivers/clk/qcom/tcsrcc-glymur.c
> > index 9c0edebcdbb12816d1be5249e4f04bcaf02048aa..585f87b23af2d92daef1787b2f38911681c0d8ee 100644
> > --- a/drivers/clk/qcom/tcsrcc-glymur.c
> > +++ b/drivers/clk/qcom/tcsrcc-glymur.c
> > @@ -4,265 +4,115 @@
> >   */
> >  
> >  #include <linux/clk-provider.h>
> > +#include <linux/clk/qcom.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/module.h>
> > +#include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> >  
> >  #include <dt-bindings/clock/qcom,glymur-tcsr.h>
> >  
> > -#include "clk-alpha-pll.h"
> > -#include "clk-branch.h"
> > -#include "clk-pll.h"
> > -#include "clk-rcg.h"
> > -#include "clk-regmap.h"
> > -#include "clk-regmap-divider.h"
> > -#include "clk-regmap-mux.h"
> > -#include "common.h"
> > -#include "gdsc.h"
> > -#include "reset.h"
> > -
> > -enum {
> > -	DT_BI_TCXO_PAD,
> > -};
> > -
> > -static struct clk_branch tcsr_edp_clkref_en = {
> > -	.halt_reg = 0x60,
> > -	.halt_check = BRANCH_HALT_DELAY,
> > -	.clkr = {
> > -		.enable_reg = 0x60,
> > -		.enable_mask = BIT(0),
> > -		.hw.init = &(const struct clk_init_data) {
> > -			.name = "tcsr_edp_clkref_en",
> > -			.parent_data = &(const struct clk_parent_data){
> > -				.index = DT_BI_TCXO_PAD,
> > -			},
> > -			.num_parents = 1,
> > -			.ops = &clk_branch2_ops,
> > -		},
> > +static const char * const tcsr_pcie_1_regulators[] = {
> > +	"vdda-refgen-0p9",
> > +	"vdda-refgen-1p2",
> > +	"vdda-qrefrx5-0p9",
> > +	"vdda-qreftx0-0p9",
> > +	"vdda-qreftx0-1p2",
> > +};
> > +
> > +static const char * const tcsr_pcie_2_regulators[] = {
> > +	"vdda-refgen-0p9",
> > +	"vdda-refgen-1p2",
> > +	"vdda-qreftx1-0p9",
> > +	"vdda-qrefrpt0-0p9",
> > +	"vdda-qrefrpt1-0p9",
> > +	"vdda-qrefrpt2-0p9",
> > +	"vdda-qrefrx2-0p9",
> > +};
> > +
> > +static const char * const tcsr_pcie_3_regulators[] = {
> > +	"vdda-refgen-0p9",
> > +	"vdda-refgen-1p2",
> > +	"vdda-qreftx1-0p9",
> > +	"vdda-qrefrpt0-0p9",
> > +	"vdda-qrefrpt1-0p9",
> > +	"vdda-qrefrx1-0p9",
> > +};
> > +
> > +static const char * const tcsr_pcie_4_regulators[] = {
> > +	"vdda-refgen-0p9",
> > +	"vdda-refgen-1p2",
> > +	"vdda-qreftx1-0p9",
> > +	"vdda-qrefrpt0-0p9",
> > +	"vdda-qrefrpt1-0p9",
> > +	"vdda-qrefrpt2-0p9",
> > +	"vdda-qrefrx2-0p9",
> > +};
> > +
> 
> TCSR clock refs are just not for PCIe alone, they would have supplies
> for all the ref clocks. These supplies can also be shared across other
> clock refs. I think it is not the correct way to handle the supplies, as
> TCSR does not have the complete supplies map.
>
We have complete supplies map. You can get it on ipcatlog. Here is example
for other instances eg USB and EDP:
- Glymur (eDP): CXO PAD -> TX0 -> RPT0 -> RX0 -> eDP
- Glymur (USB4_2): CXO PAD -> TX0 -> RPT0 -> RPT1 -> RX1 -> USB4_2
- Glymur (USB3): CXO PAD -> TX0 -> RPT3 -> RPT4 -> RX4 -> USB3_SS3

I only add supplies for PCIe in this series because USB and EDP vote these
LDO in their PHY driver. They can remove them in PHY dts node and add same
regulator list here.

- Qiang Yu
> 
> > +static const struct qcom_clk_ref_desc tcsr_cc_glymur_clk_descs[] = {
> > +	[TCSR_EDP_CLKREF_EN] = {
> > +		.name = "tcsr_edp_clkref_en",
> > +		.offset = 0x60,
> >  	},
> > -};
> >
> 
> 
> -- 
> Thanks,
> Taniya Das
>