| Message ID | 20211203104645.1476704-1-horatiu.vultur@microchip.com |
|---|---|
| Headers | show |
| Series | net: lan966x: Add switchdev and vlan support | expand |
On Fri, 3 Dec 2021 11:46:42 +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> > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(struct lan966x *lan966x, > + const unsigned char *mac, > + u16 vid, u16 port_index) > +{ > + struct lan966x_mac_entry *mac_entry; > + > + mac_entry = devm_kzalloc(lan966x->dev, > + sizeof(*mac_entry), GFP_ATOMIC); Is it really necessary to use devm_ allocation for the mac entries? It's 2x memory overhead. Also why GFP_ATOMIC? Memory allocations are _a lot_ less likely with 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_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); Double space before = > +} > + > +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; > + unsigned long flags; > + char mac[ETH_ALEN]; > + 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 founded = false; s/founded/found/ > + 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) && You need to add __aligned(2) to mac, ether_addr_equal() needs aligned arguments. > + mac_entry->port_index == dest_idx) { > + raw_entries[column].process = true; > + founded = true; > + break; > + } > + }
The 12/06/2021 18:24, Jakub Kicinski wrote: Hi Jakub, > > On Fri, 3 Dec 2021 11:46:42 +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> > > > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(struct lan966x *lan966x, > > + const unsigned char *mac, > > + u16 vid, u16 port_index) > > +{ > > + struct lan966x_mac_entry *mac_entry; > > + > > + mac_entry = devm_kzalloc(lan966x->dev, > > + sizeof(*mac_entry), GFP_ATOMIC); > > Is it really necessary to use devm_ allocation for the mac entries? > It's 2x memory overhead. It is not necessary. > > Also why GFP_ATOMIC? Memory allocations are _a lot_ less likely with > GFP_KERNEL. Initially I thought this is called also in some context where it couldn't sleep. But I was wrong. I will change these in the next version. > > > + 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_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); > > Double space before = > > > +} > > + > > +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; > > + unsigned long flags; > > + char mac[ETH_ALEN]; > > + 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 founded = false; > > s/founded/found/ > > > + 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) && > > You need to add __aligned(2) to mac, ether_addr_equal() needs aligned > arguments. > > > + mac_entry->port_index == dest_idx) { > > + raw_entries[column].process = true; > > + founded = true; > > + break; > > + } > > + }
This patch series extends lan966x with switchdev and vlan support. The first patches just adds new registers and extend the MAC table to handle the interrupts when a new address is learn/forget. The last 2 patches adds the vlan and the switchdev support. Horatiu Vultur (6): net: lan966x: Add registers that are used for switch and vlan functionality dt-bindings: net: lan966x: Extend with the analyzer interrupt net: lan966x: add support for interrupts from analyzer net: lan966x: More MAC table functionality net: lan966x: Add vlan support net: lan966x: Add switchdev support .../net/microchip,lan966x-switch.yaml | 2 + .../net/ethernet/microchip/lan966x/Makefile | 3 +- .../ethernet/microchip/lan966x/lan966x_mac.c | 337 +++++++++++ .../ethernet/microchip/lan966x/lan966x_main.c | 92 ++- .../ethernet/microchip/lan966x/lan966x_main.h | 71 ++- .../ethernet/microchip/lan966x/lan966x_regs.h | 129 +++++ .../microchip/lan966x/lan966x_switchdev.c | 544 ++++++++++++++++++ .../ethernet/microchip/lan966x/lan966x_vlan.c | 439 ++++++++++++++ 8 files changed, 1602 insertions(+), 15 deletions(-) create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c