Re: [PATCH 3/6] ptp: qcom: Add PTP driver for the Qualcomm TSC hardware

[Imran Shaik <imran.shaik@oss.qualcomm.com>]

On 27-07-2026 09:23 pm, Krzysztof Kozlowski wrote:
> On 27/07/2026 16:10, Imran Shaik wrote:
>> Add a PTP Hardware Clock driver for the Qualcomm Timestamp Counter (TSC)
>> hardware found on Qualcomm Lemans and QDU1000 SoCs. The TSC is a free
>> running hardware counter used for time synchronization, clocked by an AHB
>> configuration clock and a global counter clock, with the counter resolution
>> varying across SoCs.
>>
>> Co-developed-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/ptp/Kconfig        |  10 ++
>>   drivers/ptp/Makefile       |   1 +
>>   drivers/ptp/ptp_qcom_tsc.c | 413 +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 424 insertions(+)
>>
>> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
>> index b93640ca08b7286fd4298519a17d1daf19056498..d3ceb1ee01a505cdd0bd056649d245308fc9f140 100644
>> --- a/drivers/ptp/Kconfig
>> +++ b/drivers/ptp/Kconfig
>> @@ -263,4 +263,14 @@ config PTP_NETC_V4_TIMER
>>   	  synchronization. It also supports periodic output signal (e.g. PPS)
>>   	  and external trigger timestamping.
>>   
>> +config PTP_QCOM_TSC
>> +	tristate "Qualcomm TSC as PTP clock"
> 
> Missing depensd on ARCH_QCOM
> 

Will fix it in the next series.

>> +	depends on COMMON_CLK && PTP_1588_CLOCK
>> +	help
>> +	  This driver adds support for using the Qualcomm Timestamp Counter
>> +	  Subsystem (TSCSS) as a PTP clock.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called ptp_qcom_tsc.
>> +
>>   endmenu
> 
> ...
> 
>> +
>> +static const struct of_device_id qcom_tsc_of_match[] = {
>> +	{ .compatible = "qcom,lemans-tscss", .data = &qcom_tsc_lemans },
>> +	{ .compatible = "qcom,qdu1000-tscss", .data = &qcom_tsc_qdu1000 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_tsc_of_match);
>> +
>> +static void qcom_ptp_tsc_remove(struct platform_device *pdev)
> 
> Completely broken order of ID table, remove() and probe(). Remove ALWAYS
> follows probe. Table is next to them. Please look at other drivers.
> 

Sure, will correct the order in the next series.

>> +{
>> +	struct qcom_tsc *tsc = platform_get_drvdata(pdev);
>> +
>> +	if (tsc && tsc->ptp) {
>> +		ptp_clock_unregister(tsc->ptp);
>> +		tsc->ptp = NULL;
>> +	}
>> +}
>> +
>> +static int qcom_ptp_tsc_probe(struct platform_device *pdev)
>> +{
>> +	const struct qcom_tsc_soc_data *soc;
>> +	struct qcom_tsc *tsc;
>> +	struct resource *r_mem;
>> +	int ret;
>> +
>> +	soc = of_device_get_match_data(&pdev->dev);
>> +	if (!soc)
>> +		return -ENODEV;
>> +
>> +	tsc = devm_kzalloc(&pdev->dev, sizeof(*tsc), GFP_KERNEL);
>> +	if (!tsc)
>> +		return -ENOMEM;
>> +
>> +	tsc->dev = &pdev->dev;
>> +	tsc->soc = soc;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &tsc->lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	r_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tsc");
>> +	if (!r_mem) {
>> +		dev_err(&pdev->dev, "no IO resource defined\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	tsc->base = devm_ioremap_resource(&pdev->dev, r_mem);
>> +	if (IS_ERR(tsc->base))
>> +		return PTR_ERR(tsc->base);
>> +
>> +	tsc->ahb_clk = devm_clk_get_enabled(&pdev->dev, "ahb");
>> +	if (IS_ERR(tsc->ahb_clk))
>> +		return PTR_ERR(tsc->ahb_clk);
>> +
>> +	tsc->cntr_clk = devm_clk_get_enabled(&pdev->dev, "cntr");
>> +	if (IS_ERR(tsc->cntr_clk))
>> +		return PTR_ERR(tsc->cntr_clk);
>> +
>> +	/* ETU clock is optional, only required when the ETU block is used */
>> +	tsc->etu_clk = devm_clk_get_optional_enabled(&pdev->dev, "etu");
>> +	if (IS_ERR(tsc->etu_clk))
>> +		return PTR_ERR(tsc->etu_clk);
>> +
>> +	tsc->ptp_info = qcom_ptp_info;
>> +
>> +	tsc->ptp = ptp_clock_register(&tsc->ptp_info, &pdev->dev);
>> +	if (IS_ERR(tsc->ptp)) {
>> +		ret = PTR_ERR(tsc->ptp);
>> +		dev_err(&pdev->dev, "Failed to register ptp clock\n");
>> +		return ret;
> 
> Why aren't you using dev_err_probe?
> 

Sure, will use the dev_err_probe in the next series.

Thanks,
Imran

>> +	}
>> +
>> +	platform_set_drvdata(pdev, tsc);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver qcom_ptp_tsc_driver = {
>> +	.probe  = qcom_ptp_tsc_probe,
>> +	.remove = qcom_ptp_tsc_remove,
>> +	.driver = {
>> +		.name = "qcom_ptp_tsc",
>> +		.of_match_table = qcom_tsc_of_match,
>> +	},
>> +};
>> +module_platform_driver(qcom_ptp_tsc_driver);
>> +
>> +MODULE_DESCRIPTION("PTP QCOM TSC driver");
>> +MODULE_LICENSE("GPL");
>>
> 
> 
> Best regards,
> Krzysztof