[PATCH net 0/2] Fixes for perout configuration in IEP driver

[PATCH net 0/2] Fixes for perout configuration in IEP driver

[Meghana Malladi]

IEP driver supports both pps and perout signal generation using testptp
application. Currently the driver is missing to incorporate the perout
signal configuration. This series introduces fixes in the driver to
configure perout signal based on the arguments passed by the perout
request.

Meghana Malladi (2):
  net: ti: icss-iep: Fix pwidth configuration for perout signal
  net: ti: icss-iep: Fix phase offset configuration for perout signal

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


base-commit: 8e248f2dbb1885647e259837d38200942f3591a3
-- 
2.43.0

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

[Meghana Malladi]

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.

Fixes: c1e0230eeaab2 ("net: ti: icss-iep: Add IEP driver")
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
 drivers/net/ethernet/ti/icssg/icss_iep.c | 28 ++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index 768578c0d958..789516683ca9 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,
+				     (u32)(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,
+				     (u32)(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);
@@ -478,6 +488,12 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
 
 	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;
@@ -486,6 +502,12 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
 	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;
@@ -572,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

[PATCH net 2/2] net: ti: icss-iep: Fix phase offset configuration for perout signal

[Meghana Malladi]

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.

In case the app passes a valid phase offset value, the signal should
start toggling after that phase offset, else start immediately or
as soon as possible. ICSS_IEP_SYNC_START_REG register take number of
clock cycles to wait before starting the signal after activation time.
Set appropriate value to this register to support phase offset.

Fixes: c1e0230eeaab2 ("net: ti: icss-iep: Add IEP driver")
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
 drivers/net/ethernet/ti/icssg/icss_iep.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index 789516683ca9..69bb619db31a 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -407,6 +407,7 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 				     struct ptp_perout_request *req, int on)
 {
 	struct timespec64 ts;
+	u64 ns_start;
 	u64 ns_width;
 	int ret;
 	u64 cmp;
@@ -416,6 +417,14 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 	ts.tv_nsec = req->on.nsec;
 	ns_width = timespec64_to_ns(&ts);
 
+	if (req->flags & PTP_PEROUT_PHASE) {
+		ts.tv_sec = req->phase.sec;
+		ts.tv_nsec = req->phase.nsec;
+		ns_start = timespec64_to_ns(&ts);
+	} else {
+		ns_start = 0;
+	}
+
 	if (iep->ops && iep->ops->perout_enable) {
 		ret = iep->ops->perout_enable(iep->clockops_data, req, on, &cmp);
 		if (ret)
@@ -430,7 +439,8 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 			regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
 				     (u32)(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_START_REG,
+				     (u32)(ns_start / iep->def_inc));
 			regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
 			/* Enable CMP 1 */
 			regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
@@ -457,6 +467,8 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
 
 			regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
 				     (u32)(ns_width / iep->def_inc));
+			regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
+				     (u32)(ns_start / 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);
@@ -489,7 +501,8 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
 	mutex_lock(&iep->ptp_clk_mutex);
 
 	/* Reject requests with unsupported flags */
-	if (req->flags & ~PTP_PEROUT_DUTY_CYCLE) {
+	if (req->flags & ~(PTP_PEROUT_DUTY_CYCLE |
+			  PTP_PEROUT_PHASE)) {
 		ret = -EOPNOTSUPP;
 		goto exit;
 	}
@@ -590,6 +603,7 @@ static int icss_iep_pps_enable(struct icss_iep *iep, int on)
 	if (on) {
 		ns = icss_iep_gettime(iep, NULL);
 		ts = ns_to_timespec64(ns);
+		rq.perout.flags = 0;
 		rq.perout.period.sec = 1;
 		rq.perout.period.nsec = 0;
 		rq.perout.start.sec = ts.tv_sec + 2;
-- 
2.43.0

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

[Paolo Abeni]

On 2/11/25 11:35 AM, Meghana Malladi wrote:
> @@ -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,
> +				     (u32)(ns_width / iep->def_inc));

This causes build errors on 32bits:

ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
undefined!
make[3]: *** [../scripts/Makefile.modpost:147: Module.symvers] Error 1
make[2]: *** [/home/nipa/net/wt-0/Makefile:1944: modpost] Error 2
make[1]: *** [/home/nipa/net/wt-0/Makefile:251: __sub-make] Error 2
make: *** [Makefile:251: __sub-make] Error 2
ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]

You should use div_u64()

/P

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

[Malladi, Meghana]

On 2/13/2025 4:53 PM, Paolo Abeni wrote:
> On 2/11/25 11: 35 AM, Meghana Malladi wrote: > @@ -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 &
> 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! 
> updqdzavl0dbisXOnfkDHxHqGlQUHEro3tgnljLa7x4DRPBIRKu8Nqm3bW1LeMtXFyqz6yM7_tLlrvUmslKj9m_IL0hUlNU$>
> ZjQcmQRYFpfptBannerEnd
> 
> On 2/11/25 11:35 AM, Meghana Malladi wrote:
>> @@ -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,
>> +				     (u32)(ns_width / iep->def_inc));
> 
> This causes build errors on 32bits:
> 
> ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
> undefined!
> make[3]: *** [../scripts/Makefile.modpost:147: Module.symvers] Error 1
> make[2]: *** [/home/nipa/net/wt-0/Makefile:1944: modpost] Error 2
> make[1]: *** [/home/nipa/net/wt-0/Makefile:251: __sub-make] Error 2
> make: *** [Makefile:251: __sub-make] Error 2
> ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
> 
> You should use div_u64()
> 

I see, thanks.
Can you tell me how can I reproduce this on my end for 32 bits.
Will fix this in v2.

> /P
> 

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

[Simon Horman]

On Fri, Feb 14, 2025 at 11:35:07AM +0530, Malladi, Meghana wrote:
> 
> 
> On 2/13/2025 4:53 PM, Paolo Abeni wrote:
> > On 2/11/25 11: 35 AM, Meghana Malladi wrote: > @@ -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 &
> > 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! updqdzavl0dbisXOnfkDHxHqGlQUHEro3tgnljLa7x4DRPBIRKu8Nqm3bW1LeMtXFyqz6yM7_tLlrvUmslKj9m_IL0hUlNU$>
> > ZjQcmQRYFpfptBannerEnd
> > 
> > On 2/11/25 11:35 AM, Meghana Malladi wrote:
> > > @@ -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,
> > > +				     (u32)(ns_width / iep->def_inc));
> > 
> > This causes build errors on 32bits:
> > 
> > ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
> > undefined!
> > make[3]: *** [../scripts/Makefile.modpost:147: Module.symvers] Error 1
> > make[2]: *** [/home/nipa/net/wt-0/Makefile:1944: modpost] Error 2
> > make[1]: *** [/home/nipa/net/wt-0/Makefile:251: __sub-make] Error 2
> > make: *** [Makefile:251: __sub-make] Error 2
> > ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
> > 
> > You should use div_u64()
> > 
> 
> I see, thanks.
> Can you tell me how can I reproduce this on my end for 32 bits.
> Will fix this in v2.

Hi Meghana,

FWIIW, I was able to reproduce this problem running the following
on an x86_64 system:

ARCH=i386 make tinyconfig

echo CONFIG_COMPILE_TEST=y >> .config
echo CONFIG_PCI=y >> .config
echo CONFIG_SOC_TI=y >> .config
echo CONFIG_TI_PRUSS=y >> .config
echo CONFIG_NET=y >> .config
echo CONFIG_NETDEVICES=y >> .config

yes "" | ARCH=i386 make oldconfig

ARCH=i386 make -j$(nproc)

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

[Malladi, Meghana]

Hi Simon

On 2/16/2025 9:41 PM, Simon Horman wrote:
> On Fri, Feb 14, 2025 at 11:35:07AM +0530, Malladi, Meghana wrote:
>>
>>
>> On 2/13/2025 4:53 PM, Paolo Abeni wrote:
>>> On 2/11/25 11: 35 AM, Meghana Malladi wrote: > @@ -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 &
>>> 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! updqdzavl0dbisXOnfkDHxHqGlQUHEro3tgnljLa7x4DRPBIRKu8Nqm3bW1LeMtXFyqz6yM7_tLlrvUmslKj9m_IL0hUlNU$>
>>> ZjQcmQRYFpfptBannerEnd
>>>
>>> On 2/11/25 11:35 AM, Meghana Malladi wrote:
>>>> @@ -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,
>>>> +				     (u32)(ns_width / iep->def_inc));
>>>
>>> This causes build errors on 32bits:
>>>
>>> ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
>>> undefined!
>>> make[3]: *** [../scripts/Makefile.modpost:147: Module.symvers] Error 1
>>> make[2]: *** [/home/nipa/net/wt-0/Makefile:1944: modpost] Error 2
>>> make[1]: *** [/home/nipa/net/wt-0/Makefile:251: __sub-make] Error 2
>>> make: *** [Makefile:251: __sub-make] Error 2
>>> ERROR: modpost: "__udivdi3" [drivers/net/ethernet/ti/icssg/icss_iep.ko]
>>>
>>> You should use div_u64()
>>>
>>
>> I see, thanks.
>> Can you tell me how can I reproduce this on my end for 32 bits.
>> Will fix this in v2.
> 
> Hi Meghana,
> 
> FWIIW, I was able to reproduce this problem running the following
> on an x86_64 system:
> 
> ARCH=i386 make tinyconfig
> 
> echo CONFIG_COMPILE_TEST=y >> .config
> echo CONFIG_PCI=y >> .config
> echo CONFIG_SOC_TI=y >> .config
> echo CONFIG_TI_PRUSS=y >> .config
> echo CONFIG_NET=y >> .config
> echo CONFIG_NETDEVICES=y >> .config
> 
> yes "" | ARCH=i386 make oldconfig
> 
> ARCH=i386 make -j$(nproc)

Thanks for sharing this, I was able to reproduce and fix this locally.