[REQUEST] net: fec: Not yet ported PTP patches

[REQUEST] net: fec: Not yet ported PTP patches

[Csókás Bence]

Hi!

The following upstream commits have not been queued for stable due to 
missing `Fixes:` tags, but are strongly recommended for correct PTP 
operation on the i.MX 6 SoC family, please pick them. Our first priority 
would be 6.6, the last LTS, but obviously all stable versions would benefit.

* 4374a1fe580a ("net: fec: Move `fec_ptp_read()` to the top of the file")
* 713ebaed68d8 ("net: fec: Remove duplicated code")
* bf8ca67e2167 [tree: net-next] ("net: fec: refactor PPS channel 
configuration")

Bence

[PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Csókás, Bence]

This function is used in `fec_ptp_enable_pps()` through
struct cyclecounter read(). Moving the declaration makes
it clearer, what's happening.

Suggested-by: Frank Li <Frank.li@nxp.com>
Link: https://lore.kernel.org/netdev/20240805144754.2384663-1-csokas.bence@prolan.hu/T/#ma6c21ad264016c24612048b1483769eaff8cdf20
Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20240812094713.2883476-1-csokas.bence@prolan.hu
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 4374a1fe580a14f6152752390c678d90311df247)
---
 drivers/net/ethernet/freescale/fec_ptp.c | 50 ++++++++++++------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 2e4f3e1782a2..7f4ccd1ade5b 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -90,6 +90,30 @@
 #define FEC_PTP_MAX_NSEC_PERIOD		4000000000ULL
 #define FEC_PTP_MAX_NSEC_COUNTER	0x80000000ULL
 
+/**
+ * fec_ptp_read - read raw cycle counter (to be used by time counter)
+ * @cc: the cyclecounter structure
+ *
+ * this function reads the cyclecounter registers and is called by the
+ * cyclecounter structure used to construct a ns counter from the
+ * arbitrary fixed point registers
+ */
+static u64 fec_ptp_read(const struct cyclecounter *cc)
+{
+	struct fec_enet_private *fep =
+		container_of(cc, struct fec_enet_private, cc);
+	u32 tempval;
+
+	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
+	tempval |= FEC_T_CTRL_CAPTURE;
+	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
+
+	if (fep->quirks & FEC_QUIRK_BUG_CAPTURE)
+		udelay(1);
+
+	return readl(fep->hwp + FEC_ATIME);
+}
+
 /**
  * fec_ptp_enable_pps
  * @fep: the fec_enet_private structure handle
@@ -136,7 +160,7 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
 		 * NSEC_PER_SEC - ts.tv_nsec. Add the remaining nanoseconds
 		 * to current timer would be next second.
 		 */
-		tempval = fep->cc.read(&fep->cc);
+		tempval = fec_ptp_read(&fep->cc);
 		/* Convert the ptp local counter to 1588 timestamp */
 		ns = timecounter_cyc2time(&fep->tc, tempval);
 		ts = ns_to_timespec64(ns);
@@ -271,30 +295,6 @@ static enum hrtimer_restart fec_ptp_pps_perout_handler(struct hrtimer *timer)
 	return HRTIMER_NORESTART;
 }
 
-/**
- * fec_ptp_read - read raw cycle counter (to be used by time counter)
- * @cc: the cyclecounter structure
- *
- * this function reads the cyclecounter registers and is called by the
- * cyclecounter structure used to construct a ns counter from the
- * arbitrary fixed point registers
- */
-static u64 fec_ptp_read(const struct cyclecounter *cc)
-{
-	struct fec_enet_private *fep =
-		container_of(cc, struct fec_enet_private, cc);
-	u32 tempval;
-
-	tempval = readl(fep->hwp + FEC_ATIME_CTRL);
-	tempval |= FEC_T_CTRL_CAPTURE;
-	writel(tempval, fep->hwp + FEC_ATIME_CTRL);
-
-	if (fep->quirks & FEC_QUIRK_BUG_CAPTURE)
-		udelay(1);
-
-	return readl(fep->hwp + FEC_ATIME);
-}
-
 /**
  * fec_ptp_start_cyclecounter - create the cycle counter from hw
  * @ndev: network device
-- 
2.34.1

Re: [PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

Found matching upstream commit: 4374a1fe580a14f6152752390c678d90311df247

WARNING: Author mismatch between patch and found commit:
Backport author: =?UTF-8?q?Cs=C3=B3k=C3=A1s=2C=20Bence?= <csokas.bence@prolan.hu>
Commit author: Csókás, Bence <csokas.bence@prolan.hu>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.11.y | Present (different SHA1: 97f35652e0e8)
6.6.y | Present (different SHA1: 1e1eb62c40e1)

Note: The patch differs from the upstream commit:
---
--- -	2024-11-22 08:39:00.262357742 -0500
+++ /tmp/tmp.2jyCFuEVB2	2024-11-22 08:39:00.254181034 -0500
@@ -1,19 +1,16 @@
-This function is used in `fec_ptp_enable_pps()` through
-struct cyclecounter read(). Moving the declaration makes
-it clearer, what's happening.
-
 Suggested-by: Frank Li <Frank.li@nxp.com>
 Link: https://lore.kernel.org/netdev/20240805144754.2384663-1-csokas.bence@prolan.hu/T/#ma6c21ad264016c24612048b1483769eaff8cdf20
 Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
 Reviewed-by: Andrew Lunn <andrew@lunn.ch>
 Link: https://patch.msgid.link/20240812094713.2883476-1-csokas.bence@prolan.hu
 Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+(cherry picked from commit 4374a1fe580a14f6152752390c678d90311df247)
 ---
  drivers/net/ethernet/freescale/fec_ptp.c | 50 ++++++++++++------------
  1 file changed, 25 insertions(+), 25 deletions(-)
 
 diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
-index 2e4f3e1782a25..7f4ccd1ade5b1 100644
+index 2e4f3e1782a2..7f4ccd1ade5b 100644
 --- a/drivers/net/ethernet/freescale/fec_ptp.c
 +++ b/drivers/net/ethernet/freescale/fec_ptp.c
 @@ -90,6 +90,30 @@
@@ -87,3 +84,7 @@
  /**
   * fec_ptp_start_cyclecounter - create the cycle counter from hw
   * @ndev: network device
+-- 
+2.34.1
+
+
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Failed     |  N/A       |
| stable/linux-6.1.y        |  Failed     |  N/A       |

Re: [PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Csókás Bence]

Hi,

On 2024. 11. 22. 14:51, Sasha Levin wrote:
> [ Sasha's backport helper bot ]
> 
> Hi,
> 
> Found matching upstream commit: 4374a1fe580a14f6152752390c678d90311df247
> 
> WARNING: Author mismatch between patch and found commit:
> Backport author: =?UTF-8?q?Cs=C3=B3k=C3=A1s=2C=20Bence?= <csokas.bence@prolan.hu>
> Commit author: Csókás, Bence <csokas.bence@prolan.hu>
> 
> 
> Status in newer kernel trees:
> 6.12.y | Present (exact SHA1)
> 6.11.y | Present (different SHA1: 97f35652e0e8)
> 6.6.y | Present (different SHA1: 1e1eb62c40e1)

1/3 and 2/3 of this series was already applied by Greg, only 3/3 was not.

commit: bf8ca67e2167 ("net: fec: refactor PPS channel configuration")

Bence

Re: [PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Sasha Levin]

On Fri, Nov 22, 2024 at 02:56:40PM +0100, Csókás Bence wrote:
>Hi,
>
>On 2024. 11. 22. 14:51, Sasha Levin wrote:
>>[ Sasha's backport helper bot ]
>>
>>Hi,
>>
>>Found matching upstream commit: 4374a1fe580a14f6152752390c678d90311df247
>>
>>WARNING: Author mismatch between patch and found commit:
>>Backport author: =?UTF-8?q?Cs=C3=B3k=C3=A1s=2C=20Bence?= <csokas.bence@prolan.hu>
>>Commit author: Csókás, Bence <csokas.bence@prolan.hu>
>>
>>
>>Status in newer kernel trees:
>>6.12.y | Present (exact SHA1)
>>6.11.y | Present (different SHA1: 97f35652e0e8)
>>6.6.y | Present (different SHA1: 1e1eb62c40e1)
>
>1/3 and 2/3 of this series was already applied by Greg, only 3/3 was not.
>
>commit: bf8ca67e2167 ("net: fec: refactor PPS channel configuration")

Dare I ask why we need it to begin with? Commit message says:

	Preparation patch to allow for PPS channel configuration, no functional
	change intended.

-- 
Thanks,
Sasha

Re: [PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Csókás Bence]

On 2024. 11. 23. 15:36, Sasha Levin wrote:
> On Fri, Nov 22, 2024 at 02:56:40PM +0100, Csókás Bence wrote:
>> Hi,
>>
>> On 2024. 11. 22. 14:51, Sasha Levin wrote:
>>> [ Sasha's backport helper bot ]
>>>
>>> Hi,
>>>
>>> Found matching upstream commit: 4374a1fe580a14f6152752390c678d90311df247
>>>
>>> WARNING: Author mismatch between patch and found commit:
>>> Backport author: =?UTF-8?q?Cs=C3=B3k=C3=A1s=2C=20Bence?= 
>>> <csokas.bence@prolan.hu>
>>> Commit author: Csókás, Bence <csokas.bence@prolan.hu>
>>>
>>>
>>> Status in newer kernel trees:
>>> 6.12.y | Present (exact SHA1)
>>> 6.11.y | Present (different SHA1: 97f35652e0e8)
>>> 6.6.y | Present (different SHA1: 1e1eb62c40e1)
>>
>> 1/3 and 2/3 of this series was already applied by Greg, only 3/3 was not.
>>
>> commit: bf8ca67e2167 ("net: fec: refactor PPS channel configuration")
> 
> Dare I ask why we need it to begin with? Commit message says:
> 
>      Preparation patch to allow for PPS channel configuration, no 
> functional
>      change intended.

We have patches that depend on it, that we need to maintain on top of 6.6.y.

Bence

Re: [PATCH 6.6 1/3] net: fec: Move `fec_ptp_read()` to the top of the file

[Sasha Levin]

On Sun, Nov 24, 2024 at 11:47:41AM +0100, Csókás Bence wrote:
>On 2024. 11. 23. 15:36, Sasha Levin wrote:
>>On Fri, Nov 22, 2024 at 02:56:40PM +0100, Csókás Bence wrote:
>>>Hi,
>>>
>>>On 2024. 11. 22. 14:51, Sasha Levin wrote:
>>>>[ Sasha's backport helper bot ]
>>>>
>>>>Hi,
>>>>
>>>>Found matching upstream commit: 4374a1fe580a14f6152752390c678d90311df247
>>>>
>>>>WARNING: Author mismatch between patch and found commit:
>>>>Backport author: =?UTF-8?q?Cs=C3=B3k=C3=A1s=2C=20Bence?= 
>>>><csokas.bence@prolan.hu>
>>>>Commit author: Csókás, Bence <csokas.bence@prolan.hu>
>>>>
>>>>
>>>>Status in newer kernel trees:
>>>>6.12.y | Present (exact SHA1)
>>>>6.11.y | Present (different SHA1: 97f35652e0e8)
>>>>6.6.y | Present (different SHA1: 1e1eb62c40e1)
>>>
>>>1/3 and 2/3 of this series was already applied by Greg, only 3/3 was not.
>>>
>>>commit: bf8ca67e2167 ("net: fec: refactor PPS channel configuration")
>>
>>Dare I ask why we need it to begin with? Commit message says:
>>
>>     Preparation patch to allow for PPS channel configuration, no 
>>functional
>>     change intended.
>
>We have patches that depend on it, that we need to maintain on top of 6.6.y.

It doesn't align with the rules at
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html,
right?

-- 
Thanks,
Sasha

[PATCH 6.6 2/3] net: fec: Remove duplicated code

[Csókás, Bence]

`fec_ptp_pps_perout()` reimplements logic already
in `fec_ptp_read()`. Replace with function call.

Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20240812094713.2883476-2-csokas.bence@prolan.hu
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 713ebaed68d88121cbaf5e74104e2290a9ea74bd)
---
 drivers/net/ethernet/freescale/fec_ptp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 7f4ccd1ade5b..4cffda363a14 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -235,13 +235,7 @@ static int fec_ptp_pps_perout(struct fec_enet_private *fep)
 	timecounter_read(&fep->tc);
 
 	/* Get the current ptp hardware time counter */
-	temp_val = readl(fep->hwp + FEC_ATIME_CTRL);
-	temp_val |= FEC_T_CTRL_CAPTURE;
-	writel(temp_val, fep->hwp + FEC_ATIME_CTRL);
-	if (fep->quirks & FEC_QUIRK_BUG_CAPTURE)
-		udelay(1);
-
-	ptp_hc = readl(fep->hwp + FEC_ATIME);
+	ptp_hc = fec_ptp_read(&fep->cc);
 
 	/* Convert the ptp local counter to 1588 timestamp */
 	curr_time = timecounter_cyc2time(&fep->tc, ptp_hc);
-- 
2.34.1

[PATCH 6.6 3/3] net: fec: refactor PPS channel configuration

[Csókás, Bence]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

Preparation patch to allow for PPS channel configuration, no functional
change intended.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Csókás, Bence <csokas.bence@prolan.hu>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

(cherry picked from commit bf8ca67e21671e7a56e31da45360480b28f185f1)
---
 drivers/net/ethernet/freescale/fec_ptp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 4cffda363a14..ce7aa2c38c7f 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -84,8 +84,7 @@
 #define FEC_CC_MULT	(1 << 31)
 #define FEC_COUNTER_PERIOD	(1 << 31)
 #define PPS_OUPUT_RELOAD_PERIOD	NSEC_PER_SEC
-#define FEC_CHANNLE_0		0
-#define DEFAULT_PPS_CHANNEL	FEC_CHANNLE_0
+#define DEFAULT_PPS_CHANNEL	0
 
 #define FEC_PTP_MAX_NSEC_PERIOD		4000000000ULL
 #define FEC_PTP_MAX_NSEC_COUNTER	0x80000000ULL
@@ -524,8 +523,9 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp,
 	unsigned long flags;
 	int ret = 0;
 
+	fep->pps_channel = DEFAULT_PPS_CHANNEL;
+
 	if (rq->type == PTP_CLK_REQ_PPS) {
-		fep->pps_channel = DEFAULT_PPS_CHANNEL;
 		fep->reload_period = PPS_OUPUT_RELOAD_PERIOD;
 
 		ret = fec_ptp_enable_pps(fep, on);
@@ -536,10 +536,9 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp,
 		if (rq->perout.flags)
 			return -EOPNOTSUPP;
 
-		if (rq->perout.index != DEFAULT_PPS_CHANNEL)
+		if (rq->perout.index != fep->pps_channel)
 			return -EOPNOTSUPP;
 
-		fep->pps_channel = DEFAULT_PPS_CHANNEL;
 		period.tv_sec = rq->perout.period.sec;
 		period.tv_nsec = rq->perout.period.nsec;
 		period_ns = timespec64_to_ns(&period);
-- 
2.34.1

Re: [PATCH 6.6 3/3] net: fec: refactor PPS channel configuration

[Greg Kroah-Hartman]

On Wed, Oct 16, 2024 at 10:01:57AM +0200, Csókás, Bence wrote:
> From: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> Preparation patch to allow for PPS channel configuration, no functional
> change intended.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Csókás, Bence <csokas.bence@prolan.hu>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> (cherry picked from commit bf8ca67e21671e7a56e31da45360480b28f185f1)

Not a valid commit id in Linus's tree :(

Re: [PATCH 6.6 3/3] net: fec: refactor PPS channel configuration

[Csókás Bence]

Hi,

On 2024. 10. 18. 12:37, Greg Kroah-Hartman wrote:
> On Wed, Oct 16, 2024 at 10:01:57AM +0200, Csókás, Bence wrote:
>> From: Francesco Dolcini <francesco.dolcini@toradex.com>
>>
>> Preparation patch to allow for PPS channel configuration, no functional
>> change intended.
>>
>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>> Reviewed-by: Csókás, Bence <csokas.bence@prolan.hu>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>
>> (cherry picked from commit bf8ca67e21671e7a56e31da45360480b28f185f1)
> 
> Not a valid commit id in Linus's tree :(

It is now merged by Linus, please pick it when you can.

Re: [PATCH 6.6 3/3] net: fec: refactor PPS channel configuration

[Greg Kroah-Hartman]

On Fri, Nov 22, 2024 at 10:10:38AM +0100, Csókás Bence wrote:
> Hi,
> 
> On 2024. 10. 18. 12:37, Greg Kroah-Hartman wrote:
> > On Wed, Oct 16, 2024 at 10:01:57AM +0200, Csókás, Bence wrote:
> > > From: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > 
> > > Preparation patch to allow for PPS channel configuration, no functional
> > > change intended.
> > > 
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > Reviewed-by: Csókás, Bence <csokas.bence@prolan.hu>
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > 
> > > (cherry picked from commit bf8ca67e21671e7a56e31da45360480b28f185f1)
> > 
> > Not a valid commit id in Linus's tree :(
> 
> It is now merged by Linus, please pick it when you can.
> 

Please resend it, it is long-gone from my review queue.

thanks,

greg k-h