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

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

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@nvidia.com>
>>
>> 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@nvidia.com>
>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> ---
>> 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