On 10/11/2022 05:19, Jakub Kicinski wrote: > On Tue, 8 Nov 2022 15:56:43 +0200 Roger Quadros wrote: >> If an entry was FREE then we don't have to restore it. > > Motivation? Does it make the restore faster? Yes, since this would be called during system suspend/resume path. I will update the commit message to mention this. > >> Signed-off-by: Roger Quadros <rogerq@xxxxxxxxxx> >> --- >> >> Patch depends on >> https://lore.kernel.org/netdev/20221104132310.31577-3-rogerq@xxxxxxxxxx/T/ >> >> drivers/net/ethernet/ti/cpsw_ale.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c >> index 0c5e783e574c..41bcf34a22f8 100644 >> --- a/drivers/net/ethernet/ti/cpsw_ale.c >> +++ b/drivers/net/ethernet/ti/cpsw_ale.c >> @@ -1452,12 +1452,15 @@ void cpsw_ale_dump(struct cpsw_ale *ale, u32 *data) >> } >> } >> >> +/* ALE table should be cleared (ALE_CLEAR) before cpsw_ale_restore() */ > > Maybe my tree is old but I see we clear only if there is a netdev that This patch depends on this series https://lore.kernel.org/netdev/20221104132310.31577-3-rogerq@xxxxxxxxxx/T/ > needs to be opened but then always call ale_restore(). Is that okay? If netdev is closed and opened ale_restore() is not called. ale_restore() is only called during system suspend/resume path since CPSW-ALE might have lost context during suspend and we want to restore all valid ALE entries. I have a question here. How should ageable entries be treated in this case? > > I'd also s/should/must/ OK, will fix. > >> void cpsw_ale_restore(struct cpsw_ale *ale, u32 *data) >> { >> - int i; >> + int i, type; >> >> for (i = 0; i < ale->params.ale_entries; i++) { >> - cpsw_ale_write(ale, i, data); >> + type = cpsw_ale_get_entry_type(data); >> + if (type != ALE_TYPE_FREE) >> + cpsw_ale_write(ale, i, data); >> data += ALE_ENTRY_WORDS; >> } >> } > cheers, -roger
