Re: [PATCH v7 8/9] PCI: tegra194: Add core monitor clock support

[Manikanta Maddireddy]

On 05/03/26 4:42 pm, Manivannan Sadhasivam wrote:
> On Tue, Mar 03, 2026 at 12:27:57PM +0530, Manikanta Maddireddy wrote:
> > From: Vidya Sagar <vidyas@xxxxxxxxxx>
> >
> > Tegra supports PCIe core clock monitoring for any rate changes that may be
> > happening because of the link speed changes. This is useful in tracking
> > any changes in the core clock that are not initiated by the software. This
> > patch adds support to parse the monitor clock info from device-tree and
> > enable it if present.
>
> Reword the description in imperative mood and avoid 'This patch...'.
>
> > Reviewed-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> > Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> > ---
> > Changes V1 -> V7: None
> >
> >   drivers/pci/controller/dwc/pcie-tegra194.c | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> > index 070eb7f4058d..e0455d322166 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -249,6 +249,7 @@ struct tegra_pcie_dw {
> >   	struct resource *atu_dma_res;
> >   	void __iomem *appl_base;
> >   	struct clk *core_clk;
> > +	struct clk *core_clk_m;
> >   	struct reset_control *core_apb_rst;
> >   	struct reset_control *core_rst;
> >   	struct dw_pcie pci;
> > @@ -945,6 +946,8 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
> >   	}
> >   
> >   	clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ);
> > +	if (clk_prepare_enable(pcie->core_clk_m))
> > +		dev_err(pci->dev, "Failed to enable core monitor clock\n");
> >   
> >   	return 0;
> >   }
> > @@ -1017,6 +1020,12 @@ static int tegra_pcie_dw_start_link(struct dw_pcie *pci)
> >   		val &= ~PCI_DLF_EXCHANGE_ENABLE;
> >   		dw_pcie_writel_dbi(pci, offset + PCI_DLF_CAP, val);
> >   
> > +		/*
> > +		 * core_clk_m is enabled as part of host_init callback in
> > +		 * dw_pcie_host_init(). Disable the clock since below
> > +		 * tegra_pcie_dw_host_init() will enable it again.
> > +		 */
> > +		clk_disable_unprepare(pcie->core_clk_m);
>
> Again, this change should be in a separate patch.
This patch is handling only one feature, which is enabling monitor 
clock. Monitor clock is enabled in tegra_pcie_dw_host_init(), this 
function is executed twice when handling DLFE fix, so this line is 
disabling the clock before executing tegra_pcie_dw_host_init() 2nd time.
I think one patch is sufficient for this, otherwise it will introduce 
unbalanced clock enable error.

- Manikanta
> >   		tegra_pcie_dw_host_init(pp);
> >   		dw_pcie_setup_rc(pp);
> >   
> > @@ -1610,6 +1619,7 @@ static void tegra_pcie_dw_pme_turnoff(struct tegra_pcie_dw *pcie)
> >   
> >   static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie)
> >   {
> > +	clk_disable_unprepare(pcie->core_clk_m);
> >   	dw_pcie_host_deinit(&pcie->pci.pp);
> >   	tegra_pcie_dw_pme_turnoff(pcie);
> >   	tegra_pcie_unconfig_controller(pcie);
> > @@ -2161,6 +2171,13 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
> >   		return PTR_ERR(pcie->core_clk);
> >   	}
> >   
> > +	pcie->core_clk_m = devm_clk_get_optional(dev, "core_m");
> > +	if (IS_ERR(pcie->core_clk_m)) {
> > +		dev_err(dev, "Failed to get monitor clock: %ld\n",
> > +			PTR_ERR(pcie->core_clk_m));
>
> To simplify, just do:
>
> 		return dev_err_probe();
>
> - Mani

--
nvpublic