From: Siddharth Vadapalli <s-vadapalli@ti.com>
To: Roger Quadros <rogerq@kernel.org>
Cc: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
<pabeni@redhat.com>, <robh+dt@kernel.org>,
<krzysztof.kozlowski@linaro.org>,
<krzysztof.kozlowski+dt@linaro.org>, <nm@ti.com>,
<kristo@kernel.org>, <vigneshr@ti.com>, <nsekhar@ti.com>,
<netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>, <srk@ti.com>,
<s-vadapalli@ti.com>
Subject: Re: [PATCH net-next 2/5] net: ethernet: ti: am65-cpts: add pps support
Date: Mon, 16 Jan 2023 11:53:42 +0530 [thread overview]
Message-ID: <60c3af09-f3ed-c721-e1fe-cc8bf272b424@ti.com> (raw)
In-Reply-To: <6e691ad5-a919-75d8-ff65-c11820b253ee@kernel.org>
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@ti.com>
>>
>> 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@ti.com>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>> 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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-01-16 6:25 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-11 11:44 [PATCH net-next 0/5] Add PPS support to am65-cpts driver Siddharth Vadapalli
2023-01-11 11:44 ` [PATCH net-next 1/5] dt-binding: net: ti: am65x-cpts: add 'ti,pps' property Siddharth Vadapalli
2023-01-13 19:16 ` Rob Herring
2023-01-11 11:44 ` [PATCH net-next 2/5] net: ethernet: ti: am65-cpts: add pps support Siddharth Vadapalli
2023-01-13 9:57 ` Roger Quadros
2023-01-16 6:23 ` Siddharth Vadapalli [this message]
2023-01-11 11:44 ` [PATCH net-next 3/5] net: ethernet: ti: am65-cpts: adjust pps following ptp changes Siddharth Vadapalli
2023-01-11 11:44 ` [PATCH net-next 4/5] arm64: dts: ti: k3-am62-main: Add timesync router node Siddharth Vadapalli
2023-05-16 13:31 ` Nishanth Menon
2023-01-11 11:44 ` [PATCH net-next 5/5] arm64: dts: ti: k3-am625-sk: Add cpsw3g cpts PPS support Siddharth Vadapalli
2023-01-13 10:18 ` Roger Quadros
2023-01-16 7:12 ` Siddharth Vadapalli
2023-01-16 16:05 ` Roger Quadros
2023-01-16 16:30 ` Vignesh Raghavendra
2023-01-17 5:28 ` Siddharth Vadapalli
2023-01-17 9:30 ` Roger Quadros
2023-01-17 9:39 ` Siddharth Vadapalli
2023-01-13 9:48 ` [PATCH net-next 0/5] Add PPS support to am65-cpts driver Roger Quadros
2023-01-13 9:56 ` Siddharth Vadapalli
2023-01-13 10:21 ` Roger Quadros
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60c3af09-f3ed-c721-e1fe-cc8bf272b424@ti.com \
--to=s-vadapalli@ti.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@google.com \
--cc=kristo@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nm@ti.com \
--cc=nsekhar@ti.com \
--cc=pabeni@redhat.com \
--cc=robh+dt@kernel.org \
--cc=rogerq@kernel.org \
--cc=srk@ti.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).