Thread (23 messages) 23 messages, 2 authors, 2025-06-30

Re: [net-next v13 04/11] net: mtip: The L2 switch driver for imx287

From: Lukasz Majewski <lukma@denx.de>
Date: 2025-06-24 21:04:46
Also in: imx, linux-arm-kernel, linux-devicetree, lkml

Hi Paolo,
On 6/22/25 11:37 AM, Lukasz Majewski wrote:
quoted
+static void mtip_aging_timer(struct timer_list *t)
+{
+	struct switch_enet_private *fep = timer_container_of(fep,
t,
+
timer_aging); +
+	fep->curr_time = mtip_timeincrement(fep->curr_time);
+
+	mod_timer(&fep->timer_aging,
+		  jiffies +
msecs_to_jiffies(LEARNING_AGING_INTERVAL)); +}  
It's unclear to me why you decided to maintain this function and timer
while you could/should have used a macro around jiffies instead.
This is a bit more tricky than just getting value from jiffies.

The current code provides a monotonic, starting from 0 time "base" for
learning and managing entries in internal routing tables for MTIP.

To be more specific - the fep->curr_time is a value incremented after
each ~10ms.

Simple masking of jiffies would not provide such features.

However, I've rewritten relevant portions where GENMASK() could be used
to simplify and make the code more readable.
[...]
quoted
+static int mtip_sw_learning(void *arg)
+{
+	struct switch_enet_private *fep = arg;
+
+	while (!kthread_should_stop()) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		/* check learning record valid */
+		mtip_atable_dynamicms_learn_migration(fep,
fep->curr_time,
+						      NULL, NULL);
+		schedule_timeout(HZ / 100);
+	}
+
+	return 0;
+}  
Why are you using a full blown kernel thread here? 
The MTIP IP block requires the thread for learning. It is a HW based
switching accelerator, but the learning feature must be performed by
SW (by writing values to its registers).
Here a timer could
possibly make more sense.
Unfortunately, not - the code (in
mtip_atable_dynamicms_learn_migration() must be called). This function
has another role - it updates internal routing table with timestamps
(provided by timer mentioned above).
Why are checking the table every 10ms, while
the learning intervall is 100ms? 
Yes, this is correct. In 10ms interval the internal routing table is
updated. 100 ms is for learning.
I guess you could/should align the
frequency here with such interval.
IMHO learning with 10ms interval would bring a lot of overhead.

Just to mention - the MTIP IP block can generate interrupt for
learning event. However, it has been advised (bu NXP support), that a
thread with 100ms interval shall be used to avoid too many interrupts.
Side note: I think you should move the buffer management to a later
patch: this one is still IMHO too big.
And this is problematic - the most time I've spent for v13 to separate
the code - i.e. I exclude one function, then there are warnings that
other function is unused (and of course WARNINGS in a separate patches
are a legitimate reason to call for another patch set revision).

I've already excluded bridge and mgnt patches. Also moved out some code
from this one.
/P

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

Attachments

  • (unnamed) [application/pgp-signature] 488 bytes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help