[v3] Add perout configuration support in IEP driver

[PATCH net-next v3 1/2] net: ti: icss-iep: Add pwidth configuration for perout signal

Posted by Meghana Malladi 11 months, 1 week ago

icss_iep_perout_enable_hw() is a common function for generating
both pps and perout signals. When enabling pps, the application needs
to only pass enable/disable argument, whereas for perout it supports
different flags to configure the signal.

But icss_iep_perout_enable_hw() function is missing to hook the
configuration params passed by the app, causing perout to behave
same a pps (except being able to configure the period). As duty cycle
is also one feature which can configured for perout, incorporate this
in the function to get the expected signal.

Signed-off-by: Meghana Malladi <m-malladi@ti.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
---
Changes from v2 (v3-v2):
- Posted patch to net-next as feature addition
- Collected RB tag from Jacob Keller <jacob.e.keller@intel.com>

 drivers/net/ethernet/ti/icssg/icss_iep.c | 49 ++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index d59c1744840a..0411438a3b5d 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -406,9 +406,16 @@ static void icss_iep_update_to_next_boundary(struct icss_iep *iep, u64 start_ns)
 static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 				     struct ptp_perout_request *req, int on)
 {
+	struct timespec64 ts;
+	u64 ns_width;
 	int ret;
 	u64 cmp;
 
+	/* Calculate width of the signal for PPS/PEROUT handling */
+	ts.tv_sec = req->on.sec;
+	ts.tv_nsec = req->on.nsec;
+	ns_width = timespec64_to_ns(&ts);
+
 	if (iep->ops && iep->ops->perout_enable) {
 		ret = iep->ops->perout_enable(iep->clockops_data, req, on, &cmp);
 		if (ret)
@@ -419,8 +426,9 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 			regmap_write(iep->map, ICSS_IEP_CMP1_REG0, lower_32_bits(cmp));
 			if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
 				regmap_write(iep->map, ICSS_IEP_CMP1_REG1, upper_32_bits(cmp));
-			/* Configure SYNC, 1ms pulse width */
-			regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG, 1000000);
+			/* Configure SYNC, based on req on width */
+			regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
+				     div_u64(ns_width, iep->def_inc));
 			regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0);
 			regmap_write(iep->map, ICSS_IEP_SYNC_START_REG, 0);
 			regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
@@ -447,6 +455,8 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 				   + req->period.nsec;
 			icss_iep_update_to_next_boundary(iep, start_ns);
 
+			regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
+				     div_u64(ns_width, iep->def_inc));
 			/* Enable Sync in single shot mode  */
 			regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG,
 				     IEP_SYNC_CTRL_SYNC_N_EN(0) | IEP_SYNC_CTRL_SYNC_EN);
@@ -474,7 +484,38 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 static int icss_iep_perout_enable(struct icss_iep *iep,
 				  struct ptp_perout_request *req, int on)
 {
-	return -EOPNOTSUPP;
+	int ret = 0;
+
+	mutex_lock(&iep->ptp_clk_mutex);
+
+	/* Reject requests with unsupported flags */
+	if (req->flags & ~PTP_PEROUT_DUTY_CYCLE) {
+		ret = -EOPNOTSUPP;
+		goto exit;
+	}
+
+	if (iep->pps_enabled) {
+		ret = -EBUSY;
+		goto exit;
+	}
+
+	if (iep->perout_enabled == !!on)
+		goto exit;
+
+	/* Set default "on" time (1ms) for the signal if not passed by the app */
+	if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
+		req->on.sec = 0;
+		req->on.nsec = NSEC_PER_MSEC;
+	}
+
+	ret = icss_iep_perout_enable_hw(iep, req, on);
+	if (!ret)
+		iep->perout_enabled = !!on;
+
+exit:
+	mutex_unlock(&iep->ptp_clk_mutex);
+
+	return ret;
 }
 
 static void icss_iep_cap_cmp_work(struct work_struct *work)
@@ -553,6 +594,8 @@ static int icss_iep_pps_enable(struct icss_iep *iep, int on)
 		rq.perout.period.nsec = 0;
 		rq.perout.start.sec = ts.tv_sec + 2;
 		rq.perout.start.nsec = 0;
+		rq.perout.on.sec = 0;
+		rq.perout.on.nsec = NSEC_PER_MSEC;
 		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);
 	} else {
 		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);
-- 
2.43.0

Re: [PATCH net-next v3 1/2] net: ti: icss-iep: Add pwidth configuration for perout signal

Posted by Kory Maincent 11 months, 1 week ago

On Mon, 3 Mar 2025 19:21:23 +0530
Meghana Malladi <m-malladi@ti.com> wrote:

> icss_iep_perout_enable_hw() is a common function for generating
> both pps and perout signals. When enabling pps, the application needs
> to only pass enable/disable argument, whereas for perout it supports
> different flags to configure the signal.
> 
> But icss_iep_perout_enable_hw() function is missing to hook the
> configuration params passed by the app, causing perout to behave
> same a pps (except being able to configure the period). As duty cycle
> is also one feature which can configured for perout, incorporate this
> in the function to get the expected signal.

...

> IEP_SYNC_CTRL_SYNC_EN); @@ -474,7 +484,38 @@ static int
> icss_iep_perout_enable_hw(struct icss_iep *iep, static int
> icss_iep_perout_enable(struct icss_iep *iep, struct ptp_perout_request *req,
> int on) {
> -	return -EOPNOTSUPP;
> +	int ret = 0;
> +
> +	mutex_lock(&iep->ptp_clk_mutex);
> +
> +	/* Reject requests with unsupported flags */
> +	if (req->flags & ~PTP_PEROUT_DUTY_CYCLE) {
> +		ret = -EOPNOTSUPP;
> +		goto exit;
> +	}

The flags check does not need to be in the mutex lock.
With this change:
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

> +	if (iep->pps_enabled) {
> +		ret = -EBUSY;
> +		goto exit;
> +	}
> +
> +	if (iep->perout_enabled == !!on)
> +		goto exit;
> +
> +	/* Set default "on" time (1ms) for the signal if not passed by the
> app */
> +	if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
> +		req->on.sec = 0;
> +		req->on.nsec = NSEC_PER_MSEC;
> +	}
> +
> +	ret = icss_iep_perout_enable_hw(iep, req, on);
> +	if (!ret)
> +		iep->perout_enabled = !!on;
> +
> +exit:
> +	mutex_unlock(&iep->ptp_clk_mutex);
> +
> +	return ret;
>  }
>  
>  static void icss_iep_cap_cmp_work(struct work_struct *work)
> @@ -553,6 +594,8 @@ static int icss_iep_pps_enable(struct icss_iep *iep, int
> on) rq.perout.period.nsec = 0;
>  		rq.perout.start.sec = ts.tv_sec + 2;
>  		rq.perout.start.nsec = 0;
> +		rq.perout.on.sec = 0;
> +		rq.perout.on.nsec = NSEC_PER_MSEC;
>  		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);
>  	} else {
>  		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);



-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [EXTERNAL] Re: [PATCH net-next v3 1/2] net: ti: icss-iep: Add pwidth configuration for perout signal

Posted by Malladi, Meghana 11 months, 1 week ago


On 3/3/2025 10:33 PM, Kory Maincent wrote:
> On Mon, 3 Mar 2025 19: 21: 23 +0530 Meghana Malladi <m-malladi@ ti. com> 
> wrote: > icss_iep_perout_enable_hw() is a common function for generating 
>  > both pps and perout signals. When enabling pps, the application needs 
>  > to only pass
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source 
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK! 
> uVdqXRfP1m17KmZFPNPjnPB9kIuFmkbGwUjWeOt4PhpPuyUdhbWQXCWjPIg3CE7zH4vA7aR5lJOdoby6lh8SIqLqdFhb$>
> ZjQcmQRYFpfptBannerEnd
> 
> On Mon, 3 Mar 2025 19:21:23 +0530
> Meghana Malladi <m-malladi@ti.com> wrote:
> 
>> icss_iep_perout_enable_hw() is a common function for generating
>> both pps and perout signals. When enabling pps, the application needs
>> to only pass enable/disable argument, whereas for perout it supports
>> different flags to configure the signal.
>> 
>> But icss_iep_perout_enable_hw() function is missing to hook the
>> configuration params passed by the app, causing perout to behave
>> same a pps (except being able to configure the period). As duty cycle
>> is also one feature which can configured for perout, incorporate this
>> in the function to get the expected signal.
> 
> ...
> 
>> IEP_SYNC_CTRL_SYNC_EN); @@ -474,7 +484,38 @@ static int
>> icss_iep_perout_enable_hw(struct icss_iep *iep, static int
>> icss_iep_perout_enable(struct icss_iep *iep, struct ptp_perout_request *req,
>> int on) {
>> -	return -EOPNOTSUPP;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&iep->ptp_clk_mutex);
>> +
>> +	/* Reject requests with unsupported flags */
>> +	if (req->flags & ~PTP_PEROUT_DUTY_CYCLE) {
>> +		ret = -EOPNOTSUPP;
>> +		goto exit;
>> +	}
> 
> The flags check does not need to be in the mutex lock.
> With this change:
> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
> 

Yes agreed, will move it outside the mutex lock. Thanks.

>> +	if (iep->pps_enabled) {
>> +		ret = -EBUSY;
>> +		goto exit;
>> +	}
>> +
>> +	if (iep->perout_enabled == !!on)
>> +		goto exit;
>> +
>> +	/* Set default "on" time (1ms) for the signal if not passed by the
>> app */
>> +	if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
>> +		req->on.sec = 0;
>> +		req->on.nsec = NSEC_PER_MSEC;
>> +	}
>> +
>> +	ret = icss_iep_perout_enable_hw(iep, req, on);
>> +	if (!ret)
>> +		iep->perout_enabled = !!on;
>> +
>> +exit:
>> +	mutex_unlock(&iep->ptp_clk_mutex);
>> +
>> +	return ret;
>>  }
>>  
>>  static void icss_iep_cap_cmp_work(struct work_struct *work)
>> @@ -553,6 +594,8 @@ static int icss_iep_pps_enable(struct icss_iep *iep, int
>> on) rq.perout.period.nsec = 0;
>>  		rq.perout.start.sec = ts.tv_sec + 2;
>>  		rq.perout.start.nsec = 0;
>> +		rq.perout.on.sec = 0;
>> +		rq.perout.on.nsec = NSEC_PER_MSEC;
>>  		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);
>>  	} else {
>>  		ret = icss_iep_perout_enable_hw(iep, &rq.perout, on);
> 
> 
> 
> -- 
> Köry Maincent, Bootlin
> Embedded Linux and kernel engineering
> https://urldefense.com/v3/__https://bootlin.com__;!!G3vK! 
> TJ19I0FAUivlehxg6fb4ka96Q2RiJDOZNyHXmEeRdYIPu4Cthp- 
> iiGCHOwcUnPvn8Ek_htiuIef9PMEWGUfBEA$ <https://urldefense.com/v3/__https://bootlin.com__;!!G3vK!TJ19I0FAUivlehxg6fb4ka96Q2RiJDOZNyHXmEeRdYIPu4Cthp-iiGCHOwcUnPvn8Ek_htiuIef9PMEWGUfBEA$>
>

[PATCH net-next v3 1/2] net: ti: icss-iep: Add pwidth configuration for perout signal
[PATCH net-next v3 2/2] net: ti: icss-iep: Add phase offset configuration for perout signal