Hello Roger,
On 13/01/23 15:27, Roger Quadros wrote:
> Hi,
>
> On 11/01/2023 13:44, Siddharth Vadapalli wrote:
>> From: Grygorii Strashko <grygorii.strashko@xxxxxx>
>>
>> CPTS doesn't have HW support for PPS ("pulse per second”) signal
>> generation, but it can be modeled by using Time Sync Router and routing
>> GenFx (periodic signal generator) output to CPTS_HWy_TS_PUSH (hardware time
>> stamp) input, and configuring GenFx to generate 1sec pulses.
>>
>> +------------------------+
>> | CPTS |
>> | |
>> +--->CPTS_HW4_PUSH GENFx+---+
>> | | | |
>> | +------------------------+ |
>> | |
>> +--------------------------------+
>>
>> Add corresponding support to am65-cpts driver. The DT property "ti,pps"
>> has to be used to enable PPS support and configure pair
>> [CPTS_HWy_TS_PUSH, GenFx].
>>
>> Once enabled, PPS can be tested using ppstest tool:
>> # ./ppstest /dev/pps0
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@xxxxxx>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
>> ---
>> drivers/net/ethernet/ti/am65-cpts.c | 85 +++++++++++++++++++++++++++--
>> 1 file changed, 80 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
>> index 9535396b28cd..6a0f09b497d1 100644
>> --- a/drivers/net/ethernet/ti/am65-cpts.c
>> +++ b/drivers/net/ethernet/ti/am65-cpts.c
>> @@ -176,6 +176,10 @@ struct am65_cpts {
>> u32 genf_enable;
>> u32 hw_ts_enable;
>> struct sk_buff_head txq;
>> + bool pps_enabled;
>> + bool pps_present;
>> + u32 pps_hw_ts_idx;
>> + u32 pps_genf_idx;
>> /* context save/restore */
>> u64 sr_cpts_ns;
>> u64 sr_ktime_ns;
>> @@ -319,8 +323,15 @@ static int am65_cpts_fifo_read(struct am65_cpts *cpts)
>> case AM65_CPTS_EV_HW:
>> pevent.index = am65_cpts_event_get_port(event) - 1;
>> pevent.timestamp = event->timestamp;
>> - pevent.type = PTP_CLOCK_EXTTS;
>> - dev_dbg(cpts->dev, "AM65_CPTS_EV_HW p:%d t:%llu\n",
>> + if (cpts->pps_enabled && pevent.index == cpts->pps_hw_ts_idx) {
>> + pevent.type = PTP_CLOCK_PPSUSR;
>> + pevent.pps_times.ts_real = ns_to_timespec64(pevent.timestamp);
>> + } else {
>> + pevent.type = PTP_CLOCK_EXTTS;
>> + }
>> + dev_dbg(cpts->dev, "AM65_CPTS_EV_HW:%s p:%d t:%llu\n",
>> + pevent.type == PTP_CLOCK_EXTTS ?
>> + "extts" : "pps",
>> pevent.index, event->timestamp);
>>
>> ptp_clock_event(cpts->ptp_clock, &pevent);
>> @@ -507,7 +518,13 @@ static void am65_cpts_extts_enable_hw(struct am65_cpts *cpts, u32 index, int on)
>>
>> static int am65_cpts_extts_enable(struct am65_cpts *cpts, u32 index, int on)
>> {
>> - if (!!(cpts->hw_ts_enable & BIT(index)) == !!on)
>> + if (index >= cpts->ptp_info.n_ext_ts)
>> + return -ENXIO;
>> +
>> + if (cpts->pps_present && index == cpts->pps_hw_ts_idx)
>> + return -EINVAL;
>> +
>> + if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
>> return 0;
>>
>> mutex_lock(&cpts->ptp_clk_lock);
>> @@ -591,6 +608,12 @@ static void am65_cpts_perout_enable_hw(struct am65_cpts *cpts,
>> static int am65_cpts_perout_enable(struct am65_cpts *cpts,
>> struct ptp_perout_request *req, int on)
>> {
>> + if (req->index >= cpts->ptp_info.n_per_out)
>> + return -ENXIO;
>> +
>> + if (cpts->pps_present && req->index == cpts->pps_genf_idx)
>> + return -EINVAL;
>> +
>> if (!!(cpts->genf_enable & BIT(req->index)) == !!on)
>> return 0;
>>
>> @@ -604,6 +627,48 @@ static int am65_cpts_perout_enable(struct am65_cpts *cpts,
>> return 0;
>> }
>>
>> +static int am65_cpts_pps_enable(struct am65_cpts *cpts, int on)
>> +{
>> + int ret = 0;
>> + struct timespec64 ts;
>> + struct ptp_clock_request rq;
>> + u64 ns;
>> +
>> + if (!cpts->pps_present)
>> + return -EINVAL;
>> +
>> + if (cpts->pps_enabled == !!on)
>> + return 0;
>> +
>> + mutex_lock(&cpts->ptp_clk_lock);
>> +
>> + if (on) {
>> + am65_cpts_extts_enable_hw(cpts, cpts->pps_hw_ts_idx, on);
>> +
>> + ns = am65_cpts_gettime(cpts, NULL);
>> + ts = ns_to_timespec64(ns);
>> + rq.perout.period.sec = 1;
>> + rq.perout.period.nsec = 0;
>> + rq.perout.start.sec = ts.tv_sec + 2;
>> + rq.perout.start.nsec = 0;
>> + rq.perout.index = cpts->pps_genf_idx;
>> +
>> + am65_cpts_perout_enable_hw(cpts, &rq.perout, on);
>> + cpts->pps_enabled = true;
>> + } else {
>> + rq.perout.index = cpts->pps_genf_idx;
>> + am65_cpts_perout_enable_hw(cpts, &rq.perout, on);
>> + am65_cpts_extts_enable_hw(cpts, cpts->pps_hw_ts_idx, on);
>> + cpts->pps_enabled = false;
>> + }
>> +
>> + mutex_unlock(&cpts->ptp_clk_lock);
>> +
>> + dev_dbg(cpts->dev, "%s: pps: %s\n",
>> + __func__, on ? "enabled" : "disabled");
>> + return ret;
>> +}
>> +
>> static int am65_cpts_ptp_enable(struct ptp_clock_info *ptp,
>> struct ptp_clock_request *rq, int on)
>> {
>> @@ -614,6 +679,8 @@ static int am65_cpts_ptp_enable(struct ptp_clock_info *ptp,
>> return am65_cpts_extts_enable(cpts, rq->extts.index, on);
>> case PTP_CLK_REQ_PEROUT:
>> return am65_cpts_perout_enable(cpts, &rq->perout, on);
>> + case PTP_CLK_REQ_PPS:
>> + return am65_cpts_pps_enable(cpts, on);
>> default:
>> break;
>> }
>> @@ -926,6 +993,12 @@ static int am65_cpts_of_parse(struct am65_cpts *cpts, struct device_node *node)
>> if (!of_property_read_u32(node, "ti,cpts-periodic-outputs", &prop[0]))
>> cpts->genf_num = prop[0];
>>
>> + if (!of_property_read_u32_array(node, "ti,pps", prop, 2)) {
>> + cpts->pps_present = true;
>> + cpts->pps_hw_ts_idx = prop[0];
>> + cpts->pps_genf_idx = prop[1];
>
> What happens if DT provides an invalid value. e.g. out of range?
> Better to do a sanity check?
Thank you for pointing it out. The pps_hw_ts_idx values range from 0 to 7, while
the pps_genf_idx values range from 0 to 1. I will implement this check and
default to index 0 if an invalid value is provided, along with a dev_err print
to inform this error.
Regards,
Siddharth.
