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