| Message ID | 20211209094615.329379-4-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: lan966x: Add switchdev and vlan support | expand |
On Thu, Dec 09, 2021 at 10:46:12AM +0100, Horatiu Vultur wrote: > This patch adds support for handling the interrupts generated by the > analyzer. Currently, only the MAC table generates these interrupts. > The MAC table will generate an interrupt whenever it learns or forgets > an entry in the table. It is the SW responsibility figure out which > entries were added/removed. > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > .../ethernet/microchip/lan966x/lan966x_mac.c | 244 ++++++++++++++++++ > .../ethernet/microchip/lan966x/lan966x_main.c | 23 ++ > .../ethernet/microchip/lan966x/lan966x_main.h | 6 + > 3 files changed, 273 insertions(+) > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > index f6878b9f57ef..c01ab01bffbf 100644 > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0+ > > +#include <net/switchdev.h> > #include "lan966x_main.h" > > #define LAN966X_MAC_COLUMNS 4 > @@ -13,6 +14,23 @@ > #define MACACCESS_CMD_WRITE 7 > #define MACACCESS_CMD_SYNC_GET_NEXT 8 > > +#define LAN966X_MAC_INVALID_ROW -1 > + > +struct lan966x_mac_entry { > + struct list_head list; > + unsigned char mac[ETH_ALEN] __aligned(2); > + u16 vid; > + u16 port_index; > + int row; > +}; > + > +struct lan966x_mac_raw_entry { > + u32 mach; > + u32 macl; > + u32 maca; > + bool process; > +}; > + > static int lan966x_mac_get_status(struct lan966x *lan966x) > { > return lan_rd(lan966x, ANA_MACACCESS); > @@ -98,4 +116,230 @@ void lan966x_mac_init(struct lan966x *lan966x) > /* Clear the MAC table */ > lan_wr(MACACCESS_CMD_INIT, lan966x, ANA_MACACCESS); > lan966x_mac_wait_for_completion(lan966x); > + > + spin_lock_init(&lan966x->mac_lock); > + INIT_LIST_HEAD(&lan966x->mac_entries); > +} > + > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(const unsigned char *mac, > + u16 vid, u16 port_index) > +{ > + struct lan966x_mac_entry *mac_entry; > + > + mac_entry = kzalloc(sizeof(*mac_entry), GFP_KERNEL); > + if (!mac_entry) > + return NULL; > + > + memcpy(mac_entry->mac, mac, ETH_ALEN); > + mac_entry->vid = vid; > + mac_entry->port_index = port_index; > + mac_entry->row = LAN966X_MAC_INVALID_ROW; > + return mac_entry; > +} > + > +static void lan966x_fdb_call_notifiers(enum switchdev_notifier_type type, > + const char *mac, u16 vid, > + struct net_device *dev) > +{ > + struct switchdev_notifier_fdb_info info = { 0 }; > + > + info.addr = mac; > + info.vid = vid; > + info.offloaded = true; > + call_switchdev_notifiers(type, dev, &info.info, NULL); > +} > + > +void lan966x_mac_purge_entries(struct lan966x *lan966x) > +{ > + struct lan966x_mac_entry *mac_entry, *tmp; > + unsigned long flags; > + > + spin_lock_irqsave(&lan966x->mac_lock, flags); I hope I'm not wrong, but you are only using this spinlock to serialize access to the list, which isn't accessed from hardirq context anywhere (the irq is threaded). So spin_lock_irqsave could simply be spin_lock. Unless... > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, > + list) { > + lan966x_mac_forget(lan966x, mac_entry->mac, mac_entry->vid, > + ENTRYTYPE_LOCKED); Does this generate a MAC table interrupt? > + > + list_del(&mac_entry->list); > + kfree(mac_entry); > + } > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); > +} > + > +static void lan966x_mac_notifiers(struct lan966x *lan966x, > + enum switchdev_notifier_type type, > + unsigned char *mac, u32 vid, > + struct net_device *dev) > +{ > + rtnl_lock(); > + lan966x_fdb_call_notifiers(type, mac, vid, dev); > + rtnl_unlock(); > +} > + > +static void lan966x_mac_process_raw_entry(struct lan966x_mac_raw_entry *raw_entry, > + u8 *mac, u16 *vid, u32 *dest_idx) > +{ > + mac[0] = (raw_entry->mach >> 8) & 0xff; > + mac[1] = (raw_entry->mach >> 0) & 0xff; > + mac[2] = (raw_entry->macl >> 24) & 0xff; > + mac[3] = (raw_entry->macl >> 16) & 0xff; > + mac[4] = (raw_entry->macl >> 8) & 0xff; > + mac[5] = (raw_entry->macl >> 0) & 0xff; > + > + *vid = (raw_entry->mach >> 16) & 0xfff; > + *dest_idx = ANA_MACACCESS_DEST_IDX_GET(raw_entry->maca); > +} > + > +static void lan966x_mac_irq_process(struct lan966x *lan966x, u32 row, > + struct lan966x_mac_raw_entry *raw_entries) > +{ > + struct lan966x_mac_entry *mac_entry, *tmp; > + char mac[ETH_ALEN] __aligned(2); unsigned char > + unsigned long flags; > + u32 dest_idx; > + u32 column; > + u16 vid; > + > + spin_lock_irqsave(&lan966x->mac_lock, flags); > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, list) { > + bool found = false; > + > + if (mac_entry->row != row) > + continue; When the MAC table gets large, you could consider keeping separate lists per row. This way you can avoid traversing a list of elements you're sure you don't care about. > + > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { > + /* All the valid entries are at the start of the row, > + * so when get one invalid entry it can just skip the > + * rest of the columns > + */ > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) > + break; > + > + lan966x_mac_process_raw_entry(&raw_entries[column], > + mac, &vid, &dest_idx); > + WARN_ON(dest_idx > lan966x->num_phys_ports); > + > + /* If the entry in SW is found, then there is nothing > + * to do > + */ > + if (mac_entry->vid == vid && > + ether_addr_equal(mac_entry->mac, mac) && > + mac_entry->port_index == dest_idx) { > + raw_entries[column].process = true; > + found = true; > + break; > + } > + } > + > + if (!found) { > + /* Notify the bridge that the entry doesn't exist > + * anymore in the HW and remmove the entry from the SW s/remmove/remove/ > + * list > + */ > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_DEL_TO_BRIDGE, > + mac_entry->mac, mac_entry->vid, > + lan966x->ports[mac_entry->port_index]->dev); > + > + list_del(&mac_entry->list); > + kfree(mac_entry); > + } > + } > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); > + > + /* Now go to the list of columns and see if any entry was not in the SW > + * list, then that means that the entry is new so it needs to notify the > + * bridge. > + */ > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { > + /* All the valid entries are at the start of the row, so when > + * get one invalid entry it can just skip the rest of the columns > + */ > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) > + break; > + > + /* If the entry already exists then don't do anything */ > + if (raw_entries[column].process) s/process/processed/ > + continue; > + > + lan966x_mac_process_raw_entry(&raw_entries[column], > + mac, &vid, &dest_idx); > + WARN_ON(dest_idx > lan966x->num_phys_ports); > + > + mac_entry = lan966x_mac_alloc_entry(mac, vid, dest_idx); > + if (!mac_entry) > + return; > + > + mac_entry->row = row; > + > + spin_lock_irqsave(&lan966x->mac_lock, flags); > + list_add_tail(&mac_entry->list, &lan966x->mac_entries); > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); spin_lock_irqsave shouldn't be necessary from an irq handler. > + > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_ADD_TO_BRIDGE, > + mac, vid, lan966x->ports[dest_idx]->dev); > + } > +} > + > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x) > +{ > + struct lan966x_mac_raw_entry entry[LAN966X_MAC_COLUMNS] = { 0 }; > + u32 index, column; > + bool stop = true; > + u32 val; > + > + /* Check if the mac table triggered this, if not just bail out */ > + if (!(ANA_ANAINTR_INTR_GET(lan_rd(lan966x, ANA_ANAINTR)))) > + return IRQ_NONE; The interrupt isn't shared, so if we enter this condition, it means the analyzer block generated it, just not the MAC table portion of it. If we return IRQ_NONE there will be an IRQ storm because that condition will never go away. Could we ack the interrupt and return IRQ_HANDLED? > + > + /* Start the scan from 0, 0 */ > + lan_wr(ANA_MACTINDX_M_INDEX_SET(0) | > + ANA_MACTINDX_BUCKET_SET(0), > + lan966x, ANA_MACTINDX); > + > + while (1) { > + lan_rmw(ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_SYNC_GET_NEXT), > + ANA_MACACCESS_MAC_TABLE_CMD, > + lan966x, ANA_MACACCESS); > + lan966x_mac_wait_for_completion(lan966x); > + > + val = lan_rd(lan966x, ANA_MACTINDX); > + index = ANA_MACTINDX_M_INDEX_GET(val); > + column = ANA_MACTINDX_BUCKET_GET(val); > + > + /* The SYNC-GET-NEXT returns all the entries(4) in a row in > + * which is suffered a change. By change it means that new entry > + * was added or an entry was removed because of ageing. > + * It would return all the columns for that row. And after that > + * it would return the next row The stop conditions of the > + * SYNC-GET-NEXT is when it reaches 'directly' to row 0 > + * column 3. So if SYNC-GET-NEXT returns row 0 and column 0 > + * then it is required to continue to read more even if it > + * reaches row 0 and column 3. > + */ > + if (index == 0 && column == 0) > + stop = false; > + > + if (column == LAN966X_MAC_COLUMNS - 1 && > + index == 0 && stop) > + break; > + > + entry[column].mach = lan_rd(lan966x, ANA_MACHDATA); > + entry[column].macl = lan_rd(lan966x, ANA_MACLDATA); > + entry[column].maca = lan_rd(lan966x, ANA_MACACCESS); > + > + /* Once all the columns are read process them */ > + if (column == LAN966X_MAC_COLUMNS - 1) { > + lan966x_mac_irq_process(lan966x, index, entry); > + /* A row was processed so it is safe to assume that the > + * next row/column can be the stop condition > + */ > + stop = true; > + } > + } > + > + lan_rmw(ANA_ANAINTR_INTR_SET(0), > + ANA_ANAINTR_INTR, > + lan966x, ANA_ANAINTR); > + > + return IRQ_HANDLED; > } > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > index 101c1f005baf..7c6d6293611a 100644 > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > @@ -527,6 +527,13 @@ static irqreturn_t lan966x_xtr_irq_handler(int irq, void *args) > return IRQ_HANDLED; > } > > +static irqreturn_t lan966x_ana_irq_handler(int irq, void *args) > +{ > + struct lan966x *lan966x = args; > + > + return lan966x_mac_irq_handler(lan966x); > +} > + > static void lan966x_cleanup_ports(struct lan966x *lan966x) > { > struct lan966x_port *port; > @@ -554,6 +561,11 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x) > > disable_irq(lan966x->xtr_irq); > lan966x->xtr_irq = -ENXIO; > + > + if (lan966x->ana_irq) { > + disable_irq(lan966x->ana_irq); > + lan966x->ana_irq = -ENXIO; > + } > } > > static int lan966x_probe_port(struct lan966x *lan966x, u32 p, > @@ -870,6 +882,15 @@ static int lan966x_probe(struct platform_device *pdev) > return -ENODEV; > } > > + lan966x->ana_irq = platform_get_irq_byname(pdev, "ana"); > + if (lan966x->ana_irq) { > + err = devm_request_threaded_irq(&pdev->dev, lan966x->ana_irq, NULL, > + lan966x_ana_irq_handler, IRQF_ONESHOT, > + "ana irq", lan966x); > + if (err) > + return dev_err_probe(&pdev->dev, err, "Unable to use ana irq"); > + } > + > /* init switch */ > lan966x_init(lan966x); > lan966x_stats_init(lan966x); > @@ -923,6 +944,8 @@ static int lan966x_remove(struct platform_device *pdev) > destroy_workqueue(lan966x->stats_queue); > mutex_destroy(&lan966x->stats_lock); > > + lan966x_mac_purge_entries(lan966x); > + > return 0; > } > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > index 7e5a3b6f168d..ba548d65b58a 100644 > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > @@ -75,6 +75,9 @@ struct lan966x { > > u8 base_mac[ETH_ALEN]; > > + struct list_head mac_entries; > + spinlock_t mac_lock; /* lock for mac_entries list */ > + > /* stats */ > const struct lan966x_stat_layout *stats_layout; > u32 num_stats; > @@ -87,6 +90,7 @@ struct lan966x { > > /* interrupts */ > int xtr_irq; > + int ana_irq; > }; > > struct lan966x_port_config { > @@ -141,6 +145,8 @@ int lan966x_mac_forget(struct lan966x *lan966x, > int lan966x_mac_cpu_learn(struct lan966x *lan966x, const char *addr, u16 vid); > int lan966x_mac_cpu_forget(struct lan966x *lan966x, const char *addr, u16 vid); > void lan966x_mac_init(struct lan966x *lan966x); > +void lan966x_mac_purge_entries(struct lan966x *lan966x); > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x); > > static inline void __iomem *lan_addr(void __iomem *base[], > int id, int tinst, int tcnt, > -- > 2.33.0 >
The 12/09/2021 11:47, Vladimir Oltean wrote: > > On Thu, Dec 09, 2021 at 10:46:12AM +0100, Horatiu Vultur wrote: > > This patch adds support for handling the interrupts generated by the > > analyzer. Currently, only the MAC table generates these interrupts. > > The MAC table will generate an interrupt whenever it learns or forgets > > an entry in the table. It is the SW responsibility figure out which > > entries were added/removed. > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > .../ethernet/microchip/lan966x/lan966x_mac.c | 244 ++++++++++++++++++ > > .../ethernet/microchip/lan966x/lan966x_main.c | 23 ++ > > .../ethernet/microchip/lan966x/lan966x_main.h | 6 + > > 3 files changed, 273 insertions(+) > > > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > > index f6878b9f57ef..c01ab01bffbf 100644 > > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c > > @@ -1,5 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0+ > > > > +#include <net/switchdev.h> > > #include "lan966x_main.h" > > > > #define LAN966X_MAC_COLUMNS 4 > > @@ -13,6 +14,23 @@ > > #define MACACCESS_CMD_WRITE 7 > > #define MACACCESS_CMD_SYNC_GET_NEXT 8 > > > > +#define LAN966X_MAC_INVALID_ROW -1 > > + > > +struct lan966x_mac_entry { > > + struct list_head list; > > + unsigned char mac[ETH_ALEN] __aligned(2); > > + u16 vid; > > + u16 port_index; > > + int row; > > +}; > > + > > +struct lan966x_mac_raw_entry { > > + u32 mach; > > + u32 macl; > > + u32 maca; > > + bool process; > > +}; > > + > > static int lan966x_mac_get_status(struct lan966x *lan966x) > > { > > return lan_rd(lan966x, ANA_MACACCESS); > > @@ -98,4 +116,230 @@ void lan966x_mac_init(struct lan966x *lan966x) > > /* Clear the MAC table */ > > lan_wr(MACACCESS_CMD_INIT, lan966x, ANA_MACACCESS); > > lan966x_mac_wait_for_completion(lan966x); > > + > > + spin_lock_init(&lan966x->mac_lock); > > + INIT_LIST_HEAD(&lan966x->mac_entries); > > +} > > + > > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(const unsigned char *mac, > > + u16 vid, u16 port_index) > > +{ > > + struct lan966x_mac_entry *mac_entry; > > + > > + mac_entry = kzalloc(sizeof(*mac_entry), GFP_KERNEL); > > + if (!mac_entry) > > + return NULL; > > + > > + memcpy(mac_entry->mac, mac, ETH_ALEN); > > + mac_entry->vid = vid; > > + mac_entry->port_index = port_index; > > + mac_entry->row = LAN966X_MAC_INVALID_ROW; > > + return mac_entry; > > +} > > + > > +static void lan966x_fdb_call_notifiers(enum switchdev_notifier_type type, > > + const char *mac, u16 vid, > > + struct net_device *dev) > > +{ > > + struct switchdev_notifier_fdb_info info = { 0 }; > > + > > + info.addr = mac; > > + info.vid = vid; > > + info.offloaded = true; > > + call_switchdev_notifiers(type, dev, &info.info, NULL); > > +} > > + > > +void lan966x_mac_purge_entries(struct lan966x *lan966x) > > +{ > > + struct lan966x_mac_entry *mac_entry, *tmp; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&lan966x->mac_lock, flags); > > I hope I'm not wrong, but you are only using this spinlock to serialize > access to the list, which isn't accessed from hardirq context anywhere > (the irq is threaded). So spin_lock_irqsave could simply be spin_lock. > Unless... It should be OK to use spin_lock. I have chosen spin_lock_irq because that is the safest way in case it would be extended. > > > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, > > + list) { > > + lan966x_mac_forget(lan966x, mac_entry->mac, mac_entry->vid, > > + ENTRYTYPE_LOCKED); > > Does this generate a MAC table interrupt? It doesn't. > > > + > > + list_del(&mac_entry->list); > > + kfree(mac_entry); > > + } > > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); > > +} > > + > > +static void lan966x_mac_notifiers(struct lan966x *lan966x, > > + enum switchdev_notifier_type type, > > + unsigned char *mac, u32 vid, > > + struct net_device *dev) > > +{ > > + rtnl_lock(); > > + lan966x_fdb_call_notifiers(type, mac, vid, dev); > > + rtnl_unlock(); > > +} > > + > > +static void lan966x_mac_process_raw_entry(struct lan966x_mac_raw_entry *raw_entry, > > + u8 *mac, u16 *vid, u32 *dest_idx) > > +{ > > + mac[0] = (raw_entry->mach >> 8) & 0xff; > > + mac[1] = (raw_entry->mach >> 0) & 0xff; > > + mac[2] = (raw_entry->macl >> 24) & 0xff; > > + mac[3] = (raw_entry->macl >> 16) & 0xff; > > + mac[4] = (raw_entry->macl >> 8) & 0xff; > > + mac[5] = (raw_entry->macl >> 0) & 0xff; > > + > > + *vid = (raw_entry->mach >> 16) & 0xfff; > > + *dest_idx = ANA_MACACCESS_DEST_IDX_GET(raw_entry->maca); > > +} > > + > > +static void lan966x_mac_irq_process(struct lan966x *lan966x, u32 row, > > + struct lan966x_mac_raw_entry *raw_entries) > > +{ > > + struct lan966x_mac_entry *mac_entry, *tmp; > > + char mac[ETH_ALEN] __aligned(2); > > unsigned char > > > + unsigned long flags; > > + u32 dest_idx; > > + u32 column; > > + u16 vid; > > + > > + spin_lock_irqsave(&lan966x->mac_lock, flags); > > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, list) { > > + bool found = false; > > + > > + if (mac_entry->row != row) > > + continue; > > When the MAC table gets large, you could consider keeping separate lists > per row. This way you can avoid traversing a list of elements you're > sure you don't care about. Before I will change anything, I should run some measurements and see some results. > > > + > > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { > > + /* All the valid entries are at the start of the row, > > + * so when get one invalid entry it can just skip the > > + * rest of the columns > > + */ > > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) > > + break; > > + > > + lan966x_mac_process_raw_entry(&raw_entries[column], > > + mac, &vid, &dest_idx); > > + WARN_ON(dest_idx > lan966x->num_phys_ports); > > + > > + /* If the entry in SW is found, then there is nothing > > + * to do > > + */ > > + if (mac_entry->vid == vid && > > + ether_addr_equal(mac_entry->mac, mac) && > > + mac_entry->port_index == dest_idx) { > > + raw_entries[column].process = true; > > + found = true; > > + break; > > + } > > + } > > + > > + if (!found) { > > + /* Notify the bridge that the entry doesn't exist > > + * anymore in the HW and remmove the entry from the SW > > s/remmove/remove/ > > > + * list > > + */ > > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_DEL_TO_BRIDGE, > > + mac_entry->mac, mac_entry->vid, > > + lan966x->ports[mac_entry->port_index]->dev); > > + > > + list_del(&mac_entry->list); > > + kfree(mac_entry); > > + } > > + } > > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); > > + > > + /* Now go to the list of columns and see if any entry was not in the SW > > + * list, then that means that the entry is new so it needs to notify the > > + * bridge. > > + */ > > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { > > + /* All the valid entries are at the start of the row, so when > > + * get one invalid entry it can just skip the rest of the columns > > + */ > > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) > > + break; > > + > > + /* If the entry already exists then don't do anything */ > > + if (raw_entries[column].process) > > s/process/processed/ > > > + continue; > > + > > + lan966x_mac_process_raw_entry(&raw_entries[column], > > + mac, &vid, &dest_idx); > > + WARN_ON(dest_idx > lan966x->num_phys_ports); > > + > > + mac_entry = lan966x_mac_alloc_entry(mac, vid, dest_idx); > > + if (!mac_entry) > > + return; > > + > > + mac_entry->row = row; > > + > > + spin_lock_irqsave(&lan966x->mac_lock, flags); > > + list_add_tail(&mac_entry->list, &lan966x->mac_entries); > > + spin_unlock_irqrestore(&lan966x->mac_lock, flags); > > spin_lock_irqsave shouldn't be necessary from an irq handler. > > > + > > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_ADD_TO_BRIDGE, > > + mac, vid, lan966x->ports[dest_idx]->dev); > > + } > > +} > > + > > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x) > > +{ > > + struct lan966x_mac_raw_entry entry[LAN966X_MAC_COLUMNS] = { 0 }; > > + u32 index, column; > > + bool stop = true; > > + u32 val; > > + > > + /* Check if the mac table triggered this, if not just bail out */ > > + if (!(ANA_ANAINTR_INTR_GET(lan_rd(lan966x, ANA_ANAINTR)))) > > + return IRQ_NONE; > > The interrupt isn't shared, so if we enter this condition, it means the > analyzer block generated it, just not the MAC table portion of it. > If we return IRQ_NONE there will be an IRQ storm because that condition > will never go away. Could we ack the interrupt and return IRQ_HANDLED? Actually, there is a wrong comment, it doesn't check if the MAC table triggered it, but it checks if the analyzer generated it. So it can be removed and then always return IRQ_HANDLER. > > > + > > + /* Start the scan from 0, 0 */ > > + lan_wr(ANA_MACTINDX_M_INDEX_SET(0) | > > + ANA_MACTINDX_BUCKET_SET(0), > > + lan966x, ANA_MACTINDX); > > + > > + while (1) { > > + lan_rmw(ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_SYNC_GET_NEXT), > > + ANA_MACACCESS_MAC_TABLE_CMD, > > + lan966x, ANA_MACACCESS); > > + lan966x_mac_wait_for_completion(lan966x); > > + > > + val = lan_rd(lan966x, ANA_MACTINDX); > > + index = ANA_MACTINDX_M_INDEX_GET(val); > > + column = ANA_MACTINDX_BUCKET_GET(val); > > + > > + /* The SYNC-GET-NEXT returns all the entries(4) in a row in > > + * which is suffered a change. By change it means that new entry > > + * was added or an entry was removed because of ageing. > > + * It would return all the columns for that row. And after that > > + * it would return the next row The stop conditions of the > > + * SYNC-GET-NEXT is when it reaches 'directly' to row 0 > > + * column 3. So if SYNC-GET-NEXT returns row 0 and column 0 > > + * then it is required to continue to read more even if it > > + * reaches row 0 and column 3. > > + */ > > + if (index == 0 && column == 0) > > + stop = false; > > + > > + if (column == LAN966X_MAC_COLUMNS - 1 && > > + index == 0 && stop) > > + break; > > + > > + entry[column].mach = lan_rd(lan966x, ANA_MACHDATA); > > + entry[column].macl = lan_rd(lan966x, ANA_MACLDATA); > > + entry[column].maca = lan_rd(lan966x, ANA_MACACCESS); > > + > > + /* Once all the columns are read process them */ > > + if (column == LAN966X_MAC_COLUMNS - 1) { > > + lan966x_mac_irq_process(lan966x, index, entry); > > + /* A row was processed so it is safe to assume that the > > + * next row/column can be the stop condition > > + */ > > + stop = true; > > + } > > + } > > + > > + lan_rmw(ANA_ANAINTR_INTR_SET(0), > > + ANA_ANAINTR_INTR, > > + lan966x, ANA_ANAINTR); > > + > > + return IRQ_HANDLED; > > } > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > > index 101c1f005baf..7c6d6293611a 100644 > > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c > > @@ -527,6 +527,13 @@ static irqreturn_t lan966x_xtr_irq_handler(int irq, void *args) > > return IRQ_HANDLED; > > } > > > > +static irqreturn_t lan966x_ana_irq_handler(int irq, void *args) > > +{ > > + struct lan966x *lan966x = args; > > + > > + return lan966x_mac_irq_handler(lan966x); > > +} > > + > > static void lan966x_cleanup_ports(struct lan966x *lan966x) > > { > > struct lan966x_port *port; > > @@ -554,6 +561,11 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x) > > > > disable_irq(lan966x->xtr_irq); > > lan966x->xtr_irq = -ENXIO; > > + > > + if (lan966x->ana_irq) { > > + disable_irq(lan966x->ana_irq); > > + lan966x->ana_irq = -ENXIO; > > + } > > } > > > > static int lan966x_probe_port(struct lan966x *lan966x, u32 p, > > @@ -870,6 +882,15 @@ static int lan966x_probe(struct platform_device *pdev) > > return -ENODEV; > > } > > > > + lan966x->ana_irq = platform_get_irq_byname(pdev, "ana"); > > + if (lan966x->ana_irq) { > > + err = devm_request_threaded_irq(&pdev->dev, lan966x->ana_irq, NULL, > > + lan966x_ana_irq_handler, IRQF_ONESHOT, > > + "ana irq", lan966x); > > + if (err) > > + return dev_err_probe(&pdev->dev, err, "Unable to use ana irq"); > > + } > > + > > /* init switch */ > > lan966x_init(lan966x); > > lan966x_stats_init(lan966x); > > @@ -923,6 +944,8 @@ static int lan966x_remove(struct platform_device *pdev) > > destroy_workqueue(lan966x->stats_queue); > > mutex_destroy(&lan966x->stats_lock); > > > > + lan966x_mac_purge_entries(lan966x); > > + > > return 0; > > } > > > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > > index 7e5a3b6f168d..ba548d65b58a 100644 > > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h > > @@ -75,6 +75,9 @@ struct lan966x { > > > > u8 base_mac[ETH_ALEN]; > > > > + struct list_head mac_entries; > > + spinlock_t mac_lock; /* lock for mac_entries list */ > > + > > /* stats */ > > const struct lan966x_stat_layout *stats_layout; > > u32 num_stats; > > @@ -87,6 +90,7 @@ struct lan966x { > > > > /* interrupts */ > > int xtr_irq; > > + int ana_irq; > > }; > > > > struct lan966x_port_config { > > @@ -141,6 +145,8 @@ int lan966x_mac_forget(struct lan966x *lan966x, > > int lan966x_mac_cpu_learn(struct lan966x *lan966x, const char *addr, u16 vid); > > int lan966x_mac_cpu_forget(struct lan966x *lan966x, const char *addr, u16 vid); > > void lan966x_mac_init(struct lan966x *lan966x); > > +void lan966x_mac_purge_entries(struct lan966x *lan966x); > > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x); > > > > static inline void __iomem *lan_addr(void __iomem *base[], > > int id, int tinst, int tcnt, > > -- > > 2.33.0 > >
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c index f6878b9f57ef..c01ab01bffbf 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0+ +#include <net/switchdev.h> #include "lan966x_main.h" #define LAN966X_MAC_COLUMNS 4 @@ -13,6 +14,23 @@ #define MACACCESS_CMD_WRITE 7 #define MACACCESS_CMD_SYNC_GET_NEXT 8 +#define LAN966X_MAC_INVALID_ROW -1 + +struct lan966x_mac_entry { + struct list_head list; + unsigned char mac[ETH_ALEN] __aligned(2); + u16 vid; + u16 port_index; + int row; +}; + +struct lan966x_mac_raw_entry { + u32 mach; + u32 macl; + u32 maca; + bool process; +}; + static int lan966x_mac_get_status(struct lan966x *lan966x) { return lan_rd(lan966x, ANA_MACACCESS); @@ -98,4 +116,230 @@ void lan966x_mac_init(struct lan966x *lan966x) /* Clear the MAC table */ lan_wr(MACACCESS_CMD_INIT, lan966x, ANA_MACACCESS); lan966x_mac_wait_for_completion(lan966x); + + spin_lock_init(&lan966x->mac_lock); + INIT_LIST_HEAD(&lan966x->mac_entries); +} + +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(const unsigned char *mac, + u16 vid, u16 port_index) +{ + struct lan966x_mac_entry *mac_entry; + + mac_entry = kzalloc(sizeof(*mac_entry), GFP_KERNEL); + if (!mac_entry) + return NULL; + + memcpy(mac_entry->mac, mac, ETH_ALEN); + mac_entry->vid = vid; + mac_entry->port_index = port_index; + mac_entry->row = LAN966X_MAC_INVALID_ROW; + return mac_entry; +} + +static void lan966x_fdb_call_notifiers(enum switchdev_notifier_type type, + const char *mac, u16 vid, + struct net_device *dev) +{ + struct switchdev_notifier_fdb_info info = { 0 }; + + info.addr = mac; + info.vid = vid; + info.offloaded = true; + call_switchdev_notifiers(type, dev, &info.info, NULL); +} + +void lan966x_mac_purge_entries(struct lan966x *lan966x) +{ + struct lan966x_mac_entry *mac_entry, *tmp; + unsigned long flags; + + spin_lock_irqsave(&lan966x->mac_lock, flags); + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, + list) { + lan966x_mac_forget(lan966x, mac_entry->mac, mac_entry->vid, + ENTRYTYPE_LOCKED); + + list_del(&mac_entry->list); + kfree(mac_entry); + } + spin_unlock_irqrestore(&lan966x->mac_lock, flags); +} + +static void lan966x_mac_notifiers(struct lan966x *lan966x, + enum switchdev_notifier_type type, + unsigned char *mac, u32 vid, + struct net_device *dev) +{ + rtnl_lock(); + lan966x_fdb_call_notifiers(type, mac, vid, dev); + rtnl_unlock(); +} + +static void lan966x_mac_process_raw_entry(struct lan966x_mac_raw_entry *raw_entry, + u8 *mac, u16 *vid, u32 *dest_idx) +{ + mac[0] = (raw_entry->mach >> 8) & 0xff; + mac[1] = (raw_entry->mach >> 0) & 0xff; + mac[2] = (raw_entry->macl >> 24) & 0xff; + mac[3] = (raw_entry->macl >> 16) & 0xff; + mac[4] = (raw_entry->macl >> 8) & 0xff; + mac[5] = (raw_entry->macl >> 0) & 0xff; + + *vid = (raw_entry->mach >> 16) & 0xfff; + *dest_idx = ANA_MACACCESS_DEST_IDX_GET(raw_entry->maca); +} + +static void lan966x_mac_irq_process(struct lan966x *lan966x, u32 row, + struct lan966x_mac_raw_entry *raw_entries) +{ + struct lan966x_mac_entry *mac_entry, *tmp; + char mac[ETH_ALEN] __aligned(2); + unsigned long flags; + u32 dest_idx; + u32 column; + u16 vid; + + spin_lock_irqsave(&lan966x->mac_lock, flags); + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, list) { + bool found = false; + + if (mac_entry->row != row) + continue; + + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { + /* All the valid entries are at the start of the row, + * so when get one invalid entry it can just skip the + * rest of the columns + */ + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) + break; + + lan966x_mac_process_raw_entry(&raw_entries[column], + mac, &vid, &dest_idx); + WARN_ON(dest_idx > lan966x->num_phys_ports); + + /* If the entry in SW is found, then there is nothing + * to do + */ + if (mac_entry->vid == vid && + ether_addr_equal(mac_entry->mac, mac) && + mac_entry->port_index == dest_idx) { + raw_entries[column].process = true; + found = true; + break; + } + } + + if (!found) { + /* Notify the bridge that the entry doesn't exist + * anymore in the HW and remmove the entry from the SW + * list + */ + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_DEL_TO_BRIDGE, + mac_entry->mac, mac_entry->vid, + lan966x->ports[mac_entry->port_index]->dev); + + list_del(&mac_entry->list); + kfree(mac_entry); + } + } + spin_unlock_irqrestore(&lan966x->mac_lock, flags); + + /* Now go to the list of columns and see if any entry was not in the SW + * list, then that means that the entry is new so it needs to notify the + * bridge. + */ + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) { + /* All the valid entries are at the start of the row, so when + * get one invalid entry it can just skip the rest of the columns + */ + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca)) + break; + + /* If the entry already exists then don't do anything */ + if (raw_entries[column].process) + continue; + + lan966x_mac_process_raw_entry(&raw_entries[column], + mac, &vid, &dest_idx); + WARN_ON(dest_idx > lan966x->num_phys_ports); + + mac_entry = lan966x_mac_alloc_entry(mac, vid, dest_idx); + if (!mac_entry) + return; + + mac_entry->row = row; + + spin_lock_irqsave(&lan966x->mac_lock, flags); + list_add_tail(&mac_entry->list, &lan966x->mac_entries); + spin_unlock_irqrestore(&lan966x->mac_lock, flags); + + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_ADD_TO_BRIDGE, + mac, vid, lan966x->ports[dest_idx]->dev); + } +} + +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x) +{ + struct lan966x_mac_raw_entry entry[LAN966X_MAC_COLUMNS] = { 0 }; + u32 index, column; + bool stop = true; + u32 val; + + /* Check if the mac table triggered this, if not just bail out */ + if (!(ANA_ANAINTR_INTR_GET(lan_rd(lan966x, ANA_ANAINTR)))) + return IRQ_NONE; + + /* Start the scan from 0, 0 */ + lan_wr(ANA_MACTINDX_M_INDEX_SET(0) | + ANA_MACTINDX_BUCKET_SET(0), + lan966x, ANA_MACTINDX); + + while (1) { + lan_rmw(ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_SYNC_GET_NEXT), + ANA_MACACCESS_MAC_TABLE_CMD, + lan966x, ANA_MACACCESS); + lan966x_mac_wait_for_completion(lan966x); + + val = lan_rd(lan966x, ANA_MACTINDX); + index = ANA_MACTINDX_M_INDEX_GET(val); + column = ANA_MACTINDX_BUCKET_GET(val); + + /* The SYNC-GET-NEXT returns all the entries(4) in a row in + * which is suffered a change. By change it means that new entry + * was added or an entry was removed because of ageing. + * It would return all the columns for that row. And after that + * it would return the next row The stop conditions of the + * SYNC-GET-NEXT is when it reaches 'directly' to row 0 + * column 3. So if SYNC-GET-NEXT returns row 0 and column 0 + * then it is required to continue to read more even if it + * reaches row 0 and column 3. + */ + if (index == 0 && column == 0) + stop = false; + + if (column == LAN966X_MAC_COLUMNS - 1 && + index == 0 && stop) + break; + + entry[column].mach = lan_rd(lan966x, ANA_MACHDATA); + entry[column].macl = lan_rd(lan966x, ANA_MACLDATA); + entry[column].maca = lan_rd(lan966x, ANA_MACACCESS); + + /* Once all the columns are read process them */ + if (column == LAN966X_MAC_COLUMNS - 1) { + lan966x_mac_irq_process(lan966x, index, entry); + /* A row was processed so it is safe to assume that the + * next row/column can be the stop condition + */ + stop = true; + } + } + + lan_rmw(ANA_ANAINTR_INTR_SET(0), + ANA_ANAINTR_INTR, + lan966x, ANA_ANAINTR); + + return IRQ_HANDLED; } diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c index 101c1f005baf..7c6d6293611a 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c @@ -527,6 +527,13 @@ static irqreturn_t lan966x_xtr_irq_handler(int irq, void *args) return IRQ_HANDLED; } +static irqreturn_t lan966x_ana_irq_handler(int irq, void *args) +{ + struct lan966x *lan966x = args; + + return lan966x_mac_irq_handler(lan966x); +} + static void lan966x_cleanup_ports(struct lan966x *lan966x) { struct lan966x_port *port; @@ -554,6 +561,11 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x) disable_irq(lan966x->xtr_irq); lan966x->xtr_irq = -ENXIO; + + if (lan966x->ana_irq) { + disable_irq(lan966x->ana_irq); + lan966x->ana_irq = -ENXIO; + } } static int lan966x_probe_port(struct lan966x *lan966x, u32 p, @@ -870,6 +882,15 @@ static int lan966x_probe(struct platform_device *pdev) return -ENODEV; } + lan966x->ana_irq = platform_get_irq_byname(pdev, "ana"); + if (lan966x->ana_irq) { + err = devm_request_threaded_irq(&pdev->dev, lan966x->ana_irq, NULL, + lan966x_ana_irq_handler, IRQF_ONESHOT, + "ana irq", lan966x); + if (err) + return dev_err_probe(&pdev->dev, err, "Unable to use ana irq"); + } + /* init switch */ lan966x_init(lan966x); lan966x_stats_init(lan966x); @@ -923,6 +944,8 @@ static int lan966x_remove(struct platform_device *pdev) destroy_workqueue(lan966x->stats_queue); mutex_destroy(&lan966x->stats_lock); + lan966x_mac_purge_entries(lan966x); + return 0; } diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h index 7e5a3b6f168d..ba548d65b58a 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h @@ -75,6 +75,9 @@ struct lan966x { u8 base_mac[ETH_ALEN]; + struct list_head mac_entries; + spinlock_t mac_lock; /* lock for mac_entries list */ + /* stats */ const struct lan966x_stat_layout *stats_layout; u32 num_stats; @@ -87,6 +90,7 @@ struct lan966x { /* interrupts */ int xtr_irq; + int ana_irq; }; struct lan966x_port_config { @@ -141,6 +145,8 @@ int lan966x_mac_forget(struct lan966x *lan966x, int lan966x_mac_cpu_learn(struct lan966x *lan966x, const char *addr, u16 vid); int lan966x_mac_cpu_forget(struct lan966x *lan966x, const char *addr, u16 vid); void lan966x_mac_init(struct lan966x *lan966x); +void lan966x_mac_purge_entries(struct lan966x *lan966x); +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x); static inline void __iomem *lan_addr(void __iomem *base[], int id, int tinst, int tcnt,
This patch adds support for handling the interrupts generated by the analyzer. Currently, only the MAC table generates these interrupts. The MAC table will generate an interrupt whenever it learns or forgets an entry in the table. It is the SW responsibility figure out which entries were added/removed. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- .../ethernet/microchip/lan966x/lan966x_mac.c | 244 ++++++++++++++++++ .../ethernet/microchip/lan966x/lan966x_main.c | 23 ++ .../ethernet/microchip/lan966x/lan966x_main.h | 6 + 3 files changed, 273 insertions(+)